firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Fix PopupMenuItem & CheckedPopupMenuItem has redundant ListTile padding and update default horizontal padding for Material 3" (#132457)

Browse Source 
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.
This commit is contained in:
Casey Hillers 2023-08-13 14:27:01 -07:00 committed by GitHub
parent 6b3eefc286
commit 7ce2d83b84
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 11 additions and 197 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
dev/tools/gen_defaults/lib/popup_menu_template.dart
View File

@ -42,9 +42,5 @@ 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);
}''';
}

38
packages/flutter/lib/src/material/popup_menu.dart
View File

@ -14,7 +14,6 @@ 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';
@ -33,6 +32,7 @@ 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,11 +255,7 @@ 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.
///
/// 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 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
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 @@ 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,
),
child: item,
),
),
);
@ -547,17 +540,14 @@ 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),
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 @@ class _PopupMenuDefaultsM2 extends PopupMenuThemeData {
@override
TextStyle? get textStyle => _textTheme.subtitle1;
static EdgeInsets menuHorizontalPadding = const EdgeInsets.symmetric(horizontal: 16.0);
}
// BEGIN GENERATED TOKEN PROPERTIES - PopupMenu
@ -1477,9 +1465,5 @@ 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

166
packages/flutter/test/material/popup_menu_test.dart
View File

@ -1553,82 +1553,6 @@ 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;
@ -3491,96 +3415,6 @@ 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 {