Revert "Fix `PopupMenuItem` & `CheckedPopupMenuItem` has redundant `ListTile` padding and update default horizontal padding for Material 3" (#132457)
Reverts flutter/flutter#131609
b/295497265 - This broke Google Testing. We'll need the internal patch
before landing as there's a large number of customer codebases impacted.
diff --git a/dev/tools/gen_defaults/lib/popup_menu_template.dart b/dev/tools/gen_defaults/lib/popup_menu_template.dart
index bed11d0..f11b89c 100644
--- a/dev/tools/gen_defaults/lib/popup_menu_template.dart
+++ b/dev/tools/gen_defaults/lib/popup_menu_template.dart
@@ -42,9 +42,5 @@
@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);
}''';
}
diff --git a/packages/flutter/lib/src/material/popup_menu.dart b/packages/flutter/lib/src/material/popup_menu.dart
index 7a3f7a5..ef8f436 100644
--- a/packages/flutter/lib/src/material/popup_menu.dart
+++ b/packages/flutter/lib/src/material/popup_menu.dart
@@ -14,7 +14,6 @@
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';
@@ -33,6 +32,7 @@
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,11 +255,7 @@
/// 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.
///
- /// 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.
+ /// When null, the horizontal padding defaults to 16.0 on both sides.
final EdgeInsets? padding;
/// The text style of the popup menu item.
@@ -376,7 +372,7 @@
child: Container(
alignment: AlignmentDirectional.centerStart,
constraints: BoxConstraints(minHeight: widget.height),
- padding: widget.padding ?? (theme.useMaterial3 ? _PopupMenuDefaultsM3.menuHorizontalPadding : _PopupMenuDefaultsM2.menuHorizontalPadding),
+ padding: widget.padding ?? const EdgeInsets.symmetric(horizontal: _kMenuHorizontalPadding),
child: buildChild(),
),
);
@@ -397,10 +393,7 @@
onTap: widget.enabled ? handleTap : null,
canRequestFocus: widget.enabled,
mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor),
- child: ListTileTheme.merge(
- contentPadding: EdgeInsets.zero,
- child: item,
- ),
+ child: item,
),
),
);
@@ -547,17 +540,14 @@
?? popupMenuTheme.labelTextStyle
?? defaults.labelTextStyle;
return IgnorePointer(
- child: ListTileTheme.merge(
- contentPadding: EdgeInsets.zero,
- child: ListTile(
- enabled: widget.enabled,
- titleTextStyle: effectiveLabelTextStyle?.resolve(states),
- leading: FadeTransition(
- opacity: _opacity,
- child: Icon(_controller.isDismissed ? null : Icons.done),
- ),
- title: widget.child,
+ child: ListTile(
+ enabled: widget.enabled,
+ titleTextStyle: effectiveLabelTextStyle?.resolve(states),
+ leading: FadeTransition(
+ opacity: _opacity,
+ child: Icon(_controller.isDismissed ? null : Icons.done),
),
+ title: widget.child,
),
);
}
@@ -1436,8 +1426,6 @@
@override
TextStyle? get textStyle => _textTheme.subtitle1;
-
- static EdgeInsets menuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 16.0);
}
// BEGIN GENERATED TOKEN PROPERTIES - PopupMenu
@@ -1477,9 +1465,5 @@
@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
diff --git a/packages/flutter/test/material/popup_menu_test.dart b/packages/flutter/test/material/popup_menu_test.dart
index 60c5767..f26ed5a 100644
--- a/packages/flutter/test/material/popup_menu_test.dart
+++ b/packages/flutter/test/material/popup_menu_test.dart
@@ -1553,82 +1553,6 @@
);
});
- 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;
@@ -3491,96 +3415,6 @@
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 {
