Fix Linux numpad shortcuts on web #148988
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
github-actions
bot
added
a: text input
bleroux
force-pushed
the
fix_linux_numpad_shortcuts_on_web
branch
from
7234146 to
b8c3f3c
Compare
bleroux
force-pushed
the
fix_linux_numpad_shortcuts_on_web
branch
2 times, most recently
from
7d54c57 to
c013eb1
Compare
justinmc
approved these changes
May 24, 2024
Contributor
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM 👍 . Tricky catch. Thanks for the thorough tests.
Contributor
There was a problem hiding this comment.
Nit: Typically I'd see the closing parenthesis on the next line. Here and below.
Contributor
Author
There was a problem hiding this comment.
I did it on purpose because the analyzer was failing and complaining about skip requiring a comment.
I tried various format and that one was ok.
After seeing your comment, I had a closer look to the analyzer failure and, base on https://github.com/flutter/flutter/wiki/Tree-hygiene#skipped-tests, I figured out that the lint is about having an URL or [intended] in the comment, knowing this I was able to update the PR with a proper formatting.
bleroux
force-pushed
the
fix_linux_numpad_shortcuts_on_web
branch
from
c013eb1 to
4791efa
Compare
bleroux
force-pushed
the
fix_linux_numpad_shortcuts_on_web
branch
from
4791efa to
8069001
Compare
bleroux
added
the
autosubmit
|
"Thanks, @bleroux This is helpful." |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 26, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
May 26, 2024
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
May 26, 2024
flutter/flutter@cb26a01...6564471 2024-05-26 [email protected] Roll Flutter Engine from a207dfe8e96b to f3025ae1511b (1 revision) (flutter/flutter#149098) 2024-05-25 [email protected] Manual roll Flutter Engine from 1d154ff93a87 to a207dfe8e96b (10 revisions) (flutter/flutter#149096) 2024-05-25 [email protected] Fix Linux numpad shortcuts on web (flutter/flutter#148988) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
victorsanni
pushed a commit
to victorsanni/flutter
that referenced
this pull request
May 31, 2024
## Description This PRs fixes a Web issue on Linux related to numpad keys. In flutter#145464, I introduced numpad shortcuts for Linux. These shortcuts work well on a desktop Linux application but they broke the Linux+Web numpad logic. When I added these shortcuts, I expected them to not be active on Web (because I knew that on Web, those shortcuts are handled by the browser). But there is a trick: text editing shortcuts are still defined on Web but they are disabled at the editable text level so one can use them in components that are not `EditableText` (see flutter#103377). In this PR, I used the same approach than for other text editing shortcuts: when on web associate those shortcuts to the `DoNothingAndStopPropagationTextIntent` intent. ## Related Issue Fixes flutter#148447. ## Tests Updates 2 tests. Adds 2 tests.
|
Will this be cherry-picked to the stable branch? Our product currently depends on this fix as many of our users rely on numpad functionality, and we're experiencing a significant blockage due to this issue. We might need to downgrade to a previous stable version of Flutter that does not have this problem. Thanks for your assistance! |
Contributor
Author
m3c-fnicola
pushed a commit
to m3c-fnicola/flutter
that referenced
this pull request
Jun 24, 2024
## Description This PRs fixes a Web issue on Linux related to numpad keys. In flutter#145464, I introduced numpad shortcuts for Linux. These shortcuts work well on a desktop Linux application but they broke the Linux+Web numpad logic. When I added these shortcuts, I expected them to not be active on Web (because I knew that on Web, those shortcuts are handled by the browser). But there is a trick: text editing shortcuts are still defined on Web but they are disabled at the editable text level so one can use them in components that are not `EditableText` (see flutter#103377). In this PR, I used the same approach than for other text editing shortcuts: when on web associate those shortcuts to the `DoNothingAndStopPropagationTextIntent` intent. ## Related Issue Fixes flutter#148447. ## Tests Updates 2 tests. Adds 2 tests.
auto-submit bot
pushed a commit
that referenced
this pull request
Jul 1, 2024
Cherry pick request of #148988 to stable. Fixes an issue on Web+Linux that prevents users from inputting data using the numpad.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PRs fixes a Web issue on Linux related to numpad keys.
In #145464, I introduced numpad shortcuts for Linux. These shortcuts work well on a desktop Linux application but they broke the Linux+Web numpad logic.
When I added these shortcuts, I expected them to not be active on Web (because I knew that on Web, those shortcuts are handled by the browser). But there is a trick: text editing shortcuts are still defined on Web but they are disabled at the editable text level so one can use them in components that are not
EditableText(see #103377).In this PR, I used the same approach than for other text editing shortcuts: when on web associate those shortcuts to the
DoNothingAndStopPropagationTextIntentintent.Related Issue
Fixes #148447.
Tests
Updates 2 tests.
Adds 2 tests.