regression: Center jumped messages in virtualized list#40661
Conversation
|
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx,js}📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
**/*.spec.ts📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
Files:
🧠 Learnings (8)📚 Learning: 2026-03-27T14:52:56.865ZApplied to files:
📚 Learning: 2026-05-06T12:21:44.083ZApplied to files:
📚 Learning: 2026-02-10T16:32:42.586ZApplied to files:
📚 Learning: 2026-05-11T20:30:35.265ZApplied to files:
📚 Learning: 2026-02-24T19:22:48.358ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-02-26T19:25:44.063ZApplied to files:
📚 Learning: 2026-03-06T18:10:15.268ZApplied to files:
🔇 Additional comments (3)
WalkthroughThe PR extends the message jump-to-scroll hook to account for list offsets when preview items are present, changing the virtualizer type and adding an offset parameter that flows from MessageList through the hook to the scroll calculation. ChangesMessage list offset for centered jump scrolling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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.
Pull request overview
This PR fixes a regression in the room virtualized message list where deep-link “jump to message” could center the wrong item when the list renders a leading row (room foreword or the “load previous messages” row) ahead of the message rows.
Changes:
- Pass a rendered-list leading-item offset from
MessageListinto the jump-to-message hook. - Apply that offset when computing the target index for
virtualizer.scrollToIndex(...). - Add a unit test covering the offset case to ensure the correct rendered item is centered.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
apps/meteor/client/views/room/MessageList/MessageList.tsx |
Computes/passes the leading rendered-item offset (based on whether the list renders a top row) into the jump hook. |
apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.ts |
Adjusts the computed scroll index by the provided offset before calling scrollToIndex. |
apps/meteor/client/views/room/MessageList/hooks/useTryToJumpToMessage.spec.ts |
Adds unit coverage asserting the hook scrolls to the correct rendered index when an offset is present. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Proposed changes (including videos or screenshots)
Jumping to a message now accounts for the leading item rendered before the message rows in the virtualized message list.
That leading item is present when the room foreword or previous-message loader is shown, so using the raw message array index can center the item before the target message. The jump hook now receives the rendered-list offset from
MessageListand applies it before callingscrollToIndex.Issue(s)
Fixes #40619
Steps to test or reproduce
msgquery param.Further comments
Added unit coverage for the offset case so the virtualizer scrolls to the rendered message item instead of the raw message array index.
Summary by CodeRabbit
Bug Fixes
Tests