Skip to content

Conversation

@LongCatIsLooong
Copy link
Contributor

Fixes #138773, port autocomplete to OverlayPortal

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.
  • All existing and new tests are passing.

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Dec 17, 2023
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 👍. It's really cool to see this ported to OverlayPortal, exciting stuff.

As an aside, do you think that the use of OverlayPortal will make it easier to limit the width of the options to the width of the field (#101620)?

TextEditingController? _internalTextEditingController;
TextEditingController get _textEditingController {
return widget.textEditingController
?? (_internalTextEditingController ??= TextEditingController()..addListener(_onChangedField));
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess you don't need to remove the _onChangedField listener in dispose since that's covered by disposing the controller itself?

Copy link
Contributor Author

@LongCatIsLooong LongCatIsLooong Jan 24, 2024

Choose a reason for hiding this comment

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

Here?

https://github.com/flutter/flutter/pull/140285/files#diff-4b6f96f28cfdafbb04b797dab70576f40d7866a6291f2981bc7832b475f22355R480-R483

That's only true for internal controller/focus nodes. We can't dispose controllers or focus nodes that are not owned by this state.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I was wondering if you need to call _internalTextEditingController?.removeListener(_onChangedField); there in addition to disposing it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh sorry I misread that. The dispose method gets rid of all listeners.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok got it, thanks for clarifying 👍

FocusNode? _internalFocusNode;
FocusNode get _focusNode {
return widget.focusNode
?? (_internalFocusNode ??= FocusNode()..addListener(_updateOptionsViewVisibility));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same question for removing this listener.

_updateOptionsViewVisibility();
}
_updateActions();
_updateOverlay();
Copy link
Contributor

Choose a reason for hiding this comment

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

It's exciting that we can get rid of these update methods.

bool _floatingOptionsUpdateScheduled = false;
// Hide or show the options overlay, if needed.
void _updateOverlay() {
if (SchedulerBinding.instance.schedulerPhase == SchedulerPhase.persistentCallbacks) {
Copy link
Contributor

Choose a reason for hiding this comment

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

And great that we no longer have to deal with SchedulerBinding stuff 🤩

@LongCatIsLooong LongCatIsLooong added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 24, 2024
@auto-submit auto-submit bot merged commit b6e758a into flutter:master Jan 24, 2024
@XilaiZhang XilaiZhang added the cp: beta cherry pick this pull request to beta release candidate branch label Jan 24, 2024
flutteractionsbot pushed a commit to flutteractionsbot/flutter that referenced this pull request Jan 24, 2024
@XilaiZhang XilaiZhang removed the cp: beta cherry pick this pull request to beta release candidate branch label Jan 24, 2024
@LongCatIsLooong LongCatIsLooong deleted the autocomplete-fixes branch January 24, 2024 22:53
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 25, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jan 25, 2024
Roll Flutter from 19b06f4 to a8efa77 (38 revisions)

flutter/flutter@19b06f4...a8efa77

2024-01-25 [email protected] Roll Flutter Engine from e2014f007f61 to 7c4ed15cb271 (1 revision) (flutter/flutter#142221)
2024-01-25 [email protected] Roll Flutter Engine from 5fa2e2920274 to e2014f007f61 (1 revision) (flutter/flutter#142213)
2024-01-25 [email protected] provide command to `FakeCommand::onRun` (flutter/flutter#142206)
2024-01-25 [email protected] Roll Flutter Engine from c346fd3d9ca1 to 5fa2e2920274 (1 revision) (flutter/flutter#142212)
2024-01-25 [email protected] Roll Flutter Engine from d4d8f668159b to c346fd3d9ca1 (1 revision) (flutter/flutter#142209)
2024-01-25 [email protected] Roll Flutter Engine from 499ed00bbda2 to d4d8f668159b (2 revisions) (flutter/flutter#142205)
2024-01-25 [email protected] Roll Flutter Engine from 4880592ca5ba to 499ed00bbda2 (6 revisions) (flutter/flutter#142202)
2024-01-25 [email protected] [ci] Adds test for web hot restart with const App. (flutter/flutter#141824)
2024-01-25 [email protected] Migrate android_view to linux_android_emu platform. (flutter/flutter#142184)
2024-01-25 [email protected] Refactor `external_ui` without making any name changes (I think) (flutter/flutter#142192)
2024-01-25 [email protected] Fix text selection edge scrolling when inside a horizontal scrollable (flutter/flutter#140250)
2024-01-25 [email protected] Roll Flutter Engine from d7bf5ec1dcdd to 4880592ca5ba (2 revisions) (flutter/flutter#142186)
2024-01-24 [email protected] Roll Flutter Engine from 6a7f963dc751 to d7bf5ec1dcdd (2 revisions) (flutter/flutter#142185)
2024-01-24 [email protected] Reland "Remove hack from PageView." (flutter/flutter#142172)
2024-01-24 [email protected] Upgrade leak_tracker. (flutter/flutter#142162)
2024-01-24 [email protected] Migrate android views to devicelab. (flutter/flutter#142081)
2024-01-24 [email protected] Roll Flutter Engine from ed498f111d53 to 6a7f963dc751 (4 revisions) (flutter/flutter#142176)
2024-01-24 [email protected] Support wireless debugging for iOS 12 or earlier (flutter/flutter#141439)
2024-01-24 [email protected] Roll Flutter Engine from 4bc368ee5f74 to ed498f111d53 (5 revisions) (flutter/flutter#142167)
2024-01-24 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Refactor `external_ui` � `external_textures`" (flutter/flutter#142173)
2024-01-24 [email protected] Refactor `external_ui` â�� `external_textures` (flutter/flutter#142062)
2024-01-24 [email protected] Roll Flutter Engine from 0b9fce355df9 to 4bc368ee5f74 (3 revisions) (flutter/flutter#142157)
2024-01-24 [email protected] Update navigationBar label's maxScaleFactor to meet GAR requirement (flutter/flutter#141998)
2024-01-24 [email protected] Roll Flutter Engine from b65556fa543e to 0b9fce355df9 (1 revision) (flutter/flutter#142147)
2024-01-24 [email protected] Marks Mac_arm64 build_tests_2_4 to be unflaky (flutter/flutter#142115)
2024-01-24 [email protected] Marks Mac_x64 build_tests_2_4 to be unflaky (flutter/flutter#142111)
2024-01-24 [email protected] Marks Mac_x64 build_tests_1_4 to be unflaky (flutter/flutter#142110)
2024-01-24 [email protected] Marks Mac_x64 build_tests_3_4 to be unflaky (flutter/flutter#142112)
2024-01-24 [email protected] Revise tooltip theme docs, including more cross-references (flutter/flutter#137316)
2024-01-24 [email protected] Run some tests explicitly in both arm and x64. (flutter/flutter#141910)
2024-01-24 [email protected] Roll Flutter Engine from ba3a70ce7722 to b65556fa543e (3 revisions) (flutter/flutter#142140)
2024-01-24 [email protected] Fixes #138773, port autocomplete to OverlayPortal (flutter/flutter#140285)
2024-01-24 [email protected] Revert "[web] - Fix broken `TextField` in semantics mode when it's a sibling of `Navigator`" (flutter/flutter#142129)
2024-01-24 [email protected] Marks Mac_arm64 build_tests_1_4 to be unflaky (flutter/flutter#142114)
2024-01-24 [email protected] Roll Packages from 841fe90 to 8fbdf65 (2 revisions) (flutter/flutter#142139)
2024-01-24 [email protected] Marks Mac_arm64 build_tests_3_4 to be unflaky (flutter/flutter#142116)
2024-01-24 [email protected] Marks Mac_x64 build_tests_4_4 to be unflaky (flutter/flutter#142113)
2024-01-24 [email protected] Marks Mac_arm64 build_tests_4_4 to be unflaky (flutter/flutter#142117)

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

Labels

autosubmit Merge PR when tree becomes green via auto submit App 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.

Autocomplete and RawAutocomplete OptionsViewOpenDirection.up not working

3 participants