Make search tree context menu multiselect-aware

[andreamah]

Fixes #47166

When multiple entries are selected, if someone removes/replaces on one selected item, it happens to everything.

Note: if a whole file (uses Replace All) and entry (just uses Replace) are both selected at once and either Replace or Replace All is used, whatever replace action is valid for all selected item will run. For example, if a file and entry are all selected and the Replace button is selected on the entry, then Replace All will be run on the file and Replace will be run on the entry.

[roblourens]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

[andreamah]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

I was able to use the context menu with multiselect?

[roblourens]

Maybe a difference between using the native (on mac) vs custom (on windows) context menus. You can test with the native context menu by modifying this code:

vscode/src/vs/workbench/services/contextmenu/electron-sandbox/contextmenuService.ts

Line 49 in 6c6d447

if (!isMacintosh && getTitleBarStyle(configurationService) === 'custom') {

[joaomoreno]

Any chance we can take this opportunity and align Search will all other tree views? Ie. all other tree views open on select, but allow the user to move focus around with, for example, the keyboard. Search doesn't, as pressing DownArrow will both focus and select.

[roblourens]

all other tree views open on select, but allow the user to move focus around with, for example, the keyboard. Search doesn't, as pressing DownArrow will both focus and select.

We did this on purpose, and I think this is good in the search tree. You generally are in the search tree because you want to preview search results in the editor. Reading code in the tree view is not good, you want it in the editor. If you have to arrow, then press cmd+down to open, that's a less good experience. Actually I don't even know of a way in other trees to trigger open with the keyboard but keep the focus in the tree, so then you would have to move focus back to the tree to get to the next result. (F4 is better but a slightly different scenario, and most people don't know about it)

[roblourens]

And I notice that when I have a multiselection, then move focus around, the focused item is not revealed in an editor. I think I would probably want that same behavior here so I can still browse results while also building a selection.

[andreamah]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

@roblourens does this still happen?

[andreamah]

And I notice that when I have a multiselection, then move focus around, the focused item is not revealed in an editor. I think I would probably want that same behavior here so I can still browse results while also building a selection.

I think this is because the selection moves with focus usually, but not when you've multiselected. Do you want the editor reveal to occur with focus?

[andreamah]

And I notice that when I have a multiselection, then move focus around, the focused item is not revealed in an editor. I think I would probably want that same behavior here so I can still browse results while also building a selection.

I think this is because the selection moves with focus usually, but not when you've multiselected. Do you want the editor reveal to occur with focus?

I think that it's handled here

vscode/src/vs/workbench/contrib/search/browser/searchView.ts

Line 747 in a4f8a4e

if (options.element instanceof Match) {

but it only triggers when there is only a single selection and I'm not too sure why.

[roblourens]

I think this is because the selection moves with focus usually, but not when you've multiselected. Do you want the editor reveal to occur with focus?

I think so

[roblourens]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

@roblourens does this still happen?

Yes

[andreamah]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

@roblourens does this still happen?

Yes

Hmm, I can't replicate with the mac context menu. Do you know what else could be causing it?

[roblourens]

That issue seems to be something weird that is happening in all trees, in OSS only, so don't worry about it

[joaomoreno]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

@roblourens does this still happen?

Yes

Hmm, I can't replicate with the mac context menu. Do you know what else could be causing it?

@andreamah @roblourens Sorry for taking my time here.

This is expected behavior, and unrelated to multiple selection. You can have N elements selected and right click on another element not part of selection. When doing so, the context actions should apply to the element which was right-clicked on, not the selection. The difference between Windows and macOS is the fact that we use native menus on macOS, which steal OS focus away from the workbench window. That combined with the fact that we don't render tree focus when the workbench window doesn't have OS focus or the list itself doesn't have DOM focus, makes it appear that the element that the user has right-clicked on isn't focused which it indeed is.

Does this match what you've observed?

[andreamah]

I noticed that right-clicking on a multiselection clears the selection so that just the item I clicked on is selected. This also needs to work with the context menu

@roblourens does this still happen?

Yes

Hmm, I can't replicate with the mac context menu. Do you know what else could be causing it?

@andreamah @roblourens Sorry for taking my time here.

This is expected behavior, and unrelated to multiple selection. You can have N elements selected and right click on another element not part of selection. When doing so, the context actions should apply to the element which was right-clicked on, not the selection. The difference between Windows and macOS is the fact that we use native menus on macOS, which steal OS focus away from the workbench window. That combined with the fact that we don't render tree focus when the workbench window doesn't have OS focus or the list itself doesn't have DOM focus, makes it appear that the element that the user has right-clicked on isn't focused which it indeed is.

Does this match what you've observed?

I think one thing to add here is that Rob mentioned that he only saw this in OSS.