[CP-beta]Do not rely on Leader/Follower to position DropdownMenu menu #159436
Conversation
…#158930) ## Description This PR removes `DropdownMenu` usage of Leader/Follower. Leader/Follower positioning was introduced in flutter#154667 which uses Leader/Follower approach to fix some weird positioning issues (such as flutter#149037). Unfortunately, it also introduces some regressions, see: - flutter#157916 - flutter#158924 Because flutter#154667 is already included in the beta channel, cherry-picking this PR should be considered. ## Context This PR is not a full revert and keeps Leader/Follower usage in `MenuAnchor` because this usage is optional and doesn't cause any regression. There are some ongoing work which might fix or mitigate this problem: - flutter#157921 - flutter#158255 ## Related Issue Fixes flutter#157916 Fixes flutter#158924 Reopens flutter#123395 Reopens flutter#149037 Reopens flutter#151856
|
@justinmc please fill out the PR description above, afterwards the release team will review this request. |
github-actions
bot
added
framework
|
@bleroux can you help fill out the above form with context from your change for the cherry pick? Thanks! |
|
@Piinks I filed the form, I don't know what to answer to the last question. Not sure what 'validation' is referring to? This change is covered by tests but that's true with all changes. |
Good question! This is typically like "Run this sample code and observe there is no issue." So for this, I might provide the code from the original issue to run and show scrolling to the end is now possible. :) |
|
@Piinks does this critically need to be in the next release (or can it be in a follow-up cherry-pick release)? If so, can you (or whoever is most appropriate) review it? Thanks. |
|
@justinmc has more context on this, do you think this is critical to CP now? Or later as a hotfix? |
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
CPLGTM 👍
This PR (original #158930) is a good cherry pick candidate because it's a revert, so very low risk, and there is some complexity around the regressions that it fixed and landing follow-up fixes.
However it's not critical, as the issues it fixes have not gained tons of attention on GitHub or from critical customers. If it's easier to wait for a follow up release then I think that's ok.
christopherfujino
added
the
autosubmit
|
auto label is removed for flutter/flutter/159436, Failed to enqueue flutter/flutter/159436 with HTTP 400: GraphQL mutate failed. |
auto-submit
bot
removed
the
autosubmit
christopherfujino
merged commit d5a20a1
into
flutter:flutter-3.27-candidate.0
Just to make it clear, that this issue was not in the previous stable. The commit which introduces this regression is only on the beta channel now and was on master for maybe one month, that might explain why it did not gained attention. |
5 participants
This pull request is created by automatic cherry pick workflow
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.
Issue Link:
What is the link to the issue this cherry-pick is addressing?
#157916 and #158924
Changelog Description:
Explain this cherry pick in one line that is accessible to most Flutter developers. See best practices for examples
Restore the previous dropdown menu positioning logic.
Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)
DropdownMenu menu is misplaced and can be fully or partially hidden.
See #158930 (comment) for more context.
Workaround:
Is there a workaround for this issue?
No
Risk:
What is the risk level of this cherry-pick?
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
What are the steps to validate that this fix works?
< Replace with validation steps here >