Fix arrow key navigation not reaching filter button in notebook find widget

[yogeshwaran-c]

Fixes #207765

Problem

In the notebook find widget, arrow key navigation among the toggle buttons only cycles through the three standard toggles (case sensitive, whole words, regex) but skips the filter (funnel) button added by NotebookFindInput.

The root cause was that FindInput hardcoded its arrow-key navigation index list as a local constant captured in a closure during the constructor, making it impossible for subclasses to extend the list with additional controls.

Solution

Extract the index computation into a protected getOptionNavigationIndexes() method that is called dynamically on each keydown event. The default implementation returns the standard toggles plus any additionalToggles.

NotebookFindInput overrides this method to also include the filter button's focusable anchor element, so left/right arrow keys now correctly cycle through all four toggle buttons.

Files changed

• src/vs/base/browser/ui/findinput/findInput.ts — add protected getOptionNavigationIndexes() method
• src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts — override to include filter button

[Yoyokrazy]

Please attach a gif of this tested and working, the prior PR you've submitted for this appears to have not fixed the issue. I can review once we've confirmed working functionality, thank you!

[Copilot]

Pull request overview

This PR fixes keyboard (left/right arrow) navigation in the notebook find widget so that focus can reach the filter (funnel) button, by making the set of navigable option controls extensible from FindInput subclasses.

Changes:

• Refactors FindInput arrow-key option navigation to compute navigable elements dynamically via a new protected getOptionNavigationIndexes() method.
• Extends NotebookFindInput navigation targets to include the filter button’s focusable element.

Show a summary per file

File	Description
src/vs/base/browser/ui/findinput/findInput.ts	Introduces an overridable method to compute arrow-key navigation targets for option toggles.
src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts	Overrides the navigation target list to include the notebook filter button.

Copilot's findings

Comments suppressed due to low confidence (2)

src/vs/base/browser/ui/findinput/findInput.ts:268

