Repository navigation
fix(app): keyboard navigation previous/next message - #15047
Conversation
github-actions
Bot
added
needs:compliance
github-actions
Bot
removed
the
needs:compliance
|
Thanks for updating your PR! It now meets our contributing guidelines. 👍 |
There was a problem hiding this comment.
Pull request overview
Fixes incorrect keyboard navigation in the Session message timeline when starting from the live/bottom state, and ensures hash/shortcut jumps scroll messages fully into view (not hidden under the sticky session header).
Changes:
- Adjust message prev/next navigation to use the pinned
store.messageIdstate rather thanactiveMessage()fallback behavior. - Add a
data-session-titlemarker to the sticky session header container in the timeline. - Update hash/anchor scrolling to apply a sticky-header height inset when computing the scroll target position.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/app/src/pages/session/use-session-hash-scroll.ts | Applies sticky-header offset when scrolling to an element/message by hash or navigation. |
| packages/app/src/pages/session/message-timeline.tsx | Adds a data-session-title attribute to the sticky session header for scroll inset detection. |
| packages/app/src/pages/session.tsx | Fixes previous/next message keyboard navigation indexing from live/bottom state using store.messageId. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const b = root.getBoundingClientRect() | ||
| const top = a.top - b.top + root.scrollTop | ||
| const sticky = root.querySelector("[data-session-title]") | ||
| const inset = sticky instanceof HTMLElement ? sticky.offsetHeight : 0 | ||
| const top = Math.max(0, a.top - b.top + root.scrollTop - inset) | ||
| root.scrollTo({ top, behavior }) |
There was a problem hiding this comment.
The new sticky-header inset logic in scrollToElement isn’t covered by tests. Since this module already has a test file, consider adding a small DOM-based unit test (stubbing getBoundingClientRect/offsetHeight and asserting the scrollTo({ top }) value) to prevent regressions where hash/keyboard jumps land under the sticky header again.
Issue for this PR
Closes #15046
Type of change
What does this PR do?
Keyboard message navigation in Session view was faulty: from live/bottom state it could skip to the wrong user turn, and jumped messages could appear partially hidden under the sticky session header. Update navigation to use pinned message state (messageId) and apply sticky-header offset during scroll targeting so previous/next and hash jumps land on the correct, fully visible message.
How did you verify your code works?
Screenshots / recordings
If this is a UI change, please include a screenshot or recording.
2026-02-25.11-26-12.mp4
Checklist
If you do not follow this template your PR will be automatically rejected.