Preventing popup "Revision X not being visible in the revision grid" on right mouse button click

[h0lg]

Fixes #10250 by

• remembering clicked mouse button until node selection events are triggered and
• patching the info through to only show the popup about Revision X not being visible in the revision grid if the click did not originate from the right mouse button.

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.

[RussKie]

I don't think we should execute this path at all if the node isn't visible

[RussKie]

I believe this is all that's required:

diff --git a/GitUI/BranchTreePanel/RepoObjectsTree.cs b/GitUI/BranchTreePanel/RepoObjectsTree.cs
index 41f6e4704..6bb0a5e34 100644
--- a/GitUI/BranchTreePanel/RepoObjectsTree.cs
+++ b/GitUI/BranchTreePanel/RepoObjectsTree.cs
@@ -541,7 +541,13 @@ protected override CommandStatus ExecuteCommand(int cmd)
 
         private void OnNodeSelected(object sender, TreeViewEventArgs e)
         {
-            Node.OnNode<Node>(e.Node, node => node.OnSelected());
+            Node.OnNode<Node>(e.Node, node =>
+            {
+                if (node is not BaseBranchNode branchNode || branchNode.Visible)
+                {
+                    node.OnSelected();
+                }
+            });
         }
 

[gerhardol]

I believe this is all that's required:

No
The PR works. It is a hack, even if less hacky then what I played around with

[RussKie]

What's missing in my snippet?

[gerhardol]

What's missing in my snippet?

Have not debugged why I got the popup

[RussKie]

I ran various manual tests and observed no popups, so I'm curious what I may have missed.

[h0lg]

@RussKie I just tried your solution and it does the job - I don't get any popups either when I open the context menu on hidden refs.

The difference in behavior between the two is that your solution suppresses the popup even if I left-click on a hidden ref while mine doesn't.

@gerhardol Which is the desired behaviour?

[gerhardol]

I ran various manual tests and observed no popups, so I'm curious what I may have missed.

There are two issues:

1. There is no popup when clicking a node that is set as not visible.
   -> This is maybe something we may live with
2. The "visible" property is not always correct. The filter detection is incomplete and check against the grid is done to early.
   -> This is was I happened to test (a little too late in the night). This is fixed in SidePanel: Show Stashes #10258

[RussKie]

IMO we shouldn't try to navigate if a ref isn't visible.

[gerhardol]

IMO we shouldn't try to navigate if a ref isn't visible.

Then go with "avoid selecting", and get #10285 merged.
I would appreciate @mstv comments on the latest changes in #10285 though.

[gerhardol]

Also for 4.0

[h0lg]

IMO we shouldn't try to navigate if a ref isn't visible.

The only draw-back I see is that without the popup the user won't get any feedback as to why nothing happens on left-clicking hidden refs.
I'm personally OK with that - just wanted to point that out.

But it does feel a bit like watering down/neutering this new feedback popup feature right off the bat. You guys be the judges of whether you want to do that or not. I didn't read up on the full reasoning behind the new popup.

[h0lg]

IMO we shouldn't try to navigate if a ref isn't visible.

Are you questioning the new feedback popup feature in general? If so, you may want to revert it altogether and solve #10250 that way...

[gerhardol]

IMO we shouldn't try to navigate if a ref isn't visible.

Are you questioning the new feedback popup feature in general? If so, you may want to revert it altogether and solve #10250 that way...

I interpreted the comment as "when we know a ref is not visible" and the user have other means of seeing that.
The popup was less used in the initial version of the pop feedback.

[h0lg]

I interpreted the comment as "when we know a ref is not visible" and the user have other means of seeing that.

...and in this case the user has a visual clue by how the hidden ref is drawn in the tree (the slashed eye icon and light gray font).
I understand - thanks for clarifying. I see that RevisionGridControl.GoToRef() is used in other scenarios where the user may not have such a clue.

[h0lg]

I've reverted my solution and added @RussKie's.
Note that local/remote branch and tag nodes also all implement IGitRefAcions, see GitUI\BranchTreePanel\Trees and nodes.cd.
Maybe that's something you want to check for to keep the OnSelected() handlers firing for other nodes implementing BaseBranchNode.

[h0lg]

rebased on master and commented

[gerhardol]

Merging tomorrow

[mstv]

Thank you