feat(ui): add StreamEmojiChip, StreamEmojiChipBar and StreamListTile#58
feat(ui): add StreamEmojiChip, StreamEmojiChipBar and StreamListTile#58xsahil03x wants to merge 7 commits intomain-design-systemfrom
Conversation
…actions-view-components # Conflicts: # packages/stream_core_flutter/lib/src/components.dart
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
|
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 You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds three new UI components (StreamEmojiChip, StreamEmojiChipBar, StreamListTile), their default implementations, theming, StreamComponentFactory hooks, gallery showcases, and toolbar/debug toggle; updates exports and integrates themes into StreamTheme. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
✅ Actions performedReview triggered.
|
1 similar comment
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/design_system_gallery/lib/widgets/toolbar/baselines_toggle.dart (1)
15-18: Wrap the unawaitedperformReassemble()call withunawaited().The
unawaited_futureslint rule is enabled in the project, andperformReassemble()returnsFuture<void>. Wrapping the call withunawaited()explicitly marks the ignored future and aligns with the established pattern used elsewhere in the codebase.♻️ Proposed fix
+import 'dart:async'; import 'package:flutter/material.dart'; import 'package:flutter/rendering.dart';void _toggle() { setState(() => debugPaintBaselinesEnabled = !debugPaintBaselinesEnabled); - WidgetsBinding.instance.performReassemble(); + unawaited(WidgetsBinding.instance.performReassemble()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/design_system_gallery/lib/widgets/toolbar/baselines_toggle.dart` around lines 15 - 18, The _toggle method currently calls WidgetsBinding.instance.performReassemble() without awaiting; wrap that call with unawaited(...) to satisfy the unawaited_futures lint and mark the future as intentionally ignored. Update the _toggle implementation to call unawaited(WidgetsBinding.instance.performReassemble()) and add the appropriate import for unawaited (for example, import 'package:pedantic/pedantic.dart';) so the symbol is available; keep the existing setState line toggling debugPaintBaselinesEnabled unchanged.apps/design_system_gallery/lib/components/controls/stream_emoji_chip.dart (1)
748-759: Consider adding error handling to_byNamelookup for future-proofing.All emoji names in
_sampleEmojiscurrently exist inunicode_emojis0.5.1's dataset. However, the_byNamefunction usesfirstWherewithout anorElseclause, making initialization fragile if upstreamunicode_emojischanges emoji names or structures in future versions. Adding a fallback improves robustness:Suggested improvement
-Emoji _byName(String name) => UnicodeEmojis.allEmojis.firstWhere((e) => e.name == name); +Emoji _byName(String name) => UnicodeEmojis.allEmojis.firstWhere( + (e) => e.name == name, + orElse: () => UnicodeEmojis.allEmojis.first, + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/design_system_gallery/lib/components/controls/stream_emoji_chip.dart` around lines 748 - 759, The _byName function currently calls UnicodeEmojis.allEmojis.firstWhere without an orElse, which can throw if a name disappears; update _byName to handle missing lookups by using firstWhere(..., orElse: () => <fallback>) or catching the StateError and returning a sensible default emoji (or null) so _sampleEmojis initialization won’t crash — reference the _byName function and the _sampleEmojis list when making this change.packages/stream_core_flutter/lib/src/components/list/stream_list_tile.dart (1)
220-223: Force-unwraps on resolved state colors follow Flutter conventions but are worth documenting.The
!onresolve(states)for title/subtitle/description/icon colors will throw at runtime if a custom theme resolves tonullfor any state combination. The defaults are safe, but a consumer-providedStreamListTileThemeDatacould trip this. Consider adding a brief dartdoc note onStreamListTileThemeData(or an assert) clarifying that these properties must resolve to non-null for all states.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/list/stream_list_tile.dart` around lines 220 - 223, The force-unwrapped resolves on theme colors (e.g., the code computing effectiveTitleColor / effectiveSubtitleColor / effectiveDescriptionColor / effectiveIconColor using (theme.titleColor ?? defaults.titleColor).resolve(states)!) can throw if a consumer-supplied StreamListTileThemeData returns null for some states; add a short doc comment on StreamListTileThemeData's color properties stating they must resolve to non-null for all states and/or add defensive asserts where those properties are defined (e.g., in StreamListTileThemeData constructor or its color getters) to enforce non-null resolution at runtime so the resolve(...)! calls are safe.packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip.dart (1)
210-219: Hardcoded icon size of 20 — consider deriving from the theme.The
_AddEmojiIconuses a hardcodedsize: 20, while the chip'siconSize(line 165) isStreamEmojiSize.sm.value. If these are meant to match, coupling them would prevent future drift. Minor since it's a private widget.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip.dart` around lines 210 - 219, Replace the hardcoded IconThemeData(size: 20) in _AddEmojiIcon.build with a derived size that matches the chip's iconSize (use StreamEmojiSize.sm.value) so the add-reaction icon stays in sync; update the IconThemeData to use StreamEmojiSize.sm.value (or pull the same constant from wherever the chip uses it) and keep the rest of the widget unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dart`:
- Around line 169-170: The ButtonStyle in StreamEmojiButton currently uses
tapTargetSize: .shrinkWrap causing sizes below xl to violate the 48×48dp
touch-target minimum; update the style in the StreamEmojiButton widget (where
ButtonStyle is built) to ensure accessibility by either switching tapTargetSize
to MaterialTapTargetSize.padded for all buttons or, if you must keep shrinkWrap
visually, add conditional padding for smaller sizes (e.g., add
EdgeInsets.symmetric(horizontal/vertical ~8–12dp) when size != xl or when
diameter < 48) so that the effective touch target is at least 48×48dp; modify
the ButtonStyle construction (tapTargetSize and/or padding) accordingly in the
build/constructor that creates the emoji button.
In
`@packages/stream_core_flutter/lib/src/theme/components/stream_emoji_chip_theme.dart`:
- Around line 128-131: The constructor currently force-casts the parameter side
(type WidgetStateProperty<BorderSide?>?) to WidgetStateBorderSide? which will
throw for non-WidgetStateBorderSide inputs; update the constructor for safety by
either 1) changing the parameter type from WidgetStateProperty<BorderSide?>? to
WidgetStateBorderSide? (making the contract explicit and removing the cast), or
2) keeping the broader type and replacing the forced cast with a defensive
conversion: if side is WidgetStateBorderSide use it directly, else wrap
non-WidgetStateBorderSide instances (e.g. WidgetStatePropertyAll<BorderSide?>)
into a new WidgetStateBorderSide wrapper instance or convert them into an
equivalent WidgetStateBorderSide implementation before assigning the field;
update the constructor assignment for the field side accordingly and ensure any
downstream code expecting WidgetStateBorderSide handles the wrapped type.
---
Nitpick comments:
In `@apps/design_system_gallery/lib/components/controls/stream_emoji_chip.dart`:
- Around line 748-759: The _byName function currently calls
UnicodeEmojis.allEmojis.firstWhere without an orElse, which can throw if a name
disappears; update _byName to handle missing lookups by using firstWhere(...,
orElse: () => <fallback>) or catching the StateError and returning a sensible
default emoji (or null) so _sampleEmojis initialization won’t crash — reference
the _byName function and the _sampleEmojis list when making this change.
In `@apps/design_system_gallery/lib/widgets/toolbar/baselines_toggle.dart`:
- Around line 15-18: The _toggle method currently calls
WidgetsBinding.instance.performReassemble() without awaiting; wrap that call
with unawaited(...) to satisfy the unawaited_futures lint and mark the future as
intentionally ignored. Update the _toggle implementation to call
unawaited(WidgetsBinding.instance.performReassemble()) and add the appropriate
import for unawaited (for example, import 'package:pedantic/pedantic.dart';) so
the symbol is available; keep the existing setState line toggling
debugPaintBaselinesEnabled unchanged.
In
`@packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip.dart`:
- Around line 210-219: Replace the hardcoded IconThemeData(size: 20) in
_AddEmojiIcon.build with a derived size that matches the chip's iconSize (use
StreamEmojiSize.sm.value) so the add-reaction icon stays in sync; update the
IconThemeData to use StreamEmojiSize.sm.value (or pull the same constant from
wherever the chip uses it) and keep the rest of the widget unchanged.
In `@packages/stream_core_flutter/lib/src/components/list/stream_list_tile.dart`:
- Around line 220-223: The force-unwrapped resolves on theme colors (e.g., the
code computing effectiveTitleColor / effectiveSubtitleColor /
effectiveDescriptionColor / effectiveIconColor using (theme.titleColor ??
defaults.titleColor).resolve(states)!) can throw if a consumer-supplied
StreamListTileThemeData returns null for some states; add a short doc comment on
StreamListTileThemeData's color properties stating they must resolve to non-null
for all states and/or add defensive asserts where those properties are defined
(e.g., in StreamListTileThemeData constructor or its color getters) to enforce
non-null resolution at runtime so the resolve(...)! calls are safe.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (22)
apps/design_system_gallery/README.mdapps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/controls/stream_emoji_chip.dartapps/design_system_gallery/lib/components/tiles/stream_list_tile.dartapps/design_system_gallery/lib/widgets/toolbar/baselines_toggle.dartapps/design_system_gallery/lib/widgets/toolbar/toolbar.dartpackages/stream_core_flutter/lib/src/components.dartpackages/stream_core_flutter/lib/src/components/accessories/stream_emoji.dartpackages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dartpackages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip.dartpackages/stream_core_flutter/lib/src/components/list/stream_list_tile.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.g.theme.dartpackages/stream_core_flutter/lib/src/theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_emoji_chip_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_emoji_chip_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_list_tile_theme.dartpackages/stream_core_flutter/lib/src/theme/components/stream_list_tile_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme.g.theme.dartpackages/stream_core_flutter/lib/src/theme/stream_theme_extensions.dartpubspec.yaml
| tapTargetSize: .shrinkWrap, | ||
| visualDensity: .standard, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find and examine the file
fd -t f "stream_emoji_button.dart"Repository: GetStream/stream-core-flutter
Length of output: 228
🏁 Script executed:
# Get the file size first
wc -l packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dartRepository: GetStream/stream-core-flutter
Length of output: 157
🏁 Script executed:
# Read the relevant section around lines 169-170 with context
sed -n '150,190p' packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dartRepository: GetStream/stream-core-flutter
Length of output: 1769
🌐 Web query:
Flutter MaterialTapTargetSize shrinkWrap effect on touch target size interactive area
💡 Result:
MaterialTapTargetSize controls the minimum tap target (hit-test) size that certain Material widgets reserve, which affects both layout (extra empty space) and the interactive area (where a tap is accepted) for those widgets. [1]
MaterialTapTargetSize.padded(default): expands the minimum tap target to 48×48 logical pixels, so taps in the “invisible” padded area around the visual can still trigger the widget. This is intended to meet common accessibility guidance. [1]MaterialTapTargetSize.shrinkWrap: removes that 48×48 expansion and shrinks the tap target down to the smaller minimum used by the Material spec / the widget’s own minimums, so the tappable area becomes much closer to the visual bounds (and can end up smaller than 48×48). [1][2]
Practical effect: if you set ThemeData.materialTapTargetSize (or a widget’s per-control tapTargetSize) to shrinkWrap, controls like IconButton, Checkbox, Radio, Switch, TextButton/ElevatedButton/OutlinedButton, Chip, etc. will often take less space and have a smaller hit region, which can hurt touch accessibility unless you add your own padding/constraints. [1][2]
Sources
[1] Flutter Dart API: MaterialTapTargetSize docs (padded = 48×48, shrinkWrap = smaller minimum) (api.flutter.dev)
[2] Example discussion/illustration of padded vs shrinkWrap changing the tappable empty space (stackoverflow.com)
Citations:
- 1: https://api.flutter.dev/flutter/material/MaterialTapTargetSize.html?utm_source=openai
- 2: https://stackoverflow.com/questions/68450047/using-materialtaptargetsize-in-materialbutton-makes-any-difference?utm_source=openai
🌐 Web query:
Material Design minimum touch target size guidelines 48dp accessibility
💡 Result:
- Minimum touch target (Material/Android accessibility): Make every interactive element’s touch target at least 48dp × 48dp (bigger is better). [1]
- How to achieve it: The visible control can be smaller, as long as padding/min size makes the focusable/touchable area reach 48dp in both dimensions (e.g., via
minWidth/minHeight+ padding). [1] - Buttons specifically (Material Design): A button’s visual height can be 36dp, but the touchable target should still be 48dp minimum. [2]
- Compose note: Android/Compose guidance also reiterates 48dp × 48dp as the minimum for clickable elements (many Material components apply this automatically). [3]
Sources: Android Developers accessibility guidance [1], Material Design buttons accessibility section [2], Android Developers accessibility testing codelab [3].
[1] (developer.android.com)
[2] (m1.material.io)
[3] (developer.android.com)
Citations:
- 1: https://developer.android.com/training/accessibility/accessible-app.html?utm_source=openai
- 2: https://m1.material.io/components/buttons.html?utm_source=openai
- 3: https://developer.android.com/codelabs/basic-android-kotlin-compose-test-accessibility?utm_source=openai
🏁 Script executed:
# Find the StreamEmojiButtonSize enum or size definition
rg -t dart "enum StreamEmojiButtonSize|class StreamEmojiButtonSize" packages/stream_core_flutter/lib/src/components/buttons/Repository: GetStream/stream-core-flutter
Length of output: 55
🏁 Script executed:
# Look for the defaults class and size definitions
sed -n '1,150p' packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dart | tail -80Repository: GetStream/stream-core-flutter
Length of output: 2471
🏁 Script executed:
# Search for StreamEmojiSize to understand emoji size mapping
rg -t dart "class StreamEmojiSize|enum StreamEmojiSize" packages/stream_core_flutter/lib/src/Repository: GetStream/stream-core-flutter
Length of output: 173
🏁 Script executed:
# Get the _StreamEmojiButtonThemeDefaults to see default sizes
rg -A 20 "_StreamEmojiButtonThemeDefaults" packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dartRepository: GetStream/stream-core-flutter
Length of output: 1944
🏁 Script executed:
# Find StreamEmojiButtonSize enum definition
rg -t dart "enum StreamEmojiButtonSize|class StreamEmojiButtonSize" -A 15 packages/stream_core_flutter/Repository: GetStream/stream-core-flutter
Length of output: 1798
🏁 Script executed:
# Search for StreamEmojiButtonSize definition in size-related files
fd -t f "size.dart|button.dart" packages/stream_core_flutter/lib/src | grep -i emojiRepository: GetStream/stream-core-flutter
Length of output: 153
🏁 Script executed:
# Broader search for StreamEmojiButtonSize
rg -t dart "StreamEmojiButtonSize" packages/stream_core_flutter/lib/src/theme/ -A 2 -B 2Repository: GetStream/stream-core-flutter
Length of output: 3930
Emoji button sizes below xl violate the 48×48dp touch target accessibility minimum when using shrinkWrap.
The ButtonStyle at lines 169–170 uses tapTargetSize: .shrinkWrap with padding: EdgeInsets.zero, which sets the touch target size equal to the visual button diameter. This causes:
xl(48px): meets minimum exactly, but with no safety marginlg(40px): 40×40dp — below the 48×48dp minimummd(32px): 32×32dp — below the 48×48dp minimum
Per Material Design and accessibility guidelines (48×48dp minimum), either restore tapTargetSize: .padded or add 8–12dp of padding for smaller sizes to meet touch target requirements.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/stream_core_flutter/lib/src/components/buttons/stream_emoji_button.dart`
around lines 169 - 170, The ButtonStyle in StreamEmojiButton currently uses
tapTargetSize: .shrinkWrap causing sizes below xl to violate the 48×48dp
touch-target minimum; update the style in the StreamEmojiButton widget (where
ButtonStyle is built) to ensure accessibility by either switching tapTargetSize
to MaterialTapTargetSize.padded for all buttons or, if you must keep shrinkWrap
visually, add conditional padding for smaller sizes (e.g., add
EdgeInsets.symmetric(horizontal/vertical ~8–12dp) when size != xl or when
diameter < 48) so that the effective touch target is at least 48×48dp; modify
the ButtonStyle construction (tapTargetSize and/or padding) accordingly in the
build/constructor that creates the emoji button.
| WidgetStateProperty<BorderSide?>? side, | ||
| }) | ||
| // TODO: Fix this or try to find something better | ||
| : side = side as WidgetStateBorderSide?; |
There was a problem hiding this comment.
Unsafe downcast of side will throw at runtime for non-WidgetStateBorderSide inputs.
The constructor accepts WidgetStateProperty<BorderSide?>? but force-casts to WidgetStateBorderSide? on line 131. Any caller passing a plain WidgetStatePropertyAll<BorderSide?> or a custom WidgetStateProperty<BorderSide?> will hit a TypeError.
Consider either:
- Narrowing the parameter type to
WidgetStateBorderSide?directly (breaking the broader acceptance but making the contract explicit), or - Adding a defensive check that wraps non-
WidgetStateBorderSidevalues.
Option 1: Narrow the parameter type
const StreamEmojiChipThemeStyle({
this.backgroundColor,
this.foregroundColor,
this.overlayColor,
this.textStyle,
this.minimumSize,
this.maximumSize,
this.padding,
this.shape,
- WidgetStateProperty<BorderSide?>? side,
- })
- // TODO: Fix this or try to find something better
- : side = side as WidgetStateBorderSide?;
+ this.side,
+ });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| WidgetStateProperty<BorderSide?>? side, | |
| }) | |
| // TODO: Fix this or try to find something better | |
| : side = side as WidgetStateBorderSide?; | |
| WidgetStateProperty<BorderSide?>? side, | |
| }) | |
| // TODO: Fix this or try to find something better | |
| : side = side as WidgetStateBorderSide?; |
| WidgetStateProperty<BorderSide?>? side, | |
| }) | |
| // TODO: Fix this or try to find something better | |
| : side = side as WidgetStateBorderSide?; | |
| const StreamEmojiChipThemeStyle({ | |
| this.backgroundColor, | |
| this.foregroundColor, | |
| this.overlayColor, | |
| this.textStyle, | |
| this.minimumSize, | |
| this.maximumSize, | |
| this.padding, | |
| this.shape, | |
| this.side, | |
| }); |
| WidgetStateProperty<BorderSide?>? side, | |
| }) | |
| // TODO: Fix this or try to find something better | |
| : side = side as WidgetStateBorderSide?; | |
| this.side, | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/stream_core_flutter/lib/src/theme/components/stream_emoji_chip_theme.dart`
around lines 128 - 131, The constructor currently force-casts the parameter side
(type WidgetStateProperty<BorderSide?>?) to WidgetStateBorderSide? which will
throw for non-WidgetStateBorderSide inputs; update the constructor for safety by
either 1) changing the parameter type from WidgetStateProperty<BorderSide?>? to
WidgetStateBorderSide? (making the contract explicit and removing the cast), or
2) keeping the broader type and replacing the forced cast with a defensive
conversion: if side is WidgetStateBorderSide use it directly, else wrap
non-WidgetStateBorderSide instances (e.g. WidgetStatePropertyAll<BorderSide?>)
into a new WidgetStateBorderSide wrapper instance or convert them into an
equivalent WidgetStateBorderSide implementation before assigning the field;
update the constructor assignment for the field side accordingly and ensure any
downstream code expecting WidgetStateBorderSide handles the wrapped type.
xsahil03x
changed the title
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (1.54%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main-design-system #58 +/- ##
=====================================================
Coverage ? 30.47%
=====================================================
Files ? 106
Lines ? 3308
Branches ? 0
=====================================================
Hits ? 1008
Misses ? 2300
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/design_system_gallery/lib/components/controls/stream_emoji_chip_bar.dart (1)
68-74: State mutation inbuildmethod is unconventional.Modifying
_itemsand_selecteddirectly inbuildworks with Widgetbook's knob-triggered rebuilds but is generally not recommended Flutter practice. State changes should ideally occur insetStateor lifecycle methods.For demo code this is acceptable, but consider using
didChangeDependenciesor a knob listener callback for cleaner separation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/design_system_gallery/lib/components/controls/stream_emoji_chip_bar.dart` around lines 68 - 74, The build method mutates state (_items and _selected) when itemCount changes; move that logic out of build and into a lifecycle or handler: detect itemCount changes in didChangeDependencies (or a knob listener callback) and call setState to update _items = _buildItems(itemCount) and clear _selected when its value is no longer present; ensure the build method only reads _items/_selected while state updates happen via setState in didChangeDependencies or the knob change handler.packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip_bar.dart (1)
199-208: Consider using list equality instead of reference equality for items comparison.Using
identical(line 202) only detects when the list instance changes, not when items are modified in place. If a caller mutates the same list instance, GlobalKeys won't be recreated. This is documented behavior (callers should provide new lists), but using deep equality (listEqualsfromfoundation.dart) would be more robust.🔧 Optional: Use list equality for more robust comparison
+import 'package:flutter/foundation.dart'; `@override` void didUpdateWidget(DefaultStreamEmojiChipBar<T> oldWidget) { super.didUpdateWidget(oldWidget); - if (!identical(oldWidget.props.items, props.items)) { + if (!listEquals(oldWidget.props.items, props.items)) { _valueKeys = {for (final item in props.items) item.value: GlobalKey()}; }Note: This would require
StreamEmojiChipItemto implement proper equality.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip_bar.dart` around lines 199 - 208, The current didUpdateWidget in DefaultStreamEmojiChipBar<T> uses identical(oldWidget.props.items, props.items) which only checks reference equality so in-place mutations of props.items won't recreate _valueKeys; replace this with a deep list comparison using listEquals from package:flutter/foundation.dart (ensure StreamEmojiChipItem implements ==/hashCode if needed), i.e., import foundation.dart and use listEquals(oldWidget.props.items, props.items) to decide when to rebuild _valueKeys (which maps item.value to GlobalKey()), while keeping the existing widget.props.selected vs oldWidget.props.selected check that triggers _scrollToSelected().
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@apps/design_system_gallery/lib/components/controls/stream_emoji_chip_bar.dart`:
- Around line 68-74: The build method mutates state (_items and _selected) when
itemCount changes; move that logic out of build and into a lifecycle or handler:
detect itemCount changes in didChangeDependencies (or a knob listener callback)
and call setState to update _items = _buildItems(itemCount) and clear _selected
when its value is no longer present; ensure the build method only reads
_items/_selected while state updates happen via setState in
didChangeDependencies or the knob change handler.
In
`@packages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip_bar.dart`:
- Around line 199-208: The current didUpdateWidget in
DefaultStreamEmojiChipBar<T> uses identical(oldWidget.props.items, props.items)
which only checks reference equality so in-place mutations of props.items won't
recreate _valueKeys; replace this with a deep list comparison using listEquals
from package:flutter/foundation.dart (ensure StreamEmojiChipItem implements
==/hashCode if needed), i.e., import foundation.dart and use
listEquals(oldWidget.props.items, props.items) to decide when to rebuild
_valueKeys (which maps item.value to GlobalKey()), while keeping the existing
widget.props.selected vs oldWidget.props.selected check that triggers
_scrollToSelected().
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/design_system_gallery/lib/app/gallery_app.directories.g.dartapps/design_system_gallery/lib/components/controls/stream_emoji_chip_bar.dartpackages/stream_core_flutter/lib/src/components.dartpackages/stream_core_flutter/lib/src/components/controls/stream_emoji_chip_bar.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.dartpackages/stream_core_flutter/lib/src/factory/stream_component_factory.g.theme.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_core_flutter/lib/src/components.dart
1 participant
Summary by CodeRabbit
New Features
Improvements
Documentation