Skip to content

Commit

Permalink
Fix PopupMenuItem with a ListTile doesn't use the correct style. …
Browse files Browse the repository at this point in the history
…(#133141)

fixes [`PopupMenuItem` with a `ListTile`  doesn't use the correct text style.](flutter/flutter#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)
  • Loading branch information
TahaTesser authored Aug 25, 2023
1 parent 612117a commit ea00521
Show file tree
Hide file tree
Showing 2 changed files with 168 additions and 11 deletions.
1 change: 1 addition & 0 deletions packages/flutter/lib/src/material/popup_menu.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ class PopupMenuItemState<T, W extends PopupMenuItem<T>> extends State<W> {
mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor),
child: ListTileTheme.merge(
contentPadding: EdgeInsets.zero,
titleTextStyle: style,
child: item,
),
),
Expand Down
178 changes: 167 additions & 11 deletions packages/flutter/test/material/popup_menu_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -3423,23 +3423,25 @@ void main() {
await tester.tapAt(const Offset(20.0, 20.0));
await tester.pumpAndSettle();

// Test custom text theme text style.
// Test custom text theme.
const TextStyle customTextStyle = TextStyle(
fontSize: 20.0,
fontWeight: FontWeight.bold,
fontStyle: FontStyle.italic,
);
theme = theme.copyWith(
textTheme: theme.textTheme.copyWith(
labelLarge: const TextStyle(
fontSize: 20.0,
fontWeight: FontWeight.bold,
)
),
textTheme: const TextTheme(labelLarge: customTextStyle),
);
await tester.pumpWidget(buildMenu());

// Show the menu.
await tester.tap(find.byKey(popupMenuButtonKey));
await tester.pumpAndSettle();

expect(_labelStyle(tester, 'Item 1')!.fontSize, 20.0);
expect(_labelStyle(tester, 'Item 1')!.fontWeight, FontWeight.bold);
// Test 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('CheckedPopupMenuItem.labelTextStyle resolve material states', (WidgetTester tester) async {
Expand Down Expand Up @@ -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();
await tester.pumpWidget(
MaterialApp(
Expand Down Expand Up @@ -3537,7 +3539,7 @@ void main() {
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();
await tester.pumpWidget(
MaterialApp(
Expand Down Expand Up @@ -3581,6 +3583,160 @@ void main() {
expect(getItemSafeArea('Item 0').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 {
Expand Down

0 comments on commit ea00521

Please sign in to comment.