| Summary: | [client] Compare With Each Other should be text rather than icon | ||
|---|---|---|---|
| Product: | [ECD] Orion | Reporter: | John Arthorne <john.arthorne> |
| Component: | Git | Assignee: | 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
"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 Fixed with 1f845182b4aba0e84ed7c6e3f2c0a338bea5744f. 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. 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.) |