Left panel: Make "Expand all" and "Collapse all" based on selected node

[pmiossec]

Fixes #8873

Proposed changes

• "Expand all" and "Collapse all" now has an effect only on selected node (collapse all tree could still be made through menu icon)

Screenshots

Before

After

Test methodology

• Manual

Test environment(s)

• Git Extensions 33.33.33
• Build 873e9ed
• Git 2.28.0.windows.1 (recommended: 2.30.0 or later)
• Microsoft Windows NT 10.0.17134.0
• .NET Framework 4.8.4311.0
• DPI 192dpi (200% scaling)

[codecov]

Codecov Report

Merging #8874 (f47e84e) into master (b9cd2b5) will increase coverage by 0.00%.
The diff coverage is 75.00%.

@@           Coverage Diff           @@
##           master    #8874   +/-   ##
=======================================
  Coverage   56.08%   56.08%           
=======================================
  Files         925      925           
  Lines       65872    65873    +1     
  Branches    12084    12086    +2     
=======================================
+ Hits        36942    36945    +3     
+ Misses      25919    25912    -7     
- Partials     3011     3016    +5     

Flag	Coverage Δ
production	43.33% <71.42%> (+<0.01%)	⬆️
tests	94.86% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

[gerhardol]

The actions are now the same as clicking the nodes.
The change still makes sense

The same should be done for RevFile tree, removing current Expand all (and aligning Collapse all action)

[pmiossec]

The same should be done for RevFile tree, removing current Expand all (and aligning Collapse all action)

I don't think it would be a good idea.

I agree that "Expand all" is in most (perhaps all) repositories of no use and that we could remove it because it clutters the contextual menu.

But expand and collapse are not symmetric in the use we do of it.
Most of the time, we want to expand a node, then perhaps some others and, when that become a mess, we want to collapse everything.

And because there is not a single root, you want to keep the current "Collapse all" behavior that collapse everything.

In the left panel, that's not a problem because there is the "Collapse all" that act on the whole tree in the left panel toolbar.
In the file tree, you don't have this button.

[RussKie]

I can appreciate where this change is stemming from, and I'm not against it. But in its current form it creates a confusing experience - "all" implies all nodes. If we want this behaviour I think we need to change the label to "Expand" and "Collapse" when the menu is invoked on a node. Similar to Test Explorer in VS:

[RussKie]

The same should be done for RevFile tree, removing current Expand all (and aligning Collapse all action)

Agree on the point of changing the label to "Expand" and "Expand all". And drop "(takes...)". Not expecting as part of this PR.

[pmiossec]

If I sum up my point of view....

Preamble: First, I should remind that left panel and file tree can't be managed exactly the same way because in left panel we display things that are not linked together (branches, remotes) and we could want to act one without having a effect on others.

Left panel (LP):

ShortName (0)	Action (0b)	Usage frequency (1)	What I propose to do
LP-Exp	Expand (sub nodes)	Sometimes	Keep it ✔️
LP-Coll	Collapse (sub nodes) (2)	rarely (3)	Intended to have it because of what is described in preamble (Already made by this PR. OK ❓ ). Make sense for symmetry purpose with LP-Exp also
LP-ExpAll	Expand All	Never (because that expand things not linked. cf preamble)	Don't propose this feature i.e Remove it (Already made by this PR. OK ❓ )
LP-CollAll	Collapse All	Often	Don't propose this feature in contextual menu only because we have the button in the toolbar and that makes more sense there. (Remove already ade by this PR).

File tree (FT):

ShortName (0)	Action	Usage frequency (1)	What I propose to do
FT-Exp	Expand (sub nodes)	Sometimes	Keep it ✔️
FT-Coll	Collapse (sub nodes) (2)	rarely (3)	It could be useful sometimes. Make sense for symmetry purpose with FT-Exp also and for harmonization with LP. Intended to have it (TODO. OK ❓ )
FT-ExpAll	Expand All	Never (because most of the times, that expand too many things)	Don't propose this feature i.e Remove it (TODO. OK ❓ )
FT-CollAll	Collapse All	Often	Propose this feature here only because don't have a better place to put it to.

