Add scroll-to-first-unread pill to the message view#6409
Merged
Conversation
Contributor
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
WalkthroughThe PR introduces a "scroll to first unread" button UI element with supporting state management. It adds a new ChangesScroll to First Unread Feature
Sequence DiagramsequenceDiagram
participant User
participant UI as ScrollToFirstUnreadButton
participant Compose as MessageList
participant ViewModel as MessageListViewModel
participant Controller as MessageListController
participant State as MessageListState
User->>UI: Click scroll to first unread
UI->>Compose: onScrollToFirstUnread()
Compose->>ViewModel: scrollToFirstUnreadMessage()
ViewModel->>Controller: scrollToFirstUnreadMessage()
Controller->>State: Update unreadLabel visibility
User->>UI: Click dismiss (X)
UI->>Compose: onDismissUnreadLabel()
Compose->>ViewModel: disableUnreadLabelButton()
ViewModel->>Controller: disableUnreadLabelButton()
Controller->>Controller: Emit false to showUnreadButtonState
Controller->>Controller: Set unreadLabel.buttonVisibility = false
Controller->>State: updateUnreadLabel()
State->>UI: unreadLabel state updated
UI->>UI: Re-render (button hidden)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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}
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 (1)
stream-chat-android-ui-common/api/stream-chat-android-ui-common.api (1)
2740-2755:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBinary-incompatible constructor/
copysignature change — confirm SDK consumers are awareThe
MessageListStateprimary constructor andcopymethod have changed from 11 to 12 parameters (addingunreadLabel). This is a binary-incompatible change for:
- Java callers invoking the 11-arg constructor directly.
- Kotlin callers using positional (non-named) arguments on
copy.- Any consumer compiled against the old API — they will need to recompile with updated call sites.
The no-arg constructor (Line 2739) is preserved, and the
DefaultConstructorMarkeron the synthetic constructor confirmsunreadLabelhas a default value, so Kotlin callers using named parameters or defaults are unaffected. This is expected behaviour for data-class extension, but worth communicating explicitly in the release notes / changelog if this module is published as a versioned library.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/api/stream-chat-android-ui-common.api` around lines 2740 - 2755, MessageListState's primary constructor and copy method gained a 12th parameter unreadLabel (symbols: MessageListState constructor, copy, copy$default, unreadLabel), which is a binary-incompatible change for consumers compiled against the previous 11-arg signatures; to fix, either: (a) add a binary-compatible 11-argument overload/compat shim that delegates to the new constructor/copy (preserving old signature for Java callers and positional Kotlin usage), or (b) if you intentionally break binary compatibility, update the release notes/changelog and migration guide to call out the added unreadLabel parameter and instruct consumers to recompile, and add a `@Deprecated` note on the old usages if you remove the shim so callers are clearly warned.
🧹 Nitpick comments (1)
stream-chat-android-ui-common/api/stream-chat-android-ui-common.api (1)
2740-2741: ⚖️ Poor tradeoff
MessageListController$UnreadLabelleaks the feature layer into the state layer's public APIThe type
MessageListController$UnreadLabel(package...feature.messages.list) now appears throughoutMessageListState's public surface (constructor,copy,component12,getUnreadLabel). A KotlintypealiasinUnreadLabel.kthelps source-level ergonomics but is transparent at the bytecode level — the API dump (and therefore Java callers and binary consumers) still seesMessageListController$UnreadLabel. This forces any consumer ofMessageListStateto take a transitive dependency onMessageListControllerjust to read/pass the label value.Relocating the
UnreadLabeldata class to thestate.messages.listpackage (rather than nesting it insideMessageListController) would cleanly sever this cross-layer coupling without requiring any typealias workaround. The typealias approach works today but can be confusing to new contributors and adds an unnecessary indirection.Also applies to: 2754-2755, 2765-2765
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-common/api/stream-chat-android-ui-common.api` around lines 2740 - 2741, The public API is leaking the feature-layer nested type MessageListController$UnreadLabel into MessageListState (seen in the constructor, copy, component12, getUnreadLabel); move the UnreadLabel data class out of MessageListController into the state.messages.list package so MessageListState can reference a state-level io.getstream.chat.android.ui.common.state.messages.list.UnreadLabel type. Create the UnreadLabel data class in the state.messages.list package (preserving fields, equals/hashCode/copy semantics), update imports/usages in MessageListState and any callers to use the new UnreadLabel, remove or replace the nested class inside MessageListController (and any typealias) to avoid the transitive dependency, and run consumers/serialization checks to ensure binary compatibility is preserved.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@stream-chat-android-compose/api/stream-chat-android-compose.api`:
- Line 3465: The public ChatComponentFactory interface was extended with new
methods ScrollToFirstUnreadButton and ScrollToBottomButton which breaks binary
compatibility for precompiled custom implementors; to fix, enable JVM default
method generation by adding the Kotlin compiler option -Xjvm-default=all to the
stream-chat-android-compose module so these new members are emitted as
JVM-default methods, or alternatively document this as a breaking change
requiring consumers to recompile custom ChatComponentFactory implementations;
update build configuration accordingly and ensure CI/build docs note the change.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/list/Messages.kt`:
- Around line 389-401: The visibility check uses the numeric
unreadSeparatorIndex which can be invalid when the lazy list contains
non-message items; instead compute the unique key for the UnreadSeparator item
(e.g., derive unreadSeparatorKey from messages.first { it is
UnreadSeparatorItemState } or from the same keying logic used in itemsIndexed)
and change isUnreadSeparatorVisible (currently using
lazyListState.layoutInfo.visibleItemsInfo.any { it.index == unreadSeparatorIndex
}) to check visibleItemsInfo.any { it.key == unreadSeparatorKey }; update the
variables unreadSeparatorIndex and isUnreadSeparatorVisible and keep the call to
isScrollToFirstUnreadVisible unchanged so visibility is driven by the keyed
check.
In
`@stream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.kt`:
- Around line 638-640: The disableUnreadLabelButton currently forces
showUnreadButtonState.tryEmit(false) even when unreadLabelState.value is null,
which latches suppression; update disableUnreadLabelButton so it first checks
unreadLabelState.value != null and only then calls
showUnreadButtonState.tryEmit(false) and sets unreadLabelState.value =
unreadLabelState.value.copy(buttonVisibility = false); avoid emitting or
mutating state when unreadLabelState.value is null to prevent suppressing future
unread-pill visibility.
In
`@stream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt`:
- Around line 1048-1050: The assertion uses a safe-call that can hide a null
unreadLabelState and produce false positives; change the test to assert that
controller.unreadLabelState.value is non-null before asserting its
buttonVisibility is false — specifically, add an explicit non-null check on
controller.unreadLabelState.value (e.g., assertNotNull or equivalent) and then
assert on unreadLabelState.value.buttonVisibility in MessageListControllerTests
so the post-update assertion cannot pass when the state is null.
---
Outside diff comments:
In `@stream-chat-android-ui-common/api/stream-chat-android-ui-common.api`:
- Around line 2740-2755: MessageListState's primary constructor and copy method
gained a 12th parameter unreadLabel (symbols: MessageListState constructor,
copy, copy$default, unreadLabel), which is a binary-incompatible change for
consumers compiled against the previous 11-arg signatures; to fix, either: (a)
add a binary-compatible 11-argument overload/compat shim that delegates to the
new constructor/copy (preserving old signature for Java callers and positional
Kotlin usage), or (b) if you intentionally break binary compatibility, update
the release notes/changelog and migration guide to call out the added
unreadLabel parameter and instruct consumers to recompile, and add a `@Deprecated`
note on the old usages if you remove the shim so callers are clearly warned.
---
Nitpick comments:
In `@stream-chat-android-ui-common/api/stream-chat-android-ui-common.api`:
- Around line 2740-2741: The public API is leaking the feature-layer nested type
MessageListController$UnreadLabel into MessageListState (seen in the
constructor, copy, component12, getUnreadLabel); move the UnreadLabel data class
out of MessageListController into the state.messages.list package so
MessageListState can reference a state-level
io.getstream.chat.android.ui.common.state.messages.list.UnreadLabel type. Create
the UnreadLabel data class in the state.messages.list package (preserving
fields, equals/hashCode/copy semantics), update imports/usages in
MessageListState and any callers to use the new UnreadLabel, remove or replace
the nested class inside MessageListController (and any typealias) to avoid the
transitive dependency, and run consumers/serialization checks to ensure binary
compatibility is preserved.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 41d8a675-f966-4dff-8333-f88667be786e
⛔ Files ignored due to path filters (1)
stream-chat-android-compose/src/test/snapshots/images/io.getstream.chat.android.compose.ui.components.messages_ScrollToFirstUnreadButtonTest_pill_with_unread_count.pngis excluded by!**/*.png
📒 Files selected for processing (17)
stream-chat-android-compose/api/stream-chat-android-compose.apistream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/internal/MediaGalleryPage.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/attachments/preview/internal/MediaGalleryPhotosMenu.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/components/messages/ScrollToFirstUnreadButton.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/list/MessageList.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/list/Messages.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactory.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/theme/ChatComponentFactoryParams.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/ModifierUtils.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/MessageListViewModel.ktstream-chat-android-compose/src/main/res/values/strings.xmlstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/ui/components/messages/ScrollToFirstUnreadButtonTest.ktstream-chat-android-ui-common/api/stream-chat-android-ui-common.apistream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListController.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/messages/list/MessageListState.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/state/messages/list/UnreadLabel.ktstream-chat-android-ui-common/src/test/kotlin/io/getstream/chat/android/ui/common/feature/messages/list/MessageListControllerTests.kt
Contributor
SDK Size Comparison 📏
|
andremion
force-pushed
the
feat/scroll-to-first-unread-indicator
branch
from
5cef529 to
f7815c4
Compare
andremion
changed the title
gpunto
approved these changes
May 5, 2026
Contributor
gpunto
left a comment
gpunto
left a comment
There was a problem hiding this comment.
LG! Just a couple of doubts
Adds an UnreadLabel field on MessageListState so UI layers can react to the sticky unread-boundary signal without depending on the live unreadCount, which collapses to 0 once the SDK auto-marks the latest message as read on chat open. The new field mirrors MessageListController.unreadLabelState via a collector. Declares UnreadLabel as a typealias in the state package (state/messages/list/UnreadLabel.kt) pointing at MessageListController's nested data class, so MessageListState references the type from its own package without reaching into feature/messages/list. Both names resolve to the same JVM class — existing consumers of MessageListController.UnreadLabel keep compiling.
Lets the UI dismiss the floating unread-label button (e.g. from a close affordance on the scroll-to-first-unread pill) without affecting the inline unread separator.
Introduces the floating pill primitive used to jump to the first unread message when the unread boundary sits outside the viewport. The pill exposes two distinct interactions: tapping the label area scrolls to the boundary, while tapping the trailing close icon dismisses the pill without scrolling. Adds the ScrollToFirstUnreadButtonParams holder, the ChatComponentFactory slot, the default composable, and the supporting string resources. The slot is unwired in this commit; the wiring lands in the follow-up.
Renders the floating pill inside DefaultMessagesHelperContent using the sticky unread label exposed on MessageListState. Visibility derives from unreadLabel.buttonVisibility plus whether the inline unread separator is in the visible viewport, so the pill stays correct even after the SDK auto-marks the latest visible message as read on chat open. Tap on the pill body invokes MessageListViewModel.scrollToFirstUnreadMessage (loading the boundary if needed); tap on the close affordance invokes MessageListViewModel.disableUnreadLabelButton. Both actions are exposed as trailing defaulted callbacks on the public MessageList overloads so that state-only consumers can opt in. The visibility derivedState observes only lazyListState.layoutInfo (a State) and re-keys on unreadSeparatorIndex; buttonVisibility is read on each recomposition from the parameter so it never relies on a stale capture.
disableUnreadLabelButton() flipped buttonVisibility on the current label, but observeUnreadLabelState recomputes the label whenever lastReadMessageId changes and the calculator restored buttonVisibility=true. After scrollToFirstUnreadMessage, the auto-mark-read on the now-visible boundary triggered exactly that recomputation and the pill returned. Also tryEmit(false) on showUnreadButtonState so the suppression flows through the calculator on subsequent recomputes. The suppression is per-controller, so it resets when the user leaves and re-enters the channel — matching the Figma spec. A regression test in MessageListControllerTests locks the new contract: after disableUnreadLabelButton, pushing a new lastReadMessageId on channelState.read must not flip buttonVisibility back to true.
Cleanup-only follow-up to the pill UI: - Replace the hardcoded 1.dp stroke with StreamTokens.borderStrokeSubtle so the border respects the design-system token. - Pass role=Role.Button directly to clickable instead of layering a separate semantics modifier; the role lands on the same semantics node either way. - Split each four-edge padding into a vertical+horizontal pair for symmetry with the rest of the codebase's padding patterns. No behaviour or API change.
Expands the KDoc on MessageListController.unreadLabelState to describe its stickiness, the dismissal contract, and to point Compose-style consumers at MessageListState.unreadLabel (the aggregated mirror) so granular subscribers and UI consumers each pick the better entry point.
The internal Modifier.clickable helper centralises the SDK's clickable surface
(explicit Material 3 ripple, optional bounded clipping). The pill was
side-stepping it by calling foundation's clickable directly, drifting from the
codebase convention. Add a defaulted role parameter to the helper so the pill
can ride the shared ripple while still announcing Role.Button.
Two pre-existing trailing-lambda call sites (MediaGalleryPage,
MediaGalleryPhotosMenu) import both the foundation and internal clickables;
the new role parameter makes the simplified ".clickable { … }" form
ambiguous between the two. Disambiguate by passing bounded = true (foundation
has no bounded), which preserves the existing ripple-bearing behaviour.
Mirrors the Figma "Message View — Opened With Unread Messages" mock, which shows a thin vertical rule between the unread count and the dismiss affordance. Reuses the same border token already applied to the pill outline so the inner rule and the outline read as a single design.
Three Paparazzi snapshots covering the unread-count surface area: 1, 9, and 999. Each renders both light and dark variants via snapshotWithDarkMode so the divider, border, and ripple-bearing label area are all visible against the themed background.
Two related fixes on the pill composable: - Bound the inner Row to Modifier.height(IntrinsicSize.Min). VerticalDivider uses fillMaxHeight() internally, which was unbounded inside the Surface preview and stretched the pill across the rendered surface. With IntrinsicSize.Min, the divider's height resolves to the tallest non-flexible child (the label row or the close icon). - Move the Stream_ScrollToFirstUnreadButton testTag from the Surface to the inner clickable Row. The Surface is not interactive, so a UI test that finds the node by tag and performs a click would land on a non-clickable node. With the tag on the click target, finding by tag and clicking now invokes the scroll handler, mirroring the dismiss tag on the X icon.
- Messages.kt: switch the pill's separator-visibility check to compare LazyListItemInfo.key against the separator item's id, instead of comparing the message-list index against the LazyList index. The two diverge when non-message items (footerContent, the load-more indicator) precede itemsIndexed in the column, which would mis-judge pill visibility. - MessageListController.disableUnreadLabelButton: early-return when there is no active unread label. The previous code emitted false on showUnreadButtonState even on a no-op call (e.g. scrollToFirstUnreadMessage with a null unreadLabelState, or the XML SDK's HideUnreadLabel event with no active label), permanently latching the suppression for the controller's lifetime and hiding any future unread pill. - MessageListControllerTests: strengthen the post-read assertion to fail when unreadLabelState becomes null. The previous safe-call chain silently passed when the chain returned null, masking the regression the test is meant to guard.
Two coverage additions for the new unread-pill surface: - MessageListViewModelTest gains a case for disableUnreadLabelButton on a fixture with no active unread label, asserting state stays untouched. This exercises the VM's delegating method and the controller's early-return guard end-to-end. - MessageListTest gains a snapshot variant rendering MessageList with an active UnreadLabel and an inline UnreadSeparatorItemState. The snapshot exercises the wired pill through the public state-only MessageList, Messages.DefaultMessagesHelperContent, and the ChatComponentFactory.ScrollToFirstUnreadButton slot in inspection mode.
The fixture put the separator at index 0 of messageItems, which with reverseLayout = true sits at the bottom of the viewport. The pill hides itself while the separator is visible, so the snapshot did not exercise the pill. Place the separator at the end of the list so it stays above the visible area.
andremion
force-pushed
the
feat/scroll-to-first-unread-indicator
branch
from
f7815c4 to
821ff7f
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.
Goal
When a channel is opened with pre-existing unread messages and the unread boundary sits above the viewport, the message view did not surface any way to jump to it. The Figma "Message View — Opened With Unread Messages (Unread Above)" spec defines a floating pill at the top of the list that shows the unread count with an upward arrow, scrolls to the first unread on tap, and dismisses with a close icon.
This PR adds that pill, surfaces the unread label on
MessageListStateso Compose UIs can observe it, and fixes a related stickiness gap indisableUnreadLabelButtonthat would otherwise let the pill reappear after the auto-mark-read fires post-scroll.As per https://www.figma.com/design/Us73erK1xFNcB5EH3hyq6Y/Chat-SDK-Design-System?node-id=19496-477662&m=dev
Implementation
MessageListState.unreadLabelmirrorsMessageListController.unreadLabelState, same pattern asunreadCount.UnreadLabelis aliased into the state package.disableUnreadLabelButtonis now sticky against subsequent read-state updates (suppression is per-controller, resets on channel re-entry) and exposed as a pass-through onMessageListViewModel.ScrollToFirstUnreadButtonChatComponentFactoryslot — pill with up arrow, count, vertical divider, and close icon. Tap label = scroll, tap X = dismiss. The internalModifier.clickablehelper gains a defaultedroleparameter so the pill rides the SDK's standard ripple.DefaultMessagesHelperContentatAlignment.TopCenter. Visibility matches the unread separator by itsLazyListItemInfo.key(raw index drifts when non-message items precedeitemsIndexed). Callbacks are trailing defaulted parameters on the publicMessageListoverloads.🎨 UI Changes
Screen_recording_20260504_165209.webm
Screen_recording_20260504_165107.webm
Testing