Unverified Commit 6d18c20a authored by Hans Muller's avatar Hans Muller Committed by GitHub

Update PopupMenu layout (#40179)

parent 8a1bf5b8
......@@ -5,6 +5,7 @@
import 'dart:async';
import 'package:flutter/foundation.dart';
import 'package:flutter/rendering.dart';
import 'package:flutter/widgets.dart';
import 'constants.dart';
......@@ -27,10 +28,8 @@ import 'theme.dart';
// void setState(VoidCallback fn) { }
const Duration _kMenuDuration = Duration(milliseconds: 300);
const double _kBaselineOffsetFromBottom = 20.0;
const double _kMenuCloseIntervalEnd = 2.0 / 3.0;
const double _kMenuHorizontalPadding = 16.0;
const double _kMenuItemHeight = kMinInteractiveDimension;
const double _kMenuDividerHeight = 16.0;
const double _kMenuMaxWidth = 5.0 * _kMenuWidthStep;
const double _kMenuMinWidth = 2.0 * _kMenuWidthStep;
......@@ -122,6 +121,49 @@ class _PopupMenuDividerState extends State<PopupMenuDivider> {
Widget build(BuildContext context) => Divider(height: widget.height);
}
// This widget only exists to enable _PopupMenuRoute to save the sizes of
// each menu item. The sizes are used by _PopupMenuRouteLayout to compute the
// y coordinate of the menu's origin so that the center of selected menu
// item lines up with the center of its PopupMenuButton.
class _MenuItem extends SingleChildRenderObjectWidget {
const _MenuItem({
Key key,
@required this.onLayout,
Widget child,
}) : assert(onLayout != null), super(key: key, child: child);
final ValueChanged<Size> onLayout;
@override
RenderObject createRenderObject(BuildContext context) {
return _RenderMenuItem(onLayout);
}
@override
void updateRenderObject(BuildContext context, covariant _RenderMenuItem renderObject) {
renderObject.onLayout = onLayout;
}
}
class _RenderMenuItem extends RenderShiftedBox {
_RenderMenuItem(this.onLayout, [RenderBox child]) : assert(onLayout != null), super(child);
ValueChanged<Size> onLayout;
@override
void performLayout() {
if (child == null) {
size = Size.zero;
} else {
child.layout(constraints, parentUsesSize: true);
size = constraints.constrain(child.size);
}
final BoxParentData childParentData = child.parentData;
childParentData.offset = Offset.zero;
onLayout(size);
}
}
/// An item in a material design popup menu.
///
/// To show a popup menu, use the [showMenu] function. To create a button that
......@@ -166,12 +208,12 @@ class PopupMenuItem<T> extends PopupMenuEntry<T> {
///
/// By default, the item is [enabled].
///
/// The `height` and `enabled` arguments must not be null.
/// The `enabled` and `height` arguments must not be null.
const PopupMenuItem({
Key key,
this.value,
this.enabled = true,
this.height = _kMenuItemHeight,
this.height = kMinInteractiveDimension,
this.textStyle,
@required this.child,
}) : assert(enabled != null),
......@@ -181,19 +223,19 @@ class PopupMenuItem<T> extends PopupMenuEntry<T> {
/// The value that will be returned by [showMenu] if this entry is selected.
final T value;
/// Whether the user is permitted to select this entry.
/// Whether the user is permitted to select this item.
///
/// Defaults to true. If this is false, then the item will not react to
/// touches.
final bool enabled;
/// The height of the entry.
/// The minimum height height of the menu item.
///
/// Defaults to kMinInteractiveDimension pixels.
/// Defaults to [kMinInteractiveDimension] pixels.
@override
final double height;
/// The text style of the popup menu entry.
/// The text style of the popup menu item.
///
/// If this property is null, then [PopupMenuThemeData.textStyle] is used.
/// If [PopupMenuThemeData.textStyle] is also null, then [ThemeData.textTheme.subhead] is used.
......@@ -262,12 +304,14 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
Widget item = AnimatedDefaultTextStyle(
style: style,
duration: kThemeChangeDuration,
child: Baseline(
baseline: widget.height - _kBaselineOffsetFromBottom,
baselineType: style.textBaseline ?? TextBaseline.alphabetic,
child: Container(
alignment: AlignmentDirectional.centerStart,
constraints: BoxConstraints(minHeight: widget.height),
padding: const EdgeInsets.symmetric(horizontal: _kMenuHorizontalPadding),
child: buildChild(),
),
);
if (!widget.enabled) {
final bool isDark = theme.brightness == Brightness.dark;
item = IconTheme.merge(
......@@ -278,11 +322,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
return InkWell(
onTap: widget.enabled ? handleTap : null,
child: Container(
height: widget.height,
padding: const EdgeInsets.symmetric(horizontal: _kMenuHorizontalPadding),
child: item,
),
child: item,
);
}
}
......@@ -293,8 +333,8 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
/// shows a popup menu, consider using [PopupMenuButton].
///
/// A [CheckedPopupMenuItem] is kMinInteractiveDimension pixels high, which
/// matches the default height of a [PopupMenuItem]. The horizontal layout uses
/// a [ListTile]; the checkmark is an [Icons.done] icon, shown in the
/// matches the default minimum height of a [PopupMenuItem]. The horizontal
/// layout uses [ListTile]; the checkmark is an [Icons.done] icon, shown in the
/// [ListTile.leading] position.
///
/// {@tool sample}
......@@ -460,10 +500,17 @@ class _PopupMenu<T> extends StatelessWidget {
child: item,
);
}
children.add(FadeTransition(
opacity: opacity,
child: item,
));
children.add(
_MenuItem(
onLayout: (Size size) {
route.itemSizes[i] = size;
},
child: FadeTransition(
opacity: opacity,
child: item,
),
),
);
}
final CurveTween opacity = CurveTween(curve: const Interval(0.0, 1.0 / 3.0));
......@@ -518,15 +565,18 @@ class _PopupMenu<T> extends StatelessWidget {
// Positioning of the menu on the screen.
class _PopupMenuRouteLayout extends SingleChildLayoutDelegate {
_PopupMenuRouteLayout(this.position, this.selectedItemOffset, this.textDirection);
_PopupMenuRouteLayout(this.position, this.itemSizes, this.selectedItemIndex, this.textDirection);
// Rectangle of underlying button, relative to the overlay's dimensions.
final RelativeRect position;
// The distance from the top of the menu to the middle of selected item.
//
// This will be null if there's no item to position in this way.
final double selectedItemOffset;
// The sizes of each item are computed when the menu is laid out, and before
// the route is laid out.
List<Size> itemSizes;
// The index of the selected item, or null if PopupMenuButton.initialValue
// was not specified.
final int selectedItemIndex;
// Whether to prefer going to the left or to the right.
final TextDirection textDirection;
......@@ -549,10 +599,12 @@ class _PopupMenuRouteLayout extends SingleChildLayoutDelegate {
// getConstraintsForChild.
// Find the ideal vertical position.
double y;
if (selectedItemOffset == null) {
y = position.top;
} else {
double y = position.top;
if (selectedItemIndex != null && itemSizes != null) {
double selectedItemOffset = _kMenuVerticalPadding;
for (int index = 0; index < selectedItemIndex; index += 1)
selectedItemOffset += itemSizes[index].height;
selectedItemOffset += itemSizes[selectedItemIndex].height / 2;
y = position.top + (size.height - position.top - position.bottom) / 2.0 - selectedItemOffset;
}
......@@ -592,7 +644,15 @@ class _PopupMenuRouteLayout extends SingleChildLayoutDelegate {
@override
bool shouldRelayout(_PopupMenuRouteLayout oldDelegate) {
return position != oldDelegate.position;
// If called when the old and new itemSizes have been initialized then
// we expect them to have the same length because there's no practical
// way to change length of the items list once the menu has been shown.
assert(itemSizes.length == oldDelegate.itemSizes.length);
return position != oldDelegate.position
|| selectedItemIndex != oldDelegate.selectedItemIndex
|| textDirection != oldDelegate.textDirection
|| !listEquals(itemSizes, oldDelegate.itemSizes);
}
}
......@@ -610,10 +670,11 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
this.color,
this.showMenuContext,
this.captureInheritedThemes,
});
}) : itemSizes = List<Size>(items.length);
final RelativeRect position;
final List<PopupMenuEntry<T>> items;
final List<Size> itemSizes;
final dynamic initialValue;
final double elevation;
final ThemeData theme;
......@@ -647,15 +708,12 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
@override
Widget buildPage(BuildContext context, Animation<double> animation, Animation<double> secondaryAnimation) {
double selectedItemOffset;
int selectedItemIndex;
if (initialValue != null) {
double y = _kMenuVerticalPadding;
for (PopupMenuEntry<T> entry in items) {
if (entry.represents(initialValue)) {
selectedItemOffset = y + entry.height / 2.0;
break;
}
y += entry.height;
for (int index = 0; selectedItemIndex == null && index < items.length; index += 1) {
if (items[index].represents(initialValue))
selectedItemIndex = index;
}
}
......@@ -681,7 +739,8 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
return CustomSingleChildLayout(
delegate: _PopupMenuRouteLayout(
position,
selectedItemOffset,
itemSizes,
selectedItemIndex,
Directionality.of(context),
),
child: menu,
......
......@@ -690,6 +690,160 @@ void main() {
await tester.tap(find.text('Menu Button'));
});
testWidgets('PopupMenuItem child height is a minimum, child is vertically centered', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
final Type menuItemType = const PopupMenuItem<String>(child: Text('item')).runtimeType;
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
// This menu item's height will be 48 because the the default minimum height
// is 48 and the height of the text is less than 48.
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
// This menu item's height parameter specifies its minium height. The
// overall height of the menu item will be 50 because the child's
// height 40, is less than 50.
const PopupMenuItem<String>(
height: 50,
value: '1',
child: SizedBox(
height: 40,
child: Text('Item 1'),
),
),
// This menu item's height parameter specifies its minium height, so the
// overall height of the menu item will be 75.
const PopupMenuItem<String>(
height: 75,
value: '2',
child: SizedBox(
child: Text('Item 2'),
),
),
// This menu item's height will be 100.
const PopupMenuItem<String>(
value: '3',
child: SizedBox(
height: 100,
child: Text('Item 3'),
),
),
];
},
),
),
),
),
);
// Show the menu
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// The menu items and their InkWells should have the expected vertical size
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 0')).height, 48);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 1')).height, 50);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 2')).height, 75);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 3')).height, 100);
expect(tester.getSize(find.widgetWithText(InkWell, 'Item 0')).height, 48);
expect(tester.getSize(find.widgetWithText(InkWell, 'Item 1')).height, 50);
expect(tester.getSize(find.widgetWithText(InkWell, 'Item 2')).height, 75);
expect(tester.getSize(find.widgetWithText(InkWell, 'Item 3')).height, 100);
// Menu item children which whose height is less than the PopupMenuItem
// are vertically centered.
expect(
tester.getRect(find.widgetWithText(menuItemType, 'Item 0')).center.dy,
tester.getRect(find.text('Item 0')).center.dy,
);
expect(
tester.getRect(find.widgetWithText(menuItemType, 'Item 2')).center.dy,
tester.getRect(find.text('Item 2')).center.dy,
);
});
testWidgets('Update PopupMenuItem layout while the menu is visible', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
final Type menuItemType = const PopupMenuItem<String>(child: Text('item')).runtimeType;
Widget buildFrame({
TextDirection textDirection = TextDirection.ltr,
double fontSize = 24,
}) {
return MaterialApp(
builder: (BuildContext context, Widget child) {
return Directionality(
textDirection: textDirection,
child: PopupMenuTheme(
data: PopupMenuTheme.of(context).copyWith(
textStyle: Theme.of(context).textTheme.subhead.copyWith(fontSize: fontSize),
),
child: child,
),
);
},
home: Scaffold(
body: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
const PopupMenuItem<String>(
value: '1',
child: Text('Item 1'),
),
];
},
),
),
);
}
// Show the menu
await tester.pumpWidget(buildFrame());
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// The menu items should have their default heights and horizontal alignment.
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 0')).height, 48);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 1')).height, 48);
expect(tester.getTopLeft(find.text('Item 0')).dx, 24);
expect(tester.getTopLeft(find.text('Item 1')).dx, 24);
// While the menu is up, change its font size to 64 (default is 16).
await tester.pumpWidget(buildFrame(fontSize: 64));
await tester.pumpAndSettle(); // Theme changes are animated.
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 0')).height, 128);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 1')).height, 128);
expect(tester.getSize(find.text('Item 0')).height, 128);
expect(tester.getSize(find.text('Item 1')).height, 128);
expect(tester.getTopLeft(find.text('Item 0')).dx, 24);
expect(tester.getTopLeft(find.text('Item 1')).dx, 24);
// While the menu is up, change the textDirection to rtl. Now menu items
// will be aligned right.
await tester.pumpWidget(buildFrame(textDirection: TextDirection.rtl));
await tester.pumpAndSettle(); // Theme changes are animated.
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 0')).height, 48);
expect(tester.getSize(find.widgetWithText(menuItemType, 'Item 1')).height, 48);
expect(tester.getTopLeft(find.text('Item 0')).dx, 72);
expect(tester.getTopLeft(find.text('Item 1')).dx, 72);
});
}
class TestApp extends StatefulWidget {
......
......@@ -78,20 +78,20 @@ void main() {
Offset bottomLeft = tester.getBottomLeft(find.text('hello, world'));
Offset bottomRight = tester.getBottomRight(find.text('hello, world'));
expect(topLeft, const Offset(392.0, 298.3999996185303));
expect(topRight, const Offset(596.0, 298.3999996185303));
expect(bottomLeft, const Offset(392.0, 315.3999996185303));
expect(bottomRight, const Offset(596.0, 315.3999996185303));
expect(topLeft, const Offset(392.0, 299.5));
expect(topRight, const Offset(596.0, 299.5));
expect(bottomLeft, const Offset(392.0, 316.5));
expect(bottomRight, const Offset(596.0, 316.5));
topLeft = tester.getTopLeft(find.text('你好,世界'));
topRight = tester.getTopRight(find.text('你好,世界'));
bottomLeft = tester.getBottomLeft(find.text('你好,世界'));
bottomRight = tester.getBottomRight(find.text('你好,世界'));
expect(topLeft, const Offset(392.0, 346.3999996185303));
expect(topRight, const Offset(477.0, 346.3999996185303));
expect(bottomLeft, const Offset(392.0, 363.3999996185303));
expect(bottomRight, const Offset(477.0, 363.3999996185303));
expect(topLeft, const Offset(392.0, 347.5));
expect(topRight, const Offset(477.0, 347.5));
expect(bottomLeft, const Offset(392.0, 364.5));
expect(bottomRight, const Offset(477.0, 364.5));
}, skip: !isLinux);
testWidgets('Text baseline with EN locale', (WidgetTester tester) async {
......@@ -165,19 +165,19 @@ void main() {
Offset bottomRight = tester.getBottomRight(find.text('hello, world'));
expect(topLeft, const Offset(392.0, 299.19999980926514));
expect(topRight, const Offset(584.0, 299.19999980926514));
expect(bottomLeft, const Offset(392.0, 315.19999980926514));
expect(bottomRight, const Offset(584.0, 315.19999980926514));
expect(topLeft, const Offset(392.0, 300.0));
expect(topRight, const Offset(584.0, 300.0));
expect(bottomLeft, const Offset(392.0, 316));
expect(bottomRight, const Offset(584.0, 316));
topLeft = tester.getTopLeft(find.text('你好,世界'));
topRight = tester.getTopRight(find.text('你好,世界'));
bottomLeft = tester.getBottomLeft(find.text('你好,世界'));
bottomRight = tester.getBottomRight(find.text('你好,世界'));
expect(topLeft, const Offset(392.0, 347.19999980926514));
expect(topRight, const Offset(472.0, 347.19999980926514));
expect(bottomLeft, const Offset(392.0, 363.19999980926514));
expect(bottomRight, const Offset(472.0, 363.19999980926514));
expect(topLeft, const Offset(392.0, 348.0));
expect(topRight, const Offset(472.0, 348.0));
expect(bottomLeft, const Offset(392.0, 364.0));
expect(bottomRight, const Offset(472.0, 364.0));
}, skip: !isLinux);
}
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