Skip to content

F
Front-End
Unverified Commit 23315f1b authored by Taha Tesser's avatar Taha Tesser Committed by GitHub
Browse files
Options

[Reland] #131609 (#132555) 

This relands https://github.com/flutter/flutter/pull/131609

---

fixes [`PopupMenuItem` adds redundant padding when using `ListItem`](https://github.com/flutter/flutter/issues/128553)
parent 6971dcb7
Show whitespace changes
Inline Side-by-side
Showing with 197 additions and 11 deletions
dev/tools/gen_defaults/lib/popup_menu_template.dart
View file @ 23315f1b
......@@ -42,5 +42,9 @@ class _${blockName}DefaultsM3 extends PopupMenuThemeData {
@override
ShapeBorder? get shape => ${shape("md.comp.menu.container")};
// TODO(tahatesser): This is taken from https://m3.material.io/components/menus/specs
// Update this when the token is available.
static EdgeInsets menuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 12.0);
}''';
}
packages/flutter/lib/src/material/popup_menu.dart
View file @ 23315f1b
......@@ -14,6 +14,7 @@ import 'icon_button.dart';
import 'icons.dart';
import 'ink_well.dart';
import 'list_tile.dart';
import 'list_tile_theme.dart';
import 'material.dart';
import 'material_localizations.dart';
import 'material_state.dart';
......@@ -32,7 +33,6 @@ import 'tooltip.dart';
const Duration _kMenuDuration = Duration(milliseconds: 300);
const double _kMenuCloseIntervalEnd = 2.0 / 3.0;
const double _kMenuHorizontalPadding = 16.0;
const double _kMenuDividerHeight = 16.0;
const double _kMenuMaxWidth = 5.0 * _kMenuWidthStep;
const double _kMenuMinWidth = 2.0 * _kMenuWidthStep;
......@@ -255,7 +255,11 @@ class PopupMenuItem<T> extends PopupMenuEntry<T> {
/// If a [height] greater than the height of the sum of the padding and [child]
/// is provided, then the padding's effect will not be visible.
///
/// When null, the horizontal padding defaults to 16.0 on both sides.
/// If this is null and [ThemeData.useMaterial3] is true, the horizontal padding
/// defaults to 12.0 on both sides.
///
/// If this is null and [ThemeData.useMaterial3] is false, the horizontal padding
/// defaults to 16.0 on both sides.
final EdgeInsets? padding;
/// The text style of the popup menu item.
......@@ -372,7 +376,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
child: Container(
alignment: AlignmentDirectional.centerStart,
constraints: BoxConstraints(minHeight: widget.height),
padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _kMenuHorizontalPadding),
padding: widget.padding ?? (theme.useMaterial3 ? _PopupMenuDefaultsM3.menuHorizontalPadding : _PopupMenuDefaultsM2.menuHorizontalPadding),
child: buildChild(),
),
);
......@@ -393,9 +397,12 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
onTap: widget.enabled ? handleTap : null,
canRequestFocus: widget.enabled,
mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor),
child: ListTileTheme.merge(
contentPadding: EdgeInsets.zero,
child: item,
),
),
),
);
}
}
......@@ -540,6 +547,8 @@ class _CheckedPopupMenuItemState<T> extends PopupMenuItemState<T, CheckedPopupMe
?? popupMenuTheme.labelTextStyle
?? defaults.labelTextStyle;
return IgnorePointer(
child: ListTileTheme.merge(
contentPadding: EdgeInsets.zero,
child: ListTile(
enabled: widget.enabled,
titleTextStyle: effectiveLabelTextStyle?.resolve(states),
......@@ -549,6 +558,7 @@ class _CheckedPopupMenuItemState<T> extends PopupMenuItemState<T, CheckedPopupMe
),
title: widget.child,
),
),
);
}
}
......@@ -1426,6 +1436,8 @@ class _PopupMenuDefaultsM2 extends PopupMenuThemeData {
@override
TextStyle? get textStyle => _textTheme.subtitle1;
static EdgeInsets menuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 16.0);
}
// BEGIN GENERATED TOKEN PROPERTIES - PopupMenu
......@@ -1465,5 +1477,9 @@ class _PopupMenuDefaultsM3 extends PopupMenuThemeData {
@override
ShapeBorder? get shape => const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(4.0)));
// TODO(tahatesser): This is taken from https://m3.material.io/components/menus/specs
// Update this when the token is available.
static EdgeInsets menuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 12.0);
}
// END GENERATED TOKEN PROPERTIES - PopupMenu
packages/flutter/test/material/popup_menu_test.dart
View file @ 23315f1b
......@@ -1553,6 +1553,82 @@ void main() {
);
});
testWidgets('Material3 - PopupMenuItem default padding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(useMaterial3: true),
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
enabled: false,
child: Text('Item 0'),
),
const PopupMenuItem<String>(
value: '1',
child: Text('Item 1'),
),
];
},
),
),
),
),
);
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 0')).padding, const EdgeInsets.symmetric(horizontal: 12.0));
expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 1')).padding, const EdgeInsets.symmetric(horizontal: 12.0));
});
testWidgets('Material2 - PopupMenuItem default padding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(useMaterial3: false),
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
enabled: false,
child: Text('Item 0'),
),
const PopupMenuItem<String>(
value: '1',
child: Text('Item 1'),
),
];
},
),
),
),
),
);
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 0')).padding, const EdgeInsets.symmetric(horizontal: 16.0));
expect(tester.widget<Container>(find.widgetWithText(Container, 'Item 1')).padding, const EdgeInsets.symmetric(horizontal: 16.0));
});
testWidgets('PopupMenuItem custom padding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
final Type menuItemType = const PopupMenuItem<String>(child: Text('item')).runtimeType;
......@@ -3415,6 +3491,96 @@ void main() {
labelTextStyle.resolve(<MaterialState>{MaterialState.selected})
);
});
testWidgets('CheckedPopupMenuItem overrides redundant ListTile content padding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(useMaterial3: false),
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const CheckedPopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
const CheckedPopupMenuItem<String>(
value: '1',
checked: true,
child: Text('Item 1'),
),
];
},
),
),
),
),
);
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
SafeArea getItemSafeArea(String label) {
return tester.widget<SafeArea>(find.ancestor(
of: find.text(label),
matching: find.byType(SafeArea),
));
}
expect(getItemSafeArea('Item 0').minimum, EdgeInsets.zero);
expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero);
});
testWidgets('PopupMenuItem overrides redundant ListTile content padding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(useMaterial3: false),
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: '0',
enabled: false,
child: ListTile(title: Text('Item 0')),
),
const PopupMenuItem<String>(
value: '1',
child: ListTile(title: Text('Item 1')),
),
];
},
),
),
),
),
);
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
SafeArea getItemSafeArea(String label) {
return tester.widget<SafeArea>(find.ancestor(
of: find.text(label),
matching: find.byType(SafeArea),
));
}
expect(getItemSafeArea('Item 0').minimum, EdgeInsets.zero);
expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero);
});
}
class TestApp extends StatelessWidget {
......
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

HIAST - GitLab