Skip to content

[material_ui] Port PR (#187366) from flutter/flutter to material_ui - #12821

Open
ishivamg wants to merge 3 commits into
flutter:mainfrom
ishivamg:port/187366-dropdown-orientation-crash
Open

ishivamg wants to merge 3 commits into
flutter:mainfrom
ishivamg:port/187366-dropdown-orientation-crash

Conversation

@ishivamg

@ishivamg ishivamg commented Sep 10, 2026

Copy link
Copy Markdown

This PR ports flutter/flutter#187366 to material_ui.

When a DropdownButton menu is open (especially inside a dialog) and the device orientation changes, _DropdownButtonState.build() dismissed the route with Navigator.removeRoute() during build. That triggers setState() or markNeedsBuild() called during build.

The route is now removed synchronously unless we are in SchedulerPhase.persistentCallbacks, in which case removal is deferred to a post-frame callback.

Fixes flutter/flutter#171011
Follows flutter/flutter#188444

Pre-Review Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools.
  • I read the [Tree Hygiene] page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran [the auto-formatter].
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I [linked to at least one issue that this PR fixes] in the description above.
  • I followed [the version and CHANGELOG instructions], using [semantic versioning] and the [repository CHANGELOG style], or I have commented below to indicate which documented exception this PR falls under[^1].
  • I updated/added any relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or I have commented below to indicate which [test exemption] this PR falls under[^1].
  • All existing and new tests are passing.

…nu is open

Defer Navigator.removeRoute until after the current frame when orientation
changes during build. This ports the reviewed fix from flutter/flutter#187366.

Fixes flutter/flutter#171011
@github-actions github-actions Bot added p: material_ui triage-design Should be looked at in design triage labels Sep 10, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request defers the dismissal of the dropdown route during orientation changes if the scheduler is in the persistent callbacks phase, preventing Navigator mutation during the build phase. It also adds a corresponding widget test to verify this behavior and a changelog entry. There are no review comments, and I have no feedback to provide.

@navaronbracke navaronbracke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM with nit

Comment thread packages/material_ui/test/dropdown_test.dart Outdated
Use DropdownButton in the test name so it is easier to find when those
tests move to dropdown_button_test.dart.
@navaronbracke

Copy link
Copy Markdown
Contributor

Hi @ishivamg

I recently finished up my work for splitting up a relevant test file for DropdownButton
See #12782

To avoid conflicts, I would suggest if you could create the dropdown_button_test.dart file, with just your test in?

Then I won't have a conflict on dropdown_test.dart and can also easily cherry pick your change to the dropdown_button_test.dart test file before rebasing to save myself some work :)

Place the regression test in its own DropdownButton test file so it does
not conflict with the upcoming dropdown_test.dart split.
@ishivamg

Copy link
Copy Markdown
Author

Hi @ishivamg

I recently finished up my work for splitting up a relevant test file for DropdownButton See #12782

To avoid conflicts, I would suggest if you could create the dropdown_button_test.dart file, with just your test in?

Then I won't have a conflict on dropdown_test.dart and can also easily cherry pick your change to the dropdown_button_test.dart test file before rebasing to save myself some work :)

Moved the regression test into dropdown_button_test.dart as suggested.

@navaronbracke navaronbracke added the CICD Run CI/CD label Sep 14, 2026
@navaronbracke

Copy link
Copy Markdown
Contributor

Oh, I see see that the PR checklist is missing from this PR (it was present in the older one you ported)

I restored the checklist and checked the boxes that are definitely present. Can you cross check if you did the rest of the checklist?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD p: material_ui triage-design Should be looked at in design triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Opening dropdownMenu from alertDialog and rotating the screen throws error.

2 participants