Reland [#166645] Fix DropdownButtonFormField focusing when replacing FocusNode #170761
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…xternal FocusNode
github-actions
bot
added
framework
justinmc
reviewed
Jun 18, 2025
Contributor
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
Thanks for following up after this was reverted. Looks like CI is indeed catching a failure here that will need to be fixed before we can reland.
justinmc
reviewed
Jun 18, 2025
| ); | ||
|
|
||
| await tester.pumpWidget(buildFormField()); | ||
| final FocusNode internalNode = tester.widget<Focus>(find.byType(Focus).last).focusNode!; |
Contributor
There was a problem hiding this comment.
Actually the failure seems to be coming from here.
Contributor
Author
There was a problem hiding this comment.
That’s true. I’m not sure what changed that caused it to fail.
Anyway, I’ve updated it to use find.byKey to access the DropdownButton’s focusNode.
justinmc
reviewed
Jun 20, 2025
Contributor
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
What do you think about looking up via descendant instead of key? Otherwise looks good.
|
|
||
| await tester.pumpWidget(buildFormField()); | ||
| final FocusNode internalNode = | ||
| tester.widget<Focus>(find.byKey(const Key('dropdown-focus'))).focusNode!; |
Contributor
There was a problem hiding this comment.
My gut says we should avoid this pattern so we don't end up having key collisions with keys in people's app codebases. Though searching through the codebase it looks like we do use this pattern in a few places, e.g. in PopupMenu.
But I think something like this is a better approach:
find.descendant(
of: find.byType(DropdownButton),
matching: find.byType(Focus),
);
Contributor
There was a problem hiding this comment.
Maybe we shouldn't do this in the framework at all? #170952
Contributor
Author
There was a problem hiding this comment.
Yes, I totally agree since using descendant is safe and reliable enough.
Contributor
Author
There was a problem hiding this comment.
Well, there are two things to note here:
1- We should specify the type as DropdownButton<String>.
2- We have to use .first because there are multiple Focus widgets under DropdownButton.
I'll push the update anyway, and if using .first turns out to be unreliable, we can revert it.
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 26, 2025
…70952) This PR investigates moving away from the few places where we rely on Keys to instead solely look up a widget in a test. Reasons: * No extra stuff in users' apps that they don't need for production. * Avoid collisions between Keys in users' apps that might have the same identifier string. I'm definitely still up for discussion on whether or not this is a good idea! Written in response to the discussion on #170761 (comment).
bleroux
approved these changes
Jun 27, 2025
Contributor
bleroux
left a comment
bleroux
left a comment
There was a problem hiding this comment.
LGTM! Thanks for this reland 🙏
bleroux
added
the
autosubmit
flutter-dashboard
bot
removed
the
autosubmit
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 27, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jun 27, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 2, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 3, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 3, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 3, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 3, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 3, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 4, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 5, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 5, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 6, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Jul 7, 2025
…replacing FocusNode (flutter/flutter#170761)
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Jul 21, 2025
…utter#170952) This PR investigates moving away from the few places where we rely on Keys to instead solely look up a widget in a test. Reasons: * No extra stuff in users' apps that they don't need for production. * Avoid collisions between Keys in users' apps that might have the same identifier string. I'm definitely still up for discussion on whether or not this is a good idea! Written in response to the discussion on flutter#170761 (comment).
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Jul 21, 2025
…lacing FocusNode (flutter#170761) This is a reland of flutter#166645 fixes flutter#166642 **The newly added tests verify the following behaviors:** 1. DropdownButtonFormField can be focused if it was unfocused and we replaced FocusNode. 2. DropdownButtonFormField can be unfocused if it was focused and we replaced FocusNode. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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.
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 14, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 15, 2025
…replacing FocusNode (flutter/flutter#170761)
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Aug 16, 2025
…replacing FocusNode (flutter/flutter#170761)
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
…utter#170952) This PR investigates moving away from the few places where we rely on Keys to instead solely look up a widget in a test. Reasons: * No extra stuff in users' apps that they don't need for production. * Avoid collisions between Keys in users' apps that might have the same identifier string. I'm definitely still up for discussion on whether or not this is a good idea! Written in response to the discussion on flutter#170761 (comment).
lucaantonelli
pushed a commit
to lucaantonelli/flutter
that referenced
this pull request
Nov 21, 2025
…lacing FocusNode (flutter#170761) This is a reland of flutter#166645 fixes flutter#166642 **The newly added tests verify the following behaviors:** 1. DropdownButtonFormField can be focused if it was unfocused and we replaced FocusNode. 2. DropdownButtonFormField can be unfocused if it was focused and we replaced FocusNode. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a reland of #166645
fixes #166642
The newly added tests verify the following behaviors:
Pre-launch Checklist
///).