Reorder items in View menu #10189

gerhardol · 2022-09-09T21:59:32Z

Proposed changes

Reorder the menu items to reflect how the items are used
This menu has grown without much consideration over the years. The sorting options were recently changed in #10172 to be decoupled.
Changes the muscle memory, but even I have a problem understanding the type of setting today...

Branch filter mode (reverse order to how they are prioritized)
Reflog <- current branch <- filtered branches <- All
(Except for reflog visible in Advanced filter)
Note: Any branch filter could be ticked at the same time as Reflog, previously
Reflog added to the branch filter dropdown.
Further revision filters, mostly independent to filter modes
Show merges, first parents
(Other revision filters in Advanced filter)
History simplification, how to handle history with filters
Full history, simplify merges.
(Simplify by decoration in Advanced filter handled as a revision filter despite Git description https://git-scm.com/docs/git-log#Documentation/git-log.txt---simplify-by-decoration)
Highlighting etc
Added commits in grid
Artificial commits, stash, Git notes
Labels in Grid
Remote branches, tags, superproject info
Specific details in the grid
Columns to show
Sorting
Advanced filter form

~~Submenus could be added to explain the groups, should be a separate PR~~
Added group headers to explain usage

This will require an update in the doc too (a few other changes needed as well)
https://git-extensions-documentation.readthedocs.io/en/release-4.0/browse_repository.html

There is some overlap between the Advanced filter and View menu.
All revision filters and simplify history should be moved there as well as the reflog branch revision filter option be added.
That should be a separate PR

Screenshots

Before	After

(Before displays Show stashes instead of Show latest stashes as it is from a branch, no other changes.)

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-09-10T21:30:55Z

Cleaning up the filterhandling slightly in addition to the menu (a few clarifications too)
Updated description and screenshots and listed some follow ups

mstv · 2022-09-11T10:56:32Z

👍

Perhaps we should move down options which are configured once or very rarely. I.e. move up the others - which might be changed forth and back in different usecases - resulting in e.g.:

Branch filter mode
Advanced filter form
Sorting
Highlighting etc
- Highlight selected branch
- Draw non-relatives gray
Show ...

Although the git argument is --first-parents, we should be more explicative: Show first parents only.

Reflog should probably be added to the branch filter dropdown.

I agree.

mstv · 2022-09-11T11:07:10Z

Submenus could be added to explain the groups, should be a separate PR.

Only if it does not fit on the screen, which is not the case. It is hard to find items in submenus.

There is some overlap between the Advanced filter and View menu.
All revision filters and simplify history should be moved there as well as the reflog branch revision filter option be added.
That should be a separate PR

(I think it is good to provide some of the options redundantly for quick access.)
Basically the Advanced Filter should become the superset of all filter options (in a separate PR).

gerhardol · 2022-09-11T19:30:50Z

My first priority with this PR is that it is included with next 4.0 public build.
@RussKie was to make a build next week, I want this included.
Other discussions in a follow up if they can wait.

Although the git argument is --first-parents, we should be more explicative: Show first parents only.

OK, but next commit, since I want to remove that option from this menu and that will require the translation to be changed twice.

Submenus could be added to explain the groups, should be a separate PR.

Only if it does not fit on the screen, which is not the case. It is hard to find items in submenus.

It is a way to describe what the group is doing, so it could be adding value also if the items exist.

(I think it is good to provide some of the options redundantly for quick access.)

I want to move the following to the Advanced filter form - they are not important and not really "View" either:

Show first parents (button exists too)
- Show merge commits* (Completely meaningless before this PR, quite meaningless after)
Full history (not expected to change often)
Simplify merges (dependent on the previous)

Branch revision filters are available in the toolbar as well as Advanced filter (except for reflog that is not in the form but in the toolbar even not in the dropdown).
They should probably be removed from View.

Perhaps we should move down options which are configured once or very rarely. I.e. move up the others - which might be changed forth and back in different usecases - resulting in e.g.:

Suggested order with motivation. This is what I would like to settle the most right now.
Proposed submenu names in bold.

Branch filter revision
If they are in the menu (I suggest the first three proposed groups are removed in a followup), they should be first.
Highlighting etc
Highlight selected branch
Draw non-relatives gray
OK to move this up, changing the PR (they will be on top)
*Additional commits
Artificial, stash, notes
Labels
branches and tags
Columns
Contents (Better name needed...)
Sorting
Often shown close to last in our menus
Advanced filter form
A button on the toolbar and a shortcut exists
Toolbars (as now)

mstv · 2022-09-11T20:06:29Z

Regardless of what designers think, I know that submenus give a bad UX to me. Aiming and hovering is OK for tooltips. Being forced to wait one second until info is faded in, is a real pain (e.g. Windows taskbar).
I understand your point regarding headings. Though they can be added directly:

It stand by my argument regarding the logical linkage and usage frequency of the sections.

gerhardol · 2022-09-11T20:12:55Z

Regardless of what designers think, I know that submenus give a bad UX to me. Aiming and hovering is OK for tooltips. Being forced to wait one second until info is faded in, is a real pain (e.g. Windows taskbar). I understand your point regarding headings. Though they can be added directly:

That is a hack, will consider to add to this PR still

It stand by my argument regarding the logical linkage and usage frequency of the sections.

So do I...
Pushed moving Highlight further up (not updated the screenshot yet)

gerhardol · 2022-09-11T22:18:14Z

Pushed changes after latest discussions with @mstv (not agreeing on the group order in View menu)
Follow up PR to move filter items to Advanced filter
(This was meant to be a quick fix...)

@RussKie @pmiossec
I would like to include this in next 4.0 build

RussKie · 2022-09-11T22:29:47Z

I'm currently away, but I'll try to review this in the next few days.

RussKie

I like what I see on the screenshots. Assuming these are current - 👍

pmiossec · 2022-09-12T12:28:15Z

I'm OK to have it included in v4.0.
I'm testing it and it's good from my first uses.
I don't have a strong opinion on the sorting, I nearly never go to this menu, except very rarely for one of the first entries...

The disabled titles are a good idea but I wonder if we should not add a colon at the end to make it even clearer that it's not a menu entry that you could enable it one way or the other.

Another remark on the View menu but I think it will surely be for later (because maybe it won't be possible for all the options, for example "Commit message body"): I like how the menuitems in the "Toolbars" submenu are handled where it doesn't close the View menu and you could see it applied in "live". I would love to have the same behavior for most of the "Grid Info" and "Columns" sections menuitems. It will make the use of these options much easier especially for beginners when you don't understand the effects and have to try and revert...

A consequence to get consistency if we put in place this behavior is that maybe these sections should be put in submenus.

gerhardol · 2022-09-12T13:49:10Z

I like what I see on the screenshots. Assuming these are current

Yes, updated

The disabled titles are a good idea but I wonder if we should not add a colon at the end to make it even clearer that it's not a menu entry that you could enable it one way or the other.

We do not have colons for other fake headings like in Copy, some in RevDiff
Not important for me, will add if someone agree

Another remark on the View menu but I think it will surely be for later (because maybe it won't be possible for all the options, for example "Commit message body"): I like how the menuitems in the "Toolbars" submenu are handled where it doesn't close the View menu and you could see it applied in "live". I would love to have the same behavior for most of the "Grid Info" and "Columns" sections menuitems. It will make the use of these options much easier especially for beginners when you don't understand the effects and have to try and revert...

The problem here is that the grid need to be refreshed, the required data is not calculated if not needed. Maybe columns and a few more could be handled this way, most requires to read all data from scratch.

--

Will fix the failing test, as well allow toggling with the reflog button (now it resets the branch pattern to AllBranches).

RussKie · 2022-09-13T01:51:29Z

Another remark on the View menu but I think it will surely be for later (because maybe it won't be possible for all the options, for example "Commit message body"): I like how the menuitems in the "Toolbars" submenu are handled where it doesn't close the View menu and you could see it applied in "live". I would love to have the same behavior for most of the "Grid Info" and "Columns" sections menuitems. It will make the use of these options much easier especially for beginners when you don't understand the effects and have to try and revert...

I'd be advising against any UI/UX-related changes that deviate from the standard Windows UI/UX paradigms (e.g., this is not how Windows Explorer works). In cases where a live preview would be required, we should come up with different presentation mechanisms.

RussKie · 2022-09-13T01:57:06Z

GitUI/UserControls/RevisionGrid/RevisionGridControl.cs

@@ -1724,6 +1730,20 @@ public void ShowFilteredBranches()
            PerformRefreshRevisions();
        }

+        public void ShowCurrentBranchOnly()


[nit] Can we please retain ShowCurrentBranchOnly where it was (i.e., lines:1690-...) and place ShowReflogs method here (i.e., after ShowFilteredBranches)?

I reordered them to match how they are used, but can revert.

RussKie · 2022-09-13T02:00:52Z

The menu is getting longer, and I think it's worth considering how we can separate the grid configuration related items (e.g., the column visibility) from the graph rendering. However, this is a good step. Thank you.

gerhardol · 2022-09-13T08:22:02Z

The menu is getting longer, and I think it's worth considering how we can separate the grid configuration related items (e.g., the column visibility) from the graph rendering. However, this is a good step. Thank you.

The first I would like to do is do move the four revision filter items to Advanced form, possibly the branch revision filter too. Then all revision filter handling can be moved to FilterInfo and some simplification and bug fixes (or "limitations") can be done. (Maybe even for 4.0.) #10189 (comment)
Then the size is manageable - for now.

But before merging I would like a mediation with @mstv comments in #10189 (comment)

mstv · 2022-09-13T20:27:32Z

Thank you for this PR. My intention was to avoid additional updates of the docs for follow-up PRs.

Should the headings have an italic font style as in the other menus?

What do you think about "Show first parents only"?

Sorry, yet another discussion:

ordered by amount of displayed revisions
ordered by ascending priority

gerhardol · 2022-09-13T22:20:04Z

Should the headings have an italic font style as in the other menus?

OK, implemented a further hack

What do you think about "Show first parents only"?

Fine, but I want to move the option to Advanced form (keep the button).

* ordered by amount of displayed revisions
* ordered by ascending priority

I had changed to the first alternative, not updated the screenshot, done now

pmiossec · 2022-09-13T22:28:45Z

* ordered by amount of displayed revisions
* ordered by ascending priority
I had changed to the first alternative, not updated the screenshot, done now

That brings for me another question (sorry also): if there are in fact the same options, should we also add the same icons in the view menu?

gerhardol · 2022-09-13T22:36:04Z

* ordered by amount of displayed revisions
* ordered by ascending priority
I had changed to the first alternative, not updated the screenshot, done now
That brings for me another question (sorry also): if there are in fact the same options, should we also add the same icons in the view menu?

The same options. Images could be the same, or the options removed
(better to ask before than after)

pmiossec · 2022-09-13T22:41:41Z

The same options. Images could be the same, or the options removed
(better to ask before than after)

I prefer adding the icons. I imagine that some users are used to go in the view menus to toggle it (even if I find the toolbar more convenient). But let's see what the others think about it...

mstv

+1 after updating translation https://ci.appveyor.com/api/buildjobs/joyxw2k74hpbihte/artifacts/English.xlf

gerhardol · 2022-09-14T20:07:57Z

Changed the "Group headers in italics" to use some more common code from an existing class
Added images to branch filters
Updated screenshot

Time to squash?

pmiossec · 2022-09-12T14:46:31Z

GitUI/UserControls/FilterToolBar.cs

@@ -24,9 +24,11 @@ internal partial class FilterToolBar : ToolStripEx
        public FilterToolBar()
        {
            InitializeComponent();
+            tsmiShowReflogs.ToolTipText = TranslatedStrings.ShowReflog;


Tooltip has been renamed to remove the 's' so maybe remove it in the names tsmiShowReflog for consistency (here and everywhere else)?

The Git command is "reflog" but that doc is stating Reference logs, or "reflogs"
Maybe add it instead?
Should be consistent
https://git-scm.com/docs/git-reflog

Google for "git reflog" 2.6M and "git reflogs" 0.9M.
Changing to reflog (except for fsck-objects --no-reflogs)

Rebased on master to resolve conflicts

gerhardol · 2022-09-14T22:36:10Z

Mockup of Advanced filter after options are moved there - future PR
I do not want to delay a release though...

mstv · 2022-09-15T20:01:33Z

Time to squash?

Yes, please.

Group the menu items to reflect how the items are used, separated with group headings A number of related fixes: * stash and notes could be shown with current and filtered branches This normally did not occur as they were filtered, but it affected tests * Decouple ShowMergeCommits and ShowRevisionGridGraphColumn * Reflog was not coupled with other branch revision filters Even if Reflog dominated the other branch revision filters, this was not reflected in the menus. Also toggling the reflog (with the button) would set the branchfilter to all branches. * Add reflog to branch filter dropdown, to show current filter * Consistently name reflog/reflogs to "reflog" Except for "noreflogs" for "fsck-objects --no-reflog"

gerhardol · 2022-09-15T22:18:53Z

Time to squash?

Yes, please.

Left a reference in tmp/reorder-view-menu

RussKie · 2022-09-16T07:01:58Z

Mockup of Advanced filter after options are moved there - future PR I do not want to delay a release though...

Any reason not to use PropertyGrid similar to how we configure Scripts?

gerhardol · 2022-09-16T09:11:16Z

Any reason not to use PropertyGrid similar to how we configure Scripts?

More work to restructure

gerhardol · 2022-09-20T21:25:12Z

Merging this tomorrow

Group the menu items to reflect how the items are used, separated with group headings A number of related fixes: * stash and notes could be shown with current and filtered branches This normally did not occur as they were filtered, but it affected tests. * Decouple ShowMergeCommits and ShowRevisionGridGraphColumn (column can be shown when this filter is enabled) * Reflog was not coupled with other branch revision filters Even if Reflog dominated the other branch revision filters, this was not reflected in the menus. Also toggling the reflog (with the button) would set the branchfilter to all branches. * Add reflog to branch filter dropdown, to show current filter * Consistently name reflog/reflogs to "reflog" Except for "noreflogs" for "fsck-objects --no-reflogs" (cherry picked from commit 480458b)

RussKie · 2022-10-11T07:30:29Z

Maybe use PripertyGrid control instead? Like we do for the scripts management

gerhardol · 2022-10-11T07:57:31Z

Yes, but that is a redesign, the primary intent here was to cleanup

RussKie · 2022-10-11T12:32:28Z

👀 I sent that email several weeks ago....

gerhardol · 2022-10-11T14:07:59Z

My priority is not very high on such a change, there are more urgent changes. And I get very frustrated by UI changes, which makes my wife even less happy so I do not see I will change this in the near future.

ghost assigned gerhardol Sep 9, 2022

gerhardol requested a review from mstv September 9, 2022 22:05

gerhardol force-pushed the feature/reorder-view-menu branch from 0f85266 to 893d531 Compare September 10, 2022 21:15

RussKie reviewed Sep 12, 2022

View reviewed changes

RussKie approved these changes Sep 13, 2022

View reviewed changes

gerhardol force-pushed the feature/reorder-view-menu branch from eda228e to 39c0db4 Compare September 13, 2022 22:36

mstv reviewed Sep 14, 2022

View reviewed changes

pmiossec approved these changes Sep 14, 2022

View reviewed changes

gerhardol force-pushed the feature/reorder-view-menu branch from 7b00072 to 07a1b5e Compare September 15, 2022 20:01

gerhardol force-pushed the feature/reorder-view-menu branch from 07a1b5e to ab8f46f Compare September 15, 2022 22:10

gerhardol force-pushed the feature/reorder-view-menu branch from ab8f46f to f631ded Compare September 15, 2022 22:16

gerhardol merged commit 480458b into gitextensions:master Sep 21, 2022

gerhardol deleted the feature/reorder-view-menu branch September 21, 2022 20:22

ghost added this to the vNext milestone Sep 21, 2022

gerhardol modified the milestones: vNext, 4.0.0 Sep 21, 2022

gerhardol mentioned this pull request Sep 22, 2022

Move revision filters to advanced filter form #10205

Merged

Reorder items in View menu #10189

Reorder items in View menu #10189

Conversation

gerhardol commented Sep 9, 2022 • edited

Proposed changes

Screenshots

Test methodology

Merge strategy

gerhardol commented Sep 10, 2022

mstv commented Sep 11, 2022

mstv commented Sep 11, 2022

gerhardol commented Sep 11, 2022 • edited

mstv commented Sep 11, 2022

gerhardol commented Sep 11, 2022

gerhardol commented Sep 11, 2022

RussKie commented Sep 11, 2022 via email • edited by ghost

RussKie left a comment

Choose a reason for hiding this comment

pmiossec commented Sep 12, 2022 • edited

gerhardol commented Sep 12, 2022

RussKie commented Sep 13, 2022

RussKie Sep 13, 2022

Choose a reason for hiding this comment

gerhardol Sep 13, 2022

Choose a reason for hiding this comment

RussKie commented Sep 13, 2022

gerhardol commented Sep 13, 2022

mstv commented Sep 13, 2022

gerhardol commented Sep 13, 2022

pmiossec commented Sep 13, 2022

gerhardol commented Sep 13, 2022

pmiossec commented Sep 13, 2022

mstv left a comment

Choose a reason for hiding this comment

gerhardol commented Sep 14, 2022

pmiossec Sep 12, 2022

Choose a reason for hiding this comment

gerhardol Sep 14, 2022

Choose a reason for hiding this comment

gerhardol Sep 15, 2022

Choose a reason for hiding this comment

gerhardol commented Sep 14, 2022

mstv commented Sep 15, 2022

gerhardol commented Sep 15, 2022

RussKie commented Sep 16, 2022

gerhardol commented Sep 16, 2022

gerhardol commented Sep 20, 2022

RussKie commented Oct 11, 2022 via email • edited by ghost

gerhardol commented Oct 11, 2022

RussKie commented Oct 11, 2022

gerhardol commented Oct 11, 2022

gerhardol commented Sep 9, 2022 •

edited

gerhardol commented Sep 11, 2022 •

edited

RussKie commented Sep 11, 2022 via email •

edited by ghost

pmiossec commented Sep 12, 2022 •

edited

RussKie commented Oct 11, 2022 via email •

edited by ghost