Commit b770f344 authored by Hans Muller's avatar Hans Muller Committed by Shi-Hao Hong

Reprise: Dropdown Menu layout respects menu items intrinsic sizes (#42033)

* Reprise: Dropdown Menu layout respects menu items intrinsic sizes

* updated per review feedback

* updated per review feedback
parent 1d2eaaf2
......@@ -4,6 +4,7 @@
import 'dart:math' as math;
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'button_theme.dart';
......@@ -36,6 +37,7 @@ class _DropdownMenuPainter extends CustomPainter {
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 +52,12 @@ class _DropdownMenuPainter extends CustomPainter {
final int elevation;
final int selectedIndex;
final Animation<double> resize;
final ValueGetter<double> getSelectedItemOffset;
final BoxPainter _painter;
@override
void paint(Canvas canvas, Size size) {
final double selectedItemOffset = selectedIndex * _kMenuItemHeight + kMaterialListPadding.top;
final double selectedItemOffset = getSelectedItemOffset();
final Tween<double> top = Tween<double>(
begin: selectedItemOffset.clamp(0.0, size.height - _kMenuItemHeight),
end: 0.0,
......@@ -165,7 +167,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 +181,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: route.getSelectedItemOffset,
),
child: Semantics(
scopesRoute: true,
......@@ -194,7 +199,6 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
child: ListView(
controller: widget.route.scrollController,
padding: kMaterialListPadding,
itemExtent: _kMenuItemHeight,
shrinkWrap: true,
children: children,
),
......@@ -210,14 +214,12 @@ class _DropdownMenuState<T> extends State<_DropdownMenu<T>> {
class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
_DropdownMenuRouteLayout({
@required this.buttonRect,
@required this.menuTop,
@required this.menuHeight,
@required this.route,
@required this.textDirection,
});
final Rect buttonRect;
final double menuTop;
final double menuHeight;
final _DropdownRoute<T> route;
final TextDirection textDirection;
@override
......@@ -240,14 +242,16 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
@override
Offset getPositionForChild(Size size, Size childSize) {
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, size.height);
assert(() {
final Rect container = Offset.zero & size;
if (container.intersect(buttonRect) == buttonRect) {
// If the button was entirely on-screen, then verify
// that the menu is also on-screen.
// If the button was a bit off-screen, then, oh well.
assert(menuTop >= 0.0);
assert(menuTop + menuHeight <= size.height);
assert(menuLimits.top >= 0.0);
assert(menuLimits.top + menuLimits.height <= size.height);
}
return true;
}());
......@@ -261,15 +265,13 @@ class _DropdownMenuRouteLayout<T> extends SingleChildLayoutDelegate {
left = buttonRect.left.clamp(0.0, size.width - childSize.width);
break;
}
return Offset(left, menuTop);
return Offset(left, menuLimits.top);
}
@override
bool shouldRelayout(_DropdownMenuRouteLayout<T> oldDelegate) {
return buttonRect != oldDelegate.buttonRect
|| menuTop != oldDelegate.menuTop
|| menuHeight != oldDelegate.menuHeight
|| textDirection != oldDelegate.textDirection;
return buttonRect != oldDelegate.buttonRect || textDirection != oldDelegate.textDirection;
}
}
......@@ -293,6 +295,14 @@ class _DropdownRouteResult<T> {
int get hashCode => result.hashCode;
}
class _MenuLimits {
const _MenuLimits(this.top, this.bottom, this.height, this.scrollOffset);
final double top;
final double bottom;
final double height;
final double scrollOffset;
}
class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
_DropdownRoute({
this.items,
......@@ -303,16 +313,20 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
this.theme,
@required this.style,
this.barrierLabel,
}) : assert(style != null);
this.itemHeight,
}) : assert(style != null),
itemHeights = List<double>.filled(items.length, itemHeight ?? kMinInteractiveDimension);
final List<DropdownMenuItem<T>> items;
final List<_MenuItem<T>> items;
final EdgeInsetsGeometry padding;
final Rect buttonRect;
final int selectedIndex;
final int elevation;
final ThemeData theme;
final TextStyle style;
final double itemHeight;
final List<double> itemHeights;
ScrollController scrollController;
@override
......@@ -349,40 +363,27 @@ class _DropdownRoute<T> extends PopupRoute<_DropdownRouteResult<T>> {
void _dismiss() {
navigator?.removeRoute(this);
}
}
class _DropdownRoutePage<T> extends StatelessWidget {
const _DropdownRoutePage({
Key key,
this.route,
this.constraints,
this.items,
this.padding,
this.buttonRect,
this.selectedIndex,
this.elevation = 8,
this.theme,
this.style,
}) : super(key: key);
final _DropdownRoute<T> route;
final BoxConstraints constraints;
final List<DropdownMenuItem<T>> items;
final EdgeInsetsGeometry padding;
final Rect buttonRect;
final int selectedIndex;
final int elevation;
final ThemeData theme;
final TextStyle style;
double getSelectedItemOffset() {
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;
}
@override
Widget build(BuildContext context) {
assert(debugCheckHasDirectionality(context));
final double availableHeight = constraints.maxHeight;
// Returns the vertical extent of the menu and the initial scrollOffset
// for the ListView that contains the menu items. The vertical center of the
// selected item is aligned with the button's vertical center, as far as
// that's possible given availableHeight.
_MenuLimits getMenuLimits(Rect buttonRect, double availableHeight) {
final double maxMenuHeight = availableHeight - 2.0 * _kMenuItemHeight;
final double buttonTop = buttonRect.top;
final double buttonBottom = math.min(buttonRect.bottom, availableHeight);
final double selectedItemOffset = getSelectedItemOffset();
// 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.
......@@ -391,15 +392,14 @@ 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;
double menuTop = (buttonTop - selectedItemOffset) - (_kMenuItemHeight - buttonRect.height) / 2.0;
final double preferredMenuHeight = (items.length * _kMenuItemHeight) + kMaterialListPadding.vertical;
double menuTop = (buttonTop - selectedItemOffset) - (itemHeights[selectedIndex] - buttonRect.height) / 2.0;
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.
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
......@@ -415,14 +415,56 @@ class _DropdownRoutePage<T> extends StatelessWidget {
menuTop = menuBottom - menuHeight;
}
// If all of the menu items will not fit within availableHeight then
// 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
// shown - subsequently we leave the scroll offset where the user left
// it. This scroll offset is only accurate for fixed height menu items
// (the default).
final double scrollOffset = preferredMenuHeight <= maxMenuHeight ? 0 :
math.max(0.0, selectedItemOffset - (buttonTop - menuTop));
return _MenuLimits(menuTop, menuBottom, menuHeight, scrollOffset);
}
}
class _DropdownRoutePage<T> extends StatelessWidget {
const _DropdownRoutePage({
Key key,
this.route,
this.constraints,
this.items,
this.padding,
this.buttonRect,
this.selectedIndex,
this.elevation = 8,
this.theme,
this.style,
}) : super(key: key);
final _DropdownRoute<T> route;
final BoxConstraints constraints;
final List<_MenuItem<T>> items;
final EdgeInsetsGeometry padding;
final Rect buttonRect;
final int selectedIndex;
final int elevation;
final ThemeData theme;
final TextStyle style;
@override
Widget build(BuildContext context) {
assert(debugCheckHasDirectionality(context));
// Computing the initialScrollOffset now, before the items have been laid
// out. This only works if the item heights are effectively fixed, i.e. either
// DropdownButton.itemHeight is specified or DropdownButton.itemHeight is null
// and all of the items' intrinsic heights are less than kMinInteractiveDimension.
// Otherwise the initialScrollOffset is just a rough approximation based on
// treating the items as if their heights were all equal to kMinInteractveDimension.
if (route.scrollController == null) {
// 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;
route.scrollController = ScrollController(initialScrollOffset: scrollOffset);
final _MenuLimits menuLimits = route.getMenuLimits(buttonRect, constraints.maxHeight);
route.scrollController = ScrollController(initialScrollOffset: menuLimits.scrollOffset);
}
final TextDirection textDirection = Directionality.of(context);
......@@ -445,8 +487,7 @@ class _DropdownRoutePage<T> extends StatelessWidget {
return CustomSingleChildLayout(
delegate: _DropdownMenuRouteLayout<T>(
buttonRect: buttonRect,
menuTop: menuTop,
menuHeight: menuHeight,
route: route,
textDirection: textDirection,
),
child: menu,
......@@ -457,6 +498,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), 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
......@@ -485,7 +564,7 @@ class DropdownMenuItem<T> extends StatelessWidget {
@override
Widget build(BuildContext context) {
return Container(
height: _kMenuItemHeight,
constraints: const BoxConstraints(minHeight: _kMenuItemHeight),
alignment: AlignmentDirectional.centerStart,
child: child,
);
......@@ -617,11 +696,13 @@ class DropdownButton<T> extends StatefulWidget {
this.iconSize = 24.0,
this.isDense = false,
this.isExpanded = false,
this.itemHeight = kMinInteractiveDimension,
}) : assert(items == null || items.isEmpty || value == null || items.where((DropdownMenuItem<T> item) => item.value == value).length == 1),
assert(elevation != null),
assert(iconSize != null),
assert(isDense != null),
assert(isExpanded != null),
assert(itemHeight == null || itemHeight >= kMinInteractiveDimension),
super(key: key);
/// The list of items the user can select.
......@@ -805,6 +886,19 @@ class DropdownButton<T> extends StatefulWidget {
/// surrounding container.
final bool isExpanded;
/// If null, then the menu item heights will vary according to each menu item's
/// intrinsic height.
///
/// The default value is [kMinInteractiveDimension], which is also the minimum
/// height for menu items.
///
/// If this value is null and there isn't enough vertical room for the menu,
/// then the menu's initial scroll offset may not align the selected item with
/// the dropdown button. That's because, in this case, the initial scroll
/// offset is computed as if all of the menu item heights were
/// [kMinInteractiveDimension].
final double itemHeight;
@override
_DropdownButtonState<T> createState() => _DropdownButtonState<T>();
}
......@@ -871,9 +965,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,
......@@ -881,6 +985,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
theme: Theme.of(context, shadowThemeOnly: true),
style: _textStyle,
barrierLabel: MaterialLocalizations.of(context).modalBarrierDismissLabel,
itemHeight: widget.itemHeight,
);
Navigator.push(context, _dropdownRoute).then<void>((_DropdownRouteResult<T> newValue) {
......@@ -944,7 +1049,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,
);
......@@ -957,7 +1062,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
if (widget.hint != null || (!_enabled && widget.disabledHint != null)) {
final Widget emplacedHint = _enabled
? widget.hint
: DropdownMenuItem<Widget>(child: widget.disabledHint ?? widget.hint);
: widget.disabledHint ?? widget.hint;
hintIndex = items.length;
items.add(DefaultTextStyle(
style: _textStyle.copyWith(color: Theme.of(context).hintColor),
......@@ -982,7 +1087,11 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
innerItemsWidget = IndexedStack(
index: index,
alignment: AlignmentDirectional.centerStart,
children: items,
children: widget.isDense ? items : items.map((Widget item) {
return widget.itemHeight != null
? SizedBox(height: widget.itemHeight, child: item)
: Column(mainAxisSize: MainAxisSize.min, children: <Widget>[item]);
}).toList(),
);
}
......@@ -1014,7 +1123,7 @@ class _DropdownButtonState<T> extends State<DropdownButton<T>> with WidgetsBindi
);
if (!DropdownButtonHideUnderline.at(context)) {
final double bottom = widget.isDense ? 0.0 : 8.0;
final double bottom = (widget.isDense || widget.itemHeight == null) ? 0.0 : 8.0;
result = Stack(
children: <Widget>[
result,
......@@ -1074,12 +1183,14 @@ class DropdownButtonFormField<T> extends FormField<T> {
double iconSize = 24.0,
bool isDense = false,
bool isExpanded = false,
double itemHeight,
}) : assert(items == null || items.isEmpty || value == null || items.where((DropdownMenuItem<T> item) => item.value == value).length == 1),
assert(decoration != null),
assert(elevation != null),
assert(iconSize != null),
assert(isDense != null),
assert(isExpanded != null),
assert(itemHeight == null || itemHeight > 0),
super(
key: key,
onSaved: onSaved,
......@@ -1108,6 +1219,7 @@ class DropdownButtonFormField<T> extends FormField<T> {
iconSize: iconSize,
isDense: isDense,
isExpanded: isExpanded,
itemHeight: itemHeight,
),
),
);
......
......@@ -1683,4 +1683,171 @@ 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,
itemHeight: null,
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();
// 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);
});
testWidgets('DropdownButton hint is selected item', (WidgetTester tester) async {
const double hintPaddingOffset = 8;
const List<String> itemValues = <String>['item0', 'item1', 'item2', 'item3'];
String selectedItem = 'item0';
Widget buildFrame() {
return MaterialApp(
home: Scaffold(
body: ButtonTheme(
alignedDropdown: true,
child: DropdownButtonHideUnderline(
child: Center(
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
// The pretzel below is from an actual app. The price
// of limited configurability is keeping this working.
return DropdownButton<String>(
isExpanded: true,
elevation: 2,
value: null,
hint: LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
// Stack with a positioned widget is used to override the
// hard coded 16px margin in the dropdown code, so that
// this hint aligns "properly" with the menu.
return Stack(
alignment: Alignment.topCenter,
overflow: Overflow.visible,
children: <Widget>[
PositionedDirectional(
width: constraints.maxWidth + hintPaddingOffset,
start: -hintPaddingOffset,
top: 4.0,
child: Text('-$selectedItem-'),
),
],
);
},
),
onChanged: (String value) {
setState(() { selectedItem = value; });
},
icon: Container(),
items: itemValues.map<DropdownMenuItem<String>>((String value) {
return DropdownMenuItem<String>(
child: Text(value),
);
}).toList(),
);
},
),
),
),
),
),
);
}
await tester.pumpWidget(buildFrame());
expect(tester.getTopLeft(find.text('-item0-')).dx, 8);
// Show the popup menu.
await tester.tap(find.text('-item0-'));
await tester.pumpAndSettle();
expect(tester.getTopLeft(find.text('-item0-')).dx, 8);
});
}
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