Unverified Commit ea005219 authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `PopupMenuItem` with a `ListTile` doesn't use the correct style. (#133141)

fixes [`PopupMenuItem` with a `ListTile`  doesn't use the correct text style.](https://github.com/flutter/flutter/issues/133138)

### Description

This fixes an issue text style issue for `PopupMenuItem` with a `ListTile` (for an elaborate popup menu) 

https://api.flutter.dev/flutter/material/PopupMenuItem-class.html

<details> 
<summary>expand to view the code sample</summary> 

```dart
import 'package:flutter/material.dart';

/// Flutter code sample for [PopupMenuButton].

// This is the type used by the popup menu below.
enum SampleItem { itemOne, itemTwo, itemThree }

void main() => runApp(const PopupMenuApp());

class PopupMenuApp extends StatelessWidget {
  const PopupMenuApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      theme: ThemeData(
        useMaterial3: true,
        textTheme: const TextTheme(
          labelLarge: TextStyle(
            fontStyle: FontStyle.italic,
            fontWeight: FontWeight.bold,
          ),
        ),
      ),
      home: const PopupMenuExample(),
    );
  }
}

class PopupMenuExample extends StatefulWidget {
  const PopupMenuExample({super.key});

  @override
  State<PopupMenuExample> createState() => _PopupMenuExampleState();
}

class _PopupMenuExampleState extends State<PopupMenuExample> {
  SampleItem? selectedMenu;

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      body: Center(
        child: SizedBox(
          width: 300,
          height: 130,
          child: Align(
            alignment: Alignment.topLeft,
            child: PopupMenuButton<SampleItem>(
              initialValue: selectedMenu,
              // Callback that sets the selected popup menu item.
              onSelected: (SampleItem item) {
                setState(() {
                  selectedMenu = item;
                });
              },
              itemBuilder: (BuildContext context) =>
                  <PopupMenuEntry<SampleItem>>[
                const PopupMenuItem<SampleItem>(
                  value: SampleItem.itemOne,
                  child: Text('PopupMenuItem'),
                ),
                const CheckedPopupMenuItem<SampleItem>(
                  checked: true,
                  value: SampleItem.itemTwo,
                  child: Text('CheckedPopupMenuItem'),
                ),
                const PopupMenuItem<SampleItem>(
                  value: SampleItem.itemOne,
                  child: ListTile(
                    leading: Icon(Icons.cloud),
                    title: Text('ListTile'),
                    contentPadding: EdgeInsets.zero,
                    trailing: Icon(Icons.arrow_right_rounded),
                  ),
                ),
              ],
            ),
          ),
        ),
      ),
    );
  }
}
``` 

</details>

### Before

![Screenshot 2023-08-23 at 14 08 48](https://github.com/flutter/flutter/assets/48603081/434ac95e-2981-4ab5-9843-939b39d771a2)

### After

![Screenshot 2023-08-23 at 14 08 32](https://github.com/flutter/flutter/assets/48603081/f6aba7e0-3d03-454f-8e0b-c031492f3f2d)
parent 612117a6
...@@ -399,6 +399,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> { ...@@ -399,6 +399,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor), mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor),
child: ListTileTheme.merge( child: ListTileTheme.merge(
contentPadding: EdgeInsets.zero, contentPadding: EdgeInsets.zero,
titleTextStyle: style,
child: item, child: item,
), ),
), ),
......
...@@ -3423,14 +3423,14 @@ void main() { ...@@ -3423,14 +3423,14 @@ void main() {
await tester.tapAt(const Offset(20.0, 20.0)); await tester.tapAt(const Offset(20.0, 20.0));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
// Test custom text theme text style. // Test custom text theme.
theme = theme.copyWith( const TextStyle customTextStyle = TextStyle(
textTheme: theme.textTheme.copyWith(
labelLarge: const TextStyle(
fontSize: 20.0, fontSize: 20.0,
fontWeight: FontWeight.bold, fontWeight: FontWeight.bold,
) fontStyle: FontStyle.italic,
), );
theme = theme.copyWith(
textTheme: const TextTheme(labelLarge: customTextStyle),
); );
await tester.pumpWidget(buildMenu()); await tester.pumpWidget(buildMenu());
...@@ -3438,8 +3438,10 @@ void main() { ...@@ -3438,8 +3438,10 @@ void main() {
await tester.tap(find.byKey(popupMenuButtonKey)); await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle(); await tester.pumpAndSettle();
expect(_labelStyle(tester, 'Item 1')!.fontSize, 20.0); // Test custom text theme.
expect(_labelStyle(tester, 'Item 1')!.fontWeight, FontWeight.bold); expect(_labelStyle(tester, 'Item 1')!.fontSize, customTextStyle.fontSize);
expect(_labelStyle(tester, 'Item 1')!.fontWeight, customTextStyle.fontWeight);
expect(_labelStyle(tester, 'Item 1')!.fontStyle, customTextStyle.fontStyle);
}); });
testWidgets('CheckedPopupMenuItem.labelTextStyle resolve material states', (WidgetTester tester) async { testWidgets('CheckedPopupMenuItem.labelTextStyle resolve material states', (WidgetTester tester) async {
...@@ -3492,7 +3494,7 @@ void main() { ...@@ -3492,7 +3494,7 @@ void main() {
); );
}); });
testWidgets('CheckedPopupMenuItem overrides redundant ListTile content padding', (WidgetTester tester) async { testWidgets('CheckedPopupMenuItem overrides redundant ListTile.contentPadding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey(); final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
...@@ -3537,7 +3539,7 @@ void main() { ...@@ -3537,7 +3539,7 @@ void main() {
expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero); expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero);
}); });
testWidgets('PopupMenuItem overrides redundant ListTile content padding', (WidgetTester tester) async { testWidgets('PopupMenuItem overrides redundant ListTile.contentPadding', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey(); final Key popupMenuButtonKey = UniqueKey();
await tester.pumpWidget( await tester.pumpWidget(
MaterialApp( MaterialApp(
...@@ -3581,6 +3583,160 @@ void main() { ...@@ -3581,6 +3583,160 @@ void main() {
expect(getItemSafeArea('Item 0').minimum, EdgeInsets.zero); expect(getItemSafeArea('Item 0').minimum, EdgeInsets.zero);
expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero); expect(getItemSafeArea('Item 1').minimum, EdgeInsets.zero);
}); });
testWidgets('Material3 - PopupMenuItem overrides ListTile.titleTextStyle', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
ThemeData theme = ThemeData(useMaterial3: true);
Widget buildMenu() {
return MaterialApp(
theme: theme,
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
// Popup menu item with a Text widget.
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
// Popup menu item with a ListTile widget.
const PopupMenuItem<String>(
value: '1',
child: ListTile(title: Text('Item 1')),
),
];
},
),
),
),
);
}
await tester.pumpWidget(buildMenu());
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// Test popup menu item with a Text widget.
expect(_labelStyle(tester, 'Item 0')!.fontSize, 14.0);
expect(_labelStyle(tester, 'Item 0')!.color, theme.colorScheme.onSurface);
// Test popup menu item with a ListTile widget.
expect(_labelStyle(tester, 'Item 1')!.fontSize, 14.0);
expect(_labelStyle(tester, 'Item 1')!.color, theme.colorScheme.onSurface);
// Close the menu.
await tester.tapAt(const Offset(20.0, 20.0));
await tester.pumpAndSettle();
// Test custom text theme.
const TextStyle customTextStyle = TextStyle(
fontSize: 20.0,
fontWeight: FontWeight.bold,
fontStyle: FontStyle.italic,
);
theme = theme.copyWith(
textTheme: const TextTheme(labelLarge: customTextStyle),
);
await tester.pumpWidget(buildMenu());
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// Test popup menu item with a Text widget with custom text theme.
expect(_labelStyle(tester, 'Item 0')!.fontSize, customTextStyle.fontSize);
expect(_labelStyle(tester, 'Item 0')!.fontWeight, customTextStyle.fontWeight);
expect(_labelStyle(tester, 'Item 0')!.fontStyle, customTextStyle.fontStyle);
// Test popup menu item with a ListTile widget with custom text theme.
expect(_labelStyle(tester, 'Item 1')!.fontSize, customTextStyle.fontSize);
expect(_labelStyle(tester, 'Item 1')!.fontWeight, customTextStyle.fontWeight);
expect(_labelStyle(tester, 'Item 1')!.fontStyle, customTextStyle.fontStyle);
});
testWidgets('Material2 - PopupMenuItem overrides ListTile.titleTextStyle', (WidgetTester tester) async {
final Key popupMenuButtonKey = UniqueKey();
ThemeData theme = ThemeData(useMaterial3: false);
Widget buildMenu() {
return MaterialApp(
theme: theme,
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
key: popupMenuButtonKey,
child: const Text('button'),
onSelected: (String result) { },
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<String>>[
// Popup menu item with a Text widget.
const PopupMenuItem<String>(
value: '0',
child: Text('Item 0'),
),
// Popup menu item with a ListTile widget.
const PopupMenuItem<String>(
value: '1',
child: ListTile(title: Text('Item 1')),
),
];
},
),
),
),
);
}
await tester.pumpWidget(buildMenu());
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// Test popup menu item with a Text widget.
expect(_labelStyle(tester, 'Item 0')!.fontSize, 16.0);
expect(_labelStyle(tester, 'Item 0')!.color, theme.textTheme.subtitle1!.color);
// Test popup menu item with a ListTile widget.
expect(_labelStyle(tester, 'Item 1')!.fontSize, 16.0);
expect(_labelStyle(tester, 'Item 1')!.color, theme.textTheme.subtitle1!.color);
// Close the menu.
await tester.tapAt(const Offset(20.0, 20.0));
await tester.pumpAndSettle();
// Test custom text theme.
const TextStyle customTextStyle = TextStyle(
fontSize: 20.0,
fontWeight: FontWeight.bold,
fontStyle: FontStyle.italic,
);
theme = theme.copyWith(
textTheme: const TextTheme(subtitle1: customTextStyle),
);
await tester.pumpWidget(buildMenu());
// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();
// Test popup menu item with a Text widget with custom text style.
expect(_labelStyle(tester, 'Item 0')!.fontSize, customTextStyle.fontSize);
expect(_labelStyle(tester, 'Item 0')!.fontWeight, customTextStyle.fontWeight);
expect(_labelStyle(tester, 'Item 0')!.fontStyle, customTextStyle.fontStyle);
// Test popup menu item with a ListTile widget with custom text style.
expect(_labelStyle(tester, 'Item 1')!.fontSize, customTextStyle.fontSize);
expect(_labelStyle(tester, 'Item 1')!.fontWeight, customTextStyle.fontWeight);
expect(_labelStyle(tester, 'Item 1')!.fontStyle, customTextStyle.fontStyle);
});
} }
class TestApp extends StatelessWidget { 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