List: Support a different navigation mode where focus and selection move together

[joaomoreno]

• When focus === selection && size 1
• Else, navigation should only move focus

[rebornix]

I found that some of the tree is already doing that, for example the Problems View, which is an ObjectTree, will move both focus and select down when arrow down key is pressed, even if we turn on multipleSelectionSupport for it.

This is because focusDown command already has a special logic for tree: pass in a fake keyboard event

vscode/src/vs/workbench/browser/actions/listCommands.ts

Line 51 in e9533c2

tree.focusNext(count, loop, fakeKeyboardEvent);

and then openOnFocus option will decide if selection should be set to the same as focus

vscode/src/vs/platform/list/browser/listService.ts

Lines 617 to 619 in 828f99e

	if (this.openOnFocus) {
	this._register(Event.filter(this.widget.onDidChangeFocus, e => e.browserEvent instanceof KeyboardEvent)(e => this.onFocusFromKeyboard(e)));
	}

vscode/src/vs/platform/list/browser/listService.ts

Lines 631 to 633 in 828f99e

	private onFocusFromKeyboard(event: ITreeEvent<any>): void {
	const focus = this.widget.getFocus();
	this.widget.setSelection(focus, event.browserEvent);

The problem with this special logic is, shift+arrowdown doesn't work anymore, even with "multipleSelectionSupport": true. It will move the focus down and then move the selection down as well.

---

As we discussed this morning, a new flag/option will be introduced for this behavior and we should get rid of focused instanceof List || focused instanceof PagedList || focused instanceof Table checks in the list commands, and use the new option to decide what's the expected behavior. And also make sure shift+arrowdown still works.

[joaomoreno]

The new selectionNavigation list/tree/table option will make selection move along with focus, when navigation commands are executed: ie UpArrow, PageUp, Home, et al.

@roblourens @sandy081 This can now be adopted in Search and Problems and you can very likely remove some annoying code you must have created to keep both focus and selection in sync.

@rebornix When adopted, can you mark as verified?

[roblourens]

I forgot that the openOnFocus flag exists, which opens and selects items when they become focused, so I already get this behavior. Any benefit to using both flags?

[joaomoreno]

Oh I forgot about that, that's exactly what search, problems, etc are using. Let me see if I can clean up that one, I do look at it more as a hack, and adopt the new one instead.

[joaomoreno]

This is now adopted in search, peek references, bulk edit, comments and markers. Thanks @roblourens for reminding me of openOnFocus. Watch out for issues. cc @jrieken @sandy081

[sandy081]

Thanks @joaomoreno

[roblourens]

Now search doesn't open on focus anymore. Is there something else I need to do?

[joaomoreno]

Now search doesn't open on focus anymore.

What do you mean? How can I repro?

[roblourens]

Use the arrow keys to move focus around the search view tree, this is expected to fire onDidOpen

Mar-16-2021.10-52-26.mp4

[roblourens]

Oh I see, this only works if there is already a selection. If there is no selection then one is not added. I am getting here by starting with focus in the search input then pressing cmd+down to move focus to the tree. Should i work around it by havingn SearchView setting a selection when this happens, or do you expect the tree to handle this?

[joaomoreno]

I think the tree should handle it, I can fix it!

[roblourens]

Thanks!

[roblourens]

I think this is basically the same case but test F4 navigation as well: #119199

[joaomoreno]

Looking into the code, I ended up fixing it at search level. @roblourens could you maybe verify?