feat(ui): add configurable StreamBackButton unread count - #2816
feat(ui): add configurable StreamBackButton unread count#2816VelikovPetar wants to merge 11 commits into
StreamBackButton unread count#2816Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
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:
📝 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
🚥 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 72.83% 72.84% +0.01%
==========================================
Files 428 428
Lines 27657 27681 +24
==========================================
+ Hits 20144 20165 +21
- Misses 7513 7516 +3 ☔ 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>
| appBar: const StreamChannelHeader( | ||
| automaticallyImplyLeading: false, | ||
| leading: StreamBackButton(showUnreadCount: false), | ||
| leading: StreamBackButton(unreadIndicator: null), |
There was a problem hiding this comment.
We can remove it as its null be default
There was a problem hiding this comment.
Also, not sure if it does anything rn as we only check for non null case and otherwise fallback to the deprecated props.
if (unreadIndicator case final effective?) return effective;
There was a problem hiding this comment.
Fixing the first remark!
I am not sure I fully understand what do you mean with the 2nd comment here though, can you give some more details?
There was a problem hiding this comment.
I meant, even if we pass null here in order to not show the unread count, it won't always behave correctly as we fallback to the deprecated showUnreadCount boolean in case its null currently. So currently passing null does not always mean we are hiding the unread count.
There was a problem hiding this comment.
Ah I see what you mean. I think we have to do it like this, in order to be backwards compatible (unreadIndicator: null -> fallback to legacy showUnreadCount)
| leading = StreamBackButton(channelId: channel?.cid, showUnreadCount: true); | ||
| final cid = channel?.cid; | ||
| leading = StreamBackButton( | ||
| unreadIndicator: cid != null ? StreamUnreadIndicator.channels(cid: cid) : const StreamUnreadIndicator(), |
There was a problem hiding this comment.
:nit: lets use a switch statement here, will make it a bit cleaner
# Conflicts: # packages/stream_chat_flutter/CHANGELOG.md
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
unreadIndicatorsupport for back buttons, enabling total unread badges (optionally excluding the current channel) or per-channel unread badges.unreadIndicatorfor leading back button badges.unreadIndicator, including deprecation of legacy unread-count options.excludeCidand precedence behavior.