Closed
Bug 38367
Opened 24 years ago
Closed 21 years ago
keyboard shortcuts in menus should be aligned
Categories
(Core :: XUL, defect, P3)
Tracking
()
RESOLVED
FIXED
Future
People
(Reporter: bugzilla, Assigned: mikepinkerton)
References
Details
(Keywords: polish)
Attachments
(2 files, 3 obsolete files)
9.90 KB,
image/jpeg
|
Details | |
1.46 KB,
patch
|
janv
:
review+
bzbarsky
:
superreview+
|
Details | Diff | Splinter Review |
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?
Comment 1•24 years ago
|
||
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.
Reporter | ||
Comment 3•24 years ago
|
||
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."
Comment 4•24 years ago
|
||
ben, as owner of the Mozilla UI can you please assign this to the appropriate person(s). Thanks.
Comment 5•24 years ago
|
||
updating component and owner.
Assignee: asadotzler → pinkerton
Component: Browser-General → XP Toolkit/Widgets: Menus
OS: Windows 98 → All
QA Contact: jelwell → sairuh
Assignee | ||
Comment 6•24 years ago
|
||
polish. pushing out.
Status: NEW → ASSIGNED
Target Milestone: --- → M20
Comment 7•24 years ago
|
||
Mass-moving all M20-M30 XPToolkit bugs to Future
Target Milestone: M20 → Future
Reporter | ||
Comment 8•24 years ago
|
||
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
Comment 9•24 years ago
|
||
*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
Comment 10•24 years ago
|
||
Anyone got a filename where I could start looking?
Comment 11•23 years ago
|
||
blake - are you still seeing this? I can't repro on linux or win32
Comment 12•23 years ago
|
||
I still see this. Look at the shortcut keys in the View and Go menus, for example.
Comment 13•23 years ago
|
||
*** Bug 88212 has been marked as a duplicate of this bug. ***
Comment 14•23 years ago
|
||
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?
Comment 15•23 years ago
|
||
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.
Comment 16•23 years ago
|
||
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.
Comment 17•23 years ago
|
||
Comment 18•23 years ago
|
||
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).
Comment 19•23 years ago
|
||
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.)
Comment 20•23 years ago
|
||
Any chance the latest round of XUL changes help us here?
Comment 21•23 years ago
|
||
<hyatt> dean, no, that's a real problem.
Comment 22•22 years ago
|
||
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.
Comment 23•22 years ago
|
||
Neil: attach away, I wouldn't mind looking.
Comment 24•22 years ago
|
||
*** Bug 154555 has been marked as a duplicate of this bug. ***
Comment 25•22 years ago
|
||
Neil, do you still have that patch that you mentioned in comment 22? I'd be interested in taking a look.
Comment 26•22 years ago
|
||
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"
Comment 27•22 years ago
|
||
For some reason this causes extra overflows so I've had to work around that :-(
Updated•22 years ago
|
Summary: Make ctrl key combos align on menus → keyboard shortcuts in menus should be aligned
Comment 28•22 years ago
|
||
+ <handler event="popupshown" phase="target"> Does it reduce the overflows if you change the event to "popupshowing", or did I misunderstand what you meant?
Comment 29•22 years ago
|
||
No, I think I tried it both ways (I normally use showing), but I'll double-check.
Comment 30•22 years ago
|
||
Attachment #89563 -
Attachment is obsolete: true
Updated•22 years ago
|
Comment 31•22 years ago
|
||
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?
Comment 32•22 years ago
|
||
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.
Comment 33•22 years ago
|
||
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!
Comment 34•22 years ago
|
||
Attachment #98124 -
Flags: review+
Comment 35•22 years ago
|
||
Neil, have you asked for an sr= of this?
Updated•22 years ago
|
Attachment #98124 -
Flags: superreview?(bzbarsky)
Comment 36•21 years ago
|
||
Is anyone willing to sr= the patch provided for this bug?
Comment 37•21 years ago
|
||
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 38•21 years ago
|
||
Comment on attachment 98124 [details] [diff] [review] Only align items with accel keys sr=alecf
Attachment #98124 -
Flags: superreview?(alecf) → superreview+
Updated•21 years ago
|
Attachment #98124 -
Flags: approval1.4a?
Comment 39•21 years ago
|
||
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-
Comment 40•21 years ago
|
||
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
Comment 41•21 years ago
|
||
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 42•21 years ago
|
||
Comment on attachment 119329 [details] [diff] [review] Fix the JS warning r=varga
Attachment #119329 -
Flags: review+
Updated•21 years ago
|
Attachment #119329 -
Flags: superreview+
Comment 43•21 years ago
|
||
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.
Description
•