AdvancedFilter: Disable filters button dropdown #9807

gerhardol · 2022-01-07T23:30:33Z

Part of #9796

Proposed changes

Change Advanced filter to a split button and add "Disable filters"
to the dropdown.

To indicate that a filter is active, the "Checked" property can no longer be used.
This is indicated with a new icon instead.

Screenshots

Before

After

Test methodology

Manual

Merge strategy

I agree that the maintainer squash merge this PR (if the commit message is clear).

✒️ I contribute this code under The Developer Certificate of Origin.

gerhardol · 2022-01-11T23:22:02Z

Changing the icon when a filter is active, I believe this is good enough to indicate that there is a filter
Maybe Advanced filter need to be a dropdown too, if yyou only click the arrow to the right you may miss the actual filter...

pmiossec · 2022-01-12T01:02:28Z

What I was thinking is to have in the drop-down a way to disable the one related to the file path because it will be set every time you use the 'file history' feature.

And I find it boring to have to open the pop-up every times to disable it. And 'Disable all' could only be used if you don't have other filters that you want to keep (for example the number of commits to retrieve)

After that I let you decide if, for homogeneity, we must add every filter in the drop-down. Maybe overkill...

gerhardol · 2022-01-12T07:16:00Z

After that I let you decide if, for homogeneity, we must add every filter in the drop-down. Maybe overkill...

Adding a special for disabling path filters only.
For the other filters only Branch filter can be activated outside of this dialog and the branch filter can be disabled already in the toolbar.

RussKie · 2022-01-12T13:32:09Z

I propose the following:

Though perhaps a funnel with a cross may work better.

gerhardol · 2022-01-12T17:27:02Z

I propose the following:

Even longer toolbar, but a button with image and tooltip would be shorter.
OK for me, about the same as a tool strip button.

But @pmiossec wanted to have a special for path filters, another for all filters for which a tool strip button is preferred. If that is a requirement I prefer one button for all.

pmiossec · 2022-01-12T18:02:03Z

But @pmiossec wanted to have a special for path filters, another for all filters for which a tool strip button is preferred. If that is a requirement I prefer one button for all.

Yes, I prefer to have at least one specific for path filters because it will be set automatically quite often and so you will have to clear it often.

