Unverified Commit e623d93d authored by YeungKC's avatar YeungKC Committed by GitHub

Fix dropdown menu overscroll (#73654)

parent 97878a2f
...@@ -116,8 +116,8 @@ class _DropdownMenuItemButton<T> extends StatefulWidget { ...@@ -116,8 +116,8 @@ class _DropdownMenuItemButton<T> extends StatefulWidget {
final _DropdownRoute<T> route; final _DropdownRoute<T> route;
final EdgeInsets? padding; final EdgeInsets? padding;
final Rect? buttonRect; final Rect buttonRect;
final BoxConstraints? constraints; final BoxConstraints constraints;
final int itemIndex; final int itemIndex;
@override @override
...@@ -138,7 +138,7 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>> ...@@ -138,7 +138,7 @@ class _DropdownMenuItemButtonState<T> extends State<_DropdownMenuItemButton<T>>
if (focused && inTraditionalMode) { if (focused && inTraditionalMode) {
final _MenuLimits menuLimits = widget.route.getMenuLimits( final _MenuLimits menuLimits = widget.route.getMenuLimits(
widget.buttonRect!, widget.constraints!.maxHeight, widget.itemIndex); widget.buttonRect, widget.constraints.maxHeight, widget.itemIndex);
widget.route.scrollController!.animateTo( widget.route.scrollController!.animateTo(
menuLimits.scrollOffset, menuLimits.scrollOffset,
curve: Curves.easeInOut, curve: Curves.easeInOut,
...@@ -204,15 +204,15 @@ class _DropdownMenu<T> extends StatefulWidget { ...@@ -204,15 +204,15 @@ class _DropdownMenu<T> extends StatefulWidget {
Key? key, Key? key,
this.padding, this.padding,
required this.route, required this.route,
this.buttonRect, required this.buttonRect,
this.constraints, required this.constraints,
this.dropdownColor, this.dropdownColor,
}) : super(key: key); }) : super(key: key);
final _DropdownRoute<T> route; final _DropdownRoute<T> route;
final EdgeInsets? padding; final EdgeInsets? padding;
final Rect? buttonRect; final Rect buttonRect;
final BoxConstraints? constraints; final BoxConstraints constraints;
final Color? dropdownColor; final Color? dropdownColor;
@override @override
...@@ -288,12 +288,21 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> { ...@@ -288,12 +288,21 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
textStyle: route.style, textStyle: route.style,
child: ScrollConfiguration( child: ScrollConfiguration(
behavior: const _DropdownScrollBehavior(), behavior: const _DropdownScrollBehavior(),
child: Scrollbar( child: PrimaryScrollController(
child: ListView( controller: widget.route.scrollController!,
controller: widget.route.scrollController, child: LayoutBuilder(
padding: kMaterialListPadding, builder: (BuildContext context, BoxConstraints constraints) {
shrinkWrap: true, final double menuTotalHeight = widget.route.itemHeights.reduce((double total, double height) => total + height);
children: children, final bool isScrollable = kMaterialListPadding.vertical + menuTotalHeight > constraints.maxHeight;
return Scrollbar(
isAlwaysShown: isScrollable,
child: ListView(
padding: kMaterialListPadding,
shrinkWrap: true,
children: children,
),
);
},
), ),
), ),
), ),
...@@ -512,14 +521,21 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> { ...@@ -512,14 +521,21 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
menuTop = menuBottom - menuHeight; menuTop = menuBottom - menuHeight;
} }
double scrollOffset = 0;
// If all of the menu items will not fit within availableHeight then // If all of the menu items will not fit within availableHeight then
// compute the scroll offset that will line the selected menu item up // compute the scroll offset that will line the selected menu item up
// with the select item. This is only done when the menu is first // with the select item. This is only done when the menu is first
// shown - subsequently we leave the scroll offset where the user left // shown - subsequently we leave the scroll offset where the user left
// it. This scroll offset is only accurate for fixed height menu items // it. This scroll offset is only accurate for fixed height menu items
// (the default). // (the default).
final double scrollOffset = preferredMenuHeight <= maxMenuHeight ? 0 : if (preferredMenuHeight > maxMenuHeight) {
math.max(0.0, selectedItemOffset - (buttonTop - menuTop)); // The offset should be zero if the selected item is in view at the beginning
// of the menu. Otherwise, the scroll offset should center the item if possible.
scrollOffset = math.max(0.0, selectedItemOffset - (buttonTop - menuTop));
// If the selected item's scroll offset is greater than the maximum scroll offset,
// set it instead to the maximum allowed scroll offset.
scrollOffset = math.min(scrollOffset, preferredMenuHeight - menuHeight);
}
return _MenuLimits(menuTop, menuBottom, menuHeight, scrollOffset); return _MenuLimits(menuTop, menuBottom, menuHeight, scrollOffset);
} }
......
...@@ -991,6 +991,42 @@ void main() { ...@@ -991,6 +991,42 @@ void main() {
); );
}); });
testWidgets('Dropdown menu scrolls to last item in long lists', (WidgetTester tester) async {
final Key buttonKey = UniqueKey();
await tester.pumpWidget(buildFrame(
buttonKey: buttonKey,
value: '99',
items: List<String>.generate(/*length=*/ 100, (int index) => index.toString()),
onChanged: onChanged,
));
await tester.tap(find.byKey(buttonKey));
await tester.pump();
final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
// Make sure there is no overscroll
expect(scrollController.offset, scrollController.position.maxScrollExtent);
// Find the selected item in the scrollable dropdown list
final Finder menuItemFinder = find.byType(Scrollable);
final RenderBox menuItemContainer = tester.renderObject<RenderBox>(menuItemFinder);
final RenderBox selectedItem = tester.renderObject<RenderBox>(
find.descendant(
of: menuItemFinder,
matching: find.byKey(const ValueKey<String>('99')),
),
);
// kMaterialListPadding.vertical is 8.
const Offset menuPaddingOffset = Offset(0.0, -8.0);
final Offset selectedItemOffset = selectedItem.localToGlobal(Offset.zero);
final Offset menuItemContainerOffset = menuItemContainer.localToGlobal(menuPaddingOffset);
// Selected item should be aligned to the bottom of the dropdown menu.
expect(
selectedItem.size.bottomCenter(selectedItemOffset).dy,
menuItemContainer.size.bottomCenter(menuItemContainerOffset).dy,
);
});
testWidgets('Size of DropdownButton with null value', (WidgetTester tester) async { testWidgets('Size of DropdownButton with null value', (WidgetTester tester) async {
final Key buttonKey = UniqueKey(); final Key buttonKey = UniqueKey();
String? value; String? value;
...@@ -1833,8 +1869,7 @@ void main() { ...@@ -1833,8 +1869,7 @@ void main() {
double getMenuScroll() { double getMenuScroll() {
double scrollPosition; double scrollPosition;
final ListView listView = tester.element(find.byType(ListView)).widget as ListView; final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
final ScrollController scrollController = listView.controller!;
assert(scrollController != null); assert(scrollController != null);
scrollPosition = scrollController.position.pixels; scrollPosition = scrollController.position.pixels;
assert(scrollPosition != null); assert(scrollPosition != null);
...@@ -1870,8 +1905,7 @@ void main() { ...@@ -1870,8 +1905,7 @@ void main() {
double getMenuScroll() { double getMenuScroll() {
double scrollPosition; double scrollPosition;
final ListView listView = tester.element(find.byType(ListView)).widget as ListView; final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
final ScrollController scrollController = listView.controller!;
assert(scrollController != null); assert(scrollController != null);
scrollPosition = scrollController.position.pixels; scrollPosition = scrollController.position.pixels;
assert(scrollPosition != null); assert(scrollPosition != null);
...@@ -1907,8 +1941,7 @@ void main() { ...@@ -1907,8 +1941,7 @@ void main() {
double getMenuScroll() { double getMenuScroll() {
double scrollPosition; double scrollPosition;
final ListView listView = tester.element(find.byType(ListView)).widget as ListView; final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
final ScrollController scrollController = listView.controller!;
assert(scrollController != null); assert(scrollController != null);
scrollPosition = scrollController.position.pixels; scrollPosition = scrollController.position.pixels;
assert(scrollPosition != null); assert(scrollPosition != null);
...@@ -1944,8 +1977,7 @@ void main() { ...@@ -1944,8 +1977,7 @@ void main() {
double getMenuScroll() { double getMenuScroll() {
double scrollPosition; double scrollPosition;
final ListView listView = tester.element(find.byType(ListView)).widget as ListView; final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
final ScrollController scrollController = listView.controller!;
assert(scrollController != null); assert(scrollController != null);
scrollPosition = scrollController.position.pixels; scrollPosition = scrollController.position.pixels;
assert(scrollPosition != null); assert(scrollPosition != null);
...@@ -2887,4 +2919,18 @@ void main() { ...@@ -2887,4 +2919,18 @@ void main() {
expect(find.text('first').hitTestable(), findsNothing); expect(find.text('first').hitTestable(), findsNothing);
expect(find.text('second').hitTestable(), findsNothing); expect(find.text('second').hitTestable(), findsNothing);
}); });
testWidgets('Dropdown menu should persistently show a scrollbar if it is scrollable', (WidgetTester tester) async {
await tester.pumpWidget(buildFrame(
value: '0',
items: List<String>.generate(/*length=*/100, (int index) => index.toString()),
onChanged: onChanged,
));
await tester.tap(find.text('0'));
await tester.pumpAndSettle();
final ScrollController scrollController = PrimaryScrollController.of(tester.element(find.byType(ListView)))!;
expect(scrollController.position.maxScrollExtent > 0, isTrue);
expect(find.byType(Scrollbar), paints..rect());
});
} }
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment