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

RESOLVED FIXED in Thunderbird 20.0

Status

Thunderbird
General
--
major
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: Mikeyy, Assigned: Paenglab)

Tracking

(Blocks: 2 bugs)

17 Branch
Thunderbird 20.0
Dependency tree / graph

Thunderbird Tracking Flags

(thunderbird17 wontfix, thunderbird18 fixed, thunderbird19 fixed, thunderbird20 fixed, thunderbird-esr17 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Blocks: 789883
(Reporter)

Comment 1

5 years ago
Changed importance to minor, was set to major by mistake.
Severity: major → minor
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.
Severity: minor → normal
Summary: New menu doesn't list access key next to menu item → New app menu doesn't list keyboard shortcuts next to menu items
Depends on: 714031, 713979

Comment 3

5 years ago
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Windows 7 → All
Hardware: x86_64 → All

Updated

5 years ago
Blocks: 713979, 714031, 650170
No longer depends on: 713979, 714031

Comment 4

5 years ago
To be more specific, the "Edit" and "Save As" submenus already list keyboard shortcuts, others like "Message" don't.
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.
Severity: normal → major
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).
(Assignee)

Comment 7

5 years ago
Created attachment 684960 [details] [diff] [review]
proposed fix

This patch adds all available shortcuts in AppMenu submenus.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #684960 - Flags: review?(mconley)
Richard, thanks a lot for the rapid patch, that's great! :)
Comment on attachment 684960 [details] [diff] [review]
proposed fix

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

I'm happy with this - thanks Richard!
Attachment #684960 - Flags: review?(mconley) → review+
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
(Assignee)

Comment 10

5 years ago
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.
Attachment #684960 - Flags: approval-comm-esr17?
Attachment #684960 - Flags: approval-comm-beta?
Attachment #684960 - Flags: approval-comm-aurora?
(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
https://hg.mozilla.org/comm-central/rev/b439022b1aa1
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 20.0
Attachment #684960 - Flags: approval-comm-esr17?
Attachment #684960 - Flags: approval-comm-esr17+
Attachment #684960 - Flags: approval-comm-beta?
Attachment #684960 - Flags: approval-comm-beta+
Attachment #684960 - Flags: approval-comm-aurora?
Attachment #684960 - Flags: approval-comm-aurora+
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+
Keywords: checkin-needed
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
status-thunderbird17: --- → wontfix
status-thunderbird18: --- → fixed
status-thunderbird19: --- → fixed
status-thunderbird20: --- → fixed
status-thunderbird-esr17: --- → fixed
Keywords: checkin-needed
(Reporter)

Updated

5 years ago
Depends on: 832679
(Reporter)

Updated

5 years ago
No longer depends on: 832679
You need to log in before you can comment on or make changes to this bug.