Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add viewId to TextInputConfiguration #145708

Merged
merged 5 commits into from
Mar 28, 2024
Merged

Add viewId to TextInputConfiguration #145708

merged 5 commits into from
Mar 28, 2024

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Mar 25, 2024

In order for text fields to work correctly in multi-view on the web, we need to have the viewId information sent to the engine (TextInput.setClient). And while the text field is active, if it somehow moves to a new View, we need to inform the engine about such change (TextInput.updateConfig).

Engine PR: flutter/engine#51099
Fixes #137344

@github-actions github-actions bot added a: text input Entering text in a text field or keyboard related problems framework flutter/packages/flutter repository. See also f: labels. labels Mar 25, 2024
justinmc
justinmc approved these changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with nits 👍

One nit about docs, and another about a bug with changing viewId that I'm not sure is really a problem or not 😁 .

packages/flutter/lib/src/services/text_input.dart Show resolved Hide resolved
packages/flutter/lib/src/widgets/editable_text.dart Show resolved Hide resolved
packages/flutter/test/widgets/editable_text_test.dart Show resolved Hide resolved
@mdebbar mdebbar requested a review from justinmc March 25, 2024 19:48
justinmc
justinmc approved these changes Mar 25, 2024
View reviewed changes
Copy link
Contributor

@justinmc justinmc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renewing my LGTM 👍

I wouldn't worry about initState for readability purposes. It looks like all is well since you added the updateConfig check in the test.

goderbauer
goderbauer approved these changes Mar 25, 2024
View reviewed changes
Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

packages/flutter/lib/src/services/autofill.dart Outdated Show resolved Hide resolved
packages/flutter/lib/src/services/text_input.dart Outdated
@@ -13,6 +14,7 @@ import 'dart:ui' show
TextDirection;

import 'package:flutter/foundation.dart';
import 'package:flutter/widgets.dart' show View;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a layering violation. The services layer is below widgets and therefore it cannot import widgets.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you're not actually using View outside of docs. So, everything should still work if you just remove this import.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's right, I'm only importing it for the docs.

Without the import, VS Code can't navigate to the definition. How do you usually deal with this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without the import, VS Code can't navigate to the definition.

Yeah, that is a current limitation of dartdoc. Afaik, there is no work around. You could upvote dart-lang/sdk#49073 and/or dart-lang/sdk#50702 to get this fixed in dartdocs.

packages/flutter/lib/src/services/text_input.dart Outdated Show resolved Hide resolved
@goderbauer
Copy link
Member

Looks like some checks are failing, though...

ditman
ditman approved these changes Mar 27, 2024
View reviewed changes
Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If masters of the framework like this, ship!

