Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl #45522

Satsrag · 2023-09-07T00:35:24Z

When registering customized TextInputControl, the _PlatformTextInputControl sends inputType = TextInputType.none to the engine. After receiving TextInputType.none, the engine on the Web will create a <input> element instead of <textarea>. So there is no way to input \n(multiline).

This is my solution. I tested in Android Chrome, iOS Safari, and macOS Chrome. It works for me. But I'm not sure about other use cases. I pleasure, If someone gives me suggestions.

flutter-dashboard · 2023-09-07T00:35:26Z

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

google-cla · 2023-09-07T00:35:27Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

mdebbar

Thanks for the contribution!

It makes sense to fix this bug, but I'm not sure if we should always treat TextInputType.none as multiline.

lib/web_ui/lib/src/engine/text_editing/input_type.dart

lib/web_ui/lib/src/engine/text_editing/text_editing.dart

lib/web_ui/lib/src/engine/text_editing/input_type.dart

Satsrag · 2023-12-25T01:30:49Z

I'm sorry about asking for review again and again. But this time, I think that all work is done. Ask for a review again.
In framework PR, forceMultiline was renamed to isMultiline. So here I was renamed to isMultiline and added tests.

htoor3

Thank you for this! This lgtm. Before merging, let's verify the framework approach is the correct one with @Renzo-Olivares

htoor3 · 2023-12-26T18:30:38Z

lib/web_ui/lib/src/engine/text_editing/input_type.dart

  const EngineInputType();

