Treeview with multiselect mishandles right-click outside the selection

[eamodio]

I've been adding multi-select support to GitLens treeview and noticed a few issues with tree views with canSelectMany enabled

So in that image, I have a group of commits selected, and then right-clicked a commit outside that group. And in that case, VS Code leaves the original selection in place (unlike if you left-click), but doesn't change the listMultiSelection context key (which besides listDoubleSelection is the only content key that really can be used to shape a content menu for multi-select) and also when a command is executed doesn't provide the selected items, only the item you right-clicked on.

Ideally VS Code would either:

• Clear the previous selection (as if you left-clicked on the item) and only have the right-clicked item selected OR
• Change listMultiSelection to be false so that the content will match how the commands will get executed.

/cc @alexr00

[alexr00]

I think instead it should keep the selection, then include the node that was right-clicked-on as part of the selection. Does that seem reasonable?

Regarding how to shape the context menu: with https://github.com/microsoft/vscode/pull/210754, I've made it so only command that apply to all the selected nodes (and the one you right clicked on) are included in the context menu.

[alexr00]

Hmm, actually, after looking at how the Explorer view works I've changed my mind:

• keep the multi select
• when the command is executed use the selection, not the right click target.

[alexr00]

and also when a command is executed doesn't provide the selected items, only the item you right-clicked on.

Will fix this bug.

[alexr00]

Behavior when right click is done in a tree where there's a selection:

• First argument is always the right click target
• Second argument is always the selection array, even when the right click target isn't part of the selection
• Context menu options are scoped to the selection, so if the right click target isn't part of the selection then it's context menu actions will not be included.

[justschen]

I believe i verified the original issue, but there is some weirdness on selection using shift to select multiple items in the tree view. i don't know if this is a big issue tho 🔢 https://github.com/microsoft/vscode/assets/54879025/1052625a-aff6-49fc-bb07-8d8958b976ad

[alexr00]

Looks like https://github.com/microsoft/vscode/issues/208802, I'll see if I can repo using the steps you followed!

[eamodio]

So the experience here will be, if I have 1-3 rows selected, and then I right click on row 5 -- the context menus will behave as I clicked rows 1-3? This seems very confusing from a user point of view.

Both Mac (finder) and Windows (explorer) handle this differently, but they are consistent that if you right-click outside of the selection you get actions related to the item you right-clicked on. Windows clears the previous selection, so that the only thing is the right-clicked item, while Mac leaves the previous selection.

IMO VS Code should do one of those 2 things -- either clear the selection and the menu options will be provided for the single right-clicked item OR leave the selection intact but still only provide menu options for the single right-clicked item.

[alexr00]

Discussed during standup, and the proper behavior should be:

• When the right click occurs within the selection, show context menu actions that apply only to the entire selection. The command should get the selection as an argument.
• When the right click occurs outside the selection, show only context menu actions for the right click target. The command should not receive the selection as an argument.

[alexr00]

I've reopened this as the question of listMultiSelection has not been addressed. @eamodio is this as important now that for multi-select + right click in the selection the context menu only shows commands which apply to all the elements in the selection?

[eamodio]

@alexr00 not sure I'm following. The behavior of right-clicking on an item within the current selected items was correct (listMultiSelection was correct and the callback with the items was correct). But right-clicking outside the current selected items is the broken case.

[alexr00]

@eamodio above you had:

Ideally VS Code would either: Clear the previous selection (as if you left-clicked on the item) and only have the right-clicked item selected OR Change listMultiSelection to be false so that the content will match how the commands will get executed.

I haven't changed the behavior to match either of these, and I would like to continue to leave the behavior as is since visually it matches our other trees and there is still a multi selection in the tree.

You were using listMultiSelection as a context to determine what needs to go in a context menu. With https://github.com/microsoft/vscode/pull/210754, we only show the menu items that apply to the selection (or if you right click outside the selection, then the context menu items that apply to the right click target). I'm wondering if you still care about clearing the previous selection or changing listMultiSelection given https://github.com/microsoft/vscode/pull/210754.