Desktop: add + to access 'Zoom In', allows to use the numpad #2630
Conversation
This change aligns Joplin's behavior with all other apps that have shortcuts for `Zoom In`. It therefore improves the user experience. /ref https://discourse.joplinapp.org/t/please-help-test-the-new-desktop-pre-release/6445/2?u=tessus laurent22#2165 (comment)
|
Just as an info. This came also up in the beta-testing topic. |
|
IMO this change is important, especially since the shortcut recorder (GSoC) will have to deal with such a special case: 2 menu items for the same action but only one visible. I want to get the number of open PRs down. It's time to merge/close the low hanging fruits. |
| @@ -1025,6 +1025,13 @@ class Application extends BaseApplication { | |||
| click: () => { | |||
| Setting.incValue('windowContentZoomFactor', 10); | |||
| }, | |||
| accelerator: 'CommandOrControl+Plus', | |||
| }, { | |||
| label: _('Zoom In'), | |||
There was a problem hiding this comment.
Since the item is not visible, can we remove the label? Or does it not working if there's no label?
There was a problem hiding this comment.
No, when we remove the label, it won't work. But we can set it to an empty string.
But I think we should leave it as is.
There was a problem hiding this comment.
@laurent22 not aure, if you saw my comment.
There was a problem hiding this comment.
I think this is fine, but please could you add a comment above to explain why the menu item is duplicated?
There was a problem hiding this comment.
I will do that later this evening. Sorry, I didn't see your comment earlier. I actually wanted to do that anyway, but forgot. Thanks for the reminder.
|
Can I merge this? |
|
Looks good, thanks @tessus! |
This change aligns Joplin's behavior with all other apps that have shortcuts for
Zoom In.It therefore improves the user experience.
/ref