Keyboard shortcuts should appear in grey text

RESOLVED FIXED in Firefox 11

Status

()

Firefox
Theme
RESOLVED FIXED
7 years ago
4 years ago

People

(Reporter: faaborg, Assigned: jaws)

Tracking

(Depends on: 1 bug)

Trunk
Firefox 11
All
Windows Vista
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 6 obsolete attachments)

To help reduce the visual complexity of sub-menus in the Firefox menu, keyboard shortcuts should appear in grey text.

We may also decide to apply this styling to the traditional menu on Vista and 7 as well.  On XP, there is more of a precedent for the visual design of traditional menu bars, since they are found everywhere on the platform.  So on XP we should just continue using a native appearance.

Comment 1

7 years ago
Created attachment 489644 [details] [diff] [review]
patch
Assignee: nobody → margaret.leibovic
Attachment #489644 - Flags: review?(dao)
Whiteboard: [target-betaN]
Version: unspecified → Trunk
FYI: This patch still applies cleanly.
Created attachment 572079 [details] [diff] [review]
Patch for bug 610902

I have changed the patch to use the child selector and added a new class, |appmenu_keyboard-shortcut|, to menuitems that have keys associated with them.
Assignee: margaret.leibovic → jwein
Attachment #489644 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #572079 - Flags: review?(gavin.sharp)
Attachment #489644 - Flags: review?(dao)
Attachment #572079 - Flags: feedback?(margaret.leibovic)
Comment on attachment 572079 [details] [diff] [review]
Patch for bug 610902

Extensions' menu items are going to lack the appmenu_keyboard-shortcut class, making the styling inconsistent.

I'm not sure I understand the rationale behind this bug; these menus don't seem visually more complex than other menus... So we should probably do nothing about this or do it for all menus.
Attachment #572079 - Flags: review?(gavin.sharp) → review-
Created attachment 572178 [details] [diff] [review]
Patch for bug 610902 v1.1

This patch uses Margaret's approach but extends the graytext to the "alt" menus.

How concerned should we be about the performance of the descendant selectors here?
Attachment #572079 - Attachment is obsolete: true
Attachment #572178 - Flags: review?(dao)
Attachment #572178 - Flags: feedback?(margaret.leibovic)
Attachment #572079 - Flags: feedback?(margaret.leibovic)
Comment on attachment 572178 [details] [diff] [review]
Patch for bug 610902 v1.1

>+  #appmenu-popup .menu-accel,
>+  #appmenu-popup .menu-iconic-accel,
>+  #main-menubar .menu-accel,
>+  #main-menubar .menu-iconic-accel {
>+    color: graytext;
>+  }

At this point, it looks like #appmenu-popup and #main-menubar are redundant in these selectors.
Attachment #572178 - Flags: review?(dao) → review-
Created attachment 577347 [details] [diff] [review]
Patch for bug 610902 v1.2

Thanks for the review Dao. I have updated the patch to only have the class selectors.
Attachment #572178 - Attachment is obsolete: true
Attachment #577347 - Flags: review?(dao)
Attachment #572178 - Flags: feedback?(margaret.leibovic)
Comment on attachment 577347 [details] [diff] [review]
Patch for bug 610902 v1.2

Apparently the section where you're adding this affects Windows XP, which wasn't part of the plan. Please move this to the windows-default-theme section in browser-aero.css.
Attachment #577347 - Flags: review?(dao) → review-
OS: Windows 7 → Windows Vista
Hardware: x86 → All
Whiteboard: [target-betaN]
Created attachment 577482 [details] [diff] [review]
Patch for bug 610902 v1.3

I've moved the styles to the windows-default-theme portion of browser-aero.css.
Attachment #577347 - Attachment is obsolete: true
Attachment #577482 - Flags: review?(dao)
Comment on attachment 577482 [details] [diff] [review]
Patch for bug 610902 v1.3

Move it to the end of this section:
http://mxr.mozilla.org/mozilla-central/source/browser/themes/winstripe/browser-aero.css#40

r=me with that
Attachment #577482 - Flags: review?(dao) → review+
Created attachment 577603 [details] [diff] [review]
Patch for bug 610902 v1.4 (for check-in)
Attachment #577482 - Attachment is obsolete: true
Attachment #577603 - Flags: review+
Created attachment 577604 [details] [diff] [review]
Patch for bug 610902 v1.4 (for check-in)

Moved to the end of the section.
Attachment #577603 - Attachment is obsolete: true
Attachment #577604 - Flags: review+
https://hg.mozilla.org/integration/fx-team/rev/e35dfafeee42
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/e35dfafeee42
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 11

Updated

6 years ago
Component: Menus → Theme
Depends on: 724457
QA Contact: menus → theme
You need to log in before you can comment on or make changes to this bug.