Implement keyboard navigation between list find/filter matches

[gjsjohnmurray]

This PR is a response to #178588 but I think the feature it adds has wider uses.

When using the Ctrl+F in find-mode on a list or tree, matches (which may or may not be fuzzy) get highlighted but non-matching rows remain visible. In filter-mode non-matching rows are hidden unless they represent folders whose contents have not yet been fully resolved.

In both cases it would be useful to navigate quickly between the matches. This PR adds four new commands with default keybindings:

• list.focusNextMatchDown bound to Alt+DownArrow
• list.focusNextMatchUp bound to Alt+UpArrow
• list.focusFirstMatch bound to Alt+Home
• list.focusLastMatch bound to Alt+End

Screen recording:

[gjsjohnmurray]

I tried to avoid using the any type here and in three subsequent occurrences but couldn't work out a better one. A TS expert would probably know what to do.

[ADTC]

This is awesome! I hope it will be merged soon.

[gjsjohnmurray]

Pinging @lramos15 because though this applies to all trees it is particularly relevant in Explorer and I would like it to be considered for inclusion in 1.78 before endgame starts next week.

[lramos15]

Pinging @lramos15 because though this applies to all trees it is particularly relevant in Explorer and I would like it to be considered for inclusion in 1.78 before endgame starts next week.

This one is a bit more involved so let's await @joaomoreno's initial assessment and I will provide a secondary review after the fact 👍

[gjsjohnmurray]

@lramos15 ahead of this receiving attention from @joaomoreno are you willing to add it to the April milestone?

[lramos15]

@lramos15 ahead of this receiving attention from @joaomoreno are you willing to add it to the April milestone?

That will have to be his call as the list / tree widget is his feature.

[joaomoreno]

@gjsjohnmurray This already works as expected in non-filter mode. This makes me think that adding new commands is overkill. Are you sure they are needed?

[gjsjohnmurray]

@gjsjohnmurray This already works as expected in non-filter mode.

@joaomoreno I hadn't previously noticed that. I think the last OR condition here is what makes filter mode behave differently:

vscode/src/vs/base/browser/ui/tree/abstractTree.ts

Lines 1149 to 1152 in aebaec6

	shouldAllowFocus(node: ITreeNode<T, TFilterData>): boolean {
	if (!this.widget || !this.pattern || this._mode === TreeFindMode.Filter) {
	return true;
	}

Prior to filter mode retaining unmatching unresolved folders that condition made sense because the list would only contain matches. But now it means we no longer have a guaranteed way to navigate by keyboard directly between matches.

Conversely, in non-filter mode it's not currently always possible to navigate by keyboard onto an unmatching unresolved folder in order to expand them and discover if they contain matches, unless you first close the find widget (Esc), navigate normally with Home/UpArrow/DownArrow/End, do your expansion with RightArrow, then Ctrl+F to reinstate the Find. In some cases you can get where you want by exploiting the oddity that PageUp/PageDown can get you onto an unmatching item if you press them twice.

If we remove that last OR condition we'll give filter mode the same problems, but at least things will be consistent.

Perhaps we should also add variants of list.focusDown/First/Last/Up and bind them as Alt-modified equivalents which behave as though the find widget was closed. Possible naming, list.focusDownUnfiltered etc.

[gjsjohnmurray]

@meganrogge can this Help in relation to #180221?

[joaomoreno]

Hi @gjsjohnmurray! First of all, thanks for your patience. Second, thanks for your passion and contributions!

After reasoning about this all over again I come to the conclusion that navigation should work the same way on both modes. I also argue that the default navigation should be to only navigate to the highlighted nodes. The other navigation, using Alt, would allow the user to navigate across all visible elements. Again, I argue this should be the same on both highlight and filter mode. This means I am in favor of reverting the default navigation mode when in filter mode back to what it was, but we would at least keep consistency. I hope you agree this is a better path forward.

I've updated this PR to reflect this.

Screen.Recording.2023-12-19.at.16.11.42.mov

Apart from getting consistent behavior, this solution also allows us to clear up the type strangeness from the code.

Again, many thanks! 🍻