Implement selection of a single branch or author

rgeorgiev583 commented 10 months ago

Summary of the issue:

A single branch/author cannot be selected from the drop-down with a single click. Currently, a branch/author is added to the selection when clicked, so in order to deselect the previously selected branch/author, it has to be clicked as well. This is annoying when one just wants to switch to another branch/author without bothering with the "multiple selection" feature.

Description outlining how this pull request resolves the issue:

Introduce selectMultipleBranches/selectMultipleAuthors settings which allow the toggling of the "multiple selection" feature for branches/authors. When enabled, make the respective drop-down behave as before. When disabled, restrict branch/author selection to a single entry, effectively implementing switching of branches/authors. Enable both settings by default so that the previous behavior of the drop-downs would be preserved when using the default settings.

hansu commented 10 months ago

Thanks for your contribution! I like the new behaviour! I also think this is more useful in the most cases. But sometimes I also would like to select multiple branches or authors. What you you think of a selecting a single branch/author with a single click BUT allow multiple selections with CTRL+click?

I discovered one little issue directly after switching to the new behaviour: The previous selection of multiple branches remain and it's a bit unhandy to deselect them. And also there is an error popping up when clicking on "show all". A minor thing, yes, but maybe you can have a look at this:

git-graph-pr30

hansu commented 9 months ago

@rgeorgiev583 what do you think about my annotations?

rgeorgiev583 commented 9 months ago

@hansu sorry for the belated response (I've been quite busy with work).

I like your idea where a single branch is selected using a single click (thus deselecting the previously selected branch) but multiple branches are selected using Ctrl+click (each Ctrl+click would add the highlighted branch to the current selection). Implementing it could enable the removal of the "single selection" mode for branches/authors (as well as the setting for toggling of the "multiple selection" mode). I also think that implementing things this way would resolve the issues which you've discovered (which arise from the "Select Branches/Authors" features not being designed for single selection at all).

So I think that there are two ways to proceed:

I could try to fix the two issues which you've mentioned first, and once you approve the fixes and merge this PR, I could create a new branch with the "Ctrl+click" redesign of the feature (which would also remove the setting for toggling of the "multiple selection" mode);
I could directly proceed with implementing the "Ctrl+click" redesign, which would eliminate the need for fixes for the "single selection" mode.

What do you think?

hansu commented 9 months ago

No worries, I only wasn't sure if you might missed my comment.

My concern is if we switch completely to the new version with "Ctrl+click" (without a setting) that users might think the possibility to select multiple branches is gone. If users have to enable it via a setting, they probably have read about it and know how to use it. But the downside is that they might miss the new option...

rgeorgiev583 commented 9 months ago

@hansu my idea is to re-use the existing user interface and behaviour for the "multiple selection" dropdown widget (with the checkboxes) while defaulting to selection of a single branch when a dropdown entry is clicked. Unfortunately, I think that it would be a bit unintuitive that multiple selection would be performed using Ctrl+click, since checkboxes usually imply multiple selection using a single click on them. However, having a setting to toggle the "multiple selection" mode is enough of a complexity burden already, and I don't like the idea of the contributors to this repo maintaining two separate modes for each one of the "Select Branch" and "Select Author" features.

I'm fine with whatever you decide, though. I want this feature to be usable for more people than me :)

hansu commented 9 months ago

However, having a setting to toggle the "multiple selection" mode is enough of a complexity burden already, and I don't like the idea of the contributors to this repo maintaining two separate modes for each one of the "Select Branch" and "Select Author" features.

With you PR we already have that complexity with two different methods. And i don't understand what you mean with "two separate modes for each one of the "Select Branch" and "Select Author" features" Select branch and select author should always behave in the same way. So I would suggest:

Fix the little bug I mentioned
Extend your new mode by Ctrl+click (and keep the setting like it is now in your PR)

I think this will not increase the complexity much.

hansu commented 3 months ago

@rgeorgiev583 any updates on this?

rgeorgiev583 commented 3 months ago

@hansu Hi again! I was busy with things and had totally forgotten about this pull request. I'm sorry about that.

I plan on following your recommendations in https://github.com/hansu/vscode-git-graph/pull/30#issuecomment-1952173293. Let me ask a few questions to see if I have understood your remarks correctly:

Extend your new mode by Ctrl+click

You want me to implement multiple selection using Ctrl+click in the "selection of a single branch or author" mode, is that right?

keep the setting like it is now in your PR

You want me to preserve the ability to enable multiple selection using a single mouse click (which is the old behaviour), is that right?

And i don't understand what you mean with "two separate modes for each one of the "Select Branch" and "Select Author" features" Select branch and select author should always behave in the same way.

If you want the two drop-downs to always behave the same way when it comes to selection, I'd have to unify the two separate selectMultipleBranches/selectMultipleAuthors settings into a common one. Since multiple selection would be supported in both modes after my changes, and the only difference between them would be the selection mechanism ("Ctrl+click to select" vs. "single click to select"), I plan on naming the new setting selectMultipleItemsUsingCtrlClick, and disabling it by default. This way the old behaviour will be used by default but users would have the opportunity to enable selection using Ctrl+click from the configuration. Are you OK with that?

Fix the little bug I mentioned

You want me to fix the error message popping up when clicking on "Show All", is that right?

The previous selection of multiple branches remain and it's a bit unhandy to deselect them.

When I implement the Ctrl+click feature in the single selection mode, this won't be a problem anymore since multiple selection would be supported in that mode and there would be checkboxes in the dropdown.

Once we have clarified the requirements, I'll sit down to implement them during the weekend.

hansu commented 3 months ago

Thanks for your quick reaction!

You want me to implement multiple selection using Ctrl+click in the "selection of a single branch or author" mode, is that right?

Exactly!

You want me to preserve the ability to enable multiple selection using a single mouse click (which is the old behaviour), is that right?

Yes like you already have implemented.

If you want the two drop-downs to always behave the same way when it comes to selection, I'd have to unify the two separate selectMultipleBranches/selectMultipleAuthors settings into a common one. Since multiple selection would be supported in both modes after my changes, and the only difference between them would be the selection mechanism ("Ctrl+click to select" vs. "single click to select"), I plan on naming the new setting selectMultipleItemsUsingCtrlClick, and disabling it by default. This way the old behaviour will be used by default but users would have the opportunity to enable selection using Ctrl+click from the configuration. Are you OK with that?

Not sure if we should have the ability so set a different selection mode for branch filter and author filter. there are some pros and cons. I would propose a setting name like alternativeFilterSelectionMode. selectMultipleItemsUsingCtrlClick might be misleading as it offers a single selection with a simple click AND multiple selections using CTRL+click.

Fix the little bug I mentioned

You want me to fix the error message popping up when clicking on "Show All", is that right?

Yes

The previous selection of multiple branches remain and it's a bit unhandy to deselect them.

When I implement the Ctrl+click feature in the single selection mode, this won't be a problem anymore since multiple selection would be supported in that mode and there would be checkboxes in the dropdown.

Yes!

hansu commented 2 months ago

@rgeorgiev583 just a little reminder ;-)

hansu / vscode-git-graph

Implement selection of a single branch or author #30