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

Selectively disable zoom #1602

Merged
merged 3 commits into from
Jan 18, 2022
Merged

Selectively disable zoom #1602

merged 3 commits into from
Jan 18, 2022

Conversation

RumovZ
Copy link
Collaborator

@RumovZ RumovZ commented Jan 17, 2022

This disables zoom for the top and bottom bars in the main view. Unlike the previous eventfilter, it also disables zooming via scrolling.

  • Would be great if this could be tested by a mac user. It is my understanding that zooming can also be done on a mac by holding Control and a two finger swipe. This should also be disabled.
  • I've kept the filter for QNativeGestureEvent, but isn't that a bit unspecific? Are there no other gestures we also end up disabling?
  • I couldn't reproduce any of the issues described in Selectively enable pinch-to-zoom #985.

@dae
Copy link
Member

dae commented Jan 18, 2022

Thanks Rumo, seems to work well on a Mac+touchpad here (just pinching - I'm guessing the option key is only required when using a mousewheel).

The only other gesture that might possibly make sense is the pan one, which didn't exist in Qt5: https://doc.qt.io/qt-6/qnativegestureevent.html. Might be worth waiting for someone to say they miss it first. :-)

One remaining question - should we be always allowing zooming in the main webview / editor / etc, or should we limit it to particular views (eg review) where it might be useful?

@RumovZ
Copy link
Collaborator Author

RumovZ commented Jan 18, 2022

seems to work well on a Mac+touchpad here (just pinching - I'm guessing the option key is only required when using a mousewheel).

My mistake, I meant Control (mapped to Meta in Qt). Swiping with two fingers should trigger a scroll event, ergo you should be able to zoom with Control (on all OSes) and this gesture.
But even if I'm right, it's not that important because you would usually just pinch if you have a trackpad. I'd like to fix this if I should be wrong though.

One remaining question - should we be always allowing zooming in the main webview / editor / etc, or should we limit it to particular views (eg review) where it might be useful?

Not sure. I can't imagine why you would zoom on the deck tree instead of increasing the interface size, but it doesn't do any harm either, does it?
Disabling zoom in the deck tree, but not in review would be a bit more tricky, as we would need to dis-/enable and reset zoom when the main window state changes.

@dae
Copy link
Member

dae commented Jan 18, 2022

Maybe you're referring to an accessibility feature? When I turn it off, I don't see ctrl+two fingers up/down doing anything, and with it on, the entire screen zooms in/out (a handy feature that I use a fair bit!)

Screen Shot 2022-01-18 at 7 10 14 pm

You're right that it doesn't do much harm. It might be slightly confusing if users do it accidentally, but the same thing could be said about most of Anki's features :-) Thanks for your work on this Rumo.

@dae dae merged commit fe7a8db into ankitects:main Jan 18, 2022
@RumovZ
Copy link
Collaborator Author

RumovZ commented Jan 18, 2022

Then it's different for Mac. On my machine, Ctrl+Swipe does the same as Ctrl+Mouse wheel. Thanks for clarifying!
I better fix is_zoom_event() then, or we might get surprised in the future.

@RumovZ RumovZ deleted the zooming branch January 18, 2022 09:20
@RumovZ RumovZ mentioned this pull request Jan 19, 2022
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.

None yet

2 participants