Fix DropdownMenu does not rematch initialSelection when entries have changed #155757
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
Contributor
nate-thegrate
left a comment
There was a problem hiding this comment.
Looks phenomenal—thanks for the fix and the thorough testing!
A bunch of random suggestions, most of which are due to how List<DropdownMenuEntry<T>> has a tendency to make lines super long :)
| // whose value matches the initial selection. | ||
| // Empty the text field when no entry matches the initial selection. | ||
| void matchInitialSelection() { | ||
| final int index = filteredEntries.indexWhere((DropdownMenuEntry<T> entry) => entry.value == widget.initialSelection); |
Contributor
There was a problem hiding this comment.
Suggested change
| final int index = filteredEntries.indexWhere((DropdownMenuEntry<T> entry) => entry.value == widget.initialSelection); | |
| final int index = filteredEntries.indexWhere( | |
| (DropdownMenuEntry<T> entry) => entry.value == widget.initialSelection, | |
| ); |
Contributor
Author
There was a problem hiding this comment.
Not fond of this formatting. It reduces the long lines but the style guide explictly mention that going above 80 is preferrable when the result is more readable.
Nonetheless I applied it:
- because readability is somewhat subjective.
- I used the same formatting on a very recent PR.
- maybe we will get autoformat in the coming months (and subjectivity will be gone 😄 ).
Contributor
There was a problem hiding this comment.
Not a reviewer but I came across this PR. I'm also not a fan of the indented closure formatting. How would you both feel about something like:
TextEditingValue get _initialTextEditingValue {
for (final entry in filteredEntries) {
if (entry.value == widget.initialSelection) {
return TextEditingValue(
text: entry.label,
selection: TextSelection.collapsed(offset: entry.label.length),
);
}
}
return TextEditingValue.empty;
}where _localTextEditingController?.value = _initialTextEditingValue is set in didUpdateWidget. It could also be a function rather than a getter.
I think the current function being a void 'match'-er rather than a setter made the purpose ambigious without viewing the implementation (or the comments).
That said, feel free to ignore this!
Contributor
Author
There was a problem hiding this comment.
Thanks @davidhicks980! Really like this approach, avoiding the 'macth'-er function makes the logic more understandable.
4b6c027 to
211e059
Compare
nate-thegrate
approved these changes
Sep 30, 2024
Contributor
nate-thegrate
left a comment
There was a problem hiding this comment.
LGTM 👍
Fantastic improvements all-around, thank you!
Contributor
Author
|
I will probably wait for #155252 to land (I will solve the conflicts on my side). |
c65ab4e to
4571c55
Compare
4571c55 to
18444c8
Compare
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 2, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 4, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 5, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 5, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 6, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Oct 8, 2024
auto-submit bot
pushed a commit
to flutter/packages
that referenced
this pull request
Oct 8, 2024
Manual roll Flutter from 0975e61 to ec2e12b (54 revisions) Manual roll requested by stuartmorgan@google.com flutter/flutter@0975e61...ec2e12b 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from bd44b58e3204 to 247bc68c578e (7 revisions) (flutter/flutter#156144) 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 70232fa124d0 to bd44b58e3204 (1 revision) (flutter/flutter#156124) 2024-10-03 engine-flutter-autoroll@skia.org Roll Flutter Engine from 33ac1b30ab0a to 70232fa124d0 (2 revisions) (flutter/flutter#156122) 2024-10-02 36861262+QuncCccccc@users.noreply.github.com Update `ThemeData.dialogTheme` type to accept `DialogThemeData` (flutter/flutter#155129) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 751ab9b3c5eb to 33ac1b30ab0a (4 revisions) (flutter/flutter#156118) 2024-10-02 aam@google.com Add back main() methods to benchmark benches. (flutter/flutter#156083) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156117) 2024-10-02 victorsanniay@gmail.com Add `mouseCursor` property to `CupertinoCheckbox` (flutter/flutter#151788) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 3bdc1c0a30b6 to 751ab9b3c5eb (3 revisions) (flutter/flutter#156115) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156114) 2024-10-02 bkonyi@google.com [ Cocoon ] Wait for task results to be received by the task runner before shutting down the task process (flutter/flutter#156002) 2024-10-02 58190796+MitchellGoodwin@users.noreply.github.com Allow mixing route transitions in one app. (flutter/flutter#150031) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from f20681241753 to 3bdc1c0a30b6 (5 revisions) (flutter/flutter#156107) 2024-10-02 reidbaker@google.com Update Upgrading-Engine's-Android-API-version.md to reflect code move (flutter/flutter#156108) 2024-10-02 engine-flutter-autoroll@skia.org Roll Packages from ebcc4f0 to 7c97c88 (5 revisions) (flutter/flutter#156106) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156105) 2024-10-02 120297255+PurplePolyhedron@users.noreply.github.com fix wrong test in "fixing `DropdownMenu` keyboard navigation" (flutter/flutter#156084) 2024-10-02 brackenavaron@gmail.com fix ReorderableList not passing in item extent builder (flutter/flutter#155994) 2024-10-02 98614782+auto-submit[bot]@users.noreply.github.com Reverts "integration_test: migrate to build.gradle.kts (#154125)" (flutter/flutter#156087) 2024-10-02 magder@google.com Add deprecation warning for "flutter create --ios-language" (flutter/flutter#155867) 2024-10-02 devoncarew@google.com update flutter create generated projects to use package:flutter_lints 5.0.0 (flutter/flutter#156011) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from d48c35d16814 to f20681241753 (1 revision) (flutter/flutter#156080) 2024-10-02 captainsikandar47@gmail.com [Docs] `CupertinoListTile` API Example (flutter/flutter#154548) 2024-10-02 barpac02@gmail.com integration_test: migrate to build.gradle.kts (flutter/flutter#154125) 2024-10-02 fluttergithubbot@gmail.com Marks Windows_mokey native_assets_android to be flaky (flutter/flutter#156064) 2024-10-02 leroux_bruno@yahoo.fr Fix DropdownMenu does not rematch initialSelection when entries have changed (flutter/flutter#155757) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 9b224bd2f895 to d48c35d16814 (1 revision) (flutter/flutter#156074) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 8774940b9ddc to 9b224bd2f895 (1 revision) (flutter/flutter#156065) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 21ad04948457 to 8774940b9ddc (1 revision) (flutter/flutter#156055) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 767bdc38cf51 to 21ad04948457 (1 revision) (flutter/flutter#156049) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from 055969512dc5 to 767bdc38cf51 (1 revision) (flutter/flutter#156043) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from e0f049d69240 to 055969512dc5 (2 revisions) (flutter/flutter#156042) 2024-10-02 120297255+PurplePolyhedron@users.noreply.github.com fix `DropdownMenu` keyboard navigation when entries are filtered empty (flutter/flutter#155252) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from a5bc2e2708c7 to e0f049d69240 (1 revision) (flutter/flutter#156039) 2024-10-02 christopherfujino@gmail.com mark {Linux,Windows} tool_integration_tests_* non-bringup (flutter/flutter#155773) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from a7abf7a8163e to a5bc2e2708c7 (2 revisions) (flutter/flutter#156038) 2024-10-02 engine-flutter-autoroll@skia.org Roll Flutter Engine from df1982dd4482 to a7abf7a8163e (1 revision) (flutter/flutter#156032) 2024-10-02 66697085+SuicaLondon@users.noreply.github.com Add `enableSplash` parameter to `CarouselView` (flutter/flutter#155214) 2024-10-02 137456488+flutter-pub-roller-bot@users.noreply.github.com Roll pub packages (flutter/flutter#156030) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from ab48d6d8c167 to df1982dd4482 (1 revision) (flutter/flutter#156029) 2024-10-01 fluttergithubbot@gmail.com Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be unflaky (flutter/flutter#147289) 2024-10-01 sokolovskyi.konstantin@gmail.com Add WidgetStateMouseCursor example and tests for it. (flutter/flutter#155552) 2024-10-01 49699333+dependabot[bot]@users.noreply.github.com Bump codecov/codecov-action from 4.5.0 to 4.6.0 (flutter/flutter#156024) 2024-10-01 engine-flutter-autoroll@skia.org Roll Flutter Engine from e7b3ce717006 to ab48d6d8c167 (1 revision) (flutter/flutter#156023) ...
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Dec 11, 2024
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/packages
that referenced
this pull request
Dec 12, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 21, 2024
## Description This PR reverts `DropdownMenu` changes from #155757. Automatically rematching the `initialSelection` breaks some use cases. It is more flexible to let users manipulate the text field content using the TextEditingController. ## Related Issue Fixes [Dropdown Menu Creates Infinite Build Loop](#160196) Fixes [Can no longer initialize non selectable value in DropdownMenu as of flutter version 3.27.1](#160555) ## Tests Removes 2 regression tests from #155757. Keeps 2 tests from the original PR (missing test for the initialSelection behavior). Adds 1 tests to avoid regressing this revert.
This was referenced Jan 6, 2025
auto-submit bot
pushed a commit
that referenced
this pull request
Jan 9, 2025
…n when entries have changed" (#161177) This pull request reverts the changes in #155757. The auto-formatter created conflicts in the master branch, so the revert was performed manually in #160643. For this cherry-pick PR, I was able to run `git revert 21381d8` without any problems. <br> ### Issue Links bug reports: #160196, #160555 cherry-pick request: #161176 ### Target stable ### Changelog Description Passing a list literal to a `DropdownMenu` causes the widget to reset to the `initialSelection` after each build. ### Impacted Users This affects anyone using the [DropdownMenu](https://api.flutter.dev/flutter/material/DropdownMenu-class.html) widget. ### Impact Description The impact usually consists of the text value being inconveniently reset each time the widget is rebuilt. (In some cases it can be a fatal crash: the code sample from #160196 shows how this change can lead to an infinite build loop.) ### Workaround This regression can be mitigated by caching & modifying a single list instance, rather than using a list literal for the `DropdownMenu` constructor. ### Risk low ### Test Coverage yes ### Validation Steps #160643 added a regression test for this revert. The fix can also be verified by running the code sample from #160196 and verifying that there is no infinite build loop.
9 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jun 11, 2025
## Description The current behavior of Flutter regarding `DropdownMenu` text field is very basic: `DropdownMenu` updates the text field once a selection has been made with its coordinating `label`, and nothing more. If the selection's `label` changes, the text field will still show the old value, although the menu buttons have been updated. This not only results in the need to write a lot of boilerplate code to match the label to the current selection, but it's also counterintuitive. The `DropdownMenu` should handle rematching by itself. Issue #155660, touched on this, and a solution was introduced in #155757. It was later reverted due to it restricting some use cases. That issue & solution, however, only address `initialSelection`. If another selection was made and its corresponding `label` was changed, the text field would still show the old value, bringing us back to square one. This PR should not conflict with any previous behavior of `DropdownMenu`: any new value provided by `initialSelection` or via controller would not be overwritten by rematching. However, entries with the same value will be mapped to a single label (the first matched entry's label). ## Related Issues - Fixes #155660. ## Tests Added 3 tests. ## 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.
mboetger
pushed a commit
to mboetger/flutter
that referenced
this pull request
Jul 21, 2025
## Description The current behavior of Flutter regarding `DropdownMenu` text field is very basic: `DropdownMenu` updates the text field once a selection has been made with its coordinating `label`, and nothing more. If the selection's `label` changes, the text field will still show the old value, although the menu buttons have been updated. This not only results in the need to write a lot of boilerplate code to match the label to the current selection, but it's also counterintuitive. The `DropdownMenu` should handle rematching by itself. Issue flutter#155660, touched on this, and a solution was introduced in flutter#155757. It was later reverted due to it restricting some use cases. That issue & solution, however, only address `initialSelection`. If another selection was made and its corresponding `label` was changed, the text field would still show the old value, bringing us back to square one. This PR should not conflict with any previous behavior of `DropdownMenu`: any new value provided by `initialSelection` or via controller would not be overwritten by rematching. However, entries with the same value will be mapped to a single label (the first matched entry's label). ## Related Issues - Fixes flutter#155660. ## Tests Added 3 tests. ## 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.
9 tasks
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.
Description
This PR makes DropdownMenu rematching the initialSelection when the entries are updated.
If the new entries contains one entry whose value matches
initialSelectionthis entry's label is used to initialize the inner text field, if no entries matchesinitialSelectionthe text field is emptied.Related Issue
Fixes DropdownMenu.didUpdateWidget should re-match initialSelection when dropdownMenuEntries have changed.
Tests
Adds 3 tests.