Repository navigation
Conversation
enabled to DropdownButtonFormField
|
Hi @Mairramer! Is this something you would like to move forward with? |
Yes, I’d like that, but only if the team genuinely believes it’s something worth pursuing and sees potential for me to move forward with it. |
justinmc
left a comment
There was a problem hiding this comment.
I'm on board with this approach. I agree that the situation described in #57953 is probably confusing a bunch of people.
My main question is whether this will cause breakages or not. Looks like customer tests are passing, but we should run the Google tests. If there are tons of breakages maybe we can rethink how we do this, but if not then I'm all for landing this.
|
This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled. |
# Conflicts: # packages/flutter/test/material/dropdown_test.dart
There was a problem hiding this comment.
Code Review
This pull request introduces an enabled property to DropdownButton and DropdownButtonFormField, allowing these widgets to be disabled independently of the onChanged callback. It includes data-driven fixes to migrate existing code that used onChanged: null for disabling the widgets. Review feedback suggests updating the documentation to clarify that an empty items list also disables the button and identifies a potential edge case in the migration logic where an explicit enabled: true alongside a null onChanged callback might not be handled correctly.
In this case, we would only have a small breaking change related to the conditional use of |
justinmc
left a comment
There was a problem hiding this comment.
Approving in order to kick off the Google tests.
There was a problem hiding this comment.
Thanks for including a dart fix
|
This will have to be postponed due to the code freeze, though. |
|
There are Google test failures, and they look real. |
|
I will approve to kick of Google testing to see if that is resolved, in the meantime, since this will be a breaking change, can you create a migration guide in flutter/website? We can land it when this is ready, but it will be good to have a look at the guidance and iterate there as well in the meantime. :) |
|
@Piinks Migration guide created: flutter/website#13430 |
justinmc
left a comment
There was a problem hiding this comment.
LGTM with nits 👍 . But due to the code freeze, we should not land this PR. It will have to be ported to flutter/packages after flutter/packages#11669 lands and we open up material_ui.
| /// If [enabled] is false, or the list of [DropdownButton.items] is null or empty, | ||
| /// then the dropdown button will be disabled. When disabled, the arrow is | ||
| /// displayed in grey and the button does not respond to input. |
There was a problem hiding this comment.
Should you also explain what happens if enabled is null? Which I think includes the original behavior of disabling when onChanged is null. Or maybe you are trying to encourage people to explicitly set enabled?
There was a problem hiding this comment.
In this case, it would be both. However, I’ll include the nullable enabled behavior as well.
Description of what this PR is changing or adding, and why: Adds a breaking change page for the new `enabled` property logic on `DropdownButton` and `DropdownButtonFormField`. This explains why the `onChanged` callback is no longer required and clarifies the fallback behavior for backward compatibility. It also guides developers on how to properly separate interactive state (`enabled`) from widget behavior (`onChanged`), highlighting what the `dart fix` covers and what requires manual migration (like conditional logic). Issues fixed by this PR (if any): N/A but related to flutter/flutter#57953 PRs or commits this PR depends on (if any): Depends on flutter/flutter#182419 ## Presubmit checklist - [x] If you are unwilling, or unable, to sign the CLA, even for a _tiny_, one-word PR, please file an issue instead of a PR. - [x] If this PR is not meant to land until a future stable release, mark it as draft with an explanation. - [x] This PR follows the [Google Developer Documentation Style Guidelines](https://developers.google.com/style)—for example, it doesn't use _i.e._ or _e.g._, and it avoids _I_ and _we_ (first-person pronouns). - [x] This PR uses [semantic line breaks](https://github.com/dart-lang/site-shared/blob/main/doc/writing-for-dart-and-flutter-websites.md#semantic-line-breaks) of 80 characters or fewer. --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Parker Lougheed <[email protected]>
|
Thanks for your contribution! This PR has reached completion, however we're not able to merge this PR for the moment due to the Material/Cupertino code freeze (#184093). As soon as the decoupling is complete, we will post a way to port this PR to the new packages to land. In the meantime, I'll convert this PR to a draft and apply a label so that we can quickly find it when the freeze is lifted. |
|
I've marked this PR as not ready to port to flutter/packages yet. |
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
|
Ported to flutter/packages#12610. |
Introduced the `enabled` parameter in `DropdownButtonFormField`, allowing proper disabled/read-only usage without needing to pass dummy callbacks or remove existing ones. Previously, the only way to disable a `DropdownButtonFormField` was to set `onChanged: null`. With this change, developers can explicitly control the interactive state via the `enabled` property. For example, you can set `enabled: false` while keeping `onChanged` defined, or set `enabled: true` to have an interactive field even if `onChanged` is omitted. This change is fully backward compatible. If the `enabled` property is omitted (left as `null`), the field will fall back to the legacy behavior: it will be enabled if and only if `onChanged` is non-null. Closes flutter/flutter#57953. Port of flutter/flutter#182419. ## Pre-Review Checklist **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Made
onChangedoptional and introduced theenabledparameter inDropdownButtonFormField, allowing proper disabled/read-only usage without dummy callbacks.Breaking change
Previously, setting
onChanged: nulldisabled the widget. This is no longer the case. The disabled state must now be controlled via theenabledproperty.Most simple usages are handled by the automated fix tool. Code that conditionally relied on
onChangedbeing null to disable the widget must be updated to useenabled.If not migrated, the widget will remain interactive but will not trigger a callback.
Closes #57953
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.