regression: Floating date bubble displaying wrong date#40739
Conversation
|
Looks like this PR is ready to merge! 🎉 |
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughThis PR enhances date scroll handling by threading scroll offset information through the callback pipeline and implementing threshold-based divider visibility detection. The callback contract is updated to pass offset alongside topMessage, and the hook implementation applies pixel-based visibility thresholds to filter which date dividers are matched for bubble positioning. ChangesDate Scroll Offset Threading and Visibility Detection
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
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 |
There was a problem hiding this comment.
1 issue found across 3 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Proposed changes (including videos or screenshots)
This was a tricky one to debug, as there wasn't much context on why it happened. A huge thanks for @cardoso for the work on #38200, it allowed debugging using a real server where the issue was found (open.rocket.chat), and if it weren't for that I don't think I'd have been able to patch this.
The issue is as follows:
The message bubble has 2 possibilities for retrieving the date: It can either find a Date Divider (which self-registers by using
DateListProvider, and get the information from it's dataset, or it relies on the topmost visible message if no divider matches the filtering.The issue started happening because of a fix requested on the original PR, which required that some messages to be kept on the DOM (Virtualizer's
keepMountedarray prop) in order to avoid files contained inside the message to be downloaded every time it mounted due to scrolling.The logic on the bubble date gives preference to getting the date from the date dividers, and in doing so relied on the virtualization unmounting the elementing and removing it from the list. Since the date divider is a child of the messages, if the parent message of the divider contained a file, it would never unmount the divider, thus never unregistering from the divider list, and being used as the date source since it would be the "oldest" divider on the list.
In order to fix it we now also check the divider's parent visibility relative to the scroll container. This way any divider that should not be visible will be ignored.
Issue(s)
Root cause: #40105
CORE-2248
Steps to test or reproduce
This one is tricky. We need to insert messages with old dates in order to reproduce.
Further comments
Summary by CodeRabbit