Note: For File tree a perfect solution is not achievable in term of UX, and depending on the choice we will made, we will have to choose between some symmetry in features proposed and contextual menu clutter. Difficult choice.
For File tree, my preference would go to just remove the "Expand all"...

(0): to make the discussion easier
(0b): (TODO) I propose to use this label in the application (without what is between parenthesis which is just here to be clearer)
(1) : Extrapolated from my usage. I write it too be sure that everyone share the same point of view...
(2) : Not equivalent to a collapse with a left click because if you reopen the node, all the sub-nodes are collapsed contrary to the left click that keep the collapse/expand state so it could make sense to have it
(3) : we tend to use left click to collapse but due to (2) it could make sense to have it.

[ghost]

I think File Tree panel needs tools for global actions and Collapse/Expande for node. Like in others.
Thus, we will use a single template.

[pmiossec]

I think File Tree panel needs tools for global actions and Collapse/Expande for node. Like in others.
Thus, we will use a single template.

Yes, I think we could agree in this goal but what I was asking for is a clever step ̂toward this goal because I won't do all that in this PR.

That's why I proposed to keep FT-CollAll in the contextual menu even if that's not what is done in Left panel until FT gain a kind of toolbar. Because adding a toolbar ask a lot of new questions that we can't solve now...

[ghost]

Yes, I think we could agree in this goal but what I was asking for is a clever step ̂toward this goal because I won't do all that in this PR.
That's why I proposed to keep FT-CollAll in the contextual menu even if that's not what is done in Left panel until FT gain a kind of toolbar. Because adding a toolbar ask a lot of new questions that we can't solve now...

I agree. We spend more time on discussions. Several PRs could have been done already. 😄

[RussKie]

I agree. We spend more time on discussions. Several PRs could have been done already. 😄

I fully understand your impatience and frustration. I'd love to be able to move faster.

I think in the past it was often a case - someone proposed a change, and it would get merged without much discussion and appreciation how it would fit in the overall user experience, whether there was a similar implementation somewhere else, or the same change would need to be applied in another component. This led to the current state where we have disjoint or incoherent user experiences - affecting both behaviours and UI.

We have tens (or more like hundreds) of thousands of users, and we need to be mindful of this fact, and (try to) make rational and balanced decisions that would appeal to the majority (if not all) of our users.
Even within the team we tend to have different views on what a good UI is, how we should handle error messages, etc. Which is both great, and (at times) frustrating. But, I believe, these discussions and reviews ultimately make us come up with best possible solutions.

[RussKie]

Left panel (LP):

ShortName (0)	What I propose to do
LP-Exp	"Expand"
LP-Coll	"Collapse"
LP-ExpAll	Remove
LP-CollAll	Remove

File tree (FT):

ShortName (0)	What I propose to do
FT-Exp	"Expand"
FT-Coll	"Collapse"
FT-ExpAll	Remove
FT-CollAll	Remove

[pmiossec]

@RussKie Done. Even If I will miss FT-CollAll, I think I could give a try...

[RussKie]

Wouldn't you achieve the same by clicking on the top node > Collapse?

[pmiossec]

Wouldn't you achieve the same by clicking on the top node > Collapse?

No. There is no top node for all the top folders in the FT.

That was the main goal of my big comment to explain that the LP and FT doesn't contains the same type of data (heterogeneous vs homogeneous) and so it could make sense that they behave differently...by keeping FT-CollAll (that makes more sense for me than FT-Coll) until we get the toolbar feature that @ivangrek proposed to develop 🤣

[RussKie]

so it could make sense that they behave differently...by keeping FT-CollAll (that makes more sense for me than FT-Coll) until we get the toolbar feature that @ivangrek proposed to develop 🤣

Sorry, I didn't have a GE instance to look at while I was responding earlier.
Works for me 👍

