From e47a1dc21613d67a2e2e13c4b81f98c98ee5d4f3 Mon Sep 17 00:00:00 2001 From: Hans Muller Date: Thu, 26 Sep 2019 07:18:25 -0700 Subject: [PATCH] Dropdown Menu layout respects menu items intrinsic sizes (#41120) --- .../flutter/lib/src/material/dropdown.dart | 102 +++++++++++++++--- .../flutter/test/material/dropdown_test.dart | 98 +++++++++++++++++ 2 files changed, 185 insertions(+), 15 deletions(-) diff --git a/packages/flutter/lib/src/material/dropdown.dart b/packages/flutter/lib/src/material/dropdown.dart index fd89f4d6cba..49d9f0773b5 100644 --- a/packages/flutter/lib/src/material/dropdown.dart +++ b/packages/flutter/lib/src/material/dropdown.dart @@ -4,6 +4,7 @@ import 'dart:math' as math; +import 'package:flutter/rendering.dart'; import 'package:flutter/widgets.dart'; import 'button_theme.dart'; @@ -30,12 +31,15 @@ const EdgeInsetsGeometry _kUnalignedMenuMargin = EdgeInsetsDirectional.only(star typedef DropdownButtonBuilder = List Function(BuildContext context); +typedef _GetSelectedItemOffsetCallback = double Function(int selectedIndex); + class _DropdownMenuPainter extends CustomPainter { _DropdownMenuPainter({ this.color, this.elevation, this.selectedIndex, this.resize, + this.getSelectedItemOffset, }) : _painter = BoxDecoration( // If you add an image here, you must provide a real // configuration in the paint() function and you must provide some sort @@ -50,12 +54,12 @@ class _DropdownMenuPainter extends CustomPainter { final int elevation; final int selectedIndex; final Animation resize; - + final _GetSelectedItemOffsetCallback getSelectedItemOffset; final BoxPainter _painter; @override void paint(Canvas canvas, Size size) { - final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top; + final double selectedItemOffset = getSelectedItemOffset(selectedIndex); final Tween top = Tween( begin: selectedItemOffset.clamp(0.0, size.height - _kMenuItemHeight), end: 0.0, @@ -165,7 +169,7 @@ class _DropdownMenuState extends State<_DropdownMenu> { ), onTap: () => Navigator.pop( context, - _DropdownRouteResult(route.items[itemIndex].value), + _DropdownRouteResult(route.items[itemIndex].item.value), ), ), )); @@ -179,6 +183,9 @@ class _DropdownMenuState extends State<_DropdownMenu> { elevation: route.elevation, selectedIndex: route.selectedIndex, resize: _resize, + // This offset is passed as a callback, not a value, because it must + // be retrieved at paint time (after layout), not at build time. + getSelectedItemOffset: (int index) => route.getSelectedItemOffset(index), ), child: Semantics( scopesRoute: true, @@ -194,7 +201,6 @@ class _DropdownMenuState extends State<_DropdownMenu> { child: ListView( controller: widget.route.scrollController, padding: kMaterialListPadding, - itemExtent: _kMenuItemHeight, shrinkWrap: true, children: children, ), @@ -303,9 +309,10 @@ class _DropdownRoute extends PopupRoute<_DropdownRouteResult> { this.theme, @required this.style, this.barrierLabel, - }) : assert(style != null); + }) : assert(style != null), + itemHeights = List.filled(items.length, kMinInteractiveDimension); - final List> items; + final List<_MenuItem> items; final EdgeInsetsGeometry padding; final Rect buttonRect; final int selectedIndex; @@ -313,6 +320,7 @@ class _DropdownRoute extends PopupRoute<_DropdownRouteResult> { final ThemeData theme; final TextStyle style; + final List itemHeights; ScrollController scrollController; @override @@ -349,6 +357,17 @@ class _DropdownRoute extends PopupRoute<_DropdownRouteResult> { void _dismiss() { navigator?.removeRoute(this); } + + double getSelectedItemOffset(int selectedIndex) { + double offset = kMaterialListPadding.top; + if (items.isNotEmpty && selectedIndex > 0) { + assert(items.length == itemHeights?.length); + offset += itemHeights + .sublist(0, selectedIndex) + .reduce((double total, double height) => total + height); + } + return offset; + } } class _DropdownRoutePage extends StatelessWidget { @@ -367,7 +386,7 @@ class _DropdownRoutePage extends StatelessWidget { final _DropdownRoute route; final BoxConstraints constraints; - final List> items; + final List<_MenuItem> items; final EdgeInsetsGeometry padding; final Rect buttonRect; final int selectedIndex; @@ -391,10 +410,13 @@ class _DropdownRoutePage extends StatelessWidget { final double topLimit = math.min(_kMenuItemHeight, buttonTop); final double bottomLimit = math.max(availableHeight - _kMenuItemHeight, buttonBottom); - final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top; + final List itemHeights = route.itemHeights; + final double selectedItemOffset = route.getSelectedItemOffset(selectedIndex); double menuTop = (buttonTop - selectedItemOffset) - (_kMenuItemHeight - buttonRect.height) / 2.0; - final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical; + double preferredMenuHeight = kMaterialListPadding.vertical; + if (items.isNotEmpty) + preferredMenuHeight += itemHeights.reduce((double total, double height) => total + height); // If there are too many elements in the menu, we need to shrink it down // so it is at most the maxMenuHeight. @@ -457,6 +479,44 @@ class _DropdownRoutePage extends StatelessWidget { } } +// This widget enables _DropdownRoute to look up the sizes of +// each menu item. These sizes are used to compute the offset of the selected +// item so that _DropdownRoutePage can align the vertical center of the +// selected item lines up with the vertical center of the dropdown button, +// as closely as posible. +class _MenuItem extends SingleChildRenderObjectWidget { + const _MenuItem({ + Key key, + @required this.onLayout, + @required this.item, + }) : assert(onLayout != null), assert(item != null), super(key: key, child: item); + + final ValueChanged onLayout; + final DropdownMenuItem item; + + @override + RenderObject createRenderObject(BuildContext context) { + return _RenderMenuItem(onLayout); + } + + @override + void updateRenderObject(BuildContext context, covariant _RenderMenuItem renderObject) { + renderObject.onLayout = onLayout; + } +} + +class _RenderMenuItem extends RenderProxyBox { + _RenderMenuItem(this.onLayout, [RenderBox child]) : assert(onLayout != null), super(child); + + ValueChanged onLayout; + + @override + void performLayout() { + super.performLayout(); + onLayout(size); + } +} + /// An item in a menu created by a [DropdownButton]. /// /// The type `T` is the type of the value the entry represents. All the entries @@ -484,10 +544,12 @@ class DropdownMenuItem extends StatelessWidget { @override Widget build(BuildContext context) { - return Container( - height: _kMenuItemHeight, - alignment: AlignmentDirectional.centerStart, - child: child, + return IntrinsicHeight( + child: Container( + alignment: AlignmentDirectional.centerStart, + constraints: const BoxConstraints(minHeight: kMinInteractiveDimension), + child: child, + ), ); } } @@ -826,9 +888,19 @@ class _DropdownButtonState extends State> with WidgetsBindi ? _kAlignedMenuMargin : _kUnalignedMenuMargin; + final List<_MenuItem> menuItems = List<_MenuItem>(widget.items.length); + for (int index = 0; index < widget.items.length; index += 1) { + menuItems[index] = _MenuItem( + item: widget.items[index], + onLayout: (Size size) { + _dropdownRoute.itemHeights[index] = size.height; + }, + ); + } + assert(_dropdownRoute == null); _dropdownRoute = _DropdownRoute( - items: widget.items, + items: menuItems, buttonRect: menuMargin.resolve(textDirection).inflateRect(itemRect), padding: _kMenuItemPadding.resolve(textDirection), selectedIndex: _selectedIndex ?? 0, @@ -899,7 +971,7 @@ class _DropdownButtonState extends State> with WidgetsBindi ? List.from(widget.items) : widget.selectedItemBuilder(context).map((Widget item) { return Container( - height: _kMenuItemHeight, + constraints: const BoxConstraints(minHeight: _kMenuItemHeight), alignment: AlignmentDirectional.centerStart, child: item, ); diff --git a/packages/flutter/test/material/dropdown_test.dart b/packages/flutter/test/material/dropdown_test.dart index 90d9f7e46f5..62c3afec30b 100644 --- a/packages/flutter/test/material/dropdown_test.dart +++ b/packages/flutter/test/material/dropdown_test.dart @@ -1685,4 +1685,102 @@ void main() { verifyPaintedShadow(customPaintTwo, 24); debugDisableShadows = true; }); + + testWidgets('Variable size and oversized menu items', (WidgetTester tester) async { + final List itemHeights = [30, 40, 50, 60]; + double dropdownValue = itemHeights[0]; + + Widget buildFrame() { + return MaterialApp( + home: Scaffold( + body: Center( + child: StatefulBuilder( + builder: (BuildContext context, StateSetter setState) { + return DropdownButton( + onChanged: (double value) { + setState(() { dropdownValue = value; }); + }, + value: dropdownValue, + items: itemHeights.map>((double value) { + return DropdownMenuItem( + key: ValueKey(value), + value: value, + child: Center( + child: Container( + width: 100, + height: value, + color: Colors.blue, + ), + ), + ); + }).toList(), + ); + }, + ), + ), + ), + ); + } + + final Finder dropdownIcon = find.byType(Icon); + final Finder item30 = find.byKey(const ValueKey(30)); + final Finder item40 = find.byKey(const ValueKey(40)); + final Finder item50 = find.byKey(const ValueKey(50)); + final Finder item60 = find.byKey(const ValueKey(60)); + + // Only the DropdownButton is visible. It contains the selected item + // and a dropdown arrow icon. + await tester.pumpWidget(buildFrame()); + expect(dropdownIcon, findsOneWidget); + expect(item30, findsOneWidget); + + // All menu items have a minimum height of 48. The centers of the + // dropdown icon and the selected menu item are vertically aligned + // and horizontally adjacent. + expect(tester.getSize(item30), const Size(100, 48)); + expect(tester.getCenter(item30).dy, tester.getCenter(dropdownIcon).dy); + expect(tester.getTopRight(item30).dx, tester.getTopLeft(dropdownIcon).dx); + + // Show the popup menu. + await tester.tap(item30); + await tester.pumpAndSettle(); + + // The Each item appears twice, once in the menu and once + // in the dropdown button's IndexedStack. + expect(item30.evaluate().length, 2); + expect(item40.evaluate().length, 2); + expect(item50.evaluate().length, 2); + expect(item60.evaluate().length, 2); + + // Verify that the items have the expected sizes. The width of the items + // that appear in the menu is padded by 16 on the left and right. + expect(tester.getSize(item30.first), const Size(100, 48)); + expect(tester.getSize(item40.first), const Size(100, 48)); + expect(tester.getSize(item50.first), const Size(100, 50)); + expect(tester.getSize(item60.first), const Size(100, 60)); + expect(tester.getSize(item30.last), const Size(132, 48)); + expect(tester.getSize(item40.last), const Size(132, 48)); + expect(tester.getSize(item50.last), const Size(132, 50)); + expect(tester.getSize(item60.last), const Size(132, 60)); + + // The vertical center of the selectedItem (item30) should + // line up with its button counterpart. + expect(tester.getCenter(item30.first).dy, tester.getCenter(item30.last).dy); + + // The menu items should be arranged in a column. + expect(tester.getBottomLeft(item30.last), tester.getTopLeft(item40.last)); + expect(tester.getBottomLeft(item40.last), tester.getTopLeft(item50.last)); + expect(tester.getBottomLeft(item50.last), tester.getTopLeft(item60.last)); + + // Dismiss the menu by selecting item40 and then show the menu again. + await tester.tap(item40.last); + await tester.pumpAndSettle(); + expect(dropdownValue, 40); + await tester.tap(item40.first); + await tester.pumpAndSettle(); + + // The vertical center of the selectedItem (item40) should + // line up with its button counterpart. + expect(tester.getCenter(item40.first).dy, tester.getCenter(item40.last).dy); + }); }