Repository navigation
feat(chat): unread messages divider in the messenger transcript - #1546
Merged
Merged
Conversation
The unread divider needs a count of messages someone else sent after the viewer's READ pointer. countInboundAfter takes that as a range over message_id, so a read-through message that was deleted or never stored still counts correctly, and it skips tombstones. countAfter counts every stored row past an id, including the viewer's own and tombstones. It bounds the walk that scrolls the divider into view.
…nter MessagingOperations gains resolveUnreadBoundary, which reads the viewer's READ pointer from the stored member row and counts inbound, non-deleted messages past it. A missing self row resolves to None rather than to a pointer of 0: group rosters are paged, and 0 would put the divider above the whole transcript. getSelfReadPointer keeps its 0 fallback for its existing callers. countMessagesAfter bounds the list's walk to the divider. It is separate from distanceFromNewest, which returns null when the read-through message is not stored.
The spec's placement rule, older.messageId <= readThrough < newer.messageId with newer not from the viewer, finds no gap for 1:other 2:self 3:other with the pointer at 1: the 1|2 gap fails the sender check and 2|3 fails the range. The shared table puts the divider above 3. insertSeparators only sees neighbours, so the fix is in resolution: the boundary's readThrough is one below the first message past the pointer that the viewer did not send, tombstones included. The placement rule is unchanged and the other table rows resolve to the same boundary as before.
ChatListItem gains UnreadDivider(count, date), keyed "unread-divider" since a transcript has at most one. separatorBetween puts it in the gap where older.messageId <= readThrough < newer.messageId and the newer message is not the viewer's own. When a day also changes at that gap the divider carries the date, because insertSeparators takes one item per gap; UnreadDividerRow draws the date above the band. When every stored message is unread, the divider takes the terminal gap and the oldest message's date, so MessageList skips its trailing date for it. A divider on either side of a row is the normal gap. Counts over 99 read "99+". The view model does not produce boundaries yet; this adds the pieces and their tests against the shared placement table.
… the separators ChatViewModel reads the boundary when the chat id becomes known, along with the walk budget, and holds both in State. Only UnreadBoundaryResolved sets them. AdvanceReadPointer leaves them alone, so reading the chat does not slide the divider. The mapping combine is now mappedMessages, cached, and a light stage combines it with the boundary and separator config to run insertSeparators. The separator config was read from stateFlow.value inside the lambda before; as a combine input a page can no longer take its separators from a different config than its neighbour. UnreadDividerLifetime.UntilClose ships. The reducer takes the lifetime as a parameter so UntilSend, which clears the divider on a send, is covered too.
The list lays out at the bottom first. If the divider is whole below the top bar there, the chat stays put. Otherwise it walks back with the same append loop a jump uses, bounded by the resolved walk budget, and scrolls without animation so the divider's upper edge sits at the bar's lower edge. A divider not reached in budget leaves the chat at the bottom. HandleMessageReads takes enabled, and the list keeps it off until that settles. Without the gate the bottom-anchored first frame reports the newest message read, which is the pointer the divider is there to show. With no boundary it turns on at once, as before. ChatViewModel.messages emits nothing while the boundary resolves. A page laid out before the boundary landed has no divider, and the list would decide where to open from it.
The filled band with bold text read heavier than the date separator next to it. The divider is now the date separator's caption style between two `colors.divider` hairlines, inset to the transcript's margins. `GroupTranscriptScreenshotTest.rendersUnreadDivider` renders it on its own and sharing a gap with a day change.
With pointer 5 and messages 10..20 stored, the divider went above 10 as "11 Unread Messages" even if 6..9 were unread but never fetched. The count can come out short whenever the store has a gap under the unread run. Resolution now returns `None` unless a message at or below the boundary is stored, tombstones included (`ChatMessageDao.hasAtOrBelow`). This matches iOS and adds an eighth row to the shared table: 3:other 4:other with the pointer at 1 draws no divider.
iOS encodes the same six rows. Unread means the newest visible message is past the viewer's READ pointer and someone else sent it. A group with no self row is not unread, and a DM with no self row reads its pointer as 0. The rows run straight against `FeedProjection.unreadCount`.
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
1 participant
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.
Flipchat drew an "N Unread Messages" divider in the transcript. Its producer went in
02ea3bb26and the messenger has had nothing in its place. This adds one to the messenger, where a chat now opens at the first unread message.The rule
The boundary is resolved once per visit from the viewer's READ pointer in the stored self member row, and held in
ChatViewModel.StateasUnreadBoundary.AdvanceReadPointerdoesn't touch it, so reading the chat doesn't move the divider.getSelfReadPointerOrNullis the new nullable path; the existing non-null getter reads a missing row as 0, which would put the divider above the whole transcript.ChatMessageDao.hasAtOrBelow, tombstones included): no divider. Older unread messages may never have been fetched, so the count could come out short. With pointer 5 and 10..20 stored, it would otherwise read "11 Unread Messages" above 10 even if 6..9 are unread.ChatMessageDao.countInboundAfter). Zero means no divider. Past 99 it reads "99+".older.messageId <= readThrough < newer.messageIdandnewerisn't the viewer's own. It's a range comparison, so a deleted read-through message doesn't matter, and the rows of a split message never straddle it.ChatListItem.UnreadDivider(count, date), with the date drawn above the divider. The divider itself is a caption between two hairlines, in the date separator's style.UNREAD_DIVIDER_LIFETIMEisUntilClose.UntilSendclears the divider when the viewer sends.Change from the spec: own messages right after the pointer
The placement rule alone finds no gap for
1:other 2:self 3:otherwith the pointer at 1. The 1|2 gap fails the sender check and 2|3 fails the range, but the shared table puts the divider above 3.insertSeparatorsonly sees neighbours, so the fix is in resolution:At.readThroughis one below the first message past the pointer that the viewer didn't send, tombstones included (firstNotSentByAfter). The placement rule is unchanged, and all table rows hold. iOS needs the same adjustment to agree.Opening
The list lays out at the bottom first and stays there if the divider is already whole below the top bar. Otherwise it walks back with the jump's append loop (now
walkUntil), bounded by a stored-message count past the boundary, and scrolls without animation so the divider's upper edge sits at the bar's lower edge. The spec says lower edge, but the row's top is underChatTopEdge's fade, so that would hide the label.HandleMessageReadstakesenabledand stays off until the list has positioned, so the bottom-anchored first frame doesn't mark the unread messages read.ChatViewModel.messagesalso waits for the boundary before emitting, so the list never decides where to open from a page that has no divider yet.Tests
ChatMessageDaoTest: the count and first-not-own queries on Room.UnreadBoundaryResolutionTestandUnreadDividerPlacementTest: the shared table, now eight rows with "nothing stored at or below read-through" (3:other 4:other, pointer 1: no divider).UnreadBoundaryReducerTest: resolution,AdvanceReadPointer, and both lifetimes.UnreadSeparatorsTest:insertSeparatorsoutput, including date and divider on one gap.UnreadTableTest: the shared six-row "does this chat show as unread" table againstFeedProjection.unreadCount, which iOS encodes too. No behavior change; it pins the existing rule.GroupTranscriptScreenshotTest.rendersUnreadDivider: renders the divider tobuild/screenshots/unread_divider.png, alone and under a date. Not an assertion test.UnreadDividerOpenTest: 40 messages with the pointer at 10. The chat opens with the divider near the top and reports no read past the screen. With no boundary it opens at the bottom, and while the boundary is resolving it reports nothing.Not in this change: deleting the orphaned
ui/componentsUnreadSeparator,MessageListandChatItem.