Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 351208

Summary: [client] Compare With Each Other should be text rather than icon
Product: [ECD] Orion Reporter: John Arthorne <john.arthorne>
Component: GitAssignee: Susan McCourt <susan>
Status: RESOLVED FIXED QA Contact:
Severity: minor    
Priority: P3 CC: susan, Szymon.Brandys
Version: 0.2   
Target Milestone: 0.3 M2   
Hardware: PC   
OS: Windows 7   
Whiteboard:

Description John Arthorne CLA 2011-07-05 11:48:11 EDT
In the Git Log page, "Compare With Each Other" in the toolbar is an icon. I think we have generally switched to use hyperlinked text for commands in the toolbar, so this should be switched to text to be consistent.
Comment 1 Szymon Brandys CLA 2011-07-26 12:41:08 EDT
"Compare with Each Other" is added to the selection menu. This is how menus are rendered.
1) for many commands, the name of the menu e.g. 'More actions' is rendered. Each command is available in the menu and both the label and name are rendered.
2) for one command with no icon, we still render 'More actions' and the command is available under the menu
3) for one command with an image, we do not show 'More actions' but just add the icon to the menu bar

We should:
2) for one command with no image, just add it to the menu bar
3) for a command with an image, render 'More actions' and add the command to the menu
Comment 2 Szymon Brandys CLA 2011-07-27 05:12:04 EDT
Fixed with 1f845182b4aba0e84ed7c6e3f2c0a338bea5744f.
Comment 3 Susan McCourt CLA 2011-09-20 11:41:22 EDT
I don't think this is the right fix.

(In reply to comment #1)
> "Compare with Each Other" is added to the selection menu. This is how menus are
> rendered.
> 1) for many commands, the name of the menu e.g. 'More actions' is rendered.
> Each command is available in the menu and both the label and name are rendered.
> 2) for one command with no icon, we still render 'More actions' and the command
> is available under the menu
> 3) for one command with an image, we do not show 'More actions' but just add
> the icon to the menu bar

We have a way to force a particular contribution to always use text instead of an icon.  Doing this would mean that "Compare with Each Other" shows up as a single text link rather than being in the menu.

> 
> We should:
> 2) for one command with no image, just add it to the menu bar
> 3) for a command with an image, render 'More actions' and add the command to
> the menu

Case 3 is not good for the hovering actions case.  If there is a submenu with only one item, and it is an image, we would rather show the image than take up the real estate of the menu.

I'll take a look.
Comment 4 Susan McCourt CLA 2011-09-20 12:52:33 EDT
Fixed with 12bf4a6e98cba7c68bdf4bd183d2a08b1d43cd71.

The git code was set up properly to use forceText = true when rendering items in the toolbar.

The problem was that the command framework was passing this parameter in the wrong sequence in one of its function calls, so that the code that built the dom received a "null" instead.  (And was setting a css class to a boolean!)

So I fixed this bug.  Rather than revert the old "needsMenu" code to the old code, I simplified it so that we don't use the presence of an icon or not to decide whether a menu is needed, we just decide that if there's only one item in the menu, render the item directly.  (The old code only did this for the image case under the assumption that this was where you would win back a lot of real estate.)