[Linux - TextInput] Do not add new line for send action #41895

bleroux · 2023-05-10T13:40:10Z

Description

This PR updates the Linux text input plugin to avoid adding a new line on a multiline text field when action is not set to TextInputAction.newline.

Related Issue

Linux implementation for flutter/flutter#125879

Tests

Adds 1 test.

jpnurmi · 2023-05-10T14:59:52Z

shell/platform/linux/fl_text_input_plugin.cc

      case GDK_KEY_ISO_Enter:
-        if (priv->input_type == kFlTextInputTypeMultiline) {
+        if (priv->input_type == kFlTextInputTypeMultiline &&
+            strcmp(priv->input_action, kFlTextInputActionSend) != 0) {


should we rather compare to "TextInputAction.newline" because it's probably the only text input action that should insert "\n"?

That seems to make sense to me after looking through all of the other TextInputAction values. That's also the default for multiline fields.

should we rather compare to "TextInputAction.newline" because it's probably the only text input action that should insert "\n"?

I wanted to be as conservative as possible because I'm not sure about the meaning of all the TextInputAction values on desktop.
But I agree that it would make sense to insert "\n" only when action is TextInputAction.newline.

@justinmc Can you confirm it would be preferable?

Yeah I agree 👍. So to be specific, I think you should keep the multiline check, and instead of checking that it's not kFlTextInputActionSend, check that it is the newline action.

justinmc

LGTM 👍

Will the solution be basically the same on all of the other platforms?

CC @cbracken in case you or anyone else more familiar with the engine wants to take a look.

justinmc · 2023-05-10T19:37:06Z

shell/platform/linux/fl_text_input_plugin.cc

      case GDK_KEY_ISO_Enter:
-        if (priv->input_type == kFlTextInputTypeMultiline) {
+        if (priv->input_type == kFlTextInputTypeMultiline &&
+            strcmp(priv->input_action, kFlTextInputActionSend) != 0) {


That seems to make sense to me after looking through all of the other TextInputAction values. That's also the default for multiline fields.

justinmc · 2023-05-10T19:38:02Z

shell/platform/linux/fl_text_input_plugin.cc

 static constexpr char kMultilineInputType[] = "TextInputType.multiline";
 static constexpr char kNoneInputType[] = "TextInputType.none";

+static constexpr char kFlTextInputActionSend[] = "TextInputAction.send";


Nit: Maybe kSendInputType in order to align with the other input type constants above?

bleroux · 2023-05-10T20:25:26Z

Will the solution be basically the same on all of the other platforms?

Yes. I have a PR almost ready for macOS. I worked on something similar for Flutter Web months ago. And I looked quickly at the Windows implementation, it is very similar. The main work here is to write the platform specific test.

So the main point is to agree if we want to add '\n' only for multiline text field with TextInputAction.newline? If so, I will update this PR and once it is merged I will file PRs for the other platforms.

chinmaygarde · 2023-05-11T20:14:56Z

@justinmc Do we have concensus on @bleroux s point? We can file issues for other platforms and land this.

jpnurmi

LGTM 👍

cbracken

Nice - thanks for the fix @bleroux!

…utter/engine#41895)

…126624) flutter/engine@748ef96...ef5c349 2023-05-11 [email protected] [Linux - TextInput] Do not add new line for send action (flutter/engine#41895) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll 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 Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…#41977) ## Description This PR updates the macOS text input plugin to avoid adding a new line on a multiline text field when action is not set to `TextInputAction.newline`. ## Related Issue macOS implementation for flutter/flutter#125879. (similar to the Linux implementation in #41895). ## Tests Adds 2 tests.

…lutter#126624) flutter/engine@748ef96...ef5c349 2023-05-11 [email protected] [Linux - TextInput] Do not add new line for send action (flutter/engine#41895) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll 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 Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

…ne (#42244) ## Description This PR updates the Windows text input plugin to avoid adding a new line on a multiline text field when action is not set to `TextInputAction.newline`. ## Related Issue Fixes flutter/flutter#125879 as Linux and macOS implementations are merged. Linux PR: #41895 macOS PR: #41977 ## Tests Adds 2 tests.

[Linux - TextInput] Do not add new line for send action

3c63299

flutter-dashboard bot added the platform-linux label May 10, 2023

bleroux requested review from jpnurmi and justinmc May 10, 2023 13:56

jpnurmi reviewed May 10, 2023

View reviewed changes

justinmc approved these changes May 10, 2023

View reviewed changes

Add a new line only when input action is TextInputAction.newline

e541715

bleroux requested a review from justinmc May 11, 2023 20:53

jpnurmi approved these changes May 11, 2023

View reviewed changes

cbracken approved these changes May 11, 2023

View reviewed changes

justinmc merged commit ef5c349 into flutter:main May 11, 2023

engine-flutter-autoroll mentioned this pull request May 12, 2023

Roll Flutter Engine from 748ef964b993 to ef5c349f04d8 (1 revision) flutter/flutter#126624

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request May 12, 2023

ef5c349f0 [Linux - TextInput] Do not add new line for send action (fl…

feb6909

…utter/engine#41895)

bleroux deleted the linux_do_not_add_newline_when_action_is_send branch May 12, 2023 06:45

bleroux mentioned this pull request May 12, 2023

[macOS - TextInput] Insert new line only when TextInputAction.newline #41977

Merged

bleroux mentioned this pull request May 23, 2023

[Windows - TextInput] Insert new line only when TextInputAction.newline #42244

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Linux - TextInput] Do not add new line for send action #41895

[Linux - TextInput] Do not add new line for send action #41895

Uh oh!

bleroux commented May 10, 2023 •

edited

Loading

Uh oh!

jpnurmi May 10, 2023

Uh oh!

justinmc May 10, 2023

Uh oh!

bleroux May 10, 2023

Uh oh!

justinmc May 11, 2023

Uh oh!

justinmc left a comment

Uh oh!

justinmc May 10, 2023

Uh oh!

justinmc May 10, 2023

Uh oh!

bleroux commented May 10, 2023

Uh oh!

chinmaygarde commented May 11, 2023

Uh oh!

jpnurmi left a comment

Uh oh!

cbracken left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[Linux - TextInput] Do not add new line for send action #41895

[Linux - TextInput] Do not add new line for send action #41895

Uh oh!

Conversation

bleroux commented May 10, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related Issue

Tests

Uh oh!

jpnurmi May 10, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc May 10, 2023

Choose a reason for hiding this comment

Uh oh!

bleroux May 10, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc May 11, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc May 10, 2023

Choose a reason for hiding this comment

Uh oh!

justinmc May 10, 2023

Choose a reason for hiding this comment

Uh oh!

bleroux commented May 10, 2023

Uh oh!

chinmaygarde commented May 11, 2023

Uh oh!

jpnurmi left a comment

Choose a reason for hiding this comment

Uh oh!

cbracken left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

bleroux commented May 10, 2023 •

edited

Loading

cbracken left a comment •

edited

Loading