Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

View menu #1668

Merged
merged 6 commits into from
Feb 17, 2022
Merged

View menu #1668

merged 6 commits into from
Feb 17, 2022

Conversation

RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented Feb 14, 2022

Qt won't let us define a shortcut sequence twice, so we can't have an app-wide full screen shortcut and a another one in the browser. I went with the simplest solution and just left out the browser shortcut. That means, the menu action doesn't say "F11" or whatever, but that key still works. Open to other suggestions.
I also discovered the StandardKey enum. Let me know if it doesn't work to your liking on another platform.

Closes #1530.

Copy link
Member

@dae dae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a Linux machine here, I don't see any shortcut listed next to the main window's fullscreen menu item. Maybe Qt don't have a standard shortcut for this on (some) Linux distros? We might need to hard-code a shortcut in this case (maybe F11)

ftl/qt/qt-accel.ftl Outdated Show resolved Hide resolved
on_toggle_fullscreen -> on_toggle_full_screen
@dae
Copy link
Member

dae commented Feb 17, 2022

Thanks as always Rumo :-)

@dae dae merged commit 700fc50 into ankitects:main Feb 17, 2022
@RumovZ RumovZ deleted the view-menu branch February 17, 2022 07:42
@abdnh
Copy link
Collaborator

abdnh commented Feb 17, 2022

Could Ctrl+= also be assigned as a shortcut for Zoom In so that the shortcut works on more keyboard layouts without having to press Shift (e.g. US and UK QWERTY keyboards, I think)?

The Anki Zoom add-on, which is quite popular, assigns both Ctrl++ and Ctrl+=. See https://github.com/ccz-2/Anki-Zoom/blob/6553d7b4aa6c2b1704466a334912a3c9048a7b03/__init__.py#L155

@dae
Copy link
Member

dae commented Feb 18, 2022

It looks like the editor is already using ctrl+= and ctrl+shift+= for subscript/superscript, so currently the subscript shortcut is conflicting with the zoom shortcut, and adding ctrl+= would make superscript conflict as well. 😅 I guess we're going to need to rethink one pair of these shortcuts; ideas welcome.

@RumovZ
Copy link
Collaborator Author

RumovZ commented Feb 18, 2022

Ctrl++/- for zoom are really prevalent, so I wouldn't change these shortcuts lightly. However, as Anki already has established conflicting ones, maybe we shouldn't assign them at all? I assume most users use either a mouse or a trackpad, with which zooming is much more ergonomic anyway.

@RumovZ
Copy link
Collaborator Author

RumovZ commented Feb 18, 2022

Then again, Ctrl+= and Ctrl+Shift+= are already broken for keyboards like mine, where = is Shift+0, as I just found out. 😅

@dae
Copy link
Member

dae commented Feb 18, 2022

Yeah, that issue has cropped up multiple times in the past when the subscript/superscript shortcuts were implemented in Python. Henrik's taken a different approach to handling shortcuts in the JS code that allows this case to work correctly IIRC, but not sure that's something we can do in Qt. Would removing the shortcuts and using ctrl+scrollwheel work for you instead @abdnh?

@abdnh
Copy link
Collaborator

abdnh commented Feb 18, 2022

Would removing the shortcuts and using ctrl+scrollwheel work for you instead @abdnh?

Yes, I'm fine with using ctrl+scrollwheel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add View menu to Qt toolbars
3 participants