feat(ui): add configurable StreamBackButton unread count#2816
feat(ui): add configurable StreamBackButton unread count#2816VelikovPetar wants to merge 5 commits into
StreamBackButton unread count#2816Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthrough
ChangesUnread badge migration
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Header
participant StreamBackButton
participant StreamUnreadIndicator
participant ClientState
Header->>StreamBackButton: configure unreadIndicator
StreamBackButton->>StreamUnreadIndicator: resolve total or channel indicator
StreamUnreadIndicator->>ClientState: read unread streams
ClientState-->>StreamUnreadIndicator: provide unread counts
StreamUnreadIndicator-->>StreamBackButton: render badge
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 |
VelikovPetar
changed the title
StreamBackButton unread count
| /// | ||
| /// Set [excludeCid] to omit a channel's unread messages from the total - | ||
| /// for example, the currently open channel. | ||
| const factory StreamBackButtonUnreadCount.total({String? excludeCid}) = _TotalUnreadCount; |
There was a problem hiding this comment.
Not completely sure about the naming here, let me know if you think we should use something different (.unreadMessages()perhaps, to serve as a forecast for the potentially upcoming .unreadChannels())
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/example/lib/main.dart`:
- Around line 228-232: Update the unread-count selection in the back-button
setup to use the total count excluding the current channel rather than
StreamBackButtonUnreadCount.channel(cid). Preserve the existing fallback
behavior when no channel CID is available.
🪄 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: d4e9d622-7b75-4baf-801c-03086903330e
📒 Files selected for processing (11)
docs/docs_screenshots/test/localization/localization_rtl_test.dartmigrations/redesign/headers_and_icons.mdpackages/stream_chat_flutter/CHANGELOG.mdpackages/stream_chat_flutter/example/lib/main.dartpackages/stream_chat_flutter/lib/src/channel/channel_header.dartpackages/stream_chat_flutter/lib/src/indicators/unread_indicator.dartpackages/stream_chat_flutter/lib/src/misc/back_button.dartpackages/stream_chat_flutter/lib/src/misc/thread_header.dartpackages/stream_chat_flutter/test/src/channel/channel_header_test.dartpackages/stream_chat_flutter/test/src/indicators/unread_indicator_test.dartpackages/stream_chat_flutter/test/src/misc/back_button_test.dart
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2816 +/- ##
==========================================
+ Coverage 71.35% 71.37% +0.02%
==========================================
Files 430 430
Lines 26945 26965 +20
==========================================
+ Hits 19226 19246 +20
Misses 7719 7719 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| final String? channelId; | ||
|
|
||
| /// The unread count configuration for the back button. | ||
| final StreamBackButtonUnreadCount? unreadCount; |
There was a problem hiding this comment.
Should we instead just ask for a Widget/StreamUnreadIndicator here directly? It will make it more composable and the api cleaner.
There was a problem hiding this comment.
Good idea, perhaps passing a pure Widget would be the most future-proof. Not sure if we would be opening the API a bit too much, but since it is a pattern already used in the SDK, I think we should be OK. I will draft a solution using a Widget instead of StreamBackButtonUnreadCount.
There was a problem hiding this comment.
There is a small caveat with this approach: If we rework it to pass a Widget? unreadIndicator, passing a StreamUnreadIndicator complicates the setup a bit because currently, its placement is done by nesting a child:
StreamUnreadIndicator(
offset: .zero,
excludeCid: excludeCid,
child: button, // <--- StreamBackButton
)This would mean that when building a StreamBackButton with a StreamUnreadIndicator, We will have to pass child: null in the indicator, and handle the positioning inside the StreamBackButton, something along the lines of:
if (_effectiveUnreadIndicator case final ind?) {
button = Stack(
clipBehavior: Clip.none,
children: [
button,
Positioned.fill(
child: FittedBox(
fit: BoxFit.none,
alignment: AlignmentDirectional.topEnd,
child: ind, // <---- StreamUnreadIndicator WITHOUT child
),
),
],
);
}Should be doable, but in my opinion, it makes the API pretty confusing. I wanted to bring this up, before committing to this solution.
There was a problem hiding this comment.
Should be good, just make sure we use the same position as we had before. Flutter Badge also supports a nullable child.
| this.semanticLabel, | ||
| }) : _unreadType = _UnreadChannels(cid: cid); | ||
| }) : _unreadType = _UnreadChannels(cid: cid), | ||
| excludeCid = null; |
There was a problem hiding this comment.
I think we can support excludeCid here too. wdyt?
There was a problem hiding this comment.
In theory yes, should be feasible. I decided to not do it now because it wasn't part of the initial bug report. However I have one concern: In that case, we could in theory pass both cid and excludeCid to the _UnreadChannels config -> This can become a bit confusing, but if we prioritize the setup in the following way:
- If
cid != null-> show that specific channel unread count - If
excludeCid != null-> all unread channels - 1 (if excluded channel has unread) - Else -> all unread channels
(Ideally we would have different factory methods for channels vs currentChannel, but currently the channels factory handles both cases, so I think introducing new API might be confusing).
What do you think about this?
There was a problem hiding this comment.
lets skip it until we have a usecase
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md
Replace the typed StreamBackButtonUnreadCount config with a plain unreadIndicator Widget (typically a StreamUnreadIndicator), per review feedback on #2816. StreamBackButton now overlays the indicator on its top-end corner and hides it at zero count; badge placement is unchanged, verified pixel-identical via new golden coverage for the back button and the unread indicator. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add an explicit widget-level assertion that a configured unreadIndicator renders the StreamUnreadIndicator but no StreamBadgeNotification when the count is zero, complementing the existing golden coverage. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2 participants
Submit a pull request
Linear: FLU-557
Github Issue: #
CLA
Description of the pull request
The
StreamChannelHeaderback-button unread badge counted the currently open channel in its total, so it never reflected "unread elsewhere".StreamBackButtonnow takes anunreadCountconfiguration via the newStreamBackButtonUnreadCount:.total({excludeCid})— total unread across channels, optionally excluding one channel..channel(cid)— a specific channel's unread count.StreamChannelHeadernow uses.total(excludeCid: channel.cid), so its back-button badge shows the unread messages waiting in other channels.StreamUnreadIndicatorgained an opt-inexcludeCid(bare usage is unchanged).showUnreadCountandchannelIdonStreamBackButtonare deprecated but still functional — fully backwards compatible.Test instructions: open a channel that has unread messages while other channels also have unread. The header back-button badge shows
total − thisChannel; going back to the channel list, the badge shows the full total. Covered by unit tests inback_button_test.dartandunread_indicator_test.dart(incl. the deprecated paths).Screenshots / Videos
unreads-before.mov
unreads-after.mov
Summary by CodeRabbit
New Features
unreadIndicatorsupport for back buttons to render unread badges using total counts or per-channel counts.unreadIndicator.Bug Fixes
Documentation
unreadIndicatormigration and legacy unread options deprecation.Tests
excludeCidbehavior and RTL snapshots.