ui/ux feat: chat search w/ highlight + jump #3144
Conversation
|
dosubot
bot
added
size:XXL
|
The pull request introduces a comprehensive chat search feature with highlighting and navigation capabilities across multiple components. Given the cohesive nature of the changes, it seems appropriate to keep them within a single pull request. However, if there are any unrelated changes that could be split into separate pull requests, please consider doing so for better manageability and review efficiency. Thank you for your contributions! |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| messages, | ||
| virtuosoRef, | ||
| getSearchableText, | ||
| // disableAutoScrollRef, // Removed unused prop |
There was a problem hiding this comment.
The disableAutoScrollRef prop is defined in the interface but is commented out in the hook implementation; if it’s meant to control auto-scroll behavior during search, please integrate it properly or remove it from the interface.
This comment was generated because it violated a code review rule: mrule_aQsEnH8jWdOfHq2Z.
| if (isCollapsing && isAtBottom) { | ||
| const timer = setTimeout(() => scrollToBottomAuto(), 0) | ||
| const timer = setTimeout(() => { | ||
| console.log( |
There was a problem hiding this comment.
There are several debug console.log statements in functions like toggleRowExpansion and handleRowHeightChange; consider removing or conditionally disabling these logs in production for cleaner output.
|
oh! Can |
definitely can. one idea is we could handle keyboard shortcuts by a one-click link the vs code preferences>keyboardshortcuts>prefilled search term (tested working). Then it's easy for users to use vs code inherent keyboard shortcut handling. |
|
I like this, but I really do think the icon positioning doesn't work for me. I'd probably be up for making this a keyboard-only shortcut for now, but I'm hesitant to keep crowding the visual UI (for now). |
I agree with that: It should be hidden until someone presses CTRL-F, and then it also needs to grab the focus. |
|
Hey @hannesrudolph do we have a "needs changes" status yet? |
I agree keyboard only is best, no need to crowd. (I'd still remove the extra new task button if not replacing) |
We do now @sachasayan !! |
hannesrudolph
moved this from PR [Pre Approval Review]
to PR [Draft/WIP]
in Roo Code Roadmap
|
Hey @dlab-anton, |
github-project-automation
bot
moved this from PR [Draft/WIP]
to Done
in Roo Code Roadmap
|
@dlab-anton Sorry about the premature close. My fault! Let me see if we can get this approved. |
hannesrudolph
moved this from PR [Changes Requested]
to PR [Needs Review]
in Roo Code Roadmap
|
@mrubens can you take a look at this feature and if its ok to move forwad with it I will ask @dlab-anton to rebase it and deal witht he conflicts. Otherwise there is no point. |
|
I like the idea of this feature, but I do think we need to find the right balance of code complexity and visual prominence to make it worth it. I think if we launch it with no UI the people who need it won't find it. My instinct is to do something where we put it in the expanded view of the task header, maybe next to the "download" button, along with a keyboard shortcut. 
That said, from a code review perspective this PR touches a ton of places in the code. Is there a way to do this more at the webview level so we wouldn't need to modify so many parts of the code? |
hannesrudolph
moved this from PR [Needs Review]
to PR [Draft / In Progress]
in Roo Code Roadmap
|
Are there any updates regarding this? |
Generally speaking, one feature per pull request unless they are strongly related, or if separating them would create unnecessarily merge conflicts to break them apart. Can they be cleanly separated or implemented as either independent, dependent, or follow-up pull requests? |
|
I didnt really check 3144 yet, but TaskTimeline feature moves some codeblocks within ChatView that may be relevant when merging |
|
stale |
github-project-automation
bot
moved this from PR [Draft / In Progress]
to Done
in Roo Code Roadmap
7 participants
Context
Tasks can get long, restore points are hard to find, if you lose your place in a long task you waste a lot of time trying to find it. All this takes away time, causes frustration. There are many possible solutions from minimaps to timeline panels, annotated scrollbars, etc. but the best solution is simple text search.
Search is a "need to have" fundamental usability feature for the chat interface.
Implementation
Note I replaced the X button (since the + button has the same function) but we can also hide the icon and rely on keyboard shortcut, move the icon somewhere else.
This PR introduces a comprehensive search functionality within the chat view and resolves minor linting issues.
Key Features & Changes:
In-Chat Search:
TaskHeaderor using theCtrl/Cmd + Fkeyboard shortcut.<mark>tags.Implementation Details:
useChatSearchto encapsulate search logic, state management, navigation, and the core highlighting function.useChatSearchintoChatView, passing down necessary props (searchText,highlightText,itemIndex) to child components.getSearchableTextForChatViewinChatViewto extract rich, searchable text content from various message types and structures. This includes a lookahead mechanism to associate API response text with its corresponding request row for better searchability.ChatView's scrolling logic (handleRowHeightChange,useEffecton message length,toggleRowExpansion) to respect the search state (showSearch,isNavigatingRef) and prevent unwanted auto-scrolling.TaskHeaderto include the search toggle button and manage its state.scrollIntoViewAPI within theuseChatSearchhook for efficient scrolling to matched items.ChatRow,BrowserSessionRow,CheckpointSaved,FollowUpSuggest,CodeAccordian(header): Use thehighlightTextfunction prop directly.MarkdownBlock: Uses a separateapplyHighlightingutility function that traverses the React node tree generated byreact-remark.CodeBlock: Uses a custom Shiki transformer (createSearchHighlightTransformer) that operates on the code's AST during syntax highlighting.