refactor: share the find input toggle navigation - #329128
Merged
Raymond Zhao (rzhao271) merged 2 commits intoAug 6, 2026
Merged
refactor: share the find input toggle navigation#329128Raymond Zhao (rzhao271) merged 2 commits into
Raymond Zhao (rzhao271) merged 2 commits into
Conversation
`FindInput` and `ReplaceInput` each carried their own copy of the arrow-key/escape handling used to move focus between the toggles of a find input. The two blocks were identical apart from how the toggle elements are collected. Extract the block into `navigateToggles` in findInputToggles.ts and call it from both widgets. The toggles are passed as a callback so that they keep being resolved only once one of the handled keys is pressed, as `FindInput` did before. Fixes microsoft#199892 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors find/replace toggle keyboard navigation into a shared helper, resolving #199892 without changing behavior.
Changes:
- Extracts arrow-key and Escape handling into
navigateToggles. - Reuses the helper in
FindInputandReplaceInput. - Adds focused tests for navigation, focus restoration, and lazy toggle resolution.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/vs/base/browser/ui/findinput/findInputToggles.ts |
Adds the shared navigation helper. |
src/vs/base/browser/ui/findinput/findInput.ts |
Uses the shared helper for find toggles. |
src/vs/base/browser/ui/findinput/replaceInput.ts |
Uses the shared helper for Preserve Case. |
src/vs/base/test/browser/ui/findinput/findInputToggles.test.ts |
Tests navigation and lazy resolution. |
Contributor
Author
|
@microsoft-github-policy-service agree |
Raymond Zhao (rzhao271)
approved these changes
Aug 5, 2026
Dmitriy Vasyura (dmitrivMS)
approved these changes
Aug 5, 2026
roblourens
approved these changes
Aug 5, 2026
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.
Fixes #199892
FindInputandReplaceInputeach carried their own copy of the arrow-key /Escapehandling that moves focus between the toggles of a find input. The two blocks were identical apart from how the toggle elements are collected —FindInputresolves them throughgetToggleDomNodes(),ReplaceInputcaptures[this.preserveCase.domNode]up front.This extracts the block into
navigateTogglesinfindInputToggles.tsand calls it from both widgets. The extracted body is unchanged from the code that was removed; onlythis.getToggleDomNodes(),this.domNodeandthis.inputBox.focus()became parameters, so the diff should read as a straight move.One detail worth calling out: the toggles are passed as a callback rather than as an array.
FindInputonly calledgetToggleDomNodes()after the key check, so passing the array eagerly would have resolved the toggles on every keystroke in the widget. The callback keeps the original laziness, and there is a test covering it.How to test
Open the editor find widget (
Ctrl+F), move focus into the toggles (Match Case / Match Whole Word / Use Regular Expression) and useLeft/Rightto move between them andEscapeto go back to the input. The replace input's Preserve Case toggle behaves the same way.Verification
Added
src/vs/base/test/browser/ui/findinput/findInputToggles.test.tscovering wrap-around in both directions,Escapereturning focus to the input, focus outside the toggles being ignored, and the lazy resolution described above.tsc --noEmit -p src/tsconfig.jsonandeslinton the touched files are both clean.I was not able to run a full desktop build locally — the native modules need the Spectre-mitigated MSVC libraries, which my toolchain is missing — so the keyboard behaviour above is covered by the unit tests rather than by a manual pass in a built VS Code.