Timeline: jump-to-unread FAB and mark-as-read shortcut#6694
Timeline: jump-to-unread FAB and mark-as-read shortcut#6694jennaharris7 wants to merge 14 commits intoelement-hq:developfrom
Conversation
|
Thank you for your contribution! Here are a few things to check in the PR to ensure it's reviewed as quickly as possible:
|
ganfra
left a comment
There was a problem hiding this comment.
Thanks for the PR, first comments after a quick look, I'll get back on this next week.
| val prevMostRecentItemId = rememberSaveable { mutableStateOf<UniqueId?>(null) } | ||
|
|
||
| val newEventState = remember { mutableStateOf(NewEventState.None) } | ||
| val newMessagesCount = remember { mutableIntStateOf(0) } |
There was a problem hiding this comment.
Should probably be part of NewEventState.FromOther instead
| withContext(dispatchers.computation) { | ||
| var markerIdx = -1 | ||
| var unread = 0 | ||
| for ((i, item) in items.withIndex()) { |
There was a problem hiding this comment.
This logic won't work when the marker is not in memory, which can happens pretty easily (gaps, event-cache pagination)
There was a problem hiding this comment.
The iOS sibling PR (element-hq/element-x-ios#5506) hit the same limitation — they landed on the conclusion to ship behind a feature flag and follow up with SDK-supported scrolling. Proposing the same here. The proper fix needs an SDK accessor for the fully-read marker event id (it's in m.fully_read account data but not currently surfaced when the marker is outside the loaded window), plus rewiring the FAB to use Timeline.Mode.FocusedOnEvent on click. I’ve added an edge case not covered in the pr description, apologies for not including it initially!
| } else { | ||
| stringResource(id = CommonStrings.a11y_jump_to_unread_messages) | ||
| } | ||
| TimelineFab( |
There was a problem hiding this comment.
TimelineFab name is not precise enough on the meaning of the component, maybe something like JumpToPositionButton
| count: Int, | ||
| modifier: Modifier = Modifier, | ||
| ) { | ||
| if (count <= 0) return |
There was a problem hiding this comment.
With a when would be better.
Also needs inputs from Design team
There was a problem hiding this comment.
Refactored to use when in 7148f2f, will wait for design team feedback for other updates
|
|
||
| <resources> | ||
| <string name="common_black">"Black"</string> | ||
| <string name="a11y_jump_to_unread_messages">"Jump to unread messages"</string> |
| } | ||
|
|
||
| fun jumpToReadMarker() { | ||
| if (readMarkerIndex < 0) return |
There was a problem hiding this comment.
So like said previously, this won't work if the read marker is not loaded in memory
| lazyListState.firstVisibleItemIndex < 3 && isLive | ||
| } | ||
| } | ||
| val isReadMarkerOffTop by remember { |
There was a problem hiding this comment.
Should probably be name isJumpToUnreadVisibile
| derivedStateOf { | ||
| if (!displayJumpToUnread || readMarkerIndex < 0) { | ||
| false | ||
| } else if (forceJumpToReadMarkerVisibility) { |
There was a problem hiding this comment.
Can we simplify this check? Also forceJumpToReadMarkerVisibility should win over anything else.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
7e82ede to
0daf7ec
Compare
Timeline: jump-to-unread + mark-as-read on the timeline FABs
Content
Two additions to the room timeline, gated behind a new
JumpToUnreadfeature flag (off by default; toggleable in developer options):Both FABs show a small green dot when there are unreads in their direction:
The dot is binary (visible / not visible), not a count.
Motivation
When you scroll up in a busy room, the read marker leaves the screen and there's no quick way back. The chevron-up FAB takes you back in one tap. The green dots act as lightweight "there's something here" cues without cluttering the FAB with a number. The long-press shortcut lets you clear unread state without scrolling all the way back to the bottom.
Screenshots / GIFs
*style updated to match figma:
Edge case not covered
If the read marker isn't in the loaded timeline window (e.g. focused-event timeline, paginated gaps), the chevron-up FAB won't appear. Closing this requires SDK plumbing to surface the fully-read event id so we can paginate-and-centre on it. Reasonable to land as a follow-up; this PR is gated behind
FeatureFlags.JumpToUnreadso the limited form can soak.Tests
./gradlew :features:messages:impl:testDebugUnitTest../gradlew :tests:konsist:testDebugUnitTest.Tested devices
Checklist