Correct menu position when menu is constrained #143121
Conversation
github-actions
bot
added
framework
HansMuller
left a comment
HansMuller
left a comment
There was a problem hiding this comment.
This changes the behavior of menus that don't need to scroll: before we tried to put the selected item over the menu button, now we always align the bottom of the menu with the bottom of the button. I think the original idea was that the button would look like it contained the selected item (when everything fit that is). I think this change is OK, although it may break some golden image tests.
|
|
||
| @override | ||
| Offset getPositionForChild(Size size, Size childSize) { | ||
| // size: The size of the overlay. |
There was a problem hiding this comment.
This block of comments should be moved down, below // Find the ideal horizontal position, since they aren't needed to explain the y computation.
Got it! Thanks a lot for the explanation! For next PR which implements the auto scroll, the scrolled item will always be scrolled to the top of the menu, and the top of the menu is aligned with the button. So probably it can feel similar to the original idea? But yes, this PR changes a lot of menu behaviors. video demoScreen.Recording.2024-02-07.at.3.55.58.PM.mov |
|
I think the behavior shown in #143121 (comment) is sufficiently close to the original (buggy) behavior and also to the original intended behavior. It looks like the customer tests are OK with it. It may trip up some internal tests; we'll have to see after you land it. |
QuncCccccc
added
the
autosubmit
…3118) Fixes #142895 With the change of #143121, this PR is to add auto scroll to `PopupMenuButton` so when we open the menu, it will automatically scroll to the selected item. https://github.com/flutter/flutter/assets/36861262/c2bc0395-0641-4e7a-a54d-57a8e62ee26f
2 participants
Fixes #142896
The original code below is to always place the selected item above(overlap) the popup button so that the selected item can always be visible:
flutter/packages/flutter/lib/src/material/popup_menu.dart
Lines 723 to 732 in f8a7722
But when menu height is constrained and the menu itself is super long, the selected item still assumes there is enough space to push up all the items whose index is smaller than the selected index. As a result, every time when the menu is open, the calculation provides a different result to be the offset for the selected index, and then with a constrained height, the menu looks jumping all over the place based on the different selected index.
bug_report.mov
Even though the original calculation is to make the selected item visible when open the menu, the menu doesn't auto scroll and only expands itself as much as possible to show the selected one. In this case, if the screen it too small to show the selected item, we still cannot see it. This can be fixed by using
Scrollable.ensureVisible()(#143118).So we remove the calculation in this PR and the menu will always show up based on the top left of the anchor(button).
fix_report.mov
Pre-launch Checklist
///).