[pmiossec]

so it could make sense that they behave differently...by keeping FT-CollAll (that makes more sense for me than FT-Coll) until we get the toolbar feature that @ivangrek proposed to develop 🤣

Sorry, I didn't have a GE instance to look at while I was responding earlier.
Works for me 👍

I'm not sure to understand what is "working for your".

But playing with the FT, I discovered a new strange thing that we should know before deciding...

In the LP, a left click to collapse a node keep the expanded/collapse state of subnodes (so it makes sense to have a LP-Coll).
But in the FT, a left click to collapse a node DOES NOT keep the expanded/collapse state of subnodes so having FT-Coll finally make no sense because no one will ever use it...

So I propose:

File tree (FT):

ShortName (0)	What I propose to do
FT-Exp	"Expand"
FT-Coll	Remove (because that's like left click)
FT-ExpAll	Remove
FT-CollAll	"Collapse All" (because we don't have a root node to do collapse on and until we get the toolbar)

[RussKie]

What does "Expand" mean: 1. Expand the current node only, or 2. Expand the current node and all of its children?

[pmiossec]

What does "Expand" mean:

1. Expand the current node only, or
2. Expand the current node and all of its children?

"Expand" means 2. (with children) because 1. is of course done with the left click.

That's you that bring the idea to remove the text after (to do like in Visual Studio) even if I would prefer to find some more explicit (and good English) label like:

• Expand children (nodes)
• Expand (all) subnodes
• Expand (all) subitems
• (better idea)?

I will put this question in the fact that you handle a lot of things 😉

[RussKie]

Thank you. I'm still of an opinion we should keep it short, i.e. "Expand". Users will quickly learn what it means after one or two interactions with it.

[RussKie]

Is this ready?

[gerhardol]

If the table @pmiossec did in #8874 (comment) is updated with click behavior too, I will vote...
Likely approving the changes, there are some thinking behind the options now...

[pmiossec]

Is this ready?

No, that is not. I have to finish it. But it's even harder these last few days to find some free time... (I admire all the work @gerhardol is able to deliver while I can just give my point of view on some discussions from time to time... )

[pmiossec]

Done.

Sum up of what has been done in this PR with the new knowledge ...

Left panel (LP):

ShortName (0)	Action (0b)	Done
LP-Exp	Expand (sub nodes)	Add it ✔️
LP-Coll	Collapse (sub nodes) (2)	Add it ✔️
LP-ExpAll	Expand All	Removed❌. Reason: Not useful
LP-CollAll	Collapse All	Removed❌. Reason: Already proposed in the toolbar where it makes more sense

File tree (FT):

ShortName (0)	Action	Done
FT-Exp	Expand (sub nodes)	Renamed it ✔️
FT-Coll	Collapse (sub nodes) (2)	Still not proposed❌. Reason: Default action when collapsing with the left click
FT-ExpAll	Expand All	Removed❌. Reason: Not useful
FT-CollAll	Collapse All	Keep it ✔️ Reason: Waiting to have a toolbar to put it there

Just before writing this message, I was considering this PR as finished until I noticed 2 things:

1. In LP, "Expand" is after "Collapse" (strange!). In FT, it is in the opposite. Invert in the LP?
2. In the LP, there are tooltîps "Expand/Collapse all subnodes" (see screenshot). Keep or remove? Or add to FT?

Your point of view on everything?

[RussKie]

My 2c 1. Expand first, followed by Collapse 2. Remove tooltips, if other items don't have them. Do you find tooltips useful here?

[gerhardol]

2. Remove tooltips, if other items don't have them. Do you find tooltips useful here?

In side panel, no added information

For file tree, the hint that operation can be slow could have been useful for the slow expand all, that is being removed.

[pmiossec]

1. Expand first, followed by Collapse

Done.

2. Remove tooltips, if other items don't have them. Do you find tooltips useful here?

Kept because other items have a tooltip

[RussKie]

Including into 3.5 since it won't drastically affect the translation status.