feat: Add jump-to-user-prompts navigation feature #9832
Conversation
- Add UserPromptNavigation component with up/down navigation buttons - Integrate navigation buttons near scroll-to-bottom button - Add keyboard shortcuts (Alt+Up/Down) for quick navigation - Add visual highlight animation when jumping to prompts - Add position indicator showing current prompt (e.g., "2 of 5") - Add comprehensive tests for the navigation feature - Add translation keys for new UI elements Closes #9825
Review complete. Found 3 issues that should be addressed:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // Briefly highlight the message after scrolling | ||
| setTimeout(() => { | ||
| const element = document.querySelector(`[data-message-index="${messageIndex}"]`) | ||
| if (element) { | ||
| element.classList.add("prompt-highlight") | ||
| setTimeout(() => { | ||
| element.classList.remove("prompt-highlight") | ||
| }, 1500) | ||
| } | ||
| }, 500) | ||
|
|
||
| // Clear navigation state after a delay | ||
| setTimeout(() => { | ||
| setIsNavigating(false) | ||
| }, 3000) |
There was a problem hiding this comment.
Potential memory leak: The setTimeout calls for highlighting and clearing navigation state are not cleaned up if the component unmounts. If a user navigates and then quickly switches tasks or closes the view, these timeouts will still fire and attempt to access DOM elements that may no longer exist, potentially causing errors or memory leaks. Consider storing timeout IDs in refs and cleaning them up in a useEffect cleanup function.
Fix it with Roo Code or mention @roomote and request a fix.
| // Reset navigation when messages change significantly | ||
| useEffect(() => { | ||
| if (!isNavigating) { | ||
| setCurrentPromptIndex(-1) | ||
| } | ||
| }, [messages.length, isNavigating]) |
There was a problem hiding this comment.
The messages prop is received but only used in a useEffect dependency to reset navigation state. The actual filtering and navigation logic uses visibleMessages. This creates ambiguity about which array is the source of truth. Consider removing the messages prop if it's not needed, or document why both are necessary. If messages.length changes but visibleMessages doesn't (or vice versa), the reset logic might not work as expected.
Fix it with Roo Code or mention @roomote and request a fix.
hannesrudolph
added
the
Issue/PR - Triage
| // Clear navigation state after a delay | ||
| setTimeout(() => { | ||
| setIsNavigating(false) | ||
| }, 3000) |
There was a problem hiding this comment.
Race condition risk: If users click navigation buttons rapidly, multiple setTimeout callbacks will be scheduled to set isNavigating to false after 3 seconds. This could cause the navigation state to be cleared prematurely or behave unpredictably. Consider using a ref to track the timeout ID and clearing it before scheduling a new one, similar to how the highlight timeouts should be managed.
Fix it with Roo Code or mention @roomote and request a fix.
This PR attempts to address Issue #9825 by implementing a navigation feature that allows users to quickly jump between their prompts in the chat history.
Summary
Implements the jump-to-prompt navigation feature as outlined in the approved plan. Users can now easily navigate through their conversation history using dedicated navigation buttons or keyboard shortcuts.
Changes
Features Added
Technical Implementation
UserPromptNavigationcomponent with navigation logicChatViewlayout near existing scroll controlsUser Experience
Testing
Screenshots/Demo
The navigation buttons appear near the scroll-to-bottom button when user prompts are available in the chat history.
Feedback and guidance are welcome!
Closes #9825