Unverified Commit 49c58718 authored by Abdur Rafay Saleem's avatar Abdur Rafay Saleem Committed by GitHub

feat: added custom padding in PopupMenuButton (#96657)

parent 017ed179
...@@ -528,10 +528,12 @@ class _PopupMenu<T> extends StatelessWidget { ...@@ -528,10 +528,12 @@ 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) {
...@@ -583,9 +585,7 @@ class _PopupMenu<T> extends StatelessWidget { ...@@ -583,9 +585,7 @@ class _PopupMenu<T> extends StatelessWidget {
explicitChildNodes: true, explicitChildNodes: true,
label: semanticLabel, label: semanticLabel,
child: SingleChildScrollView( child: SingleChildScrollView(
padding: const EdgeInsets.symmetric( padding: padding,
vertical: _kMenuVerticalPadding,
),
child: ListBody(children: children), child: ListBody(children: children),
), ),
), ),
...@@ -727,6 +727,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -727,6 +727,7 @@ 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,
...@@ -740,6 +741,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -740,6 +741,7 @@ 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;
...@@ -778,7 +780,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -778,7 +780,7 @@ class _PopupMenuRoute<T> extends PopupRoute<T> {
} }
} }
final Widget menu = _PopupMenu<T>(route: this, semanticLabel: semanticLabel); final Widget menu = _PopupMenu<T>(padding: padding, 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,
...@@ -850,6 +852,11 @@ class _PopupMenuRoute<T> extends PopupRoute<T> { ...@@ -850,6 +852,11 @@ 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.
...@@ -864,6 +871,7 @@ Future<T?> showMenu<T>({ ...@@ -864,6 +871,7 @@ 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,
...@@ -892,6 +900,7 @@ Future<T?> showMenu<T>({ ...@@ -892,6 +900,7 @@ 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,
...@@ -984,6 +993,7 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -984,6 +993,7 @@ 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,
...@@ -1078,6 +1088,14 @@ class PopupMenuButton<T> extends StatefulWidget { ...@@ -1078,6 +1088,14 @@ 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
...@@ -1121,6 +1139,11 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> { ...@@ -1121,6 +1139,11 @@ 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) {
...@@ -1129,6 +1152,7 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> { ...@@ -1129,6 +1152,7 @@ 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,6 +40,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -40,6 +40,7 @@ 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.
...@@ -64,6 +65,11 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -64,6 +65,11 @@ 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({
...@@ -73,6 +79,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -73,6 +79,7 @@ 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,
...@@ -81,6 +88,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -81,6 +88,7 @@ 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,
); );
} }
...@@ -100,6 +108,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -100,6 +108,7 @@ 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),
); );
} }
...@@ -111,7 +120,8 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -111,7 +120,8 @@ class PopupMenuThemeData with Diagnosticable {
elevation, elevation,
textStyle, textStyle,
enableFeedback, enableFeedback,
mouseCursor mouseCursor,
padding,
); );
} }
...@@ -127,7 +137,8 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -127,7 +137,8 @@ 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
...@@ -139,6 +150,7 @@ class PopupMenuThemeData with Diagnosticable { ...@@ -139,6 +150,7 @@ 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,6 +1471,72 @@ void main() { ...@@ -1471,6 +1471,72 @@ 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,6 +13,7 @@ PopupMenuThemeData _popupMenuTheme() { ...@@ -13,6 +13,7 @@ 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),
); );
} }
...@@ -195,6 +196,7 @@ void main() { ...@@ -195,6 +196,7 @@ 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),
...@@ -207,6 +209,7 @@ void main() { ...@@ -207,6 +209,7 @@ 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>(
...@@ -250,6 +253,12 @@ void main() { ...@@ -250,6 +253,12 @@ 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 {
...@@ -258,6 +267,8 @@ void main() { ...@@ -258,6 +267,8 @@ 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(
...@@ -275,6 +286,7 @@ void main() { ...@@ -275,6 +286,7 @@ void main() {
} }
return SystemMouseCursors.alias; return SystemMouseCursors.alias;
}), }),
padding: themePadding,
), ),
child: PopupMenuButton<void>( child: PopupMenuButton<void>(
key: popupButtonKey, key: popupButtonKey,
...@@ -333,5 +345,11 @@ void main() { ...@@ -333,5 +345,11 @@ 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