From d422e85f5be10a9067c43f803b0fd341656a6260 Mon Sep 17 00:00:00 2001 From: jslavitz Date: Mon, 8 Oct 2018 10:39:39 -0700 Subject: [PATCH] Large Dropdown Menu Fix (#22594) * Adds comments clarifying the procedure used to render the menu as well as tests verifying various dropdown menu button positioning and initial scroll states. --- .../flutter/lib/src/material/dropdown.dart | 46 ++++-- .../flutter/test/material/dropdown_test.dart | 148 ++++++++++++++++++ 2 files changed, 182 insertions(+), 12 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index 6fd94ea164..90006bb37f 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -329,25 +329,47 @@ class _DropdownRoute extends PopupRoute<_DropdownRouteResult> { assert(debugCheckHasDirectionality(context)); final double screenHeight = MediaQuery.of(context).size.height; final double maxMenuHeight = screenHeight - 2.0 * _kMenuItemHeight; - final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical; - final double menuHeight = math.min(maxMenuHeight, preferredMenuHeight); final double buttonTop = buttonRect.top; + final double buttonBottom = buttonRect.bottom; + + // If the button is placed on the bottom or top of the screen, its top or + // bottom may be less than [_kMenuItemHeight] from the edge of the screen. + // In this case, we want to change the menu limits to align with the top + // or bottom edge of the button. + final double topLimit = math.min(_kMenuItemHeight, buttonTop); + final double bottomLimit = math.max(screenHeight - _kMenuItemHeight, buttonBottom); + final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top; + double menuTop = (buttonTop - selectedItemOffset) - (_kMenuItemHeight - buttonRect.height) / 2.0; - const double topPreferredLimit = _kMenuItemHeight; - if (menuTop < topPreferredLimit) - menuTop = math.min(buttonTop, topPreferredLimit); - double bottom = menuTop + menuHeight; - final double bottomPreferredLimit = screenHeight - _kMenuItemHeight; - if (bottom > bottomPreferredLimit) { - bottom = math.max(buttonTop + _kMenuItemHeight, bottomPreferredLimit); - menuTop = bottom - menuHeight; + final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical; + + // If there are too many elements in the menu, we need to shrink it down + // so it is at most the maxMenuHeight. + final double menuHeight = math.min(maxMenuHeight, preferredMenuHeight); + + double menuBottom = menuTop + menuHeight; + + // If the computed top or bottom of the menu are outside of the range + // specified, we need to bring them into range. If the item height is larger + // than the button height and the button is at the very bottom or top of the + // screen, the menu will be aligned with the bottom or top of the button + // respectively. + if (menuTop < topLimit) + menuTop = math.min(buttonTop, topLimit); + if (menuBottom > bottomLimit) { + menuBottom = math.max(buttonBottom, bottomLimit); + menuTop = menuBottom - menuHeight; } if (scrollController == null) { - final double scrollOffset = (preferredMenuHeight > maxMenuHeight) ? - math.max(0.0, selectedItemOffset - (buttonTop - menuTop)) : 0.0; + // The limit is asymmetrical because we do not care how far positive the + // limit goes. We are only concerned about the case where the value of + // [buttonTop - menuTop] is larger than selectedItemOffset, ie. when + // the button is close to the bottom of the screen and the selected item + // is close to 0. + final double scrollOffset = preferredMenuHeight > maxMenuHeight ? math.max(0.0, selectedItemOffset - (buttonTop - menuTop)) : 0.0; scrollController = ScrollController(initialScrollOffset: scrollOffset); } diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index c061135656..fcd3b21755 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -777,4 +777,152 @@ void main() { ), ignoreId: true, ignoreRect: true, ignoreTransform: true)); semantics.dispose(); }); + + testWidgets('Dropdown in middle showing middle item', (WidgetTester tester) async { + final List> items = + List>.generate(100, (int i) => + DropdownMenuItem(value: i, child: Text('$i'))); + + final DropdownButton button = DropdownButton( + value: 50, + onChanged: (int newValue){}, + items: items, + ); + + double getMenuScroll() { + double scrollPosition; + final ListView listView = tester.element(find.byType(ListView)).widget; + final ScrollController scrollController = listView.controller; + assert(scrollController != null); + scrollPosition = scrollController.position.pixels; + assert(scrollPosition != null); + return scrollPosition; + } + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Align( + alignment: Alignment.center, + child: button, + ), + ), + ), + ); + + await tester.tap(find.text('50')); + await tester.pumpAndSettle(); + expect(getMenuScroll(), 2180.0); + }); + + testWidgets('Dropdown in top showing bottom item', (WidgetTester tester) async { + final List> items = + List>.generate(100, (int i) => + DropdownMenuItem(value: i, child: Text('$i'))); + + final DropdownButton button = DropdownButton( + value: 99, + onChanged: (int newValue){}, + items: items, + ); + + double getMenuScroll() { + double scrollPosition; + final ListView listView = tester.element(find.byType(ListView)).widget; + final ScrollController scrollController = listView.controller; + assert(scrollController != null); + scrollPosition = scrollController.position.pixels; + assert(scrollPosition != null); + return scrollPosition; + } + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Align( + alignment: Alignment.topCenter, + child: button, + ), + ), + ), + ); + + await tester.tap(find.text('99')); + await tester.pumpAndSettle(); + expect(getMenuScroll(), 4312.0); + }); + + testWidgets('Dropdown in bottom showing top item', (WidgetTester tester) async { + final List> items = + List>.generate(100, (int i) => + DropdownMenuItem(value: i, child: Text('$i'))); + + final DropdownButton button = DropdownButton( + value: 0, + onChanged: (int newValue){}, + items: items, + ); + + double getMenuScroll() { + double scrollPosition; + final ListView listView = tester.element(find.byType(ListView)).widget; + final ScrollController scrollController = listView.controller; + assert(scrollController != null); + scrollPosition = scrollController.position.pixels; + assert(scrollPosition != null); + return scrollPosition; + } + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Align( + alignment: Alignment.bottomCenter, + child: button, + ), + ), + ), + ); + + await tester.tap(find.text('0')); + await tester.pumpAndSettle(); + expect(getMenuScroll(), 0.0); + }); + + testWidgets('Dropdown in center showing bottom item', (WidgetTester tester) async { + final List> items = + List>.generate(100, (int i) => + DropdownMenuItem(value: i, child: Text('$i'))); + + final DropdownButton button = DropdownButton( + value: 99, + onChanged: (int newValue){}, + items: items, + ); + + double getMenuScroll() { + double scrollPosition; + final ListView listView = tester.element(find.byType(ListView)).widget; + final ScrollController scrollController = listView.controller; + assert(scrollController != null); + scrollPosition = scrollController.position.pixels; + assert(scrollPosition != null); + return scrollPosition; + } + + await tester.pumpWidget( + MaterialApp( + home: Material( + child: Align( + alignment: Alignment.center, + child: button, + ), + ), + ), + ); + + await tester.tap(find.text('99')); + await tester.pumpAndSettle(); + expect(getMenuScroll(), 4312.0); + }); }