feat(ui): factories for composer attachments#2566
Merged
renefloor merged 8 commits intofeat/design-refreshfrom Mar 26, 2026
Merged
feat(ui): factories for composer attachments#2566renefloor merged 8 commits intofeat/design-refreshfrom
renefloor merged 8 commits intofeat/design-refreshfrom
Conversation
renefloor
changed the title
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)
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 |
renefloor
commented
Mar 24, 2026
packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## feat/design-refresh #2566 +/- ##
=======================================================
+ Coverage 64.23% 65.12% +0.88%
=======================================================
Files 433 432 -1
Lines 26396 26056 -340
=======================================================
+ Hits 16956 16968 +12
+ Misses 9440 9088 -352 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/stream_chat_flutter/lib/src/components/message_composer/message_composer_input_header.dart (1)
208-216:⚠️ Potential issue | 🟠 MajorBug: Container result is discarded —
trailingis never assigned.The
Containerwidget is created but not assigned totrailing. The image case will always result intrailingbeingnull.🐛 Proposed fix
if (image != null) { - Container( + trailing = Container( width: 40, height: 40, decoration: BoxDecoration( borderRadius: BorderRadius.all(context.streamRadius.md), image: DecorationImage(image: image, fit: BoxFit.cover), ), ); } else if (mimeType != 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/components/message_composer/message_composer_input_header.dart` around lines 208 - 216, The Container built for the image is created but never assigned to the variable trailing, so trailing remains null; update the image branch in the component (where the Container with width:40, height:40, BoxDecoration and DecorationImage is constructed) to assign that Container to trailing (e.g., trailing = Container(...)) so the image is actually used, preserving existing properties like borderRadius: BorderRadius.all(context.streamRadius.md) and fit: BoxFit.cover.packages/stream_chat_flutter/lib/src/components/stream_chat_component_builders.dart (1)
16-32:⚠️ Potential issue | 🔴 CriticalBug: New builder parameters are declared but never added to the builders list.
The
messageComposerAttachmentListandmessageComposerAttachmentparameters are declared (lines 16-17) but not included in thebuilderslist. This means consumers cannot customize these components viastreamChatComponentBuilders().🐛 Proposed fix to include the new builders
if (messageWidget != null) StreamComponentBuilderExtension(builder: messageWidget), if (unreadIndicator != null) StreamComponentBuilderExtension(builder: unreadIndicator), + if (messageComposerAttachmentList != null) StreamComponentBuilderExtension(builder: messageComposerAttachmentList), + if (messageComposerAttachment != null) StreamComponentBuilderExtension(builder: messageComposerAttachment), ]; return builders;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_chat_flutter/lib/src/components/stream_chat_component_builders.dart` around lines 16 - 32, The new parameters messageComposerAttachmentList and messageComposerAttachment are declared but never appended to the builders list; update the builders list in the streamChatComponentBuilders function to include conditionally-wrapped StreamComponentBuilderExtension entries for messageComposerAttachmentList and messageComposerAttachment (e.g., if (messageComposerAttachmentList != null) StreamComponentBuilderExtension(builder: messageComposerAttachmentList), and similarly for messageComposerAttachment) so consumers can override those components.
🧹 Nitpick comments (4)
packages/stream_chat_flutter/test/src/message_input/message_input_attachment_list_test.dart (1)
21-23: Consider updating test group and description names to match the widget under test.The test group is named
'StreamMessageInputAttachmentList tests'but the widget being tested is nowStreamMessageComposerAttachmentList. The same applies to test descriptions at lines 23, 47, and 80.♻️ Proposed fix for consistency
- group('StreamMessageInputAttachmentList tests', () { + group('StreamMessageComposerAttachmentList tests', () { testWidgets( - 'StreamMessageInputAttachmentList should render attachments', + 'StreamMessageComposerAttachmentList should render attachments', (WidgetTester tester) async {Similar updates needed for:
- Line 47:
'StreamMessageInputAttachmentList should call onRemovePressed callback'- Line 80:
'StreamMessageInputAttachmentList should display empty box if no attachments'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stream_chat_flutter/test/src/message_input/message_input_attachment_list_test.dart` around lines 21 - 23, Update the test group and its test descriptions to reference the current widget name StreamMessageComposerAttachmentList instead of StreamMessageInputAttachmentList: change the group label string (currently 'StreamMessageInputAttachmentList tests') and the test titles ('StreamMessageInputAttachmentList should render attachments', 'StreamMessageInputAttachmentList should call onRemovePressed callback', 'StreamMessageInputAttachmentList should display empty box if no attachments') to use 'StreamMessageComposerAttachmentList' so the group and all test descriptions match the widget under test.packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment.dart (1)
85-107: Minor optimization: consolidate track lookups.The code performs two passes over the tracks list:
.any()followed by.indexWhere(). These can be combined into a single lookup.♻️ Suggested refactor
if (attachment.type == AttachmentType.audio || attachment.type == AttachmentType.voiceRecording) { if (audioPlaylistController == null) { return const SizedBox.shrink(); } - final hasTrack = audioPlaylistController!.value.tracks.any((it) => it.key == attachment); - - if (!hasTrack) { - return const SizedBox.shrink(); - } - final trackIndex = audioPlaylistController!.value.tracks.indexWhere((it) => it.key == attachment); + if (trackIndex == -1) { + return const SizedBox.shrink(); + } return SizedBox(🤖 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/stream_message_composer_attachment.dart` around lines 85 - 107, The code currently checks audioPlaylistController!.value.tracks twice using .any(...) then .indexWhere(...); replace this with a single lookup by using indexWhere once (e.g., compute trackIndex = audioPlaylistController!.value.tracks.indexWhere((it) => it.key == attachment)) and then check if trackIndex is -1 to decide to return SizedBox.shrink() or proceed to build MessageInputVoiceRecordingAttachment with controller: audioPlaylistController!, index: trackIndex, and onRemovePressed; keep the existing null check for audioPlaylistController before the lookup.packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart (1)
761-773: Consider simplifying conditional controller instantiation.The two branches duplicate most parameters. You could use a single constructor call with a spread or named parameter approach if the API supports it, or extract common params.
♻️ Suggested refactor
setState(() { final attachmentLimit = widget.attachmentLimit; - _pickerController = attachmentLimit != null - ? StreamAttachmentPickerController( - initialAttachments: _effectiveController.attachments, - initialPoll: _effectiveController.poll, - maxAttachmentCount: attachmentLimit, - maxAttachmentSize: widget.maxAttachmentSize, - ) - : StreamAttachmentPickerController( - initialAttachments: _effectiveController.attachments, - initialPoll: _effectiveController.poll, - maxAttachmentSize: widget.maxAttachmentSize, - ); + _pickerController = StreamAttachmentPickerController( + initialAttachments: _effectiveController.attachments, + initialPoll: _effectiveController.poll, + maxAttachmentSize: widget.maxAttachmentSize, + // Only pass maxAttachmentCount when limit is set + ...(attachmentLimit != null ? { maxAttachmentCount: attachmentLimit } : {}), + );Note: This depends on whether
StreamAttachmentPickerControlleraccepts null formaxAttachmentCount. If not, the current approach is fine.🤖 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/stream_message_input.dart` around lines 761 - 773, The controller instantiation for _pickerController duplicates parameters across the conditional; refactor by building a single StreamAttachmentPickerController call using common arguments (initialAttachments: _effectiveController.attachments, initialPoll: _effectiveController.poll, maxAttachmentSize: widget.maxAttachmentSize) and supply maxAttachmentCount only when widget.attachmentLimit is non-null (e.g., pass widget.attachmentLimit directly if the constructor accepts null, or construct the args map/variables and include maxAttachmentCount conditionally before calling StreamAttachmentPickerController) so you eliminate the duplicated branches while keeping the same behavior.packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment_list.dart (1)
80-81: Consider moving computed getter to State class.The
_audioAttachmentsgetter performs filtering computation on the widget's data. While functional, placing computed logic directly on aStatefulWidgetis unconventional—widgets are typically just data holders. Consider moving this to the State class or computing inline where needed for clarity.♻️ Suggested refactor
class DefaultMessageComposerAttachmentList extends StatefulWidget { // ... /// Callback called when the remove button is pressed. ValueSetter<Attachment>? get onRemovePressed => props.onRemovePressed; - List<Attachment> get _audioAttachments => - attachments.where((it) => it.type == AttachmentType.audio || it.type == AttachmentType.voiceRecording).toList(); - `@override` State<DefaultMessageComposerAttachmentList> createState() => _DefaultMessageComposerAttachmentListState(); } class _DefaultMessageComposerAttachmentListState extends State<DefaultMessageComposerAttachmentList> { - late List<Attachment> _audioAttachments = widget._audioAttachments; + late List<Attachment> _audioAttachments = _computeAudioAttachments(); + + List<Attachment> _computeAudioAttachments() => + widget.attachments.where((it) => it.type == AttachmentType.audio || it.type == AttachmentType.voiceRecording).toList(); // In didUpdateWidget: - final newAudioAttachments = widget._audioAttachments; + final newAudioAttachments = _computeAudioAttachments();🤖 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/stream_message_composer_attachment_list.dart` around lines 80 - 81, The _audioAttachments getter on the StreamMessageComposerAttachmentList widget performs filtering work on the widget class; move this computed logic into the corresponding State class (e.g., the State for StreamMessageComposerAttachmentList) or compute the filtered list inline where it's used (e.g., inside build or event handlers) so the widget remains a pure data holder; relocate the implementation of _audioAttachments (the where(... type == AttachmentType.audio || type == AttachmentType.voiceRecording) logic) into State and update all references to use the State-level member or local variable.
🤖 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_chat_flutter/lib/src/message_input/stream_message_composer_attachment_list.dart`:
- Around line 111-122: The post-frame callback that calls
_scrollController.animateTo can throw if the controller is not attached; wrap
the animateTo call in a safety guard (e.g., check _scrollController.hasClients
and mounted) before accessing _scrollController.position.maxScrollExtent or
calling animateTo in the callback for StreamMessageComposerAttachmentList; if
the guard fails, skip the animation. Ensure you keep the existing
addPostFrameCallback and only perform the animateTo when the controller is
valid.
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment.dart`:
- Around line 174-178: The current builder in ValueListenableBuilder uses
state.tracks.where((it) => it.key == attachment).first which can throw
StateError if the track was removed between updates; update the lookup in the
builder (ValueListenableBuilder, controller, state, tracks, attachment, track)
to use a safe lookup such as state.tracks.firstWhereOrNull((it) => it.key ==
attachment) (collection package is available) or otherwise check for empty
result and handle the null/missing case before accessing track to avoid runtime
exceptions during rebuilds.
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart`:
- Around line 493-500: The handler currently returns KeyEventResult.handled
whenever clearQuotedMessageKeyPredicate(node, event) is true even if no quote
was cleared; update the logic in the block inside stream_message_input.dart so
that after calling clearQuotedMessageKeyPredicate(node, event) you only return
KeyEventResult.handled when an actual action occurs (i.e., when
_effectiveController.message.quotedMessage != null &&
_effectiveController.text.isEmpty, you call
_effectiveController.clearQuotedMessage() and
widget.onQuotedMessageCleared?.call() and return handled); otherwise return
KeyEventResult.ignored so other widgets can process the key.
In `@packages/stream_chat_flutter/lib/stream_chat_flutter.dart`:
- Line 105: Update the test group and test descriptions to reference the new
class name StreamMessageComposerAttachmentList instead of the old
StreamMessageInputAttachmentList; locate the test file(s) that still mention
StreamMessageInputAttachmentList in group/test descriptions and replace those
strings (e.g., test group titles and test case descriptions) so they match the
actual widget under test (StreamMessageComposerAttachmentList) and avoid
breaking public API naming consistency.
---
Outside diff comments:
In
`@packages/stream_chat_flutter/lib/src/components/message_composer/message_composer_input_header.dart`:
- Around line 208-216: The Container built for the image is created but never
assigned to the variable trailing, so trailing remains null; update the image
branch in the component (where the Container with width:40, height:40,
BoxDecoration and DecorationImage is constructed) to assign that Container to
trailing (e.g., trailing = Container(...)) so the image is actually used,
preserving existing properties like borderRadius:
BorderRadius.all(context.streamRadius.md) and fit: BoxFit.cover.
In
`@packages/stream_chat_flutter/lib/src/components/stream_chat_component_builders.dart`:
- Around line 16-32: The new parameters messageComposerAttachmentList and
messageComposerAttachment are declared but never appended to the builders list;
update the builders list in the streamChatComponentBuilders function to include
conditionally-wrapped StreamComponentBuilderExtension entries for
messageComposerAttachmentList and messageComposerAttachment (e.g., if
(messageComposerAttachmentList != null) StreamComponentBuilderExtension(builder:
messageComposerAttachmentList), and similarly for messageComposerAttachment) so
consumers can override those components.
---
Nitpick comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment_list.dart`:
- Around line 80-81: The _audioAttachments getter on the
StreamMessageComposerAttachmentList widget performs filtering work on the widget
class; move this computed logic into the corresponding State class (e.g., the
State for StreamMessageComposerAttachmentList) or compute the filtered list
inline where it's used (e.g., inside build or event handlers) so the widget
remains a pure data holder; relocate the implementation of _audioAttachments
(the where(... type == AttachmentType.audio || type ==
AttachmentType.voiceRecording) logic) into State and update all references to
use the State-level member or local variable.
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment.dart`:
- Around line 85-107: The code currently checks
audioPlaylistController!.value.tracks twice using .any(...) then
.indexWhere(...); replace this with a single lookup by using indexWhere once
(e.g., compute trackIndex =
audioPlaylistController!.value.tracks.indexWhere((it) => it.key == attachment))
and then check if trackIndex is -1 to decide to return SizedBox.shrink() or
proceed to build MessageInputVoiceRecordingAttachment with controller:
audioPlaylistController!, index: trackIndex, and onRemovePressed; keep the
existing null check for audioPlaylistController before the lookup.
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart`:
- Around line 761-773: The controller instantiation for _pickerController
duplicates parameters across the conditional; refactor by building a single
StreamAttachmentPickerController call using common arguments
(initialAttachments: _effectiveController.attachments, initialPoll:
_effectiveController.poll, maxAttachmentSize: widget.maxAttachmentSize) and
supply maxAttachmentCount only when widget.attachmentLimit is non-null (e.g.,
pass widget.attachmentLimit directly if the constructor accepts null, or
construct the args map/variables and include maxAttachmentCount conditionally
before calling StreamAttachmentPickerController) so you eliminate the duplicated
branches while keeping the same behavior.
In
`@packages/stream_chat_flutter/test/src/message_input/message_input_attachment_list_test.dart`:
- Around line 21-23: Update the test group and its test descriptions to
reference the current widget name StreamMessageComposerAttachmentList instead of
StreamMessageInputAttachmentList: change the group label string (currently
'StreamMessageInputAttachmentList tests') and the test titles
('StreamMessageInputAttachmentList should render attachments',
'StreamMessageInputAttachmentList should call onRemovePressed callback',
'StreamMessageInputAttachmentList should display empty box if no attachments')
to use 'StreamMessageComposerAttachmentList' so the group and all test
descriptions match the widget under test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: da79dced-58ea-40b9-ab1a-bb13e2eeb3f1
📒 Files selected for processing (15)
melos.yamlpackages/stream_chat_flutter/lib/src/bottom_sheets/edit_message_sheet.dartpackages/stream_chat_flutter/lib/src/components/message_composer/message_composer_component_props.dartpackages/stream_chat_flutter/lib/src/components/message_composer/message_composer_input_header.dartpackages/stream_chat_flutter/lib/src/components/message_composer/message_composer_leading.dartpackages/stream_chat_flutter/lib/src/components/message_composer/stream_chat_message_composer.dartpackages/stream_chat_flutter/lib/src/components/stream_chat_component_builders.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment_list.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_input.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_input_attachment_list.dartpackages/stream_chat_flutter/lib/stream_chat_flutter.dartpackages/stream_chat_flutter/pubspec.yamlpackages/stream_chat_flutter/test/src/message_input/message_input_attachment_list_test.dartpackages/stream_chat_flutter/test/src/message_input/message_input_test.dart
💤 Files with no reviewable changes (3)
- packages/stream_chat_flutter/lib/src/bottom_sheets/edit_message_sheet.dart
- packages/stream_chat_flutter/test/src/message_input/message_input_test.dart
- packages/stream_chat_flutter/lib/src/message_input/stream_message_input_attachment_list.dart
packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment_list.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/message_input/stream_message_composer_attachment.dart
Show resolved
Hide resolved
packages/stream_chat_flutter/lib/src/message_input/stream_message_input.dart
Show resolved
Hide resolved
Brazol
approved these changes
Mar 26, 2026
…r-attachments # Conflicts: # melos.yaml # packages/stream_chat_flutter/pubspec.yaml
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
CLA
Description of the pull request
Main change in this PR is that it removes all the builder methods and only creates a factories for the attachment list or individual attachments.
It also re-adds some missing features and removes unused properties.
needs: GetStream/stream-core-flutter#83
Summary by CodeRabbit
Release Notes
New Features
Improvements
Refactor