-  static EngineInputType fromName(String name, {bool isDecimal = false}) {
+  static EngineInputType fromName(String name, {bool isDecimal = false, bool isMultiline= false}) {


nit: change isMultiline= false to isMultiline = false

Sorry about this. Now I have fixed it. Thank you!

@htoor3, Happy New Year! The framework PR has been merged. Is it time to merge this PR?

@LongCatIsLooong

For #125875 and flutter/engine#45522 According to [this discussion](#139446 (comment)), Just added `forceMultiline` to JSON on `_configurationToJson`. @LongCatIsLooong @Renzo-Olivares

htoor3 · 2024-01-03T18:18:35Z

lgtm, thank you @Satsrag for your contribution!

Satsrag · 2024-01-09T23:52:47Z

@mdebbar, Please have a review.

Satsrag · 2024-01-27T10:41:06Z

@htoor3 when this PR will be merged?

auto-submit · 2024-01-29T16:15:56Z

auto label is removed for flutter/engine/45522, due to This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

htoor3 · 2024-01-29T17:46:42Z

Let's get a review from @mdebbar and then we can get this merged

mdebbar

Thanks for helping make Flutter better!

The PR looks good to me with some minor suggestions.

lib/web_ui/lib/src/engine/text_editing/text_editing.dart

…customized TextInputControl

Co-authored-by: Mouad Debbar <[email protected]>

Satsrag · 2024-02-07T01:43:49Z

Thanks for helping make Flutter better!

The PR looks good to me with some minor suggestions.

@mdebbar Thank you! Fixed according to your suggestion.

…gistering customized TextInputControl (flutter/engine#45522)

…143082) flutter/engine@1ac6beb...6807342 2024-02-07 [email protected] Roll Skia from 0fb71bedffa2 to 7777976cd518 (3 revisions) (flutter/engine#50435) 2024-02-07 [email protected] Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl (flutter/engine#45522) 2024-02-07 [email protected] Roll Skia from 3cc2a15196cb to 0fb71bedffa2 (1 revision) (flutter/engine#50429) 2024-02-07 [email protected] Roll Skia from 1b266b36a4c8 to 3cc2a15196cb (1 revision) (flutter/engine#50427) 2024-02-07 [email protected] [Impeller] Always pop clips on restore. (flutter/engine#50419) 2024-02-07 [email protected] Roll Dart SDK from 35269fa71956 to 452dd17120b7 (1 revision) (flutter/engine#50424) 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://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

github-actions bot added the platform-web Code specifically for the web engine label Sep 7, 2023

Satsrag force-pushed the input_control_bug branch from 68dde85 to 0269e8a Compare September 7, 2023 02:41

Satsrag mentioned this pull request Sep 19, 2023

Cannot Input New Line '\n' When Using TextInputControl on Web flutter/flutter#125875

Closed

2 tasks

mdebbar reviewed Nov 22, 2023

View reviewed changes

lib/web_ui/lib/src/engine/text_editing/input_type.dart Show resolved Hide resolved

lib/web_ui/lib/src/engine/text_editing/text_editing.dart Outdated Show resolved Hide resolved

Satsrag force-pushed the input_control_bug branch from 0269e8a to 629765b Compare November 23, 2023 04:28

Satsrag force-pushed the input_control_bug branch from 629765b to 12c2be7 Compare December 3, 2023 12:06

Satsrag mentioned this pull request Dec 3, 2023

Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl flutter/flutter#139446

Closed

htoor3 reviewed Dec 5, 2023

View reviewed changes

lib/web_ui/lib/src/engine/text_editing/input_type.dart Outdated Show resolved Hide resolved

Satsrag commented Dec 6, 2023

View reviewed changes

lib/web_ui/lib/src/engine/text_editing/input_type.dart Show resolved Hide resolved

Satsrag force-pushed the input_control_bug branch 3 times, most recently from bb0b606 to 101df2c Compare December 8, 2023 05:43

Satsrag requested review from htoor3 and mdebbar December 9, 2023 16:26

Satsrag mentioned this pull request Dec 19, 2023

fix: cannot input new line using custom input control flutter/flutter#140356

Merged

Satsrag force-pushed the input_control_bug branch from 101df2c to 0a98e39 Compare December 20, 2023 00:18

htoor3 approved these changes Dec 26, 2023

View reviewed changes

Satsrag force-pushed the input_control_bug branch from 5e18965 to 7523e7d Compare January 3, 2024 07:51

htoor3 added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2024

auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2024

mdebbar approved these changes Feb 6, 2024

View reviewed changes

lib/web_ui/lib/src/engine/text_editing/text_editing.dart Outdated Show resolved Hide resolved

lib/web_ui/lib/src/engine/text_editing/text_editing.dart Outdated Show resolved Hide resolved

Satsrag force-pushed the input_control_bug branch from 1c18add to ef1a8db Compare February 6, 2024 17:16

Satsrag and others added 11 commits February 7, 2024 08:25

fix: On the Web, cannot support multiline inputting when registering …

7dd5fb5

…customized TextInputControl

feat: added forceMutiline to none input type

f8abce9

feat: added test for MultilineNone mode

f08864e

feat: added comment to MultilineNoTextInputType

b13a673

feat: added link MultilineNoTex issue

8804dd5

forceMultiline to isMultiline

bc99129

feat: added none multline test

e3866cb

fix: input_type.dart isMultiline format

af1a8a5

Update lib/web_ui/lib/src/engine/text_editing/text_editing.dart

0dcf5d2

Co-authored-by: Mouad Debbar <[email protected]>

Update lib/web_ui/lib/src/engine/text_editing/text_editing.dart

e754ce2

Co-authored-by: Mouad Debbar <[email protected]>

feat: MultilineNoTextInputType extend MultilineInputType

9b49e0b

Satsrag force-pushed the input_control_bug branch from ef1a8db to 9b49e0b Compare February 7, 2024 00:25

mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024

auto-submit bot merged commit 8a99fc4 into flutter:main Feb 7, 2024

engine-flutter-autoroll mentioned this pull request Feb 7, 2024

Roll Flutter Engine from 1ac6beb8a3c2 to 6807342305e4 (6 revisions) flutter/flutter#143082

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 7, 2024

8a99fc448 Fix: On the Web, cannot support multiline inputting when re…

6fe9773

…gistering customized TextInputControl (flutter/engine#45522)

Satsrag mentioned this pull request Feb 7, 2024

On Web, cannot input return '\n' Satsrag/embed_input#1

Closed

Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl #45522

Fix: On the Web, cannot support multiline inputting when registering customized TextInputControl #45522

Uh oh!

Conversation

Satsrag commented Sep 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flutter-dashboard bot commented Sep 7, 2023

Uh oh!

google-cla bot commented Sep 7, 2023

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Satsrag commented Dec 25, 2023

Uh oh!

htoor3 left a comment

Choose a reason for hiding this comment

Uh oh!

htoor3 Dec 26, 2023

Choose a reason for hiding this comment

Uh oh!

Satsrag Dec 27, 2023

Choose a reason for hiding this comment

Uh oh!

Satsrag Jan 3, 2024

Choose a reason for hiding this comment

Uh oh!

htoor3 commented Jan 3, 2024

Uh oh!

Satsrag commented Jan 9, 2024

Uh oh!

Satsrag commented Jan 27, 2024

Uh oh!

auto-submit bot commented Jan 29, 2024

Uh oh!

htoor3 commented Jan 29, 2024

Uh oh!

mdebbar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Satsrag commented Feb 7, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Satsrag commented Sep 7, 2023 •

edited

Loading