Bug 107455 - 'New Slide' and 'Delete Slide' both use _e_ for accelerator
Summary: 'New Slide' and 'Delete Slide' both use _e_ for accelerator
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
5.2.5.1 release
Hardware: All All
: medium normal
Assignee: Yousuf Philips (jay) (retired)
URL:
Whiteboard: target:5.4.0
Keywords:
Depends on:
Blocks: Shortcuts-Accelerators
  Show dependency treegraph
 
Reported: 2017-04-26 18:35 UTC by Paul Unger
Modified: 2017-05-02 18:33 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Unger 2017-04-26 18:35:51 UTC
In the S_l_ide menu (where _x_ = underline):
New Slid_e_ and 
D_e_lete Slide and
Slide Master D_e_sign and
Master _E_lements
*all* use 'e' for the keyboard shortcut.

Suggest:
_N_ew Slide and 
_D_elete Slide and 
_M_aster Elements
since neither 'n' nor 'd' nor 'm' are used elsewhere in the Slide menu.
Comment 1 Yousuf Philips (jay) (retired) 2017-04-26 21:17:08 UTC
Hi pcunger,

Thanks for pointing this out. Your suggestion looks good.

@Stuart: You gonna take this one?
Comment 2 V Stuart Foote 2017-04-26 21:53:41 UTC
(In reply to Yousuf Philips (jay) from comment #1)
> 
> @Stuart: You gonna take this one?

No, not set up to build at the moment. All yours...
Comment 3 Yousuf Philips (jay) (retired) 2017-04-27 13:50:46 UTC
Done - https://gerrit.libreoffice.org/#/c/37034/
Comment 4 Paul Unger 2017-04-27 15:43:24 UTC
Thank you! I look forward to when it's released.
Comment 5 Yousuf Philips (jay) (retired) 2017-05-02 13:07:22 UTC
(In reply to pcunger from comment #0)
> since neither 'n' nor 'd' nor 'm' are used elsewhere in the Slide menu.

This isnt accurate, n is used in "_New Master", d is used in "_Delete Master" and m is used in "Su_mmary Slide", so what should we do about this now as my patch doesnt solve the multiple entries having the same accelerator issue.
Comment 6 Paul Unger 2017-05-02 13:58:15 UTC
Doh! Sorry... They're greyed out, so I didn't pay them any attention. My mistake!

What to do? Just a suggestion, but how about:

Ne_w_ Slide
D_u_plicate Slide
_D_elete Slide

As far as I can see 'u' isn't used elsewhere (even in greyed out menu items), so that's a safe change. 'W' /is/ used elsewhere (Sho_w_ Slide), but it is greyed out, and I'm not sure in what context it would be active... 'D' is also used elsewhere, but to my mind 'Slide' functions would be used more often then 'Master' ones, so change 'Delete _M_aster' to free up 'd' (this /does/ double up 'm' with Su_m_mary Slide, but that might be OK since the latter is relatively unused in comparison to Delete Master (which probably isn't used much either?))

One thing I noticed is that even though there are a number of items that use 'e', when the Slide menu is open pressing 'e' toggles between New Slide and Delete Slide--it doesn't go to Master Slide Design or Master Elements (LO 5.2.6.2, Win7). Anyone know why?
Comment 7 Cor Nouws 2017-05-02 14:28:47 UTC
(In reply to pcunger from comment #6)
> One thing I noticed is that even though there are a number of items that use
> 'e', when the Slide menu is open pressing 'e' toggles between New Slide and
> Delete Slide--it doesn't go to Master Slide Design or Master Elements (LO
> 5.2.6.2, Win7). Anyone know why?

It just toggles between two.
more notes:
- when there is no shortcut, it is automatically assigned
- if two have the same and the first is a submenu, it won't toggle to the second

So, possibly a pretty safe and sustainable approach is to assign smart thought short cuts to important menu items, and let the software do the rest.
(I've paid much time to  figuring out short cuts and assigning them in the past. Then things were extended, changed. So try to stay safe & be smart, is what I mean ;) )
Comment 8 Yousuf Philips (jay) (retired) 2017-05-02 18:31:52 UTC
Yep its better that similar normal and master slide commands have the same accelerator rather than all of them using _e_.
Comment 9 Commit Notification 2017-05-02 18:33:22 UTC
Yousuf Philips committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=bbc6d66effa62d0ef69c0422595c43f3821cd3bb

tdf#107455 Fix accelerators in the Slide menu

It will be available in 5.4.0.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.