Add inline add more tile for gallery permissions#2553
Merged
renefloor merged 3 commits intofeat/design-refreshfrom Mar 19, 2026
Merged
Add inline add more tile for gallery permissions#2553renefloor merged 3 commits intofeat/design-refreshfrom
renefloor merged 3 commits intofeat/design-refreshfrom
Conversation
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feat/design-refresh #2553 +/- ##
=======================================================
+ Coverage 64.56% 64.67% +0.10%
=======================================================
Files 426 426
Lines 26126 26138 +12
=======================================================
+ Hits 16869 16905 +36
+ Misses 9257 9233 -24 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
renefloor
force-pushed
the
feature/improve-gallery-picker-layout
branch
from
cbc65f8 to
821e209
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart (1)
279-325:⚠️ Potential issue | 🟠 MajorUnused constructor parameters:
color,elevation,clipBehavior, andshape.The
OptionDrawerconstructor still acceptscolor,elevation,clipBehavior, andshapeparameters, but thebuildmethod no longer uses them—it only renders aContainerwithmarginandchild.This appears to be an incomplete refactor. The parameters should either be:
- Removed if they're no longer needed, or
- Used in the build method (e.g., wrapped in a
MaterialorCardwidget).Based on the context snippets, no current callers use these parameters, so removing them would be the cleaner approach.
🔧 Proposed fix: Remove unused parameters
class OptionDrawer extends StatelessWidget { /// Creates a widget that will be shown in the attachment picker. const OptionDrawer({ super.key, required this.child, - this.color, - this.elevation = 2, this.margin, - this.clipBehavior = Clip.hardEdge, - this.shape = _kDefaultOptionDrawerShape, }); /// The widget below this widget in the tree. final Widget child; - /// The background color of the options card. - /// - /// Defaults to [StreamColorTheme.barsBg]. - final Color? color; - - /// The elevation of the options card. - /// - /// The default value is 2. - final double elevation; - /// The margin of the options card. final EdgeInsetsGeometry? margin; - /// The clip behavior of the options card. - /// - /// The default value is [Clip.hardEdge]. - final Clip clipBehavior; - - /// The shape of the options card. - final ShapeBorder shape; - `@override` Widget build(BuildContext context) {Additionally, consider whether the
_kDefaultOptionDrawerShapeconstant (lines 270-275) is still needed if it's no longer referenced.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart` around lines 279 - 325, The OptionDrawer constructor exposes unused parameters (color, elevation, clipBehavior, shape) and references _kDefaultOptionDrawerShape, but build() only uses margin and child; remove the unused constructor fields and their assignments (color, elevation, clipBehavior, shape) and delete _kDefaultOptionDrawerShape if no other code references it, leaving OptionDrawer with only key, child, margin and the used defaults; alternatively, if you prefer to preserve the visual card behavior, wrap the returned Container in a Material/Card and wire the existing fields into that widget (use elevation, color, clipBehavior, and shape) — pick one approach and apply it consistently in the OptionDrawer class and its constructor.
🧹 Nitpick comments (2)
packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart (1)
158-199: Consider usingMaterial.colorfor background styling instead ofoverlayColor.The
_AddMoreTileimplementation is functionally correct. However, theoverlayColorresolver returnscolorScheme.backgroundSurfaceCardin the default state (line 176), which isn't the typical usage pattern foroverlayColor(intended for splash/highlight effects, not backgrounds).If a background color is needed, consider setting it on the
Materialwidget'scolorproperty instead:♻️ Optional: Use Material.color for background
return Material( + color: colorScheme.backgroundSurfaceCard, child: InkWell( onTap: onTap, overlayColor: WidgetStateProperty.resolveWith((states) { if (states.contains(WidgetState.pressed)) return colorScheme.statePressed; if (states.contains(WidgetState.hovered)) return colorScheme.stateHover; if (states.contains(WidgetState.focused)) return colorScheme.stateFocused; - return colorScheme.backgroundSurfaceCard; + return null; }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart` around lines 158 - 199, The overlayColor resolver in _AddMoreTile is being used to provide a default background (colorScheme.backgroundSurfaceCard) which is not idiomatic; instead set the background on the Material widget via its color property and reserve overlayColor for interactive highlights (pressed/hovered/focused) only. Update the Material(...) that wraps the InkWell to pass color: colorScheme.backgroundSurfaceCard, and change the overlayColor resolver in the InkWell to return only the state-specific colors (or null for the default case) so overlayColor is not used as the persistent background.packages/stream_chat_flutter_core/test/paged_value_grid_view_test.dart (1)
39-125: Good test coverage for the core functionality.The tests effectively verify:
- Items render starting at index 0 without leading builder
- Leading item renders before paged items
itemBuilderreceives correct (non-offset) indices- Edge case with a single paged item
Consider adding a test for the load-more indicator behavior with
leadingItemBuilderto ensure the indicator still renders at the correct position:📝 Suggested additional test
testWidgets('renders load-more indicator at correct position with leading item', (tester) async { final controller = _TestController(['a', 'b'], nextPageKey: 1); final builtIndices = <int>[]; await tester.pumpWidget( _wrap( _buildGrid( controller, leadingItemBuilder: (_) => const Text('leading'), builtIndices: builtIndices, ), ), ); await tester.pump(); expect(find.text('leading'), findsOneWidget); expect(find.text('item-0'), findsOneWidget); expect(find.text('item-1'), findsOneWidget); expect(find.text('load-more-indicator'), findsOneWidget); expect(builtIndices, [0, 1]); // itemBuilder indices should not include leading or load-more });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_chat_flutter_core/test/paged_value_grid_view_test.dart` around lines 39 - 125, Add a new widget test named "renders load-more indicator at correct position with leading item" that uses _TestController (with nextPageKey: 1) and _buildGrid with a leadingItemBuilder to assert the leading Text appears, the paged items 'item-0' and 'item-1' appear, and the 'load-more-indicator' is rendered; also verify the builtIndices list collected by the itemBuilder equals [0,1] (i.e., itemBuilder indices are not offset by the leading item or the indicator). Use the same test harness helpers (_wrap, _buildGrid, _TestController, builtIndices) and follow existing test style (pumpWidget then pump) to ensure consistent behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dart`:
- Around line 279-325: The OptionDrawer constructor exposes unused parameters
(color, elevation, clipBehavior, shape) and references
_kDefaultOptionDrawerShape, but build() only uses margin and child; remove the
unused constructor fields and their assignments (color, elevation, clipBehavior,
shape) and delete _kDefaultOptionDrawerShape if no other code references it,
leaving OptionDrawer with only key, child, margin and the used defaults;
alternatively, if you prefer to preserve the visual card behavior, wrap the
returned Container in a Material/Card and wire the existing fields into that
widget (use elevation, color, clipBehavior, and shape) — pick one approach and
apply it consistently in the OptionDrawer class and its constructor.
---
Nitpick comments:
In `@packages/stream_chat_flutter_core/test/paged_value_grid_view_test.dart`:
- Around line 39-125: Add a new widget test named "renders load-more indicator
at correct position with leading item" that uses _TestController (with
nextPageKey: 1) and _buildGrid with a leadingItemBuilder to assert the leading
Text appears, the paged items 'item-0' and 'item-1' appear, and the
'load-more-indicator' is rendered; also verify the builtIndices list collected
by the itemBuilder equals [0,1] (i.e., itemBuilder indices are not offset by the
leading item or the indicator). Use the same test harness helpers (_wrap,
_buildGrid, _TestController, builtIndices) and follow existing test style
(pumpWidget then pump) to ensure consistent behavior.
In
`@packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart`:
- Around line 158-199: The overlayColor resolver in _AddMoreTile is being used
to provide a default background (colorScheme.backgroundSurfaceCard) which is not
idiomatic; instead set the background on the Material widget via its color
property and reserve overlayColor for interactive highlights
(pressed/hovered/focused) only. Update the Material(...) that wraps the InkWell
to pass color: colorScheme.backgroundSurfaceCard, and change the overlayColor
resolver in the InkWell to return only the state-specific colors (or null for
the default case) so overlayColor is not used as the persistent background.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba5d54b8-4a08-4752-ad34-31b3be4ab06e
📒 Files selected for processing (18)
packages/stream_chat_flutter/lib/src/localization/translations.dartpackages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dartpackages/stream_chat_flutter/lib/src/message_input/attachment_picker/stream_attachment_picker.dartpackages/stream_chat_flutter/lib/src/scroll_view/photo_gallery/stream_photo_gallery.dartpackages/stream_chat_flutter_core/lib/src/paged_value_scroll_view.dartpackages/stream_chat_flutter_core/test/paged_value_grid_view_test.dartpackages/stream_chat_localizations/example/lib/add_new_lang.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_ca.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_de.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_en.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_es.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_fr.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_hi.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_it.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_ja.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_ko.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_no.dartpackages/stream_chat_localizations/lib/src/stream_chat_localizations_pt.dart
xsahil03x
approved these changes
Mar 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Submit a pull request
Fixes FLU-387
CLA
Description of the pull request
This adds a nice plus button instead of the one in the top row.
Screenshots / Videos
Summary by CodeRabbit
Release Notes
New Features
Improvements
Localization
Tests