This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Fix TextField performing both new line and input action #36893
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
flutter-dashboard
bot
added
the
platform-web
bleroux
force-pushed
the
fix_text_field_performing_both_newline_and_input_action
branch
4 times, most recently
from
4299ff1 to
30c1793
Compare
mdebbar
approved these changes
Nov 4, 2022
| @@ -1369,6 +1369,10 @@ abstract class DefaultTextEditingStrategy with CompositionAwareMixin implements | |||
| final DomKeyboardEvent event = e as DomKeyboardEvent; | |||
| if (event.keyCode == _kReturnKeyCode) { | |||
| onAction!(inputConfiguration.inputAction); | |||
| // Stop key event propagation if the input type is not multiline. | |||
There was a problem hiding this comment.
e.stopPropagation() is different from e.preventDefault(), and your comment could be confused for the former.
You could phrase as follows: "Prevent the browser from inserting a new line when it's not a multiline input."
bleroux
force-pushed
the
fix_text_field_performing_both_newline_and_input_action
branch
from
30c1793 to
b2944cd
Compare
htoor3
reviewed
Nov 4, 2022
| @@ -390,6 +390,28 @@ Future<void> testMain() async { | |||
| expect(event.defaultPrevented, isFalse); | |||
| }); | |||
|
|
|||
| test('Triggers input action and prevent new line key event for single line field', () { | |||
There was a problem hiding this comment.
This looks great!
@mdebbar would it be worth adding a test to verify return key behavior for multiline inputs is preserved?
There was a problem hiding this comment.
There is already a test for that right above this :)
There was a problem hiding this comment.
@htoor3 If I remember well this is already covered by an existing test. (I first I implemented this change for both single line and multiline inputs and it broke several tests. I ended targeting only single line, not because of those failing tests, but because changing the behavior for multilines inputs will require some discussions. I will open a new issue to discuss this, see flutter/flutter#113559 (comment)).
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Nov 4, 2022
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Nov 4, 2022
…114696) * f3d0b6af7 Fix TextField performing both new line and input action (flutter/engine#36893) * e947833ce Roll Skia from 1272b520c082 to 7a45d3123f8b (4 revisions) (flutter/engine#37316)
schwa423
pushed a commit
to schwa423/engine
that referenced
this pull request
Nov 16, 2022
Co-authored-by: Bruno Leroux <[email protected]>
shogohida
pushed a commit
to shogohida/flutter
that referenced
this pull request
Dec 7, 2022
…lutter#114696) * f3d0b6af7 Fix TextField performing both new line and input action (flutter/engine#36893) * e947833ce Roll Skia from 1272b520c082 to 7a45d3123f8b (4 revisions) (flutter/engine#37316)
gspencergoog
pushed a commit
to gspencergoog/flutter
that referenced
this pull request
Jan 19, 2023
…lutter#114696) * f3d0b6af7 Fix TextField performing both new line and input action (flutter/engine#36893) * e947833ce Roll Skia from 1272b520c082 to 7a45d3123f8b (4 revisions) (flutter/engine#37316)
bleroux
deleted the
fix_text_field_performing_both_newline_and_input_action
branch
auto-submit bot
pushed a commit
that referenced
this pull request
Jun 20, 2024
#53453) ## Description This PR prevents new line key event from being dispatched for a multiline text field when `TextField.textInputAction` is not `TextInputAction.newline`. Since #33428, web engine does not prevent new line key events. In #36893, I fixed a similar issue for single line text fields. At that time I was not sure if we want to fix it for multiline text fields. I checked again on non-web platforms (macos, iOS, Android) and the new line is not added if the input action is not `TextInputAction.newline`. For a **multiline field**, the default text input action is `TextInputAction.newline`. If the developer sets text input action to another value: - before this PR, the action is performed and a new line is added. - after this PR, the action is performed but no new line is added. ## Related Issue Fixes flutter/flutter#145051 ## Tests Adds 1 tests, updates 3 tests.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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 PR prevents new line key event from being dispatched when
TextField.textInputActionis notTextInputAction.newlineand the text field is not multiline.Related Issue
Fixes flutter/flutter#113559
Implementation choice
Since #33428, web engine does not prevent new line key events.
Here, I choose to prevent it whether the field is multiline or not.For a single line field (the default text input action is
TextInputAction.done):TextInputAction.nextfor instance), the newly focused field inserts the new line (see [Web] TextField receiving newline on TextInputAction.next flutter#113559).For a multiline field, the default text input action is
TextInputAction.newline.If the developer sets text input action to another value:
after this PR, the action is performed but no new line is added.@mdebbar For multiline text fields, it seems that it was expected to always insert a new line.
I think that it should not when the user choose to set the input action to something else than.TextInputAction.newlineEdit: I have tested on Linux and MacOS and on both platforms a new line is inserted and the action is performed. So it is probably better to do the same on Web for the moment.
Tests
Adds 2 tests, updates 2 tests.