Dropdown Menu layout respects menu items intrinsic sizes (#41120)

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<Widget> 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<double> 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<double> top = Tween<double>(
       begin: selectedItemOffset.clamp(0.0, size.height - _kMenuItemHeight),
       end: 0.0,
@@ -165,7 +169,7 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
           ),
           onTap: () => Navigator.pop(
             context,
-            _DropdownRouteResult<T>(route.items[itemIndex].value),
+            _DropdownRouteResult<T>(route.items[itemIndex].item.value),
           ),
         ),
       ));
@@ -179,6 +183,9 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
           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<T> extends State<_DropdownMenu<T>> {
                 child: ListView(
                   controller: widget.route.scrollController,
                   padding: kMaterialListPadding,
-                  itemExtent: _kMenuItemHeight,
                   shrinkWrap: true,
                   children: children,
                 ),
@@ -303,9 +309,10 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
     this.theme,
     @required this.style,
     this.barrierLabel,
-  }) : assert(style != null);
+  }) : assert(style != null),
+       itemHeights = List<double>.filled(items.length, kMinInteractiveDimension);
 
-  final List<DropdownMenuItem<T>> items;
+  final List<_MenuItem<T>> items;
   final EdgeInsetsGeometry padding;
   final Rect buttonRect;
   final int selectedIndex;
@@ -313,6 +320,7 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
   final ThemeData theme;
   final TextStyle style;
 
+  final List<double> itemHeights;
   ScrollController scrollController;
 
   @override
@@ -349,6 +357,17 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
   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<T> extends StatelessWidget {
@@ -367,7 +386,7 @@ class _DropdownRoutePage<T> extends StatelessWidget {
 
   final _DropdownRoute<T> route;
   final BoxConstraints constraints;
-  final List<DropdownMenuItem<T>> items;
+  final List<_MenuItem<T>> items;
   final EdgeInsetsGeometry padding;
   final Rect buttonRect;
   final int selectedIndex;
@@ -391,10 +410,13 @@ class _DropdownRoutePage<T> 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<double> 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<T> 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<T> 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<Size> onLayout;
+  final DropdownMenuItem<T> 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<Size> 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<T> 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<T> extends State<DropdownButton<T>> with WidgetsBindi
       ? _kAlignedMenuMargin
       : _kUnalignedMenuMargin;
 
+    final List<_MenuItem<T>> menuItems = List<_MenuItem<T>>(widget.items.length);
+    for (int index = 0; index < widget.items.length; index += 1) {
+      menuItems[index] = _MenuItem<T>(
+        item: widget.items[index],
+        onLayout: (Size size) {
+          _dropdownRoute.itemHeights[index] = size.height;
+        },
+      );
+    }
+
     assert(_dropdownRoute == null);
     _dropdownRoute = _DropdownRoute<T>(
-      items: widget.items,
+      items: menuItems,
       buttonRect: menuMargin.resolve(textDirection).inflateRect(itemRect),
       padding: _kMenuItemPadding.resolve(textDirection),
       selectedIndex: _selectedIndex ?? 0,
@@ -899,7 +971,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
         ? List<Widget>.from(widget.items)
         : widget.selectedItemBuilder(context).map((Widget item) {
             return Container(
-              height: _kMenuItemHeight,
+              constraints: const BoxConstraints(minHeight: _kMenuItemHeight),
               alignment: AlignmentDirectional.centerStart,
               child: item,
             );

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<double> itemHeights = <double>[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<double>(
+                  onChanged: (double value) {
+                    setState(() { dropdownValue = value; });
+                  },
+                  value: dropdownValue,
+                  items: itemHeights.map<DropdownMenuItem<double>>((double value) {
+                    return DropdownMenuItem<double>(
+                      key: ValueKey<double>(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<double>(30));
+    final Finder item40 = find.byKey(const ValueKey<double>(40));
+    final Finder item50 = find.byKey(const ValueKey<double>(50));
+    final Finder item60 = find.byKey(const ValueKey<double>(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);
+  });
 }