Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement selection of a single branch or author #30

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

rgeorgiev583
Copy link

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.

…election only to a single branch when disabled.
…lection only to a single author when disabled.
@rgeorgiev583 rgeorgiev583 force-pushed the implement-selection-of-a-single-branch-or-author branch from 3bd8499 to 87482ab Compare January 23, 2024 18:33
@hansu
Copy link
Owner

hansu commented Jan 24, 2024

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
Copy link
Owner

hansu commented Feb 18, 2024

@rgeorgiev583 what do you think about my annotations?

@rgeorgiev583
Copy link
Author

@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
Copy link
Owner

hansu commented Feb 18, 2024

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
Copy link
Author

@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
Copy link
Owner

hansu commented Feb 19, 2024

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:

  1. Fix the little bug I mentioned
  2. 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
Copy link
Owner

hansu commented Aug 22, 2024

@rgeorgiev583 any updates on this?

@rgeorgiev583
Copy link
Author

@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 #30 (comment). 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
Copy link
Owner

hansu commented Aug 23, 2024

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
Copy link
Owner

hansu commented Sep 16, 2024

@rgeorgiev583 just a little reminder ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants