791957 - New app menu doesn't list keyboard shortcuts next to menu items

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Mihovil Stanic [:Mikeyy - L10n HR]]

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]]

Changed importance to minor, was set to major by mistake.

Comment 2

[Thomas D. (:thomas8)]

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]

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]

To be more specific, the "Edit" and "Save As" submenus already list keyboard shortcuts, others like "Message" don't.

Comment 5

[Thomas D. (:thomas8)]

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. (:thomas8)]

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)]

Attachment: proposed fix

This patch adds all available shortcuts in AppMenu submenus.

Comment 8

[Thomas D. (:thomas8)]

Richard, thanks a lot for the rapid patch, that's great! :)

Comment 9

[Mike Conley (:mconley) (:⚙️)]

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)]

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. (:thomas8)]

(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]]

https://hg.mozilla.org/comm-central/rev/b439022b1aa1

Comment 13

[Thomas D. (:thomas8)]

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+

Comment 14

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/comm-aurora/rev/7cbc4c9da278
https://hg.mozilla.org/releases/comm-beta/rev/c32dc913667f
https://hg.mozilla.org/releases/comm-esr17/rev/3edc5ae88ef7