Add adaptive vertical positioning to RenderFollowerLayer #157921
Closed
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
github-actions
bot
added
framework
|
This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again. For more guidance, visit Writing a golden file test for Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. |
bleroux
force-pushed
the
fix_dropdown_menu_bottom_overflow
branch
2 times, most recently
from
3b00bb1 to
7e2dad5
Compare
bleroux
force-pushed
the
fix_dropdown_menu_bottom_overflow
branch
from
7e2dad5 to
59c4d84
Compare
This was referenced Nov 14, 2024
github-merge-queue bot
pushed a commit
that referenced
this pull request
Nov 23, 2024
## Description This PR removes `DropdownMenu` usage of Leader/Follower. Leader/Follower positioning was introduced in #154667 which uses Leader/Follower approach to fix some weird positioning issues (such as #149037). Unfortunately, it also introduces some regressions, see: - #157916 - #158924 Because #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: - #157921 - #158255 ## Related Issue Fixes #157916 Fixes #158924 Reopens #123395 Reopens #149037 Reopens #151856
flutteractionsbot
pushed a commit
to flutteractionsbot/flutter
that referenced
this pull request
Nov 25, 2024
…#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
Contributor
Author
|
Closing and I will revisit once #158255 will land. |
Contributor
|
Hi @bleroux, was this effort abandoned? |
Contributor
Author
|
@pathconnected This is tricky to implement correctly. I might get some bandwidth to work again on this in the coming weeks. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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 adds adaptive positioning to Leader/Follower.
Doing so a follower position can be automatically modified if not enough space is available.
For the moment, only bottom overflow is implemented as it corresponds to DropdownMenu use case.
Context
Since #154667,
DropdownMenurelies on Leader/Follower approach in order to position the menu below the text field.While this change was needed to fix several positioning issues where the menu did not move while the text field did, it also introduced a regression as the menu is always positioned below the text field and might not be visible or be partially visible.
This PR aims to fix this issues by adding an adaptive positioning logic at the Leader/Follower level. Doing so this logic would be reusable for other widgets (for instance Autocomplete which faces the same problem).
Related Issue
Fixes DropdownMenu menu always appears below the TextField
Tests
Adds 6 tests.