Skip to content

Conversation

@LongCatIsLooong
Copy link
Contributor

The TextEditingController.text setter has been a major source of confusion, as using it would usually result in loss of cursor position if the text field is active. Arguably it also should not be part of the controller's interface since it is a convenience method.

There are many issues, for example: #65547

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

The `TextEditingController.text` setter has been a major source of
confusion, as using it would usually result in loss of cursor position
if the text field is active. Argubly it also should not be part of the
controller's interface since it is a convenience method.

There are many issues, for example: flutter#65547
@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. f: material design flutter/packages/flutter/material repository. f: cupertino flutter/packages/flutter/cupertino repository labels Oct 29, 2024
@LongCatIsLooong
Copy link
Contributor Author

Some takeaways from the errors I got from marking TextEditingController.text as @visibleForTesting:

  1. Some SearchDelegate variant is using it to implement query. -> Ideally we want to change that because it would reset the cursor location
  2. Some call sites are using to set text to empty which should be fine.

@LongCatIsLooong LongCatIsLooong marked this pull request as ready for review January 8, 2025 21:31
@LongCatIsLooong LongCatIsLooong changed the title Mark TextEditingController.text setter as @visibleForTesting Update TextEditingController.text documentation to recommend against using it in production code Jan 8, 2025
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

@justinmc
Copy link
Contributor

justinmc commented Jan 9, 2025

@LongCatIsLooong Sorry to bug you while you're out, but if this shouldn't be merged, please let me know! Otherwise I'll probably go ahead and merge this next week or so.

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 10, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jan 10, 2025
Merged via the queue into flutter:master with commit 9e211ca Jan 10, 2025
104 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 10, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 11, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 11, 2025
@LongCatIsLooong LongCatIsLooong deleted the TextEditingController-text-setter-test-only branch January 12, 2025 01:21
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 13, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 13, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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
maheshj01 pushed a commit to maheshj01/flutter that referenced this pull request Jan 15, 2025
…t using it in production code (flutter#157769)

The `TextEditingController.text` setter has been a major source of
confusion, as using it would usually result in loss of cursor position
if the text field is active. Arguably it also should not be part of the
controller's interface since it is a convenience method.

There are many issues, for example:
flutter#65547


## Pre-launch Checklist

- [ ] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [ ] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [ ] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [ ] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [ ] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
flutter/flutter@864d4f5...72db8f6

2025-01-13 [email protected] Roll Packages from 6554751 to 3c3bc68 (16 revisions) (flutter/flutter#161515)
2025-01-13 [email protected] Update error message for when leading/trailing width exceeds `ListTile` width and add missing test (flutter/flutter#161091)
2025-01-13 [email protected] Deprecate unused `ButtonStyleButton.iconAlignment` property (flutter/flutter#160023)
2025-01-12 [email protected] Provide monitor information. (flutter/flutter#161359)
2025-01-11 [email protected] [android_engine_test] Remove background/foreground from surface texture trampoline test. (flutter/flutter#161441)
2025-01-11 [email protected] Remove some miscellaneous references to Cirrus. (flutter/flutter#161390)
2025-01-11 [email protected] add semantics role and tab (flutter/flutter#161260)
2025-01-10 [email protected] CupertinoSheetRoute (flutter/flutter#157568)
2025-01-10 [email protected] Update `TextEditingController.text` documentation to recommend against using it in production code (flutter/flutter#157769)
2025-01-10 [email protected] ð��� [tool] Add a wirelessly connected device name as `displayName` (flutter/flutter#160497)
2025-01-10 [email protected] FixForward: method was renamed (flutter/flutter#161431)

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] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: text input Entering text in a text field or keyboard related problems f: cupertino flutter/packages/flutter/cupertino repository f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants