Last Comment Bug 791957 - New app menu doesn't list keyboard shortcuts next to menu items
: New app menu doesn't list keyboard shortcuts next to menu items
Status: RESOLVED FIXED
:
Product: Thunderbird
Classification: Client Software
Component: General (show other bugs)
: 17 Branch
: All All
: -- major (vote)
: Thunderbird 20.0
Assigned To: Richard Marti (:Paenglab)
:
Mentors:
Depends on:
Blocks: tb-keyboard-tracker tbkbd-doc-tracker TB-AppMenu 789883
  Show dependency treegraph
 
Reported: 2012-09-18 01:43 PDT by Mihovil Stanic [:Mikeyy - L10n HR]
Modified: 2013-01-19 14:26 PST (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
wontfix
fixed
fixed
fixed
fixed


Attachments
proposed fix (23.72 KB, patch)
2012-11-25 07:59 PST, Richard Marti (:Paenglab)
mconley: review+
standard8: approval‑comm‑aurora+
standard8: approval‑comm‑beta+
standard8: approval‑comm‑esr17+
Details | Diff | Splinter Review

Description Mihovil Stanic [:Mikeyy - L10n HR] 2012-09-18 01:43:54 PDT
New menu doesn't list access key next to menu item.
They are only listed in old menu which is now hidden.

New users will have hard time finding out which access key is for what if he/she doesn't discover old menu.
Comment 1 Mihovil Stanic [:Mikeyy - L10n HR] 2012-09-18 01:46:49 PDT
Changed importance to minor, was set to major by mistake.
Comment 2 Thomas D. (currently busy elsewhere; needinfo?me) 2012-09-18 02:07:45 PDT
I'm not a big fan of app menu, but surely we should continue to mention the keyboard shortcuts for individual commands in that menu, especially if app menu is meant to become default UI. We really can't expect users to dig into the old menu for finding out about the keyboard shortcuts, and notwithstanding the goodness of existing keyboard shortcuts documentation, having in-product, contextual reference of keyboard shortcuts is far superior to that.

Firefox also lists keyboard shortcuts in their current app menu.

If there are any aesthetical concerns, we could move keyboard shortcuts into tooltips.
Comment 3 rsx11m 2012-09-20 15:14:21 PDT
This might have been done on purpose for the two-column main app menu to not overcrowd it, but most of its items open submenus anyway and thus don't have a shortcut. However, the appearances of those secondary submenus are identical to any other menu, and hence at least those should have the related keyboard shortcuts listed.
Comment 4 rsx11m 2012-09-20 15:27:51 PDT
To be more specific, the "Edit" and "Save As" submenus already list keyboard shortcuts, others like "Message" don't.
Comment 5 Thomas D. (currently busy elsewhere; needinfo?me) 2012-11-25 04:24:40 PST
Richard, any reason why keyboard shortcuts did not survive in your new AppMenu (Bug 650170)?

Imho, removing virtually all in-product documentation of keyboard shortcuts (with default settings, i.e. no traditional menu bar), is a major issue.
More so because keyboard shortcuts can be expected to become *more* relevant now that the traditional menu is gone by default.

This should be a straightforward fix, and please let's have this sooner rather than later - we can even lose new customers if they assume from looking at our default AppMenu that there is almost no keyboard shortcuts support in TB.
Comment 6 Thomas D. (currently busy elsewhere; needinfo?me) 2012-11-25 04:28:45 PST
Worse, new users not knowing the shortcut keys but hitting them accidentally will be very confused and have a hard time to find out what's going on - like hitting A with a message selected (see also bug 511741).
Comment 7 Richard Marti (:Paenglab) 2012-11-25 07:59:04 PST
Created attachment 684960 [details] [diff] [review]
proposed fix

This patch adds all available shortcuts in AppMenu submenus.
Comment 8 Thomas D. (currently busy elsewhere; needinfo?me) 2012-11-25 08:51:42 PST
Richard, thanks a lot for the rapid patch, that's great! :)
Comment 9 Mike Conley (:mconley) - (Needinfo me!) 2012-11-29 10:22:43 PST
Comment on attachment 684960 [details] [diff] [review]
proposed fix

Review of attachment 684960 [details] [diff] [review]:
-----------------------------------------------------------------

I'm happy with this - thanks Richard!
Comment 10 Richard Marti (:Paenglab) 2012-11-29 10:44:00 PST
Comment on attachment 684960 [details] [diff] [review]
proposed fix

[Approval Request Comment]
I'm not sure if this should land on TB 17. But this could help for the acceptance of the AppMenu.
I think this is low risk as it only adds key shortcuts and changes no program logic.
Comment 11 Thomas D. (currently busy elsewhere; needinfo?me) 2012-11-29 11:10:58 PST
(In reply to Richard Marti [:Paenglab] from comment #10)
> Comment on attachment 684960 [details] [diff] [review]
> proposed fix
> 
> [Approval Request Comment]
> I'm not sure if this should land on TB 17. But this could help for the
> acceptance of the AppMenu.
> I think this is low risk as it only adds key shortcuts and changes no
> program logic.

+1, absolutely, to both: It should land on TB17 because
* it is vital for the acceptance of AppMenu (we can only tuck things away insofar it's easy to reach them in other ways)
* it's absolutely no-risk because it just adds existing strings to some menus
Comment 12 Ryan VanderMeulen [:RyanVM] 2012-11-29 14:18:56 PST
https://hg.mozilla.org/comm-central/rev/b439022b1aa1
Comment 13 Thomas D. (currently busy elsewhere; needinfo?me) 2012-12-04 11:48:49 PST
If I understand comment 12 correctly, this has only been checked into comm-central (Daily/Trunk) so far. So 3 further checkins are needed given we now have
approval-comm-esr17+
approval-comm-beta+
approval-comm-aurora+

Note You need to log in before you can comment on or make changes to this bug.