From ea005219394211bea4c2b87011699c63cfabb115 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Fri, 25 Aug 2023 17:49:04 +0300 Subject: [PATCH] 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
expand to view the code sample ```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 createState() => _PopupMenuExampleState(); } class _PopupMenuExampleState extends State { SampleItem? selectedMenu; @override Widget build(BuildContext context) { return Scaffold( body: Center( child: SizedBox( width: 300, height: 130, child: Align( alignment: Alignment.topLeft, child: PopupMenuButton( initialValue: selectedMenu, // Callback that sets the selected popup menu item. onSelected: (SampleItem item) { setState(() { selectedMenu = item; }); }, itemBuilder: (BuildContext context) => >[ const PopupMenuItem( value: SampleItem.itemOne, child: Text('PopupMenuItem'), ), const CheckedPopupMenuItem( checked: true, value: SampleItem.itemTwo, child: Text('CheckedPopupMenuItem'), ), const PopupMenuItem( value: SampleItem.itemOne, child: ListTile( leading: Icon(Icons.cloud), title: Text('ListTile'), contentPadding: EdgeInsets.zero, trailing: Icon(Icons.arrow_right_rounded), ), ), ], ), ), ), ), ); } } ```
### 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) --- .../flutter/lib/src/material/popup_menu.dart | 1 + .../test/material/popup_menu_test.dart | 178 ++++++++++++++++-- 2 files changed, 168 insertions(+), 11 deletions(-) diff --git a/packages/flutter/lib/src/material/popup_menu.dart b/packages/flutter/lib/src/material/popup_menu.dart index 7a3f7a5249..cc87623e63 100644 --- a/packages/flutter/lib/src/material/popup_menu.dart +++ b/packages/flutter/lib/src/material/popup_menu.dart @@ -399,6 +399,7 @@ class PopupMenuItemState> extends State { mouseCursor: _EffectiveMouseCursor(widget.mouseCursor, popupMenuTheme.mouseCursor), child: ListTileTheme.merge( contentPadding: EdgeInsets.zero, + titleTextStyle: style, child: item, ), ), diff --git a/packages/flutter/test/material/popup_menu_test.dart b/packages/flutter/test/material/popup_menu_test.dart index 60c576797d..1bfb2e028f 100644 --- a/packages/flutter/test/material/popup_menu_test.dart +++ b/packages/flutter/test/material/popup_menu_test.dart @@ -3423,14 +3423,14 @@ 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()); @@ -3438,8 +3438,10 @@ void main() { 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 { @@ -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( @@ -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( @@ -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( + key: popupMenuButtonKey, + child: const Text('button'), + onSelected: (String result) { }, + itemBuilder: (BuildContext context) { + return >[ + // Popup menu item with a Text widget. + const PopupMenuItem( + value: '0', + child: Text('Item 0'), + ), + // Popup menu item with a ListTile widget. + const PopupMenuItem( + 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( + key: popupMenuButtonKey, + child: const Text('button'), + onSelected: (String result) { }, + itemBuilder: (BuildContext context) { + return >[ + // Popup menu item with a Text widget. + const PopupMenuItem( + value: '0', + child: Text('Item 0'), + ), + // Popup menu item with a ListTile widget. + const PopupMenuItem( + 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 {