Formed in 2009, the Archive Team (not to be confused with the archive.org Archive-It Team) is a rogue archivist collective dedicated to saving copies of rapidly dying or deleted websites for the sake of history and digital heritage. The group is 100% composed of volunteers and interested parties, and has expanded into a large amount of related projects for saving online and digital history.
fix a dropdown button menu position bug #89199
Merged
+39
−5
Conversation
| menuTop = math.min(buttonTop, topLimit); | ||
| menuBottom = menuTop + menuHeight; |
xu-baolin
Aug 31, 2021
Author
Member
This is a bug and I add an assertion to check this at line 580
This is a bug and I add an assertion to check this at line 580
|
Ready for review now. |
| @@ -516,7 +516,10 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> { | |||
| // selected item is aligned with the button's vertical center, as far as | |||
| // that's possible given availableHeight. | |||
| _MenuLimits getMenuLimits(Rect buttonRect, double availableHeight, int index) { | |||
| final double maxMenuHeight = availableHeight - 2.0 * _kMenuItemHeight; | |||
| double maxMenuHeight = availableHeight - 2.0 * _kMenuItemHeight; | |||
Piinks
Sep 9, 2021
Contributor
maxMenuHeight v menuMaxHeight is a bit confusing.
What if we made this one computedMaxHeight? Or something like that?
maxMenuHeight v menuMaxHeight is a bit confusing.
What if we made this one computedMaxHeight? Or something like that?
| @@ -567,6 +577,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> { | |||
| scrollOffset = math.min(scrollOffset, preferredMenuHeight - menuHeight); | |||
| } | |||
|
|
|||
| assert(menuHeight == menuBottom - menuTop); | |||
Piinks
Sep 9, 2021
Contributor
Can you add a test that triggers this assertion?
Can you add a test that triggers this assertion?
xu-baolin
Sep 10, 2021
Author
Member
If we only add this assertion to the master branch, there will be many tests that trigger this assertion.
Fortunately, no one has used _MenuLimits.menuBottom before, so there are no functional issues.
If we only add this assertion to the master branch, there will be many tests that trigger this assertion.
Fortunately, no one has used _MenuLimits.menuBottom before, so there are no functional issues.
xu-baolin
Sep 10, 2021
Author
Member
This is just to check that the metrics calculated by this function are correct.
I didn't think of a good way to test and trigger it.
This is just to check that the metrics calculated by this function are correct.
I didn't think of a good way to test and trigger it.
fluttergithubbot
merged commit b2550fe
into
flutter:master
48 checks passed
48 checks passed
cla/google
All necessary CLAs are signed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Fixes #89029
This is an enhanced patch of #76493