And that some others filters are "long time/permanent filters". For example, I sometimes use the "Since" or "Limit" filter to display less commits and so display and refresh quicker the revision grid in big repositories (as we don't always need the whole history).

I don't want to clear these "permanent filters" when I need to quickly clear the path filter.

Side note: A "since" filter expressed in number of days instead of a date will be a nice improvement for my use case....

gerhardol · 2022-01-12T18:13:34Z

I don't want to clear these "permanent filters" when I need to quickly clear the path filter.

One or two buttons for you?
With two buttons, someone need to hand edit an icon...

Side note: A "since" filter expressed in number of days instead of a date will be a nice improvement for my use case....

Side note: How big are the repos compared to Linux, how long time to open?
Do you notice much changes in latest master?
(There were some severe regressions for a while in master but the the time to open repos should be about half in master compared to 3.5.)

gerhardol · 2022-01-12T19:22:28Z

Fugue has a funnel-minus icon too, if the two button approach is preferred

pmiossec · 2022-01-12T20:46:20Z

One or two buttons for you?

Not sure but I tend to think only one. Because, with the workflow I explained with a "permanent filter" enabled it won't change anything, you will have to click twice. The difference is that "Clear all filters" will be in one click but I don't think I will use it often (except if I stop using the "permanent filter").
But if we choose to go for 2, I think the icon you found is good enough to not display text...

I prefer to test one button during some times to see if it's painful or not but that's not as if I will have to do this action as often as it become painful.

Other side note: It is for me much more painful for me to not have 2 dissociated buttons for the fetch and pull (like in all the others git GUIs) and having to click the little arrow if I want to change the action...

Side note: How big are the repos compared to Linux, how long time to open?

Not that big. Around 25k commit. But enough to feel the difference when the filter is enabled especially when my laptop is busy doing something else.

Do you notice much changes in latest master?

I didn't paid attention to that as I don't run v3.5 but my custom version based on the last commit before switch to .net5 and I didn't used enough the master version to compare....

gerhardol · 2022-01-12T21:48:53Z

Not sure but I tend to think only one.

Updated the original proposal but did not overwrite RussKies proposal but pushed to
https://github.com/gerhardol/gitextensions/pull/new/feature/i9796-adv-filter-disable-go1

RussKie · 2022-01-12T23:08:27Z

I think a strategic solution to the problem would be to use https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.toolstripcontrolhost, and move the advanced filter dialog into it. This would probably allow us to remove the quick path filter too, because the advanced filter will be as readily available. PS. I've restarted my efforts in making toolbars movable because even on a 4K screen with user scripts the toolstrip becomes unusably long.

gerhardol · 2022-01-12T23:34:04Z

I think a strategic solution to the problem would be to use https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.toolstripcontrolhost, and move the advanced filter dialog into it.

I believe my last proposal is sufficient. Maybe the icon when a filter is active can be clearer.

pmiossec · 2022-01-17T22:12:18Z

Not sure but I tend to think only one.

Updated the original proposal but did not overwrite RussKies proposal but pushed to https://github.com/gerhardol/gitextensions/pull/new/feature/i9796-adv-filter-disable-go1

What is in this branch seems good for me in terms of behavior....
Maybe one or two adjustments needed around "Disable filters": I don't know if it is on purpose but, it is not enabled when "Limit" is checked and when clicking on it it doesn't disable "Limit" also. I was expecting that it disable all the filters...

gerhardol · 2022-01-17T22:32:00Z

What is in this branch seems good for me in terms of behavior....

I will push this then

Maybe one or two adjustments needed around "Disable filters": I don't know if it is on purpose but, it is not enabled when "Limit" is checked and when clicking on it it doesn't disable "Limit" also. I was expecting that it disable all the filters...

Limit, IgnoreCase, CurrentBranchOnlyCheck is ignored in disabling and displaying tooltip.
(ShowSimplifyByDecoration was added in #9808)
I rather skip that still...

RussKie · 2022-01-18T10:29:55Z

I want to try ToolStripControlHost (I had a brush with it as part of work investigations). I think it could be an easy win.

gerhardol · 2022-01-18T23:50:48Z

Completed renaming from Disable filters to Reset filters, that RussKie started with
Updated with toolstrip for ResetPathFilter in addition to all
Changed default hotkey to ctrl-i due to #9796 (comment) @pmiossec

mstv

:+1 for the PR
but a few comments

GitUI/Hotkey/HotkeySettingsManager.cs

GitUI/UserControls/FilterToolBar.Designer.cs

mstv · 2022-01-19T20:56:02Z

GitUI/UserControls/FilterToolBar.cs

-            RevisionGridFilter.ShowRevisionFilterDialog();
+            if (!tsmiResetAllFilters.Enabled)
+            {
+                RevisionGridFilter.ShowRevisionFilterDialog();
+            }
+            else
+            {
+                tsbtnAdvancedFilter.ShowDropDown();
+            }


If I will want to edit the advanced filter often, I might not like this extra click. Though for the main use case, it saves aiming the arrow. I am not sure about this. Not requesting a change.

I expect that if you have a filter, the most likely action is to deactivate it, not modify it.
So the dropdown is shown.
The arrow is a little narrow.

GitUI/UserControls/RevisionGrid/RevisionGridControl.cs

GitUI/UserControls/FilterToolBar.Designer.cs

GitUI/Hotkey/HotkeySettingsManager.cs

gerhardol · 2022-01-20T19:39:57Z

I think a strategic solution to the problem would be to use https://docs.microsoft.com/en-us/dotnet/api/system.windows.forms.toolstripcontrolhost

@RussKie Any opinions about the PR, is it good-enough like this?

gerhardol · 2022-01-26T19:36:24Z

Squash merge tomorrow

Change Advanced filter to a split button and add "Reset filters" to the dropdown for all and path filters. * To indicate that a filter is active, the "Checked" property can no longer be used. This is indicated with a new icon instead. * Update hotkey for filter related actions Ctrl-F is also used for search, use Ctrl-I instead. Similar for disabling filters (added in master), Ctrl-Shift-F is used for searching files and is adjusted to Ctrl-Shift-I. This PR adds a hotkey for disabling the path filter, set to Ctrl-Shift-H.

A little close to ResetRevisionFilter (for all), but better grammar...

gerhardol requested a review from pmiossec January 7, 2022 23:30

ghost assigned gerhardol Jan 7, 2022

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from ed2aead to aed6b6d Compare January 7, 2022 23:34

gerhardol mentioned this pull request Jan 8, 2022

Feedback with new Blame and File history in next version #9796

Open

gerhardol marked this pull request as ready for review January 11, 2022 23:19

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from 6141000 to 2b55c11 Compare January 11, 2022 23:28

gerhardol marked this pull request as draft January 13, 2022 08:49

gerhardol marked this pull request as ready for review January 18, 2022 23:47

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from 68f972a to 247bb48 Compare January 18, 2022 23:55

mstv reviewed Jan 19, 2022

View reviewed changes

GitUI/UserControls/FilterToolBar.Designer.cs Outdated Show resolved Hide resolved

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from 2a909e9 to 66679e2 Compare January 19, 2022 23:16

mstv approved these changes Jan 19, 2022

View reviewed changes

gerhardol commented Jan 20, 2022

View reviewed changes

GitUI/Hotkey/HotkeySettingsManager.cs Outdated Show resolved Hide resolved

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from 87147cc to 4ecd969 Compare January 23, 2022 22:50

gerhardol added 2 commits January 27, 2022 21:25

fixup! ResetPathRevisionFilter -> ResetRevisionPathFilter

710493a

A little close to ResetRevisionFilter (for all), but better grammar...

gerhardol force-pushed the feature/i9796-adv-filter-disable branch from 4ecd969 to 710493a Compare January 27, 2022 20:25

gerhardol merged commit 214be05 into gitextensions:master Jan 29, 2022

ghost added this to the vNext milestone Jan 29, 2022

gerhardol deleted the feature/i9796-adv-filter-disable branch January 29, 2022 19:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

AdvancedFilter: Disable filters button dropdown #9807

AdvancedFilter: Disable filters button dropdown #9807

gerhardol commented Jan 7, 2022 •

edited

gerhardol commented Jan 11, 2022 •

edited

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

RussKie commented Jan 12, 2022

gerhardol commented Jan 12, 2022

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

gerhardol commented Jan 12, 2022

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

RussKie commented Jan 12, 2022 via email •

edited by ghost

gerhardol commented Jan 12, 2022

pmiossec commented Jan 17, 2022

gerhardol commented Jan 17, 2022

RussKie commented Jan 18, 2022

gerhardol commented Jan 18, 2022

mstv left a comment

mstv Jan 19, 2022

gerhardol Jan 19, 2022

gerhardol commented Jan 20, 2022

gerhardol commented Jan 26, 2022

AdvancedFilter: Disable filters button dropdown #9807

AdvancedFilter: Disable filters button dropdown #9807

Conversation

gerhardol commented Jan 7, 2022 • edited

Proposed changes

Screenshots

Before

After

Test methodology

Merge strategy

gerhardol commented Jan 11, 2022 • edited

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

RussKie commented Jan 12, 2022

gerhardol commented Jan 12, 2022

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

gerhardol commented Jan 12, 2022

pmiossec commented Jan 12, 2022

gerhardol commented Jan 12, 2022

RussKie commented Jan 12, 2022 via email • edited by ghost

gerhardol commented Jan 12, 2022

pmiossec commented Jan 17, 2022

gerhardol commented Jan 17, 2022

RussKie commented Jan 18, 2022

gerhardol commented Jan 18, 2022

mstv left a comment

Choose a reason for hiding this comment

mstv Jan 19, 2022

Choose a reason for hiding this comment

gerhardol Jan 19, 2022

Choose a reason for hiding this comment

gerhardol commented Jan 20, 2022

gerhardol commented Jan 26, 2022

gerhardol commented Jan 7, 2022 •

edited

gerhardol commented Jan 11, 2022 •

edited

RussKie commented Jan 12, 2022 via email •

edited by ghost