FindWidget/TerminalFindWidget: Provide random access to search matches via an inline "Nth Match" input field. #309109
Open
boborrob wants to merge 21 commits into
Conversation
- Proof-of-concept UI changes - Add an editable numerical input field to the findWidget that allows the user to jump to matches a la random access (jump functionality pending)
- Proof-of-concept API/functionality changes - More compliance and alignment with existing data model and class hiearchy - Misc bug fixes pending - Cleanup pending
- Better file and class names - Better alignment with findController and findModel APIs - Register a new `MoveToEditableNthMatch ` EditorAction - Add More key event handlers (not finished) - Enforce width of the input box - Don't recreate the input on every update. - Consider adding scroll event listener for mouse-driven step functionality - Cleanup still pending
- Restore 'No Results' error message when the input search string yields 0 matches. - Fallback to default text input element behavior. - Add up/down arrow key support for step behavior native to numerical input elements. - Remove unnecessary comments and scar tissue.
…get . - UI/API proof-of-concept. - Build, link and troubleshoot a local build of @xterm node_module, modified to support nth match. - Before fixing focus-retention issue of NthMatchInput in terminal simpleFindWidget
- Fix focus-retention issue - Use the incumbent dom.FocusTracker to handle focus and blur events
- Fix the focus-retention issue. - Use the incumbent dom.FocusTracker to handle focus and blur events.
- Combine editor and terminal changes
…to access match instances arbitrarily.
… to access matches arbitrarily.
- On input/change, update the width of `nthMatchInput` to fit the length of its value - Test the editor and terminal finders at their pre-defined `MATCHES_LIMIT` of `19999` matches.
Author
@microsoft-github-policy-service agree |
- `findWidget`: Refer to `nthMatchInput`\'s `domNode` like the surrounding `focusTracker`s do with their respective controls. - `terminalFindWidget`: Investigate coupling of `themeService.onDidthemeColorChange` events to the `find(previous: boolean)` call . The normal `findWidget` has no such coupling. Is this intentional?
Contributor
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @kycutlerMatched files:
@jrualesMatched files:
|
… normalize placeholder text
- Consolidate and reuse input sanitization logic. - For ease of use, update the match highlight immediately when the input changes (any keydown event, as opossed to an explicit Enter keydown) - Separate placeholder text from tooltip text. - Mitigate coupling of terminal's themeChange event to find(...) call.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds an inline “Nth Match” input to the find widgets so users can jump directly to a specific match index, extending similar functionality to the terminal’s find experience (via xterm search support).
Changes:
- Introduces a reusable
NthMatchInputcontrol and wires it into the editor find widget andSimpleFindWidget-based find widgets. - Adds a new editor action to move to the match specified by the inline Nth-match input.
- Extends terminal/xterm search plumbing to support jumping to the Nth match and adds related terminal context keys/command IDs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/contrib/webview/browser/webviewFindWidget.ts | Adds required findNth/focus-tracker overrides for SimpleFindWidget (currently unimplemented). |
| src/vs/workbench/contrib/terminalContrib/find/common/terminal.find.ts | Adds FindNth command id and includes it in skip-shell list. |
| src/vs/workbench/contrib/terminalContrib/find/browser/terminalFindWidget.ts | Implements terminal-side findNth, wires nth-match focus context key, and refreshes decoration on theme changes. |
| src/vs/workbench/contrib/terminal/common/terminalContextKey.ts | Adds a terminal context key for nth-match input focus. |
| src/vs/workbench/contrib/terminal/browser/xterm/xtermTerminal.ts | Adds findNth API to the xterm terminal wrapper. |
| src/vs/workbench/contrib/terminal/browser/terminal.ts | Extends search options with nthMatchPosition, adds IXtermTerminal.findNth, adjusts highlight limit constant. |
| src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts | Adds inline Nth-match input to SimpleFindWidget result-count UI and hooks input events to navigation. |
| src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.css | Updates matches-count layout to accommodate an inline input. |
| src/vs/workbench/contrib/browserView/electron-browser/features/browserEditorFindFeature.ts | Adds required SimpleFindWidget overrides for browser find widget (currently unimplemented). |
| src/vs/editor/contrib/find/browser/findWidget.ts | Adds inline Nth-match input to the editor find widget matches-count UI and focus tracking. |
| src/vs/editor/contrib/find/browser/findModel.ts | Adds a new context key and action ID for editable nth-match navigation; adjusts focused context key naming. |
| src/vs/editor/contrib/find/browser/findController.ts | Implements and registers MoveToEditableNthMatchFindAction. |
| src/vs/base/browser/ui/findinput/nthMatchInput.ts | Introduces the NthMatchInput control and sanitization helper. |
| src/vs/base/browser/ui/findinput/nthMatchInput.css | Styles the inline nth-match input for editor/terminal usage. |
Comments suppressed due to low confidence (1)
src/vs/workbench/contrib/codeEditor/browser/find/simpleFindWidget.ts:463
- In the "results exist" branch of
updateResultCount,labelis never assigned (it stays''), but it is still passed into_announceSearchResults. This will produce an empty/incorrect ARIA announcement like " found for '…'". Please setlabelto the localized matches label (or another meaningful string) whencount.resultCount > 0.
const showRedOutline = (this.inputValue.length > 0 && count?.resultCount === 0);
this._matchesCount.classList.toggle('no-results', showRedOutline);
let label = '';
if (count?.resultCount) {
let matchesCount: string = String(count.resultCount);
- Implement proof-of-concept. - Jump to last highlighted match on button click. - Remove nthMatchInput `onJump` event since all transformative keydown events now trigger an implicit jump. - Start localizing hard-coded strings. - Before changes for Copilot code review
- Improve language for the stepping logic in `NthMatchInput`. - Rename `nthMatchPosition` to `n`. - Simplify variable names throughout. - Localize the ` of ` phrase between the `nthMatchInput` and `lastMatchButton` widgets. - Sanitize the `matchesCount` value before consuming it. - Export `MATCHES_LIMIT` from a shared, base layer. - Remove unused, nullable properties from `INthMatchInputOptions`. - Remove redundant css properties from `simpleFindWidget.css` - Use distinct css selectors for editor and terminal `NthMatchInput` widgets. - Pass N as an arg to `MoveToNthMatchFindAction` instead of querying the DOM. - Clear editor match decorations when search fails outright. - Remove "find nth" features from `WebViewFindWidget` and `BrowserEditorFindFeature` , and restore legacy find behavior by extending a `SimpleWebFindWidget` class instead of the enhanced `SimpleFindWidget`. - Roll `MoveToLastMatchFindAction` into `MoveToNthMatchFindAction` since the former is just a special case where N == `matchesCount` | `MATCHES_LIMIT`. - Add tests for Nth Match in the `findController` test suite. - Before implementing "find nth" in `NotebookFindWidget`
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
Allow users to access search results arbitrarily using an inline input field in the
findWidgetandterminalFindWidgetcomponents.Motivation:
For large sets of search results, cycling through matches sequentially can be cumbersome and time-consuming, especially when the relative location of the desired match is already known with some certainty. An input allows the user to jump arbitrarily to the desired match or to at least get closer to it much faster.
The editor exhibits a similar behavior already, but the means for invoking it are buried in the
quickInputservice. With this change, the functionality is presented upfront with no real cost to the existing UX. The information presented to the user at the outset is the same. Except now, there's an an inline "knob" the user can turn for more granular control over the search experience.The terminal editor benefits from this change too, and since it's an upstream dependency (xterm.js), the change is implemented there as well. See this PR for details.
Example Usages:
Future Enhancements:
- Jump to the last match when theDonematchesCountwidget is clicked.