Closed Bug 38367 Opened 24 years ago Closed 21 years ago

keyboard shortcuts in menus should be aligned

Categories

(Core :: XUL, defect, P3)

x86
All
defect

Tracking

()

RESOLVED FIXED
Future

People

(Reporter: bugzilla, Assigned: mikepinkerton)

References

Details

(Keywords: polish)

Attachments

(2 files, 3 obsolete files)

Throwing this one in browser gen for now...everytime I put stuff in XP menus
they're always thrown back out again :)

In all Win32 the Ctrl+(whatever) shortcuts on menus are aligned perfectly and
evenly.  In Mozilla (as especially evidenced by the Ctrl+N, Ctrl+L, and Ctrl+O
shortcuts on the File menu), the shortcut texts are not lined up at all.  This
looks sloppy.  Look at any other Windows app (IE on Win, for example) for how
much nicer it looks aligned properly.

I realize this isn't high priority.  Helpwanted?
I guess that if it isn't a bug with the menu structure but rather a bug with 
someone's poorly written XUL or CSS then it belongs to the person that wrote 
that menu rather than the owner of Menus code.  So it's worth looking at who 
wrote that particular menu and assigning it to them.  
*** Bug 38462 has been marked as a duplicate of this bug. ***
From bug 38462:

"The shortcut keys text like "Ctrl+N" and "Ctrl+W" in the File menu should be 
left aligned to the right. Currently they seems to be just right aligned."
ben, as owner of the Mozilla UI can you please assign this to the appropriate 
person(s).  Thanks.
updating component and owner.  
Assignee: asadotzler → pinkerton
Component: Browser-General → XP Toolkit/Widgets: Menus
OS: Windows 98 → All
QA Contact: jelwell → sairuh
polish. pushing out.
Status: NEW → ASSIGNED
Target Milestone: --- → M20
Mass-moving all M20-M30 XPToolkit bugs to Future
Target Milestone: M20 → Future
I don't know about other OS's, but this is standard on Win32....it'd be nice to 
see this sometime before this release.

putting on helpwanted radar, resummarizing (the caps have been annoying me)
Keywords: helpwanted, polish
Summary: Make Ctrl Key Combos Align on Menus → Make ctrl key combos align on menus
*spam*: transferring current XP Menu bugs over to jrgm, the new component owner.
feel free to add me to the cc list (unless am the Reporter) of any of these, if
you have any questions/etc.
QA Contact: sairuh → jrgm
Anyone got a filename where I could start looking?
blake - are you still seeing this? I can't repro on  linux or win32
I still see this.  Look at the shortcut keys in the View and Go menus, for 
example.
*** Bug 88212 has been marked as a duplicate of this bug. ***
How would you work this? I suppose menupopups could contain an anonymous grid,
the menuitems would be rows in the grid and the labels and accesstexts etc. columns?
I don't really know.  I just always assumed that since the shortcut keys are
right-aligned now, it wouldn't be too hard to change them to be left-aligned. 
But I haven't had a chance to explore this theory.
Neil, I think you're probably closer.  Currently the display portion of a 
menuitem is defined as:

<xul:text class="menu-text" align="left" inherits="value=label,accesskey,crop" 
crop="right"/>
<xul:text class="menu-accel" inherits="value=acceltext"/>

The only way that I could think of aligning things using the current design 
would be to wrap all objects of class "menu-text" in one vertical box (xul:box 
orient="vertical") and all of "menu-accel" in another.  But I don't know if such 
a thing is possible.

I believe that in the end there still has to be single "menu", "menuitem", 
"menuitem-iconic", and "menu-iconic" bindings that have all the properties (eg. 
label, acceltext, acceskey, disabled, etc.)

I'll attach an image that shows how the text fields lay out.  You can see that 
the "menu-accel" text only extends as far left as is needed.  As I mentioned 
above, perhaps two vertical boxes could be used to align the two text fields.
can we calculate the width of each shortcut key and then use css rightmargin to 
pad all the other keys as necessary? it would require two passes, but it should 
work.

Actually better, these things can inherit right? so make all of them 
inherit="minwidth=key_width" where key_width is assigned based on a single pass 
max(width).
It's a nice idea but it doesn't sound very skinnable to me.
(In my skin you would have to measure the width of a stack.)
Any chance the latest round of XUL changes help us here?
<hyatt> dean, no, that's a real problem.
timeless: I tried calculating the width of the accel key but it only works the
second time that you open the popup :-( I can attach a patch if you're interested.
Neil: attach away, I wouldn't mind looking.
*** Bug 154555 has been marked as a duplicate of this bug. ***
Neil, do you still have that patch that you mentioned in comment 22?  I'd be
interested in taking a look.
Consider changing the summary to include the words "keyboard shortcuts" and/or
"hotkeys" to make it more visible in queries

suggested:

"Align keyboard shortcuts (hotkeys, ctrl key combos) on menus"
Attached patch Lame hack (obsolete) — Splinter Review
For some reason this causes extra overflows so I've had to work around that :-(
Summary: Make ctrl key combos align on menus → keyboard shortcuts in menus should be aligned
+      <handler event="popupshown" phase="target">

Does it reduce the overflows if you change the event to "popupshowing", or did I
misunderstand what you meant?
No, I think I tried it both ways (I normally use showing), but I'll double-check.
Attached patch popupshowing seems to work :-) (obsolete) — Splinter Review
Attachment #89563 - Attachment is obsolete: true
Keywords: helpwantedpatch, review, ui
Neil: Oh wow, I totally missed your working patch from a month ago!  It looks to
me like you're checking the width of every accel box, regardless of there's any
acceltext set.  Have you done any tests to see how this affects the speed that
large menus take to display, like Bookmarks?
Actually I find that the bookmarks menu speed depends mostly on the time RDF
takes to build it... I haven't noticed any problems with speed even at 133Mhz.
Interesting.  I just noticed that in Office 2000 and XP shortcuts are
right-aligned, just like we do it now.  But in IE6 shortcuts are left-aligned. 
So MS isn't even standard anymore!
Attachment #98124 - Flags: review+
Neil, have you asked for an sr= of this?
Attachment #98124 - Flags: superreview?(bzbarsky)
Is anyone willing to sr= the patch provided for this bug?
Comment on attachment 98124 [details] [diff] [review]
Only align items with accel keys

>+            array[i].setAttribute("width", width);
Just noticed that this should say array[i].width = width;

And array is probably an awful name to use :-P
Attachment #98124 - Flags: superreview?(bzbarsky) → superreview?(alecf)
Comment on attachment 98124 [details] [diff] [review]
Only align items with accel keys

sr=alecf
Attachment #98124 - Flags: superreview?(alecf) → superreview+
Attachment #98124 - Flags: approval1.4a?
Comment on attachment 98124 [details] [diff] [review]
Only align items with accel keys

please land first thing in beta. thanks.
Attachment #98124 - Flags: approval1.4a? → approval1.4a-
I'm seeing a lot of :
Warning: reference to undefined property accel.label
Source File: 
Line: 9

which seems to becaused a checkin to popup.xml
This performs the checks I was trying to do last time more reliably.
Attachment #89900 - Attachment is obsolete: true
Attachment #98124 - Attachment is obsolete: true
Comment on attachment 119329 [details] [diff] [review]
Fix the JS warning

r=varga
Attachment #119329 - Flags: review+
Attachment #119329 - Flags: superreview+
Fix checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Component: XP Toolkit/Widgets: Menus → XUL
QA Contact: jrgmorrison → xptoolkit.widgets
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: