Unverified Commit 34b8cef5 authored by Greg Spencer's avatar Greg Spencer Committed by GitHub

Revert "feat: added custom padding in PopupMenuButton (#96657)" (#96781)

parent a2ac0e30
...@@ -528,12 +528,10 @@ class _PopupMenu<T> extends StatelessWidget { ...@@ -528,12 +528,10 @@ class _PopupMenu<T> extends StatelessWidget {
Key? key, Key? key,
required this.route, required this.route,
required this.semanticLabel, required this.semanticLabel,
this.padding,
}) : super(key: key); }) : super(key: key);
final _PopupMenuRoute<T> route; final _PopupMenuRoute<T> route;
final String? semanticLabel; final String? semanticLabel;
final EdgeInsetsGeometry? padding;
@override @override
Widget build(BuildContext context) { Widget build(BuildContext context) {
...@@ -585,7 +583,9 @@ class _PopupMenu<T> extends StatelessWidget { ...@@ -585,7 +583,9 @@ class _PopupMenu<T> extends StatelessWidget {
explicitChildNodes: true, explicitChildNodes: true,
label: semanticLabel, label: semanticLabel,
child: SingleChildScrollView( child: SingleChildScrollView(
padding: padding, padding: const EdgeInsets.symmetric(
vertical: _kMenuVerticalPadding,
),
child: ListBody(children: children), child: ListBody(children: children),
), ),
), ),
...@@ -727,7 +727,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -727,7 +727,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
_PopupMenuRoute({ _PopupMenuRoute({
required this.position, required this.position,
required this.items, required this.items,
this.padding,
this.initialValue, this.initialValue,
this.elevation, this.elevation,
required this.barrierLabel, required this.barrierLabel,
...@@ -741,7 +740,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -741,7 +740,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
final List<PopupMenuEntry<T>> items; final List<PopupMenuEntry<T>> items;
final List<Size?> itemSizes; final List<Size?> itemSizes;
final T? initialValue; final T? initialValue;
final EdgeInsetsGeometry? padding;
final double? elevation; final double? elevation;
final String? semanticLabel; final String? semanticLabel;
final ShapeBorder? shape; final ShapeBorder? shape;
...@@ -780,7 +778,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -780,7 +778,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
} }
} }
final Widget menu = _PopupMenu<T>(padding: padding, route: this, semanticLabel: semanticLabel); final Widget menu = _PopupMenu<T>(route: this, semanticLabel: semanticLabel);
final MediaQueryData mediaQuery = MediaQuery.of(context); final MediaQueryData mediaQuery = MediaQuery.of(context);
return MediaQuery.removePadding( return MediaQuery.removePadding(
context: context, context: context,
...@@ -852,11 +850,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -852,11 +850,6 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
/// label is not provided, it will default to /// label is not provided, it will default to
/// [MaterialLocalizations.popupMenuLabel]. /// [MaterialLocalizations.popupMenuLabel].
/// ///
/// The `padding` argument is used to add empty space around the outside
/// of the popup menu. If this property is not provided, then [PopupMenuThemeData.padding]
/// is used. If [PopupMenuThemeData.padding] is also null, then
/// EdgeInsets.symmetric(vertical: 8.0) is used.
///
/// See also: /// See also:
/// ///
/// * [PopupMenuItem], a popup menu entry for a single value. /// * [PopupMenuItem], a popup menu entry for a single value.
...@@ -871,7 +864,6 @@ Future<T?> showMenu<T>({ ...@@ -871,7 +864,6 @@ Future<T?> showMenu<T>({
required RelativeRect position, required RelativeRect position,
required List<PopupMenuEntry<T>> items, required List<PopupMenuEntry<T>> items,
T? initialValue, T? initialValue,
EdgeInsetsGeometry? padding,
double? elevation, double? elevation,
String? semanticLabel, String? semanticLabel,
ShapeBorder? shape, ShapeBorder? shape,
...@@ -900,7 +892,6 @@ Future<T?> showMenu<T>({ ...@@ -900,7 +892,6 @@ Future<T?> showMenu<T>({
position: position, position: position,
items: items, items: items,
initialValue: initialValue, initialValue: initialValue,
padding: padding,
elevation: elevation, elevation: elevation,
semanticLabel: semanticLabel, semanticLabel: semanticLabel,
barrierLabel: MaterialLocalizations.of(context).modalBarrierDismissLabel, barrierLabel: MaterialLocalizations.of(context).modalBarrierDismissLabel,
...@@ -993,7 +984,6 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -993,7 +984,6 @@ class PopupMenuButton<T> extends StatefulWidget {
this.tooltip, this.tooltip,
this.elevation, this.elevation,
this.padding = const EdgeInsets.all(8.0), this.padding = const EdgeInsets.all(8.0),
this.menuPadding,
this.child, this.child,
this.icon, this.icon,
this.iconSize, this.iconSize,
...@@ -1088,14 +1078,6 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -1088,14 +1078,6 @@ class PopupMenuButton<T> extends StatefulWidget {
/// Theme.of(context).cardColor is used. /// Theme.of(context).cardColor is used.
final Color? color; final Color? color;
/// If provided, menu padding is used for empty space around the outside
/// of the popup menu.
///
/// If this property is null, then [PopupMenuThemeData.padding] is used.
/// If [PopupMenuThemeData.padding] is also null, then
/// EdgeInsets.symmetric(vertical: 8.0) is used.
final EdgeInsetsGeometry? menuPadding;
/// Whether detected gestures should provide acoustic and/or haptic feedback. /// Whether detected gestures should provide acoustic and/or haptic feedback.
/// ///
/// For example, on Android a tap will produce a clicking sound and a /// For example, on Android a tap will produce a clicking sound and a
...@@ -1139,11 +1121,6 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> { ...@@ -1139,11 +1121,6 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
), ),
Offset.zero & overlay.size, Offset.zero & overlay.size,
); );
final EdgeInsetsGeometry menuPadding = widget.menuPadding ??
popupMenuTheme.padding ??
const EdgeInsets.symmetric(vertical: _kMenuVerticalPadding);
final List<PopupMenuEntry<T>> items = widget.itemBuilder(context); final List<PopupMenuEntry<T>> items = widget.itemBuilder(context);
// Only show the menu if there is something to show // Only show the menu if there is something to show
if (items.isNotEmpty) { if (items.isNotEmpty) {
...@@ -1152,7 +1129,6 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> { ...@@ -1152,7 +1129,6 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
elevation: widget.elevation ?? popupMenuTheme.elevation, elevation: widget.elevation ?? popupMenuTheme.elevation,
items: items, items: items,
initialValue: widget.initialValue, initialValue: widget.initialValue,
padding: menuPadding,
position: position, position: position,
shape: widget.shape ?? popupMenuTheme.shape, shape: widget.shape ?? popupMenuTheme.shape,
color: widget.color ?? popupMenuTheme.color, color: widget.color ?? popupMenuTheme.color,
......
...@@ -40,7 +40,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -40,7 +40,6 @@ class PopupMenuThemeData with Diagnosticable {
this.textStyle, this.textStyle,
this.enableFeedback, this.enableFeedback,
this.mouseCursor, this.mouseCursor,
this.padding,
}); });
/// The background color of the popup menu. /// The background color of the popup menu.
...@@ -65,11 +64,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -65,11 +64,6 @@ class PopupMenuThemeData with Diagnosticable {
/// If specified, overrides the default value of [PopupMenuItem.mouseCursor]. /// If specified, overrides the default value of [PopupMenuItem.mouseCursor].
final MaterialStateProperty<MouseCursor?>? mouseCursor; final MaterialStateProperty<MouseCursor?>? mouseCursor;
/// If specified, defines the padding for the popup menu of [PopupMenuButton].
///
/// If [PopupMenuButton.menuPadding] is provided, [padding] is ignored.
final EdgeInsetsGeometry? padding;
/// Creates a copy of this object with the given fields replaced with the /// Creates a copy of this object with the given fields replaced with the
/// new values. /// new values.
PopupMenuThemeData copyWith({ PopupMenuThemeData copyWith({
...@@ -79,7 +73,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -79,7 +73,6 @@ class PopupMenuThemeData with Diagnosticable {
TextStyle? textStyle, TextStyle? textStyle,
bool? enableFeedback, bool? enableFeedback,
MaterialStateProperty<MouseCursor?>? mouseCursor, MaterialStateProperty<MouseCursor?>? mouseCursor,
EdgeInsets? padding,
}) { }) {
return PopupMenuThemeData( return PopupMenuThemeData(
color: color ?? this.color, color: color ?? this.color,
...@@ -88,7 +81,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -88,7 +81,6 @@ class PopupMenuThemeData with Diagnosticable {
textStyle: textStyle ?? this.textStyle, textStyle: textStyle ?? this.textStyle,
enableFeedback: enableFeedback ?? this.enableFeedback, enableFeedback: enableFeedback ?? this.enableFeedback,
mouseCursor: mouseCursor ?? this.mouseCursor, mouseCursor: mouseCursor ?? this.mouseCursor,
padding: padding ?? this.padding,
); );
} }
...@@ -108,7 +100,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -108,7 +100,6 @@ class PopupMenuThemeData with Diagnosticable {
textStyle: TextStyle.lerp(a?.textStyle, b?.textStyle, t), textStyle: TextStyle.lerp(a?.textStyle, b?.textStyle, t),
enableFeedback: t < 0.5 ? a?.enableFeedback : b?.enableFeedback, enableFeedback: t < 0.5 ? a?.enableFeedback : b?.enableFeedback,
mouseCursor: t < 0.5 ? a?.mouseCursor : b?.mouseCursor, mouseCursor: t < 0.5 ? a?.mouseCursor : b?.mouseCursor,
padding: EdgeInsetsGeometry.lerp(a?.padding, b?.padding, t),
); );
} }
...@@ -120,8 +111,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -120,8 +111,7 @@ class PopupMenuThemeData with Diagnosticable {
elevation, elevation,
textStyle, textStyle,
enableFeedback, enableFeedback,
mouseCursor, mouseCursor
padding,
); );
} }
...@@ -137,8 +127,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -137,8 +127,7 @@ class PopupMenuThemeData with Diagnosticable {
&& other.shape == shape && other.shape == shape
&& other.textStyle == textStyle && other.textStyle == textStyle
&& other.enableFeedback == enableFeedback && other.enableFeedback == enableFeedback
&& other.mouseCursor == mouseCursor && other.mouseCursor == mouseCursor;
&& other.padding == padding;
} }
@override @override
...@@ -150,7 +139,6 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -150,7 +139,6 @@ class PopupMenuThemeData with Diagnosticable {
properties.add(DiagnosticsProperty<TextStyle>('text style', textStyle, defaultValue: null)); properties.add(DiagnosticsProperty<TextStyle>('text style', textStyle, defaultValue: null));
properties.add(DiagnosticsProperty<bool>('enableFeedback', enableFeedback, defaultValue: null)); properties.add(DiagnosticsProperty<bool>('enableFeedback', enableFeedback, defaultValue: null));
properties.add(DiagnosticsProperty<MaterialStateProperty<MouseCursor?>>('mouseCursor', mouseCursor, defaultValue: null)); properties.add(DiagnosticsProperty<MaterialStateProperty<MouseCursor?>>('mouseCursor', mouseCursor, defaultValue: null));
properties.add(DiagnosticsProperty<EdgeInsetsGeometry>('padding', padding, defaultValue: null));
} }
} }
......
...@@ -1471,72 +1471,6 @@ void main() { ...@@ -1471,72 +1471,6 @@ void main() {
expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 3')).padding, const EdgeInsets.all(20)); expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 3')).padding, const EdgeInsets.all(20));
}); });
testWidgets('PopupMenu custom padding', (WidgetTester tester) async {
final Key popupMenuButtonWithDefaultPaddingKey = UniqueKey();
final Key popupMenuButtonWithOverriddenPaddingKey = UniqueKey();
const EdgeInsets padding = EdgeInsets.zero;
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Column(
children: <PopupMenuButton<String>>[
PopupMenuButton<String>(
key: popupMenuButtonWithDefaultPaddingKey,
child: const Text('button'),
onSelected: (String result) {},
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
];
},
),
PopupMenuButton<String>(
menuPadding: padding,
key: popupMenuButtonWithOverriddenPaddingKey,
child: const Text('button'),
onSelected: (String result) {},
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
];
},
)
],
),
),
),
);
final Finder popupFinder = find.byType(SingleChildScrollView);
// Show the menu
await tester.tap(find.byKey(popupMenuButtonWithDefaultPaddingKey));
await tester.pumpAndSettle();
expect(tester.widget<SingleChildScrollView>(popupFinder).padding,
const EdgeInsets.symmetric(vertical: 8),
reason: 'The popup without explicit paddings should utilise the default ones.',);
// Close previous menu
await tester.tap(find.byKey(popupMenuButtonWithDefaultPaddingKey), warnIfMissed: false);
await tester.pumpAndSettle();
// Show the menu
await tester.tap(find.byKey(popupMenuButtonWithOverriddenPaddingKey));
await tester.pumpAndSettle();
expect(tester.widget<SingleChildScrollView>(popupFinder).padding,
padding,
reason: 'The popup should utilise explicitly set paddings.',);
});
testWidgets('CheckedPopupMenuItem child height is a minimum, child is vertically centered', (WidgetTester tester) async { testWidgets('CheckedPopupMenuItem child height is a minimum, child is vertically centered', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey(); final Key popupMenuButtonKey = UniqueKey();
final Type menuItemType = const CheckedPopupMenuItem<String>(child: Text('item')).runtimeType; final Type menuItemType = const CheckedPopupMenuItem<String>(child: Text('item')).runtimeType;
......
...@@ -13,7 +13,6 @@ PopupMenuThemeData _popupMenuTheme() { ...@@ -13,7 +13,6 @@ PopupMenuThemeData _popupMenuTheme() {
shape: BeveledRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(12))), shape: BeveledRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(12))),
elevation: 12.0, elevation: 12.0,
textStyle: TextStyle(color: Color(0xffffffff), textBaseline: TextBaseline.alphabetic), textStyle: TextStyle(color: Color(0xffffffff), textBaseline: TextBaseline.alphabetic),
padding: EdgeInsets.symmetric(vertical: 6, horizontal: 6),
); );
} }
...@@ -196,7 +195,6 @@ void main() { ...@@ -196,7 +195,6 @@ void main() {
); );
const double elevation = 7.0; const double elevation = 7.0;
const TextStyle textStyle = TextStyle(color: Color(0x00000000), textBaseline: TextBaseline.alphabetic); const TextStyle textStyle = TextStyle(color: Color(0x00000000), textBaseline: TextBaseline.alphabetic);
const EdgeInsets menuPadding = EdgeInsets.zero;
await tester.pumpWidget(MaterialApp( await tester.pumpWidget(MaterialApp(
theme: ThemeData(popupMenuTheme: popupMenuTheme), theme: ThemeData(popupMenuTheme: popupMenuTheme),
...@@ -209,7 +207,6 @@ void main() { ...@@ -209,7 +207,6 @@ void main() {
elevation: elevation, elevation: elevation,
color: color, color: color,
shape: shape, shape: shape,
menuPadding: menuPadding,
itemBuilder: (BuildContext context) { itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[ return <PopupMenuEntry<void>>[
PopupMenuItem<void>( PopupMenuItem<void>(
...@@ -253,12 +250,6 @@ void main() { ...@@ -253,12 +250,6 @@ void main() {
).last, ).last,
); );
expect(text.style, textStyle); expect(text.style, textStyle);
/// PopupMenu widget is private so in order to test padding the widget
/// with the popup padding is extracted.
final SingleChildScrollView popupMenu = tester.widget<SingleChildScrollView>
(find.byType(SingleChildScrollView));
expect(popupMenu.padding, menuPadding);
}); });
testWidgets('ThemeData.popupMenuTheme properties are utilized', (WidgetTester tester) async { testWidgets('ThemeData.popupMenuTheme properties are utilized', (WidgetTester tester) async {
...@@ -267,8 +258,6 @@ void main() { ...@@ -267,8 +258,6 @@ void main() {
final Key enabledPopupItemKey = UniqueKey(); final Key enabledPopupItemKey = UniqueKey();
final Key disabledPopupItemKey = UniqueKey(); final Key disabledPopupItemKey = UniqueKey();
const EdgeInsets themePadding = EdgeInsets.zero;
await tester.pumpWidget(MaterialApp( await tester.pumpWidget(MaterialApp(
key: popupButtonApp, key: popupButtonApp,
home: Material( home: Material(
...@@ -286,7 +275,6 @@ void main() { ...@@ -286,7 +275,6 @@ void main() {
} }
return SystemMouseCursors.alias; return SystemMouseCursors.alias;
}), }),
padding: themePadding,
), ),
child: PopupMenuButton<void>( child: PopupMenuButton<void>(
key: popupButtonKey, key: popupButtonKey,
...@@ -345,11 +333,5 @@ void main() { ...@@ -345,11 +333,5 @@ void main() {
await gesture.down(tester.getCenter(find.byKey(enabledPopupItemKey))); await gesture.down(tester.getCenter(find.byKey(enabledPopupItemKey)));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(RendererBinding.instance!.mouseTracker.debugDeviceActiveCursor(1), SystemMouseCursors.alias); expect(RendererBinding.instance!.mouseTracker.debugDeviceActiveCursor(1), SystemMouseCursors.alias);
/// PopupMenu widget is private so in order to test padding we extract
/// the widget which holds the padding.
final SingleChildScrollView popupMenu = tester.widget<SingleChildScrollView>
(find.byType(SingleChildScrollView));
expect(popupMenu.padding, themePadding);
}); });
} }
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