🔄️ Improve unread marker display - #6484
Merged
Merged
Conversation
AndyScherzinger
added
enhancement
AndyScherzinger
force-pushed
the
feat/noid/msgPreFetching
branch
from
August 13, 2026 15:07
f978b99 to
1a1f664
Compare
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 13, 2026 21:42
74641aa to
f0bc26d
Compare
AndyScherzinger
force-pushed
the
feat/noid/msgPreFetching
branch
from
August 13, 2026 21:43
1a1f664 to
3f6a24d
Compare
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 13, 2026 21:43
f0bc26d to
a137536
Compare
AndyScherzinger
added
3. to review
AndyScherzinger
marked this pull request as ready for review
August 13, 2026 21:47
AndyScherzinger
changed the title
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 14, 2026 15:47
a137536 to
8c9e228
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/31816236588/artifacts/9225804811 |
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 14, 2026 17:22
8c9e228 to
4f75c5e
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/31823716011/artifacts/9228623568 |
mahibi
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 18, 2026 10:14
4f75c5e to
4711f95
Compare
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32125764257/artifacts/9323067893 |
Collaborator
|
Reviewed and tested, looks good @AndyScherzinger 👍 I added 3 commits. If these make sense to you, it can be merged. |
mahibi
approved these changes
Aug 18, 2026
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32150518162/artifacts/9334362137 |
The unread marker can only be placed correctly when the chat's visible window - the latest chat block - reaches back to the last read message. For a room without any cached chat block the catch-up fetched the newest page, so any backlog larger than one page left a chat block floating entirely above the unread boundary and the marker ended up pinned to the oldest message of that block, in the middle of the unread messages. When the unread backlog is at least one page but still closable within the backlog rounds, anchor the initial fetch at lastReadMessage (including the last read message itself) and close the rest of the backlog from there, so the created chat block contains the boundary. Smaller backlogs fit into the newest page anyway and oversized backlogs could not be closed either way, so both keep the newest-messages fetch. The room list prefetch passes the conversation's lastReadMessage and unread count along, so background-prefetched rooms open with a window the marker can be placed in. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The initial load decided the cache was usable when the newest cached message id was at least the conversation's lastReadMessage. That check compares against the top of the latest chat block, not its extent: a block created by a capped newest-messages fetch (e.g. the background prefetch) floats entirely above the unread boundary and trivially passes, so the open path only closed the backlog above the block and the unread marker stayed pinned to the block's oldest message, in the middle of the unread messages. Decide on containment instead: the cache is only trusted when the latest chat block reaches back to lastReadMessage (or has no history below it). Otherwise the initial window is re-fetched via initialCatchUp, anchored at the unread boundary when the backlog is large. This also changes the handling of a cache whose latest block lies completely below lastReadMessage (read further on another device): instead of fetching the newest page into a floating block, the backlog is closed from the newest cached message, keeping the window contiguous across the boundary. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
buildChatItems latched firstUnreadMessageId on the first emission as "the first visible message newer than lastReadMessage". When the visible window did not reach back to the last read message - all visible messages unread - this resolved to the window's oldest message, far above the true boundary, and the cached value kept the marker there even after scrolling up loaded the older unread messages. Only latch the marker when a message at or below lastReadMessage is visible, proving the boundary is inside the window. Until then no marker is shown instead of a wrong one. Temporary messages have negative ids and don't count as proof. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The catch-up triggered by a push notification fetched the newest page for rooms without any cached chat block, so a large unread backlog left a chat block floating above the unread boundary — the same window shape that misplaces the unread marker on chat open. Look up the pushed room in the local conversations cache and pass its lastReadMessage and unread count to catchUpRoom, so the initial fetch is anchored at the boundary when the backlog calls for it. The boundary is room-level state and therefore not passed for thread catch-ups. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Covers loadInitialMessages' choice between trusting the cached window and re-fetching it: the backlog is closed from the newest cached message when the latest chat block reaches the last read message (or has no history below it), while a block floating above the boundary and an empty cache are repaired with a fetch anchored at lastReadMessage for large backlogs and a newest-messages fetch for small ones. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
Extracts the latch condition from buildChatItems into the pure findFirstUnreadMessageId (no behavior change) and covers it: the marker position is only derived when a message at or below lastReadMessage is visible, windows floating entirely above the boundary and temporary messages with negative ids yield no marker, and a deleted or expired boundary message is handled by any older visible message. Assisted-by: Claude Code:claude-fable-5 Signed-off-by: Andy Scherzinger <info@andy-scherzinger.de>
The check only guards against a block whose oldest message skipped past lastReadMessage entirely — it doesn't verify the block's newest message actually reaches that far, so a stale-but-not-floating block also passes it. blockNotFloatingAboveLastReadMessage describes what's actually being tested. Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 18, 2026 18:24
e5ec21f to
6ee450c
Compare
fetchNewMessages defaulted to fromMessageId=0 when no HTTP sync had happened yet for the conversation, so the insurance request tried to close the backlog from the very start of the room's history instead of skipping until there is something to verify against. For a busy room this can never close within tryCloseBacklog's round budget, burning five requests before falling back anyway. Assisted-by: Claude Code:claude-sonnet-5 Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
This also calms down the loading bar. Fix 1: When entering a chat, in onResume in ChatViewModel, fetchNewMessagesWithRetry was called. This is too aggressive pulling and should only be used when waiting for uploads to be finished. It is also not satisfied when the response is empty and will try again. UX wise this caused that the loading bar was shown for a long time. fetchNewMessagesWithRetry was replaced with chatRepository.fetchNewMessages() which does a single fetch which is enough in this case. Fix 2: As onResume is executed on every opening of a chat, it added unnecessary requests on top of the loadInitialMessages handling. As a guard to only execute it when coming back from background, isReturningFromBackground was introduced. In this way the scenario from #6313 is still fulfilled but too many requests on startup are avoided. Signed-off-by: Marcel Hibbe <dev@mhibbe.de>
AndyScherzinger
force-pushed
the
feat/noid/msgUnreadMarker
branch
from
August 18, 2026 18:30
6ee450c to
20790c7
Compare
Member
Author
|
Nice improvements @mahibi 👍 All great - will merge after CI did its thing 🚀 |
Contributor
|
APK file: https://github.com/nextcloud/talk-android/actions/runs/32171372510/artifacts/9339772535 |
AndyScherzinger
enabled auto-merge
August 18, 2026 22:21
AndyScherzinger
disabled auto-merge
August 18, 2026 22:28
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.
When entering a conversation, especially when using #6454 the read-state is not always correct / read / propagated.
This PR addresses this, implementing read-status propagation
🏁 Checklist
/backport to stable-xx.x🤖 AI (if applicable)