Remove editable_text_utils cross-imports from material and cupertino …#184519
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces local editable_text_utils.dart files to the cupertino and material test directories and updates several test files to import them. Feedback includes optimizing the findRenderEditable function to avoid late initialization errors and using the targetPlatform.name property for more idiomatic platform string parsing in sendKeys.
| import 'package:flutter_test/flutter_test.dart'; | ||
| import '../widgets/clipboard_utils.dart'; | ||
| import '../widgets/editable_text_utils.dart'; | ||
| import 'editable_text_utils.dart'; |
There was a problem hiding this comment.
Could you reorder this as well such that this import is after ../ imports?
|
|
||
| import '../widgets/clipboard_utils.dart'; | ||
| import '../widgets/editable_text_utils.dart'; | ||
| import 'editable_text_utils.dart'; |
There was a problem hiding this comment.
Could you reorder this import to be after ../ imports?
|
|
||
| import '../widgets/clipboard_utils.dart'; | ||
| import '../widgets/editable_text_utils.dart' show textOffsetToPosition; | ||
| import 'editable_text_utils.dart' show textOffsetToPosition; |
There was a problem hiding this comment.
Please reorder this one as well
|
|
||
| import '../widgets/clipboard_utils.dart'; | ||
| import '../widgets/editable_text_utils.dart'; | ||
| import 'editable_text_utils.dart'; |
|
@justinmc It'd be super nice if the packages repo had some sort of tooling to verify this file is kept in-sync between the widgets/cupertino/material tests libraries. Otherwise, I imagine folks will make edits to these files without realizing they need two update two other locations too. Does that tooling sound feasible? Let me know if I should create an issue :) Other than this maintenance concern, this change looks good to me! |
|
Thanks for the review! I've reordered local imports to come after |
ec2a81c to
1bc0f90
Compare
|
@xfce0 Looks like you'll need to run |
|
Thanks for catching that! Fixed the formatting now. |
|
@xfce0 Looks like there are merge conflicts that will need to be resolved before we can re-run tests. |
81ed3f5 to
d4e1669
Compare
loic-sharma
left a comment
There was a problem hiding this comment.
Thanks for cleaning this up!
flutter/flutter@a0924c7...05e0ae0 2026-04-08 meylis@divine.video Fix Android engine flags defaulting to true for malformed values (flutter/flutter#184631) 2026-04-08 katelovett@google.com Try one more again (flutter/flutter#184767) 2026-04-08 goderbauer@google.com Remove custom `analysis_options.yaml` from `imitation_game_flutter` (flutter/flutter#184717) 2026-04-08 victorsanniay@gmail.com Add more error handling to unawaited callsites (flutter/flutter#184526) 2026-04-08 34465683+rkishan516@users.noreply.github.com Refactor: remove material from absorb_ponter_test, container_test, lookup_boundary_test, page_view_test, router_test, semantics_clipping_test, semantics_merge_test, shadow_test, text_test (flutter/flutter#183309) 2026-04-08 73785960+xfce0@users.noreply.github.com Remove editable_text_utils cross-imports from material and cupertino … (flutter/flutter#184519) 2026-04-08 robert.ancell@canonical.com Replace hard coded max path length with system defined one. (flutter/flutter#184697) 2026-04-08 jesswon@google.com [Re-land] Add Support For Built-in Kotlin (flutter/flutter#184745) 2026-04-08 15619084+vashworth@users.noreply.github.com Manually stop and continue LLDB breakpoints on Xcode 26.4+ (flutter/flutter#184690) 2026-04-08 katelovett@google.com Code freeze workflow (flutter/flutter#184246) 2026-04-08 737941+loic-sharma@users.noreply.github.com [Dot shorthands] Migrate examples/api/lib/widgets (flutter/flutter#183965) 2026-04-08 59215665+davidhicks980@users.noreply.github.com [cupertino.dart] Implement CupertinoMenuAnchor and CupertinoMenuItem using RawMenuAnchor (flutter/flutter#182036) 2026-04-08 87018443+mayanksharma9@users.noreply.github.com [Semantics] clarify Android header docs (flutter/flutter#183573) 2026-04-08 dacoharkes@google.com [ci] mac build_test bringup false (flutter/flutter#184738) 2026-04-08 34871572+gmackall@users.noreply.github.com Reland "Apply rect clipping to surface views" (flutter/flutter#184732) 2026-04-08 bkonyi@google.com Remove bringup label for resharded Windows tool_integration_tests shards (flutter/flutter#184721) 2026-04-08 namangoyaldev@gmail.com Tool: Add search and filtering to widget preview scaffold (flutter/flutter#184023) 2026-04-08 36861262+QuncCccccc@users.noreply.github.com Update localization from translation console (flutter/flutter#184742) 2026-04-07 rmolivares@renzo-olivares.dev Revert "Add Support For Built-in Kotlin (#184227)" (flutter/flutter#184739) 2026-04-07 34871572+gmackall@users.noreply.github.com Collect HCPP adoption analytics for flutter run/build apk/build appbundle (flutter/flutter#184225) 2026-04-07 jacksongardner@google.com Add a github workflow for reverting PRs. (flutter/flutter#184593) 2026-04-07 jesswon@google.com Add Support For Built-in Kotlin (flutter/flutter#184227) 2026-04-07 34871572+gmackall@users.noreply.github.com Revert "Apply rect clipping to surface views (#184471)" (flutter/flutter#184728) 2026-04-07 jesswon@google.com [Fix-forward] Added Compose plugin to Add-to-app Integration Test (flutter/flutter#184681) 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 bmparr@google.com,stuartmorgan@google.com 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
flutter#184519) Remove cross-directory test imports of `editable_text_utils.dart` from material and cupertino tests. Part of flutter#182636. `editable_text_utils.dart` is a 143-line test utility containing `textOffsetToPosition`, `findRenderEditable`, `globalize`, `sendKeys`, `isContextMenuProvidedByPlatform`, and `OverflowWidgetTextEditingController`. Duplication is the appropriate strategy per the issue guidelines. The file is copied into `test/material/` and `test/cupertino/`, and all 10 imports are updated to reference the local copies. This also unblocks a future PR to remove `text_selection_toolbar_utils` cross-imports, which depends on `editable_text_utils`. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [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
Remove cross-directory test imports of
editable_text_utils.dartfrom material and cupertino tests.Part of #182636.
editable_text_utils.dartis a 143-line test utility containingtextOffsetToPosition,findRenderEditable,globalize,sendKeys,isContextMenuProvidedByPlatform, andOverflowWidgetTextEditingController. Duplication is the appropriate strategy per the issue guidelines. The file is copied intotest/material/andtest/cupertino/, and all 10 imports are updated toreference the local copies.
This also unblocks a future PR to remove
text_selection_toolbar_utilscross-imports, which depends oneditable_text_utils.Pre-launch Checklist
///).