Repository navigation
fix: Apply reversed axis flip by macOS on Web when scroll with modifier keys - #189506
Conversation
github-actions
Bot
added
framework
942c3fb to
3af3a0e
Compare
Gustl22
changed the title
3af3a0e to
218d9ef
Compare
github-actions
Bot
added
engine
218d9ef to
8d4dec4
Compare
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a reverse flip of the scroll axis when the Shift key is pressed on macOS with a mouse, normalizing the input received by the framework. It introduces a new isMacOS helper, exposes physical shift key constants, and adds corresponding tests. The review feedback suggests simplifying the implementation by removing the redundant debugEmulateMacOs test flag and avoiding hardcoding the pull request URL in the native macOS code comments.
|
This requires review from the web team. |
| final shiftKeyPressed = | ||
| (_keyboardConverter?.keyIsPressed(kPhysicalShiftLeft) ?? false) || | ||
| (_keyboardConverter?.keyIsPressed(kPhysicalShiftRight) ?? false); |
There was a problem hiding this comment.
Instead of relying on keyIsPressed, let's use the browser-provided event.shiftKey or event.getModifierState('Shift'). Obviously, tests will need to be changed accordingly.
There was a problem hiding this comment.
I replaced with your proposal.
As additional info: I am not 100% sure, whether the shift flip is applied on logical or physical keys. I tried to switch with Karabiner elements, but as they switch the physical key mapping, i cannot differentiate between logical and hard key in any use case. And OSX built-in Settings do not allow to remap the shift key. In the end a user barely is able to remap the Shift key in any case. But as the legacy pinch zoom modifier ctrl key also is applied logically (as far as I understood), I assume its the same for the shift key.
I also tested with varius input devices and wheels:
- Touch pad, scroll horizontally
- Touch pad, scroll vertically
- Mouse with vertical scroll wheel
- Mouse with horizontal scroll wheel
As one expects, both Physical Shift (Left or/and Right) and DOM event Shift state always are in sync and both are applied independently of the input source event. What I wanted to clarify with this, that the OS does not magically add a "Shift" state, when scrolling horizontally (either with the touch pad or the horizontal scroll mouse wheel) without actually pressing shift.
There was a problem hiding this comment.
Thanks for the extensive testing!!
105bca7 to
ca3cf2b
Compare
ca3cf2b to
625e0df
Compare
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/189506, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
flutter-zl
added
CICD
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/189506, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label. |
|
@flutter-zl not sure, how this can be progressed, it just states:
Thanks for your help |
flutter-zl
added
CICD
flutter-dashboard
Bot
removed
the
autosubmit
MacOS is flipping the vertical scroll (y) to a horizontal scroll (x), when the "Shift" modifier key is pressed.
To conform with other platforms, a reverse flip was already applied for native macOS, see: be9ae47
The same must be done for web platforms, as the event is received by the browser, but the flipping by OSX happens before.
Fixes #159868
See also #185146
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.