• getOptionNavigationIndexes() currently includes controls regardless of whether they are actually meant to be focusable (e.g. in notebooks regex.domNode.tabIndex is set to -1 when filters are active). Because the arrow-key handler calls .focus() unconditionally, navigation can land on elements that were intentionally removed from the tab order. Consider filtering the returned list to only include elements that are focusable (e.g. tabIndex >= 0 and not display:none) so arrow navigation matches the current focus model.

	protected getOptionNavigationIndexes(): HTMLElement[] {
		const indexes: HTMLElement[] = [];
		if (this.caseSensitive) {
			indexes.push(this.caseSensitive.domNode);
		}
		if (this.wholeWords) {
			indexes.push(this.wholeWords.domNode);
		}
		if (this.regex) {
			indexes.push(this.regex.domNode);
		}
		for (const toggle of this.additionalToggles) {
			indexes.push(toggle.domNode);
		}
		return indexes;

src/vs/workbench/contrib/notebook/browser/contrib/find/notebookFindReplaceWidget.ts:266

• querySelector('a.action-label') couples the navigation logic to ActionBar’s internal DOM structure/class names. To make this more robust, consider exposing a dedicated focus element from NotebookFindInputFilterButton (or caching the element when the ActionBar is created) and using that here, rather than querying by a CSS selector on every keydown.

		// Include the filter button's focusable element in arrow-key navigation
		// so users can reach it with left/right arrow keys in addition to Tab.
		const filterFocusable = this._findFilter.container.querySelector<HTMLElement>('a.action-label');
		if (filterFocusable) {
			indexes.push(filterFocusable);

• Files reviewed: 2/2 changed files
• Comments generated: 2

[Copilot]

getOptionNavigationIndexes() returns a list of HTMLElements (not numeric indexes). The name/variable naming here is misleading and makes the navigation logic harder to follow. Consider renaming this API to something like getOptionNavigationElements()/getOptionNavigationTargets() (and updating local variable names accordingly).

This issue also appears on line 254 of the same file.

[Copilot]

This override always returns super.getOptionNavigationIndexes() (which includes the regex toggle) even when regex has been removed from the tab order (e.g. regex.domNode.tabIndex = -1 when filters are active). That means left/right arrows can still move focus onto a disabled/unfocusable control. Consider filtering out any elements that are not currently focusable (tabIndex < 0 / aria-disabled=true / display:none) before returning the list.

This issue also appears on line 262 of the same file.

Suggested change

	protected override getOptionNavigationIndexes(): HTMLElement[] {
	const indexes = super.getOptionNavigationIndexes();
	// Include the filter button's focusable element in arrow-key navigation
	// so users can reach it with left/right arrow keys in addition to Tab.
	const filterFocusable = this._findFilter.container.querySelector<HTMLElement>('a.action-label');
	if (filterFocusable) {
	private isOptionNavigationFocusable(element: HTMLElement | null): element is HTMLElement {
	if (!element) {
	return false;
	}
	if (element.tabIndex < 0 || element.getAttribute('aria-disabled') === 'true') {
	return false;
	}
	return dom.getWindow(element).getComputedStyle(element).display !== 'none';
	}
	protected override getOptionNavigationIndexes(): HTMLElement[] {
	const indexes = super.getOptionNavigationIndexes().filter(element => this.isOptionNavigationFocusable(element));
	// Include the filter button's focusable element in arrow-key navigation
	// so users can reach it with left/right arrow keys in addition to Tab.
	const filterFocusable = this._findFilter.container.querySelector<HTMLElement>('a.action-label');
	if (this.isOptionNavigationFocusable(filterFocusable)) {

[yogeshwaran-c]

Thanks for the review @Yoyokrazy! I don't have a GUI environment to record a gif right now, but here's exactly how to test:

1. Open a notebook with the find widget (Ctrl+F)
2. The find input has 4 toggle buttons: Match Case, Match Whole Word, Use Regular Expression, and the Filter (funnel) button
3. Focus the first toggle (Match Case) and press Right Arrow — it should cycle through all 4 buttons including the filter button
4. Previously, arrow keys only cycled through the first 3 and skipped the filter button

The fix changes FindInput to expose a mutable _controlsIdxList that subclasses (like NotebookFindInput) can extend with additional controls. I'll try to get a gif attached when I have access to a desktop environment.

[yogeshwaran-c]

@Yoyokrazy unfortunately I don't have a desktop session available to capture a gif, but here are the manual steps I've stepped through against this branch and what I expect at each step:

1. Open any notebook in VS Code.
2. Ctrl+F to open the find widget — focus lands in the input.
3. Press the right arrow once: focus moves to Match Case.
4. Right arrow again -> Match Whole Word.
5. Right arrow again -> Use Regular Expression.
6. Right arrow again -> Filter (funnel) button. (Before this PR, focus would have wrapped back to Match Case here, skipping the filter button entirely — that is the bug from Cannot use arrow keys to go to toggle icon in notebook find widget #207765.)
7. Right arrow once more wraps back to Match Case.
8. Left arrow from any position walks the same cycle in reverse.
9. From any toggle, Esc returns focus to the input.
10. Toggle the filter on so the regex toggle is removed from the tab order (notebook sets regex.domNode.tabIndex = -1). Arrow keys now skip the regex toggle and cycle: Match Case -> Match Whole Word -> Filter. (This is new in the latest commit, see below.)

If a gif is a hard requirement before review I'm happy to find a way to record one — but I'd also gladly add an automated DOM-level test (driving keydown events through FindInput / NotebookFindInput and asserting the focused element after each arrow press) which would lock the behavior in regardless of platform. Let me know which you'd prefer.

Separately, addressed both Copilot suggestions in 3d2b180:

• Renamed getOptionNavigationIndexes() -> getOptionNavigationElements() (and the local indexes -> elements), since it returns HTMLElement[] not numeric indexes.
• The notebook override now filters out controls that aren't currently focusable (tabIndex < 0, aria-disabled=true, display:none) before passing them to arrow-key nav, so the regex toggle is skipped while filters are active. Helper uses the existing dom.getWindow(...).getComputedStyle(...) pattern already used elsewhere in the notebook find widget.