feat(ui): channel page and floating UI - #2748
Open
renefloor wants to merge 78 commits into
Open
Conversation
This reverts commit 8f3f336. # Conflicts: # packages/stream_chat_flutter/lib/src/channel/channel_page.dart
broken due to merge
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds reusable channel and thread pages, configurable message-list and composer layouts, floating app-bar avatar styling, and migrates the sample app to ChangesLibrary UI and public APIs
Sample app scaffold migration
Estimated code review effort: 4 (Complex) | ~75 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![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: 3
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/message_input/stream_message_composer.dart (1)
433-470:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing
composerLocationincopyWithmethod.The new
composerLocationfield is not included in thecopyWithmethod, breaking the expected API contract. Callers cannot override this field when copying props.🐛 Proposed fix to add missing parameter
MessageComposerProps copyWith({ void Function(Message)? onMessageSent, FutureOr<Message> Function(Message)? preMessageSending, StreamMessageComposerController? messageComposerController, FocusNode? focusNode, bool? disableAttachments, bool? canAlsoSendToChannelFromThread, bool? enableVoiceRecording, bool? sendVoiceRecordingAutomatically, AudioRecorderFeedback? voiceRecordingFeedback, UserMentionTileBuilder? userMentionsTileBuilder, ErrorListener? onError, int? attachmentLimit, List<AttachmentPickerType>? allowedAttachmentPickerTypes, Iterable<StreamAutocompleteTrigger>? customAutocompleteTriggers, bool? mentionAllAppUsers, bool? shouldKeepFocusAfterMessage, MessageValidator? validator, String? restorationId, bool? enableSafeArea, bool? enableMentionsOverlay, VoidCallback? onQuotedMessageCleared, OgPreviewFilter? ogPreviewFilter, MessageInputPlaceholderBuilder? placeholderBuilder, bool? useSystemAttachmentPicker, PollConfig? pollConfig, AttachmentPickerOptionsBuilder? attachmentPickerOptionsBuilder, OnAttachmentPickerResult? onAttachmentPickerResult, KeyEventPredicate? sendMessageKeyPredicate, KeyEventPredicate? clearQuotedMessageKeyPredicate, TextInputAction? textInputAction, TextInputType? keyboardType, TextCapitalization? textCapitalization, bool? autofocus, bool? autoCorrect, + ComposerLocation? composerLocation, }) { return MessageComposerProps( onMessageSent: onMessageSent ?? this.onMessageSent, preMessageSending: preMessageSending ?? this.preMessageSending, messageComposerController: messageComposerController ?? this.messageComposerController, focusNode: focusNode ?? this.focusNode, disableAttachments: disableAttachments ?? this.disableAttachments, canAlsoSendToChannelFromThread: canAlsoSendToChannelFromThread ?? this.canAlsoSendToChannelFromThread, enableVoiceRecording: enableVoiceRecording ?? this.enableVoiceRecording, sendVoiceRecordingAutomatically: sendVoiceRecordingAutomatically ?? this.sendVoiceRecordingAutomatically, voiceRecordingFeedback: voiceRecordingFeedback ?? this.voiceRecordingFeedback, userMentionsTileBuilder: userMentionsTileBuilder ?? this.userMentionsTileBuilder, onError: onError ?? this.onError, attachmentLimit: attachmentLimit ?? this.attachmentLimit, allowedAttachmentPickerTypes: allowedAttachmentPickerTypes ?? this.allowedAttachmentPickerTypes, customAutocompleteTriggers: customAutocompleteTriggers ?? this.customAutocompleteTriggers, mentionAllAppUsers: mentionAllAppUsers ?? this.mentionAllAppUsers, shouldKeepFocusAfterMessage: shouldKeepFocusAfterMessage ?? this.shouldKeepFocusAfterMessage, validator: validator ?? this.validator, restorationId: restorationId ?? this.restorationId, enableSafeArea: enableSafeArea ?? this.enableSafeArea, enableMentionsOverlay: enableMentionsOverlay ?? this.enableMentionsOverlay, onQuotedMessageCleared: onQuotedMessageCleared ?? this.onQuotedMessageCleared, ogPreviewFilter: ogPreviewFilter ?? this.ogPreviewFilter, placeholderBuilder: placeholderBuilder ?? this.placeholderBuilder, useSystemAttachmentPicker: useSystemAttachmentPicker ?? this.useSystemAttachmentPicker, pollConfig: pollConfig ?? this.pollConfig, attachmentPickerOptionsBuilder: attachmentPickerOptionsBuilder ?? this.attachmentPickerOptionsBuilder, onAttachmentPickerResult: onAttachmentPickerResult ?? this.onAttachmentPickerResult, sendMessageKeyPredicate: sendMessageKeyPredicate ?? this.sendMessageKeyPredicate, clearQuotedMessageKeyPredicate: clearQuotedMessageKeyPredicate ?? this.clearQuotedMessageKeyPredicate, textInputAction: textInputAction ?? this.textInputAction, keyboardType: keyboardType ?? this.keyboardType, textCapitalization: textCapitalization ?? this.textCapitalization, autofocus: autofocus ?? this.autofocus, autoCorrect: autoCorrect ?? this.autoCorrect, + composerLocation: composerLocation ?? this.composerLocation, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart` around lines 433 - 470, The copyWith method in MessageComposerProps is missing the new composerLocation field; update the copyWith signature to accept composerLocation and include it in the returned MessageComposerProps (e.g., composerLocation: composerLocation ?? this.composerLocation) alongside the other fields so callers can override composerLocation when copying props.sample_app/lib/pages/new_group_chat_screen.dart (1)
10-11:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd public API doc comment.
NewGroupChatScreenis a public widget but lacks documentation. As per coding guidelines, all public APIs must have doc comments.📝 Suggested doc comment
+/// A screen for selecting users to add to a new group chat. +/// +/// Users can search for platform members and select multiple participants +/// before proceeding to group details configuration. class NewGroupChatScreen extends StatefulWidget { const NewGroupChatScreen({super.key});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@sample_app/lib/pages/new_group_chat_screen.dart` around lines 10 - 11, Add a public Dart doc comment for the NewGroupChatScreen widget: above the class declaration for NewGroupChatScreen, insert a /// comment that briefly describes the widget's purpose (e.g., creates a UI for creating a new group chat), documents its constructor parameters (if any, like key), and any important behavior or usage notes so it complies with the public API documentation guideline.Source: Coding guidelines
🧹 Nitpick comments (3)
packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart (1)
979-984: 💤 Low valueDeprecated
axisAlignmentparameter usage.The
axisAlignmentparameter onSizeTransitionis deprecated. The comment acknowledges this, but consider migrating to the replacement API when available to avoid future deprecation warnings during builds.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart` around lines 979 - 984, Replace the deprecated axisAlignment usage in the SizeTransition inside StreamMessageComposer: remove the "axisAlignment: -1" argument and instead pass an alignment value that preserves the original behavior (axisAlignment -1 -> top alignment) using the new "alignment" parameter (e.g., Alignment.topCenter or AlignmentDirectional.topStart) on the SizeTransition that wraps _buildInlineAttachmentPicker; update the SizeTransition invocation accordingly so the widget aligns the collapsing/expanding animation to the top without using the deprecated axisAlignment.packages/stream_chat_flutter/lib/src/channel/channel_header.dart (1)
134-136: ⚡ Quick winInconsistent documentation pattern for
appBarBehaviorfields acrossStreamChannelHeader,StreamChannelListHeader, andStreamBackButton.All three widgets document their
appBarBehaviorfield with boolean phrasing ("Whether X is floating") when the field is typed asAppBarBehavior?, not a boolean. The documentation should describe the parameter's purpose (controlling visual behavior) and mention the fallback to theme defaults. Consider unifying the wording acrosschannel_header.dart,channel_list_header.dart, andback_button.dart.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/channel/channel_header.dart` around lines 134 - 136, Update the doc comments for the appBarBehavior field in StreamChannelHeader, StreamChannelListHeader, and StreamBackButton to stop using boolean phrasing and instead describe that appBarBehavior (type AppBarBehavior?) controls the header/back button visual/layout behavior (e.g., floating vs pinned) and that it falls back to the theme's default when null; make the wording consistent across the three files by using the same concise sentence describing purpose and null/theme fallback and reference the AppBarBehavior type in the comment.packages/stream_chat_flutter/lib/src/channel/channel_page.dart (1)
32-44: ⚡ Quick winPrefer
late finalnon-nullableFocusNodeto avoid force-unwraps.The
_focusNodefield is declared nullable but always initialized ininitStateand force-unwrapped at every usage site (lines 43, 50, 57). This pattern is fragile and could panic if lifecycle order changes. Declaring it aslate final FocusNodemakes the initialization contract explicit and eliminates the null-check overhead.♻️ Proposed refactor
- FocusNode? _focusNode; + late final FocusNode _focusNode; final _messageComposerController = StreamMessageComposerController(); `@override` void initState() { + super.initState(); _focusNode = FocusNode(); - super.initState(); } `@override` void dispose() { - _focusNode!.dispose(); + _focusNode.dispose(); super.dispose(); } void _reply(Message message) { _messageComposerController.quotedMessage = message; WidgetsBinding.instance.addPostFrameCallback((timeStamp) { - _focusNode!.requestFocus(); + _focusNode.requestFocus(); }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/channel/channel_page.dart` around lines 32 - 44, The _focusNode field is declared nullable but always initialized in initState and force-unwrapped elsewhere; change its declaration to use non-nullable late final (late final FocusNode _focusNode) so initialization is explicit, keep _focusNode = FocusNode() inside initState, call _focusNode.dispose() in dispose, and remove any force-unwraps (!) where _focusNode is accessed (e.g., usages in the widget build and handlers).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dart`:
- Line 78: The permission-loading branch in stream_gallery_picker.dart returns
const SizedBox.expand() when !snapshot.hasData, but the project's Empty widget
builds const SizedBox.shrink(), so replace the expand with the shrink (or return
Empty()) in the permission/loading branch to avoid introducing an unintended
expanded blank area and layout shift; locate the conditional using
snapshot.hasData in the StreamGalleryPicker build method and update the returned
widget accordingly.
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart`:
- Around line 14-16: The constant _kPickerBodyHeight is unused dead code; either
remove the declaration or apply it as the fixed height for the inline attachment
picker UI—locate the picker implementation in stream_message_composer.dart (look
for the InlineAttachmentPicker or picker body widget inside
StreamMessageComposer/MessageComposer) and replace any hard-coded height values
or SizedBox/Container height with _kPickerBodyHeight, or delete the
_kPickerBodyHeight declaration if you choose not to standardize the height.
In `@sample_app/lib/routes/app_routes.dart`:
- Around line 48-72: The onChannelAvatarPressed handler can crash because
currentUserId may be null and channelMembers.firstWhere(...) will throw if no
match; update the logic in onChannelAvatarPressed to null-guard currentUserId
and safely find the other member (use collection's firstWhereOrNull or
firstWhere with orElse returning null) when computing otherUser from
channel.state?.members, and only navigate to Routes.CHAT_INFO_SCREEN when
otherUser is non-null; fallback to the GROUP_INFO_SCREEN path otherwise. Ensure
you import the collection helper if you use firstWhereOrNull.
---
Outside diff comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart`:
- Around line 433-470: The copyWith method in MessageComposerProps is missing
the new composerLocation field; update the copyWith signature to accept
composerLocation and include it in the returned MessageComposerProps (e.g.,
composerLocation: composerLocation ?? this.composerLocation) alongside the other
fields so callers can override composerLocation when copying props.
In `@sample_app/lib/pages/new_group_chat_screen.dart`:
- Around line 10-11: Add a public Dart doc comment for the NewGroupChatScreen
widget: above the class declaration for NewGroupChatScreen, insert a /// comment
that briefly describes the widget's purpose (e.g., creates a UI for creating a
new group chat), documents its constructor parameters (if any, like key), and
any important behavior or usage notes so it complies with the public API
documentation guideline.
---
Nitpick comments:
In `@packages/stream_chat_flutter/lib/src/channel/channel_header.dart`:
- Around line 134-136: Update the doc comments for the appBarBehavior field in
StreamChannelHeader, StreamChannelListHeader, and StreamBackButton to stop using
boolean phrasing and instead describe that appBarBehavior (type AppBarBehavior?)
controls the header/back button visual/layout behavior (e.g., floating vs
pinned) and that it falls back to the theme's default when null; make the
wording consistent across the three files by using the same concise sentence
describing purpose and null/theme fallback and reference the AppBarBehavior type
in the comment.
In `@packages/stream_chat_flutter/lib/src/channel/channel_page.dart`:
- Around line 32-44: The _focusNode field is declared nullable but always
initialized in initState and force-unwrapped elsewhere; change its declaration
to use non-nullable late final (late final FocusNode _focusNode) so
initialization is explicit, keep _focusNode = FocusNode() inside initState, call
_focusNode.dispose() in dispose, and remove any force-unwraps (!) where
_focusNode is accessed (e.g., usages in the widget build and handlers).
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart`:
- Around line 979-984: Replace the deprecated axisAlignment usage in the
SizeTransition inside StreamMessageComposer: remove the "axisAlignment: -1"
argument and instead pass an alignment value that preserves the original
behavior (axisAlignment -1 -> top alignment) using the new "alignment" parameter
(e.g., Alignment.topCenter or AlignmentDirectional.topStart) on the
SizeTransition that wraps _buildInlineAttachmentPicker; update the
SizeTransition invocation accordingly so the widget aligns the
collapsing/expanding animation to the top without using the deprecated
axisAlignment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e2a3f42-fca0-422a-96f1-7cd12e8d83ee
📒 Files selected for processing (40)
docs/docs_screenshots/pubspec.yamlmelos.yamlpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/channel/channel_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_list_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_page.dartpackages/stream_chat_flutter/lib/src/channel/thread_page.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_group.dartpackages/stream_chat_flutter/lib/src/message_action/message_actions_builder.dartpackages/stream_chat_flutter/lib/src/message_input/attachment_picker/options/stream_gallery_picker.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dartpackages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dartpackages/stream_chat_flutter/lib/src/misc/back_button.dartpackages/stream_chat_flutter/lib/src/scroll_view/channel_scroll_view/stream_channel_list_skeleton_loading.dartpackages/stream_chat_flutter/lib/src/scroll_view/photo_gallery/stream_photo_gallery.dartpackages/stream_chat_flutter/lib/src/scroll_view/thread_scroll_view/stream_thread_list_skeleton_loading.dartpackages/stream_chat_flutter/lib/src/stream_chat_configuration.dartpackages/stream_chat_flutter/lib/stream_chat_flutter.dartpackages/stream_chat_flutter/pubspec.yamlsample_app/lib/app.dartsample_app/lib/config/sample_app_config.dartsample_app/lib/config/sample_app_config_screen.dartsample_app/lib/pages/advanced_options_page.dartsample_app/lib/pages/channel_file_display_screen.dartsample_app/lib/pages/channel_list_page.dartsample_app/lib/pages/channel_media_display_screen.dartsample_app/lib/pages/channel_page.dartsample_app/lib/pages/chat_info_screen.dartsample_app/lib/pages/draft_list_page.dartsample_app/lib/pages/group_chat_details_screen.dartsample_app/lib/pages/group_info_screen.dartsample_app/lib/pages/new_chat_screen.dartsample_app/lib/pages/new_group_chat_screen.dartsample_app/lib/pages/pinned_messages_screen.dartsample_app/lib/pages/thread_list_page.dartsample_app/lib/pages/thread_page.dartsample_app/lib/routes/app_routes.dartsample_app/lib/widgets/channel_list.dart
💤 Files with no reviewable changes (2)
- sample_app/lib/pages/thread_page.dart
- sample_app/lib/pages/channel_page.dart
Threads previously gated the button on the parent channel's isUpToDate stream rather than the thread's own scroll position, so it could show while already at the newest reply. Behaviour change for callers who never set showScrollToBottom, so it belongs in the changelog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Making `config` nullable moved its fallback onto an inherited-widget lookup, and `_config` was a getter, so `StreamChatConfiguration.of(context)` now ran from wherever the getter was read -- including the `_messageNewListener` stream callback. dependOnInheritedWidgetOfExactType asserts the element is active, so a message arriving while the list is deactivated (GlobalKey reparent, or racing the cancel in dispose) throws in debug. Reading widget.config, as before, had no such constraint. It also re-ran the lookup ~20x per build. Resolve into a field at the top of didChangeDependencies, and add didUpdateWidget for the case where `config` changes without dependencies changing. The didUpdateWidget test swaps the config via setState below StreamChat rather than a second pumpWidget: StreamChatConfigurationData has no `==`, so rebuilding StreamChat hands StreamChatConfiguration a fresh instance and notifies every dependent, which would re-resolve the config for the wrong reason and hide a missing didUpdateWidget. Verified the test fails with the line removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `repliesEnabled` guard on ThreadReply landed with only a mock-helper change (adding `replies: true`) so existing tests kept passing -- the new branch itself was untested. Parameterise the helper with `enableReplies` and assert the action drops out while the send-reply capability is still granted, so the test pins the config check rather than the capability check. Also assert QuotedReply survives, since it does not depend on the replies config. Verified the test fails with the guard removed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The three-tier fallback (widget-local override, then StreamAppBarTheme, then the ambient StreamAppStyle) plus an identical two-arm switch was written out three times, and the derived local was named differently in each -- showAvatarShadow, hasAvatarShadow, isFloating. Extract resolveAppBarBehavior and isFloatingAppBar into an internal util, and name the local isFloating at all three sites. Behaviour-preserving; existing header and back-button tests pass unchanged. Kept in stream_chat_flutter rather than stream_core_flutter so this stays self-contained -- noted in the doc comment that next to StreamAppBarBehavior is the better long-term home. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three unrelated nits from the review: - Move `_pillHeight` up with the other state fields in the composer; it was declared between two methods, after both of its use sites. - Drop the ignore_for_file line in channel_list_page.dart entirely. The review flagged `avoid_redundant_argument_values` as too broad; checking it turned out to have no offenders at all, and `deprecated_member_use` went stale when BottomNavigationBar was replaced with StreamBottomNavBar. - Add a `streamTopInset` / `streamBottomInset` BuildContext extension in the sample app, replacing 12 copies of `StreamScaffoldInsets.maybeOf(context)?.topPadding ?? 0.0` across 11 files. Kept in the sample: StreamScaffoldInsets lives in stream_core_flutter. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md # sample_app/lib/pages/channel_page.dart
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart (1)
837-840: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the composer-location resolution.
The three-step fallback (
props.location→ theme → app style) is duplicated verbatim inbuildand_buildMessageInput; the two can silently diverge. A private getter keeps them in sync.♻️ Proposed refactor
ComposerLocation get _effectiveComposerLocation => widget.props.location ?? StreamMessageComposerTheme.of(context).location ?? (StreamTheme.of(context).appStyle.isFloating ? ComposerLocation.floating : ComposerLocation.docked);- final effectiveComposerLocation = - widget.props.location ?? - StreamMessageComposerTheme.of(context).location ?? - (StreamTheme.of(context).appStyle.isFloating ? ComposerLocation.floating : ComposerLocation.docked); + final effectiveComposerLocation = _effectiveComposerLocation;Also applies to: 1001-1005
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart` around lines 837 - 840, Extract the duplicated composer-location fallback into a private _effectiveComposerLocation getter on the composer, resolving widget.props.location, the theme location, and app-style default in that order. Replace the inline resolution in both build and _buildMessageInput with this getter so both paths share one implementation.packages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dart (1)
174-175: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer copying the ambient
MediaQueryDataover replacing it.
MediaQueryData(padding: ...)drops every other field (notablysize, which defaults toSize.zero). It happens to work because the composer only readspaddingOf, but any futuresizeOf/textScalerread below this subtree will see degenerate values. Wrapping withMediaQuery.of(context).copyWith(padding: ...)(via aBuilder) is more robust.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dart` around lines 174 - 175, Update the MediaQuery wrapper around the home widget to copy the ambient MediaQueryData and override only its padding, using a Builder if needed to obtain the correct context. Preserve all other inherited MediaQuery fields instead of constructing a new MediaQueryData with defaults.packages/stream_chat_flutter/test/src/channel/thread_page_test.dart (1)
147-226: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared mock harness.
_pumpThreadPageduplicates almost all of the client/channel/channelState stubbing (and the_messageListView/_composerController/_composerFocusNodehelpers) frompackages/stream_chat_flutter/test/src/channel/channel_page_test.dartandpackages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dart. A shared helper (e.g. amockChatSetup()in a test util) would keep these three suites from drifting apart as the mocked surface grows.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/test/src/channel/thread_page_test.dart` around lines 147 - 226, The mock setup in _pumpThreadPage duplicates the client, channel, channelState, and related helper stubbing used by _messageListView, _composerController, and _composerFocusNode in the other test suites. Extract the shared setup into a reusable test utility such as mockChatSetup(), update all three suites to use it, and preserve each test’s suite-specific overrides and behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/stream_chat_flutter/lib/src/channel/channel_header.dart`:
- Around line 152-159: Update isFloating resolution in channel_header.dart at
lines 152-159 and channel_list_header.dart at lines 129-131 to use the
per-instance style behavior first, falling back to headerTheme.style?.behavior.
Preserve the existing avatar and app-bar behavior while honoring the local
header theme when no override is provided.
In `@sample_app/lib/widgets/location/location_aware_message_composer.dart`:
- Around line 30-35: Update the props construction in
LocationAwareMessageComposer to preserve the incoming
allowedAttachmentPickerTypes by appending LocationPickerType to that existing
collection rather than replacing it with all enum values. Also chain
props.onAttachmentPickerResult from onAttachmentPickerResult so non-location
results reach the caller’s handler while location results continue through
_onCustomAttachmentPickerResult.
---
Nitpick comments:
In
`@packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart`:
- Around line 837-840: Extract the duplicated composer-location fallback into a
private _effectiveComposerLocation getter on the composer, resolving
widget.props.location, the theme location, and app-style default in that order.
Replace the inline resolution in both build and _buildMessageInput with this
getter so both paths share one implementation.
In `@packages/stream_chat_flutter/test/src/channel/thread_page_test.dart`:
- Around line 147-226: The mock setup in _pumpThreadPage duplicates the client,
channel, channelState, and related helper stubbing used by _messageListView,
_composerController, and _composerFocusNode in the other test suites. Extract
the shared setup into a reusable test utility such as mockChatSetup(), update
all three suites to use it, and preserve each test’s suite-specific overrides
and behavior.
In
`@packages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dart`:
- Around line 174-175: Update the MediaQuery wrapper around the home widget to
copy the ambient MediaQueryData and override only its padding, using a Builder
if needed to obtain the correct context. Preserve all other inherited MediaQuery
fields instead of constructing a new MediaQueryData with defaults.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ab55b9ff-06fb-4ffe-92dd-22038fbf2be6
⛔ Files ignored due to path filters (3)
docs/docs_screenshots/test/theming/goldens/macos/theming_red.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/message_widget/goldens/ci/stream_message_deleted_dark.pngis excluded by!**/*.pngpackages/stream_chat_flutter/test/src/message_widget/goldens/ci/stream_message_deleted_light.pngis excluded by!**/*.png
📒 Files selected for processing (38)
docs/docs_screenshots/test/src/mocks.dartpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/channel/channel_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_list_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_page.dartpackages/stream_chat_flutter/lib/src/channel/thread_page.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar.dartpackages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_group.dartpackages/stream_chat_flutter/lib/src/components/message_composer/message_composer_input.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dartpackages/stream_chat_flutter/lib/src/message_list_view/message_list_view.dartpackages/stream_chat_flutter/lib/src/message_widget/components/stream_message_deleted.dartpackages/stream_chat_flutter/lib/src/misc/back_button.dartpackages/stream_chat_flutter/lib/src/utils/app_bar_behavior.dartpackages/stream_chat_flutter/test/src/channel/channel_page_test.dartpackages/stream_chat_flutter/test/src/channel/thread_page_test.dartpackages/stream_chat_flutter/test/src/message_action/message_actions_builder_test.dartpackages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dartpackages/stream_chat_flutter/test/src/message_list_view/message_list_view_test.dartpackages/stream_chat_flutter/test/src/message_widget/stream_message_deleted_test.dartpackages/stream_chat_flutter/test/src/theme/message_composer_theme_test.dartsample_app/ios/Runner.xcodeproj/project.pbxprojsample_app/lib/app.dartsample_app/lib/config/sample_app_config_screen.dartsample_app/lib/pages/advanced_options_page.dartsample_app/lib/pages/channel_file_display_screen.dartsample_app/lib/pages/channel_list_page.dartsample_app/lib/pages/channel_media_display_screen.dartsample_app/lib/pages/chat_info_screen.dartsample_app/lib/pages/group_chat_details_screen.dartsample_app/lib/pages/group_info_screen.dartsample_app/lib/pages/new_chat_screen.dartsample_app/lib/pages/new_group_chat_screen.dartsample_app/lib/pages/pinned_messages_screen.dartsample_app/lib/utils/scaffold_insets.dartsample_app/lib/widgets/channel_list.dartsample_app/lib/widgets/location/location_aware_message_composer.dart
💤 Files with no reviewable changes (1)
- sample_app/lib/pages/channel_list_page.dart
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar_group.dart
- packages/stream_chat_flutter/lib/src/components/avatar/stream_user_avatar.dart
- packages/stream_chat_flutter/lib/src/components/avatar/stream_channel_avatar.dart
- packages/stream_chat_flutter/CHANGELOG.md
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md # packages/stream_chat_flutter/lib/src/misc/back_button.dart
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/stream_chat_flutter/lib/src/channel/thread_page.dart (1)
92-104: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace deprecated
channelId/showUnreadCountwithunreadIndicator.
StreamBackButton.channelIdandStreamBackButton.showUnreadCountare marked@Deprecatedin favor ofunreadIndicator, and the annotation states they will be removed in a future version.StreamThreadHeader's own default leading already uses the non-deprecatedunreadIndicatorparameter directly. Use the same pattern here for consistency and to avoid relying on an API scheduled for removal.♻️ Proposed fix to use `unreadIndicator` instead of deprecated params
leading: switch (widget.onBackPressed) { final onBackPressed? => StreamBackButton( onPressed: onBackPressed, - channelId: channel?.cid, - showUnreadCount: true, + unreadIndicator: switch (channel?.cid) { + final cid? => StreamUnreadIndicator.channels(cid: cid), + null => const StreamUnreadIndicator(), + }, ), _ => null, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/stream_chat_flutter/lib/src/channel/thread_page.dart` around lines 92 - 104, Update the StreamBackButton construction in StreamThreadHeader’s leading switch to replace the deprecated channelId and showUnreadCount arguments with the non-deprecated unreadIndicator parameter, matching the header’s default leading behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/stream_chat_flutter/lib/src/channel/thread_page.dart`:
- Around line 92-104: Update the StreamBackButton construction in
StreamThreadHeader’s leading switch to replace the deprecated channelId and
showUnreadCount arguments with the non-deprecated unreadIndicator parameter,
matching the header’s default leading behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f10bfec3-94ee-44ce-acd2-18c043bcff15
📒 Files selected for processing (16)
packages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/lib/src/channel/channel_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_list_header.dartpackages/stream_chat_flutter/lib/src/channel/channel_page.dartpackages/stream_chat_flutter/lib/src/channel/thread_page.dartpackages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dartpackages/stream_chat_flutter/lib/src/misc/back_button.dartpackages/stream_chat_flutter/lib/src/misc/thread_header.dartpackages/stream_chat_flutter/lib/src/theme/message_composer_theme.dartpackages/stream_chat_flutter/lib/src/theme/message_composer_theme.g.theme.dartpackages/stream_chat_flutter/test/src/channel/channel_header_test.dartpackages/stream_chat_flutter/test/src/channel/channel_list_header_test.dartpackages/stream_chat_flutter/test/src/channel/thread_page_test.dartpackages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dartpackages/stream_chat_flutter/test/src/theme/message_composer_theme_test.dartsample_app/lib/widgets/location/location_aware_message_composer.dart
🚧 Files skipped from review as they are similar to previous changes (9)
- packages/stream_chat_flutter/lib/src/channel/channel_page.dart
- packages/stream_chat_flutter/CHANGELOG.md
- packages/stream_chat_flutter/test/src/message_input/message_composer_location_test.dart
- packages/stream_chat_flutter/lib/src/misc/back_button.dart
- sample_app/lib/widgets/location/location_aware_message_composer.dart
- packages/stream_chat_flutter/lib/src/theme/message_composer_theme.dart
- packages/stream_chat_flutter/lib/src/theme/message_composer_theme.g.theme.dart
- packages/stream_chat_flutter/test/src/theme/message_composer_theme_test.dart
- packages/stream_chat_flutter/lib/src/message_input/stream_message_composer.dart
xsahil03x
reviewed
Aug 4, 2026
| /// When null, falls back to | ||
| /// [StreamChatConfigurationData.messageListViewConfiguration] from the | ||
| /// nearest [StreamChatConfiguration] ancestor. | ||
| final StreamMessageListViewConfiguration? config; |
Contributor
Author
There was a problem hiding this comment.
Why? If you supplied it previously it's still going to work with what you supply. If you didn't supply anything it's still falling back to the default. Or do you think anybody will use it as getter?
Member
There was a problem hiding this comment.
anybody will use it as getter
it was non-nullable before so yeah kind of breaking
xsahil03x
reviewed
Aug 4, 2026
xsahil03x
reviewed
Aug 4, 2026
| // then the ambient app bar theme, then the app style. | ||
| child: StreamAppBarTheme( | ||
| data: headerTheme, | ||
| data: headerTheme.merge(StreamAppBarThemeData(style: style)), |
Member
There was a problem hiding this comment.
do we need to pass the style in the data? we are already passing it in the StreamAppBar below
Contributor
Author
There was a problem hiding this comment.
StreamAppBar's own style param only styles the bar itself. The header's default slot children — _DefaultChannelAvatar and StreamBackButton — are built outside the bar and resolve floating-ness from context via isFloatingAppBar(context) → StreamAppBarTheme.of(context).style?.behavior. Without the merge, StreamChannelHeader(style: ...behavior: floating) would render a floating bar with a regular (shadowless) avatar and a ghost instead of outline back button. This was exactly CodeRabbit's earlier finding (comment 3683952384), fixed by merging into the theme instead of threading isFloating through every slot.
We can instead remove the style from the StreamAppBar below.
xsahil03x
reviewed
Aug 4, 2026
| // theme, then the app style. | ||
| child: StreamAppBarTheme( | ||
| data: headerTheme, | ||
| data: headerTheme.merge(StreamAppBarThemeData(style: style)), |
Member
There was a problem hiding this comment.
do we need to pass the style in the data? we are already passing it in the StreamAppBar below
xsahil03x
reviewed
Aug 4, 2026
| // thread view, as replying in a thread that is already being viewed doesn't make sense. | ||
| // Additionally, the channel needs to support sending replies. | ||
| if (canSendReply && !isThreadMessage && !isInThreadView) { | ||
| if (canSendReply && repliesEnabled && !isThreadMessage && !isInThreadView) { |
Member
There was a problem hiding this comment.
Is canSendReply was not enough in this case?
Contributor
Author
There was a problem hiding this comment.
They're two different gates. canSendReply is the per-user own-capability (ChannelCapability.sendReply); repliesEnabled is channel.config.replies, the channel-type feature flag. The two can disagree — a channel type with threads turned off can still hand out the send-reply capability, which is what left the "Thread reply" action visible. There's a test pinning this (33eeab5): capability granted, replies: false → action must not appear (it fails if the guard is removed).
Compared with other SDK's:
iOS — capability wins, config is only a fallback
Sources/StreamChatUI/MessageActionsPopup/ChatMessageActionsVC.swift:111:
// If a channel is not set, we fallback to using channelConfig only.
let canSendReply = channel?.canSendReply ?? channelConfig.repliesEnabled
...
if canSendReply && !message.isPartOfThread && !isInsideThread {
actions.append(threadReplyActionItem())
}
So when the channel is available (the normal case), config.replies is ignored entirely for this action — canSendReply is ownCapabilities.contains(.sendReply) (Channel.swift:698). iOS does use the config flag elsewhere though: ChatMessageLayoutOptionsResolver.swift:89 gates the thread-reply footer on channel.config.repliesEnabled.
Android UI-Components (XML) — ANDs both, exactly like this PR
MessageListView.kt:377:
val isThreadEnabled = style.threadsEnabled && channel.config.isThreadEnabled
fed into CapabilitiesHelper.kt:92:
threadsEnabled && !isInThread && !message.isThreadReply() && message.isSynced() &&
ownCapabilities.contains(ChannelCapabilities.SEND_REPLY)
And Config.isThreadEnabled is mapped straight from the API's replies field (DomainMapping.kt:672). This is the same gate as canSendReply && repliesEnabled, plus a UI-level style toggle.
Android Compose — capability + a UI visibility flag, no channel config
MessageActionsOptionsVisibility.kt:44 passes threadsEnabled = isThreadReplyVisible (a developer-facing visibility flag), so config.replies never enters the Compose path.
---
What this means for the review comment: Sahil's instinct matches iOS and Compose; the PR matches Android XML. None of the three treats the capability as strictly sufficient and uses the config flag nowhere — both platforms consult config.replies for thread UI somewhere (iOS's footer, Android's FootnoteDecorator), which suggests the flag is treated as meaningful client-side rather than fully folded into capabilities server-side.
One parity gap worth mentioning separately: our thread-reply indicator (stream_message_item.dart:503) is gated only on replyCount > 0, not on repliesEnabled — iOS and Android both gate their equivalent on the config flag. So if we keep the guard, it arguably belongs there too; if we drop it, we're consistent with iOS's action list but diverge from Android.
Member
There was a problem hiding this comment.
I checked with the backend source and it said,
⏺ Verified against the backend — and the source contradicts renefloor's central claim. The send-reply capability is itself gated on the replies config:
lib/combined/internal/permissionutil/channel_capabilities.go:73
func SendReply(session *ChannelSession) bool {
if session.Channel.Frozen && !session.Check(permissions.UseFrozenChannel).Allowed {
return false
}
return session.Channel.Config.Replies && (session.Check(permissions.CreateReply).Allowed
|| session.Check(permissions.CreateMessage).Allowed)
}
And Config.Replies is exactly the client's channel.config.replies:
lib/combined/types/channel_config.go:51 → Replies bool json:"replies".
So the server only ever puts send-reply in own_capabilities when config.replies == true. There is no path where "a channel type with threads turned off still hands out the send-reply capability" — the &&
Config.Replies prevents it (the only other exit is the frozen check, also false). Contrast UpdateThread at :172, which is pure permission with no config gate — so the backend does couple send-reply to the config
on purpose.
What this means:
- Your instinct is correct. canSendReply already implies repliesEnabled, so canSendReply && repliesEnabled is redundant — the second term is always true whenever the first is.
- No realistic disagreement exists. own_capabilities and config arrive in the same channel response, and the server computes the capability from the config — so they can't drift apart. renefloor's pinning test
(capability granted + replies: false) is a hand-built state the backend never emits.
- Cross-SDK: this matches iOS's primary gate (canSendReply); Android XML's extra AND is belt-and-suspenders against that same impossible state.
So dropping the repliesEnabled guard is safe and actually more faithful to the backend contract. renefloor's SDK-parity survey is useful, but its premise — that the capability and the config are independent
gates that "can disagree" — doesn't hold against the source; the backend already folded the config into the capability.
One fair caveat to acknowledge back to him: his separate point still stands — the thread-reply indicator (stream_message_item.dart:503, gated only on replyCount > 0) is a real parity gap independent of this, and
worth its own look.
xsahil03x
reviewed
Aug 4, 2026
xsahil03x
reviewed
Aug 4, 2026
xsahil03x
reviewed
Aug 4, 2026
Comment on lines
+849
to
+869
| if (_isThreadConversation) | ||
| ValueListenableBuilder<bool>( | ||
| valueListenable: _showScrollToBottom, | ||
| child: _buildScrollToBottom(), | ||
| builder: (context, value, child) { | ||
| if (!snapshot || value) return child!; | ||
| if (value) return child!; | ||
| return const Empty(); | ||
| }, | ||
| ) | ||
| else | ||
| BetterStreamBuilder<bool>( | ||
| stream: streamChannel!.channel.state!.isUpToDateStream, | ||
| initialData: streamChannel!.channel.state!.isUpToDate, | ||
| builder: (context, snapshot) => ValueListenableBuilder<bool>( | ||
| valueListenable: _showScrollToBottom, | ||
| child: _buildScrollToBottom(), | ||
| builder: (context, value, child) { | ||
| if (!snapshot || value) return child!; | ||
| return const Empty(); | ||
| }, | ||
| ), |
Member
There was a problem hiding this comment.
Is the value of isUpToDate not correct in case of thread conversations?
Contributor
Author
There was a problem hiding this comment.
streamChannel.channel.state.isUpToDate tracks the channel's message window (false when you've jumped to an older message and the latest page isn't loaded). A thread's reply list is a separate list with no equivalent flag. So opening a thread after jumping to an old channel message left isUpToDate == false, and the thread's scroll-to-bottom button never appeared even though the thread list was perfectly scrollable. In thread mode the button is gated on _showScrollToBottom alone.
The branch is arguably ugly — an alternative is a single BetterStreamBuilder whose gate is _isThreadConversation || isUpToDate, which collapses the if/else into one widget tree.
xsahil03x
reviewed
Aug 4, 2026
xsahil03x
reviewed
Aug 4, 2026
| // per-header theme, then the ambient app bar theme, then the app style. | ||
| return StreamAppBarTheme( | ||
| data: headerTheme, | ||
| data: headerTheme.merge(StreamAppBarThemeData(style: style)), |
Member
There was a problem hiding this comment.
do we need to pass the style in the data? we are already passing it in the StreamAppBar below
xsahil03x
reviewed
Aug 4, 2026
renefloor
force-pushed
the
feat/channel-page
branch
from
August 4, 2026 11:13
af6b9b7 to
d07bcf5
Compare
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
Linear: FLU-502
CLA
Description of the pull request
This introduces the floating style on all sample app pages and moves the channel page and thread page to the sdk.
The
StreamMessageListViewConfigurationis also added to the global chat config so it's easier to use in the channel and thread page.Core PR: GetStream/stream-core-flutter#115
Screenshots / Videos
Summary by CodeRabbit
New Features
Bug Fixes