Fix scroll jump when returning to a channel after WS reconnect#6234
Open
VelikovPetar wants to merge 5 commits intov7from
Open
Fix scroll jump when returning to a channel after WS reconnect#6234VelikovPetar wants to merge 5 commits intov7from
VelikovPetar wants to merge 5 commits intov7from
Conversation
Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
…ck_from_background
…ck_from_background
Contributor
SDK Size Comparison 📏
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThis change refactors the channel message update logic to intelligently handle message gaps and discontinuities. It introduces gap detection between current and incoming messages, implements conditional refresh strategies based on state conditions, and adds a cached latest messages feature to preserve messages when gaps are detected. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ChannelLogic
participant ChannelState
participant MessageCache as Cached<br/>Messages
Client->>ChannelLogic: updateStateFromDatabase()
ChannelLogic->>ChannelLogic: shouldRefreshMessages = true
ChannelLogic->>ChannelLogic: hasGap(current, incoming)?
alt Gap Detected
ChannelLogic->>MessageCache: upsertCachedLatestMessages(incoming)
MessageCache->>MessageCache: mergeSorted by ID
MessageCache->>MessageCache: trim to LIMIT
ChannelLogic->>ChannelState: setInsideSearch(true)
ChannelLogic->>ChannelState: setEndOfNewerMessages(false)
else Inside Search & No Gap
ChannelLogic->>MessageCache: upsertCachedLatestMessages(incoming)
else Force Refresh (shouldRefreshMessages=true)
ChannelLogic->>ChannelState: setMessages(incoming)
else Contiguous Messages
ChannelLogic->>ChannelState: upsertMessages(incoming)
ChannelLogic->>ChannelState: setEndOfOlderMessages(based on limit)
end
ChannelState-->>Client: state updated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
…ck_from_background
…fter_coming_back_from_background' into bug/AND-1113_fix_channel_jumps_after_coming_back_from_background
|
gpunto
approved these changes
Mar 13, 2026
| // refresh the "jump to latest" cache with the server's current latest page. | ||
| state.upsertCachedLatestMessages(sortedMessages) | ||
| } | ||
| hasGap(currentMessages, sortedMessages) -> { |
Contributor
There was a problem hiding this comment.
Just for me to understand: this check for a gap between current and incoming, in that order, right?. What if there's a gap the other way around, i.e. all incoming are older than all current, is that a valid case?
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 the user scrolls up in a channel, puts the app in the background (WS disconnects), and returns,
updateDataForChannel()was unconditionally callingstate.setMessages()— a full list replacement. This destroyed the older messages the user had loaded and caused a visible scroll jump to the bottom.Implementation
ChannelStateImpl— AddedupsertCachedLatestMessages(messages): merges incoming messages into_cachedLatestMessagesusing the existingmergeSortedinfrastructure, capped atCACHED_LATEST_MESSAGES_LIMIT(25). This lets reconnect logic refresh the "jump to latest" cache without touching the user's active scroll position.ChannelLogicImpl.updateDataForChannel()— Replaced the unconditionalsetMessages()with a 4-branchwhen:existingMessages.isEmpty()orshouldRefreshMessagessetMessages()— initial load / explicit refresh (existing behaviour)insideSearch == trueupsertCachedLatestMessages()— user's window is already trimmed; refresh the "jump to latest" cache onlyupsertCachedLatestMessages()+setInsideSearch(true)+setEndOfNewerMessages(false)upsertMessages()— normal reconnect, scroll position preserved🎨 UI Changes
jump-before.mp4
jump-after-f.mp4
Testing
ChannelLogicImplTestcovering all four branches.ChannelStateImplMessagesTestcoveringupsertCachedLatestMessages(empty, all-filtered, normal merge).Summary by CodeRabbit
Release Notes
Bug Fixes
Tests