engine
engine copied to clipboard
flutter
Send PointerSignalKind.scale events from web
Send PointerSignalKind.scale instead of PointerSignalKind.scroll if ctrl key is pressed on wheel event.
Part of https://github.com/flutter/flutter/issues/112103
Sequence
- https://github.com/flutter/flutter/pull/112170
- https://github.com/flutter/engine/pull/36342
- https://github.com/flutter/flutter/pull/112172
- This PR
Pre-launch Checklist
- [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities.
- [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides].
- [x] I listed at least one issue that this PR fixes in the description above.
- [x] I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See [testing the engine] for instructions on writing and running engine tests.
- [x] I updated/added relevant documentation (doc comments with
///). - [x] I signed the [CLA].
- [x] All existing and new tests are passing.
To clarify, right now we are ignoring these ctrl-zoom events altogether. So this isn't causing things that would scroll before to now cause zooms.
@moffatman you are right that we ignore ctrl-zoom now, but that's not the case on Mac/iOS.
@mdebbar So on macOS, when using ctrl-scroll wheel, what's the expected behavior?
@moffatman I believe on macOS, it should just scroll even if ctrl is pressed.
@mdebbar I noticed sites which offer trackpad gestures (Ex: Figma) do not handle that properly, they will zoom-in if holding ctrl and scrolling. I guess the only option is to watch document onkey events and have a parallel tracking of the real ctrlKey. Do you think that's worth doing?
@moffatman I think, by default, we should do what the platform does (in this case, browsers on macOS scroll even if ctrl is pressed).
Apps can customize this behavior if they want to. I'm not sure if they can do such customization today with Flutter or not. Thoughts @justinmc ?
Right now I don't believe there's a straightforward way to override a keyboard shortcut and a mouse gesture combined.
@Renzo-Olivares would that be possible with your upcoming gesture shortcuts work?
Gold has detected about 8 new digest(s) on patchset 3. View them at https://flutter-engine-gold.skia.org/cl/github/36348
Right now I don't believe there's a straightforward way to override a keyboard shortcut and a mouse gesture combined.
I asked @gspencergoog about this recently, I was looking into shift+scroll to flip the scroll direction. He recommended not tying it into shortcuts, but just checking the keyboard when a scroll event comes in. So I have a proposal out that would add that key modifier on the inherited ScrollBehavior. Maybe something similar would suit here for ctrl?
https://github.com/flutter/flutter/pull/115610
@moffatman @justinmc @mdebbar @Piinks Are there still plans for this PR? It is getting a little stale...
Remaining issues
- Is there something that should be done here for future compatibility with framework-defined modifier keys
- I'm not so sure here, it doesn't make sense as the events are different types
- Using the mouse wheel while holding ctrl is not supposed to zoom on macOS, just scroll.
- macOS scroll+zooms and trackpad pinches will both have
event.ctrlKey, so we can't tell them apart from the event alone. Will need to add some sort of tracking of key-presses in order to tell if it's a truectrlKeyor not.
- macOS scroll+zooms and trackpad pinches will both have
To start with, I will fix the merge conflicts at least..
I still need to add some sort of tracking for the ctrl key press. Since right now it will zoom instead of scroll on macOS safari.
Would appreciate some review from anyone involved in web keyboard handling. I made some very lazy changes to expose what I needed to peek at the data. Maybe there is a better way?