packages/flutter/lib/src/widgets/editable_text.dart Show resolved Hide resolved
@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 28, 2024
@auto-submit auto-submit bot merged commit 42988d1 into flutter:master Mar 28, 2024
67 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
42988d1b0 Add viewId to TextInputConfiguration (flutter/flutter#1…
c16ab77 
…45708)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
42988d1b0 Add viewId to TextInputConfiguration (flutter/flutter#1…
c5990ef 
…45708)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
42988d1b0 Add viewId to TextInputConfiguration (flutter/flutter#1…
ad970e6 
…45708)
philipfranchi pushed a commit to philipfranchi/flutter-relabel-widgets that referenced this pull request Mar 28, 2024
@mdebbar @philipfranchi
Add viewId to TextInputConfiguration (flutter#145708)
7f2c567 
In order for text fields to work correctly in multi-view on the web, we need to have the `viewId` information sent to the engine (`TextInput.setClient`). And while the text field is active, if it somehow moves to a new `View`, we need to inform the engine about such change (`TextInput.updateConfig`).

Engine PR: flutter/engine#51099
Fixes flutter#137344
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 28, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
42988d1b0 Add viewId to TextInputConfiguration (flutter/flutter#1…
93acc20 
…45708)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 28, 2024
@engine-flutter-autoroll
Manual roll Flutter from dbdcead93225 to 89ea49204b37 (54 revisions) (#…
fbf44e2 
…6431)

Manual roll Flutter from dbdcead93225 to 89ea49204b37 (54 revisions)

Manual roll requested by dit@google.com

flutter/flutter@dbdcead...89ea492

2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from f71e5ad8586b to a396dc1a03a9 (3 revisions) (flutter/flutter#145928)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 043af350ae85 to f71e5ad8586b (1 revision) (flutter/flutter#145919)
2024-03-28 103135467+sealesj@users.noreply.github.com Refactor skp_generator_tests (flutter/flutter#145871)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c9d5adb6ff8 to 043af350ae85 (2 revisions) (flutter/flutter#145917)
2024-03-28 tessertaha@gmail.com Update `TabBar` and `TabBar.secondary` to use indicator height/color M3 tokens (flutter/flutter#145753)
2024-03-28 engine-flutter-autoroll@skia.org Roll Packages from e6b3e11 to 924c7e6 (5 revisions) (flutter/flutter#145915)
2024-03-28 mdebbar@google.com Add `viewId` to `TextInputConfiguration` (flutter/flutter#145708)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9df2d3a0778e to 7c9d5adb6ff8 (3 revisions) (flutter/flutter#145909)
2024-03-28 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from c602abdbae16 to 9df2d3a0778e (10 revisions) (flutter/flutter#145903)
2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) (#145877)" (flutter/flutter#145901)
2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 922c7b133bc2 to b3516c4c5683 (1 revision) (#145879)" (flutter/flutter#145900)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 922c7b133bc2 to b3516c4c5683 (1 revision) (flutter/flutter#145879)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) (flutter/flutter#145877)
2024-03-28 rmolivares@renzo-olivares.dev Remove deprecated `TextTheme` members (flutter/flutter#139255)
2024-03-27 maRci002@users.noreply.github.com [WIP] Predictive back support for routes (flutter/flutter#141373)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 73c145c9ac3a to c602abdbae16 (1 revision) (flutter/flutter#145865)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d65662541682 to 73c145c9ac3a (8 revisions) (flutter/flutter#145862)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from b7dddee939f2 to d65662541682 (2 revisions) (flutter/flutter#145851)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 00dab0d9d310 to b7dddee939f2 (2 revisions) (flutter/flutter#145841)
2024-03-27 engine-flutter-autoroll@skia.org Roll Packages from ab1630b to e6b3e11 (6 revisions) (flutter/flutter#145833)
2024-03-27 103135467+sealesj@users.noreply.github.com Refactor web long running tests (flutter/flutter#145776)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from da64c6bcbbb6 to 00dab0d9d310 (1 revision) (flutter/flutter#145830)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d6c6ba5aa157 to da64c6bcbbb6 (1 revision) (flutter/flutter#145811)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 064a4f5d9042 to d6c6ba5aa157 (1 revision) (flutter/flutter#145807)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from cad8e7a9ad70 to 064a4f5d9042 (1 revision) (flutter/flutter#145805)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 441005698702 to cad8e7a9ad70 (1 revision) (flutter/flutter#145804)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d872d50e53f4 to 441005698702 (1 revision) (flutter/flutter#145803)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 92ebd47dd8a8 to d872d50e53f4 (6 revisions) (flutter/flutter#145801)
2024-03-27 godofredoc@google.com Update localization files. (flutter/flutter#145780)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 026d8902e3b5 to 92ebd47dd8a8 (1 revision) (flutter/flutter#145788)
2024-03-26 ditman@gmail.com [web] Add BackgroundIsolateBinaryMessenger.ensureInitialized to web. (flutter/flutter#145786)
2024-03-26 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.0 to 4.1.1 (flutter/flutter#145787)
2024-03-26 christopherfujino@gmail.com Roll pub packages and regenerate gradle lockfiles (flutter/flutter#145727)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5c7aea6f20fc to 026d8902e3b5 (1 revision) (flutter/flutter#145785)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from baede78d2352 to 5c7aea6f20fc (2 revisions) (flutter/flutter#145784)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from cffd1dcfe6a5 to baede78d2352 (2 revisions) (flutter/flutter#145778)
2024-03-26 dwzrlp@163.com Correct typo: "Free" to "Three" in comments (flutter/flutter#145689)
2024-03-26 tessertaha@gmail.com Fix disabled `DropdownMenu` doesn't defer the mouse cursor (flutter/flutter#145686)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2d93a64cbc7 to cffd1dcfe6a5 (9 revisions) (flutter/flutter#145773)
2024-03-26 engine-flutter-autoroll@skia.org Roll Packages from 28d126c to ab1630b (1 revision) (flutter/flutter#145755)
2024-03-26 sokolovskyi.konstantin@gmail.com Memory leaks clean up 2 (flutter/flutter#145757)
2024-03-26 sokolovskyi.konstantin@gmail.com Fix memory leak in Overlay.wrap. (flutter/flutter#145744)
2024-03-26 jacksongardner@google.com Be tolerant of backticks around directory name in `pub` output. (flutter/flutter#145768)
2024-03-26 tessertaha@gmail.com Fix `ExpansionTile` Expanded/Collapsed announcement is interrupted by VoiceOver (flutter/flutter#143936)
...
@mdebbar mdebbar deleted the text_editing_view_id branch April 3, 2024 14:44
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 14, 2024
@engine-flutter-autoroll
42988d1b0 Add viewId to TextInputConfiguration (flutter/flutter#1…
fa17401 
…45708)
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@engine-flutter-autoroll @arc-yong
Manual roll Flutter from dbdcead93225 to 89ea49204b37 (54 revisions) (f…
7f61172 
…lutter#6431)

Manual roll Flutter from dbdcead93225 to 89ea49204b37 (54 revisions)

Manual roll requested by dit@google.com

flutter/flutter@dbdcead...89ea492

2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from f71e5ad8586b to a396dc1a03a9 (3 revisions) (flutter/flutter#145928)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 043af350ae85 to f71e5ad8586b (1 revision) (flutter/flutter#145919)
2024-03-28 103135467+sealesj@users.noreply.github.com Refactor skp_generator_tests (flutter/flutter#145871)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 7c9d5adb6ff8 to 043af350ae85 (2 revisions) (flutter/flutter#145917)
2024-03-28 tessertaha@gmail.com Update `TabBar` and `TabBar.secondary` to use indicator height/color M3 tokens (flutter/flutter#145753)
2024-03-28 engine-flutter-autoroll@skia.org Roll Packages from e6b3e11 to 924c7e6 (5 revisions) (flutter/flutter#145915)
2024-03-28 mdebbar@google.com Add `viewId` to `TextInputConfiguration` (flutter/flutter#145708)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9df2d3a0778e to 7c9d5adb6ff8 (3 revisions) (flutter/flutter#145909)
2024-03-28 engine-flutter-autoroll@skia.org Manual roll Flutter Engine from c602abdbae16 to 9df2d3a0778e (10 revisions) (flutter/flutter#145903)
2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) (#145877)" (flutter/flutter#145901)
2024-03-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Flutter Engine from 922c7b133bc2 to b3516c4c5683 (1 revision) (#145879)" (flutter/flutter#145900)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from 922c7b133bc2 to b3516c4c5683 (1 revision) (flutter/flutter#145879)
2024-03-28 engine-flutter-autoroll@skia.org Roll Flutter Engine from c602abdbae16 to 922c7b133bc2 (7 revisions) (flutter/flutter#145877)
2024-03-28 rmolivares@renzo-olivares.dev Remove deprecated `TextTheme` members (flutter/flutter#139255)
2024-03-27 maRci002@users.noreply.github.com [WIP] Predictive back support for routes (flutter/flutter#141373)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 73c145c9ac3a to c602abdbae16 (1 revision) (flutter/flutter#145865)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d65662541682 to 73c145c9ac3a (8 revisions) (flutter/flutter#145862)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from b7dddee939f2 to d65662541682 (2 revisions) (flutter/flutter#145851)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 00dab0d9d310 to b7dddee939f2 (2 revisions) (flutter/flutter#145841)
2024-03-27 engine-flutter-autoroll@skia.org Roll Packages from ab1630b to e6b3e11 (6 revisions) (flutter/flutter#145833)
2024-03-27 103135467+sealesj@users.noreply.github.com Refactor web long running tests (flutter/flutter#145776)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from da64c6bcbbb6 to 00dab0d9d310 (1 revision) (flutter/flutter#145830)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d6c6ba5aa157 to da64c6bcbbb6 (1 revision) (flutter/flutter#145811)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 064a4f5d9042 to d6c6ba5aa157 (1 revision) (flutter/flutter#145807)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from cad8e7a9ad70 to 064a4f5d9042 (1 revision) (flutter/flutter#145805)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 441005698702 to cad8e7a9ad70 (1 revision) (flutter/flutter#145804)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from d872d50e53f4 to 441005698702 (1 revision) (flutter/flutter#145803)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 92ebd47dd8a8 to d872d50e53f4 (6 revisions) (flutter/flutter#145801)
2024-03-27 godofredoc@google.com Update localization files. (flutter/flutter#145780)
2024-03-27 engine-flutter-autoroll@skia.org Roll Flutter Engine from 026d8902e3b5 to 92ebd47dd8a8 (1 revision) (flutter/flutter#145788)
2024-03-26 ditman@gmail.com [web] Add BackgroundIsolateBinaryMessenger.ensureInitialized to web. (flutter/flutter#145786)
2024-03-26 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.1.0 to 4.1.1 (flutter/flutter#145787)
2024-03-26 christopherfujino@gmail.com Roll pub packages and regenerate gradle lockfiles (flutter/flutter#145727)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from 5c7aea6f20fc to 026d8902e3b5 (1 revision) (flutter/flutter#145785)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from baede78d2352 to 5c7aea6f20fc (2 revisions) (flutter/flutter#145784)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from cffd1dcfe6a5 to baede78d2352 (2 revisions) (flutter/flutter#145778)
2024-03-26 dwzrlp@163.com Correct typo: "Free" to "Three" in comments (flutter/flutter#145689)
2024-03-26 tessertaha@gmail.com Fix disabled `DropdownMenu` doesn't defer the mouse cursor (flutter/flutter#145686)
2024-03-26 engine-flutter-autoroll@skia.org Roll Flutter Engine from b2d93a64cbc7 to cffd1dcfe6a5 (9 revisions) (flutter/flutter#145773)
2024-03-26 engine-flutter-autoroll@skia.org Roll Packages from 28d126c to ab1630b (1 revision) (flutter/flutter#145755)
2024-03-26 sokolovskyi.konstantin@gmail.com Memory leaks clean up 2 (flutter/flutter#145757)
2024-03-26 sokolovskyi.konstantin@gmail.com Fix memory leak in Overlay.wrap. (flutter/flutter#145744)
2024-03-26 jacksongardner@google.com Be tolerant of backticks around directory name in `pub` output. (flutter/flutter#145768)
2024-03-26 tessertaha@gmail.com Fix `ExpansionTile` Expanded/Collapsed announcement is interrupted by VoiceOver (flutter/flutter#143936)
...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@goderbauer goderbauer goderbauer approved these changes

@justinmc justinmc justinmc approved these changes

@ditman ditman ditman approved these changes

Assignees
No one assigned
Labels
a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[web] Text fields support multi-view
4 participants
@mdebbar @goderbauer @justinmc @ditman