Repository navigation
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Docstring Coverage |  |
No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0… |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
| Description Check | |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly summarizes the main change: updating UI and sample screens to respect asymmetric safe-area insets on iPhone Duo. It is concise and specific. |
✨ Finishing Touches
-
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
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 help to get the list of available commands.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3017 +/- ##
==========================================
+ Coverage 78.56% 79.13% +0.56%
==========================================
Files 452 452
Lines 29451 29459 +8
==========================================
+ Hits 23139 23312 +173
+ Misses 6312 6147 -165 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…opens Opening the inline attachment picker collapsed every safe-area edge toward zero, so on a device with a side inset (iPhone Duo's status rail, a notched phone in landscape) the composer and the picker ran under it, and the grid changed width on every animation frame. Only the bottom inset now gives way to the picker. The composer's top divider moves from the input row to the full-width background, so it spans the screen while the input stays inside the safe area. Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
With `enableSafeArea` on, the message list applies the insets itself, but the scroll-to-bottom button's end offset was a fixed margin, so on a device with a side inset it sat under the system area. The margin is now added on top of the end-side inset, resolved for the text direction, which lines the button up with the bubbles. Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…nsets The floating date divider and the unread indicator were centered on the list's full width, while the messages and the inline date dividers sit between the side safe-area insets. On a device with a side inset they were off-center, and the floating pill no longer lined up with the inline divider it fades into, so both showed side by side. Both pills are now padded by the side insets and centered over the list's content. Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…rea insets The Configuration, Advanced options, Chat info, Group info, Group details and New chat screens padded only the top inset and set a fixed side padding, so on a device with a side inset (iPhone Duo's status rail, a notched phone in landscape) their content ran under the system area. Each body is now wrapped in a side-only SafeArea; the scroll views keep handling the top and bottom. The language picker and the Message info sheet move from Material bottom sheets to showStreamSheet with StreamSheetHeader, so the sheets stay clear of the rail too. Message info's rows use StreamListTile and follow the other sheets' layout. Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…area insets With `enableSafeArea` on, `StreamMessageListView` now wraps its content in a `SafeArea` for the side insets and takes only the top and bottom insets as scroll padding. Messages, the floating date divider, the unread indicator and the scroll-to-bottom button stay clear of the side insets, while the background still spans the full width. The loading, empty and error states, including custom ones from `builders`, clear every inset. This replaces the per-widget padding from 706b6c6 and 81e49d7. Also renames the two composer safe-area tests to name the widget under test. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Without an explicit `padding`, `PagedValueListView` and `PagedValueGridView` inset their items by the safe area but drew their empty, loading and error states under it. Those states now clear every inset; passing a `padding` still leaves them alone. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The message list reads it since the unread banners landed, so every docs screenshot with a message list rendered an error screen. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…safe-area insets The Photos & Videos grid ran under a side inset (iPhone Duo's status rail, a notched phone in landscape), with the system indicators drawn over the thumbnails. The Files screen's month headers used a fixed side padding, so their labels could run under a left inset. The grid is now wrapped in a side-only SafeArea, and each header keeps its full-width background while its label takes the safe-area insets. Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Refs FLU-820 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
4e5e92e to
47e41ed
Compare
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md # packages/stream_chat_flutter_core/CHANGELOG.md
…-insets-on-iphone-duo-composer
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AP6bV4L7FQYjHoHZeomLxr
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AP6bV4L7FQYjHoHZeomLxr
renefloor
left a comment
renefloor
left a comment
There was a problem hiding this comment.
Reviewed this layer on its own (the rest of the stack gets separate reviews). The composer fix and the message-list safe-area tests look good — they assert real geometry and most would fail on revert. A few things inline, mainly the paged-list SafeArea, which I think is a behavioural change rather than a fix.
| // Without a padding, a non-scrolling state clears every safe-area inset. | ||
| Widget _insetState(Widget child) { | ||
| if (widget.padding != null) return child; | ||
| return SafeArea(child: child); |
There was a problem hiding this comment.
This wraps the customer's emptyBuilder / loadingBuilder / errorBuilder output in a SafeArea whenever padding is null, which is the default for StreamChannelListView, StreamThreadListView, the member/user/poll-vote lists, etc. (same in PagedValueGridView below).
A customer whose empty state is a full-bleed illustration or coloured background will see it inset after upgrading, with no compile warning, and the only opt-out (passing padding) also drops the items' automatic scroll inset. Per CLAUDE.md I'd treat this as a behavioural change: list it under 🔄 Changed (not 🐞 Fixed) with the opt-out spelled out, and mention it in the padding dartdoc.
It's also wider than what the items get: a vertical ListView with padding: null only takes the top/bottom insets, but this takes all four sides. E.g. a short horizontal strip at the top of a body without an AppBar now loses ~47pt at the top only in its empty/loading state. Consider only applying the main-axis insets, to match BoxScrollView.
There was a problem hiding this comment.
Agreed it shouldn't touch customer builders. In 954d4a0 I moved the inset out of PagedValueListView/PagedValueGridView and into our own default widgets: StreamScrollViewEmptyWidget, StreamScrollViewErrorWidget and StreamScrollViewLoadingWidget clear the safe area via StreamSafeArea(minimum: …), and the channel and thread skeleton rows clear the side insets the way StreamListTile does (top: false, bottom: false). Custom emptyBuilder/loadingBuilder/errorBuilder output is laid out exactly as before, with a test for that.
I didn't go main-axis only: the Duo's rail is a cross-axis inset, and BoxScrollView doesn't drop the cross-axis insets either. It passes them down through MediaQuery for each item to clear, which is what the rows now do.
| 🐞 Fixed | ||
|
|
||
| - Fixed `StreamDraftListController.deleteDraft` removing the wrong draft. | ||
| - Fixed `PagedValueListView` and `PagedValueGridView` drawing their empty, loading and error states under the safe area when no `padding` is given. |
There was a problem hiding this comment.
See the comment on _insetState: this changes how customer-supplied state builders are laid out, so I think it belongs under 🔄 Changed, with the opt-out (pass a padding).
There was a problem hiding this comment.
Core no longer changes in this PR (954d4a0), so the entry is gone. The UI changelog has a 🐞 Fixed line for the default states instead.
| expect(_backgroundFillFinder(tester), findsOneWidget); | ||
| }); | ||
|
|
||
| testWidgets('StreamMessageComposer runs the regular top divider under the side insets', (tester) async { |
There was a problem hiding this comment.
I think this passes for the wrong reason: the fill DecoratedBox always spanned the full width, so the width assertion was already true before this PR. It only fails on revert because _backgroundFillFinder now matches the exact BoxDecoration including the border.
It wouldn't catch the border being re-added inside StreamChatMessageInput (double border). Could it assert that no bordered box sits under StreamChatMessageInput, and that the bordered box starts at x=0?
There was a problem hiding this comment.
Good catch, confirmed: the old assertion passed with the change reverted, and a second border inside StreamChatMessageInput passed every composer test. In c5f42ff the test finds every top-only border inside the composer: one test checks it runs from x=0 to the full width under a left and right inset, and another checks there is exactly one. Both regressions now fail.
| 🔄 Changed | ||
|
|
||
| - `StreamMessageListView` now rebuilds only the message rows that changed; a custom row builder should read changing outside state through an inherited widget. | ||
| - `StreamMessageComposer`'s top border now spans the full width, including under the side safe-area insets, and also shows above the no-permission message. |
There was a problem hiding this comment.
The PR description says the composer is now 1px shorter because the border became an inside stroke. Worth adding to this line: integrators with golden tests will see diffs, as this PR's own goldens show.
| }); | ||
|
|
||
| testWidgets('StreamMessageListView centers the floating date divider over the content area', (tester) async { | ||
| const padding = EdgeInsets.only(right: 84); |
There was a problem hiding this comment.
All the safe-area tests in this PR use a right-only inset. A swapped left/right (e.g. in StreamSafeArea.driven(to: ...) in the composer) or a symmetric horizontal / 2 would still pass. Could we add a case with asymmetric insets on both sides, e.g. EdgeInsets.only(left: 47, right: 84), and assert both edges, for the message list and for the composer while the picker opens?
There was a problem hiding this comment.
Done in c5f42ff: the message list and composer tests now use left: 47, right: 84 and check both edges. Swapped or averaged insets were already caught by the composer test (it compares against the closed state), but dropping the left side wasn't, and neither was copyWith(right: 0) in the message list. Both fail now.
…uilders Moves the safe-area inset out of PagedValueListView and PagedValueGridView, where it wrapped customer emptyBuilder/loadingBuilder/errorBuilder output, into the SDK's own state widgets. StreamScrollViewEmptyWidget, StreamScrollViewErrorWidget and StreamScrollViewLoadingWidget clear the safe area with StreamSafeArea(minimum: ...), and the channel and thread skeleton rows clear the side insets the way StreamListTile does. Custom builders are laid out as before. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Names the list views a customer sees in the changelog instead of the internal skeleton widgets, says what "safe area" means in the state widgets' docs, notes the composer is 1px shorter, and drops a private comment that restated its code. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The message list and composer safe-area tests use different left and right insets and check both edges, the composer divider test fails on a second border or one inside the insets, and the safe-area tests build their own fixtures instead of sharing them through setUp. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md
xsahil03x
changed the title
This branch has not been deployed
2 participants
Resolves FLU-820. On iPhone Duo the status rail sits on one side, so the left and right safe-area insets differ (about 84pt on the right). Several places either ignored the side insets or dropped them.
enableSafeArea): content sits in aSafeAreafor the side insets and keeps the top and bottom as scroll padding. Messages, the floating date divider, the unread indicator and the scroll-to-bottom button stay clear of the rail, and the background stays full width. Loading, empty and error states, including custombuilders, clear every inset. This replaces the per-widget padding from the earlier commits in this PR.StreamScrollViewEmptyWidget,StreamScrollViewErrorWidget,StreamScrollViewLoadingWidget) clear the safe area, and the channel and thread loading rows clear the side insets likeStreamListTile. Custom state builders are laid out as before.showStreamSheet.Video controls in the media viewer depend on fluttercommunity/chewie#972.
Verified on the iPhone Duo simulator (outer display), with new widget tests for each fix.
🤖 Generated with Claude Code
Summary by CodeRabbit