From 038ec62b285d483469b87275b183cd371448a85e Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Fri, 28 Jul 2023 20:16:04 +0300 Subject: [PATCH] =?UTF-8?q?Add=20`CheckedPopupMenuItem=E2=80=8E.labelTextS?= =?UTF-8?q?tyle`=20and=20update=20default=20text=20style=20for=20Material?= =?UTF-8?q?=203=20(#131060)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fixes [Update `CheckedPopupMenuItem‎` for Material 3](https://github.com/flutter/flutter/issues/128576) ### Description - This adds the missing ``CheckedPopupMenuItem‎.labelTextStyle` parameter - Fixes default text style for `CheckedPopupMenuItem‎`. It used `ListTile`'s `bodyLarge` instead of `LabelLarge` similar to `PopupMenuItem`. ### Code sample
expand to view the code sample ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp( debugShowCheckedModeBanner: false, theme: ThemeData( useMaterial3: true, textTheme: const TextTheme( labelLarge: TextStyle( fontWeight: FontWeight.bold, fontStyle: FontStyle.italic, letterSpacing: 5.0, ), ), ), home: const Example(), ); } } class Example extends StatelessWidget { const Example({super.key}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: const Text('Sample'), actions: [ PopupMenuButton( icon: const Icon(Icons.more_vert), itemBuilder: (BuildContext context) => >[ const CheckedPopupMenuItem( // labelTextStyle: MaterialStateProperty.resolveWith( // (Set states) { // if (states.contains(MaterialState.selected)) { // return const TextStyle( // color: Colors.red, // fontStyle: FontStyle.italic, // fontWeight: FontWeight.bold, // ); // } // return const TextStyle( // color: Colors.amber, // fontStyle: FontStyle.italic, // fontWeight: FontWeight.bold, // ); // }), child: Text('Mild'), ), const CheckedPopupMenuItem( checked: true, // labelTextStyle: MaterialStateProperty.resolveWith( // (Set states) { // if (states.contains(MaterialState.selected)) { // return const TextStyle( // color: Colors.red, // fontStyle: FontStyle.italic, // fontWeight: FontWeight.bold, // ); // } // return const TextStyle( // color: Colors.amber, // fontStyle: FontStyle.italic, // fontWeight: FontWeight.bold, // ); // }), child: Text('Spicy'), ), const PopupMenuDivider(), const PopupMenuItem( value: 'Close', child: Text('Close'), ), ], ) ], ), ); } } ```
### Customized `textTheme.labelLarge` text theme. | Before | After | | --------------- | --------------- | | | | ### New `CheckedPopupMenuItem‎.labelTextStyle` parameter with material states support --- .../flutter/lib/src/material/popup_menu.dart | 11 ++ .../test/material/popup_menu_test.dart | 118 ++++++++++++++++++ .../test/material/popup_menu_theme_test.dart | 58 +++++++-- 3 files changed, 178 insertions(+), 9 deletions(-) diff --git a/packages/flutter/lib/src/material/popup_menu.dart b/packages/flutter/lib/src/material/popup_menu.dart index 1d80305e74..60081cf0a6 100644 --- a/packages/flutter/lib/src/material/popup_menu.dart +++ b/packages/flutter/lib/src/material/popup_menu.dart @@ -475,6 +475,7 @@ class CheckedPopupMenuItem extends PopupMenuItem { super.enabled, super.padding, super.height, + super.labelTextStyle, super.mouseCursor, super.child, }); @@ -529,9 +530,19 @@ class _CheckedPopupMenuItemState extends PopupMenuItemState states = { + if (widget.checked) MaterialState.selected, + }; + final MaterialStateProperty? effectiveLabelTextStyle = widget.labelTextStyle + ?? popupMenuTheme.labelTextStyle + ?? defaults.labelTextStyle; return IgnorePointer( child: ListTile( enabled: widget.enabled, + titleTextStyle: effectiveLabelTextStyle?.resolve(states), leading: FadeTransition( opacity: _opacity, child: Icon(_controller.isDismissed ? null : Icons.done), diff --git a/packages/flutter/test/material/popup_menu_test.dart b/packages/flutter/test/material/popup_menu_test.dart index ea3c47668e..3a8b4e2040 100644 --- a/packages/flutter/test/material/popup_menu_test.dart +++ b/packages/flutter/test/material/popup_menu_test.dart @@ -3307,6 +3307,117 @@ void main() { final Finder modalBottomSheet = find.text('ModalBottomSheet'); expect(modalBottomSheet, findsOneWidget); }); + + testWidgets('Material3 - CheckedPopupMenuItem.labelTextStyle uses correct text style', (WidgetTester tester) async { + final Key popupMenuButtonKey = UniqueKey(); + ThemeData theme = ThemeData(useMaterial3: true); + + Widget buildMenu() { + return MaterialApp( + theme: theme, + home: Scaffold( + appBar: AppBar( + actions: [ + PopupMenuButton( + key: popupMenuButtonKey, + itemBuilder: (BuildContext context) => >[ + const CheckedPopupMenuItem( + child: Text('Item 1'), + ), + const CheckedPopupMenuItem( + checked: true, + child: Text('Item 2'), + ), + ], + ), + ], + ), + ), + ); + } + + await tester.pumpWidget(buildMenu()); + + // Show the menu + await tester.tap(find.byKey(popupMenuButtonKey)); + await tester.pumpAndSettle(); + + // Test default text style. + 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 text style. + theme = theme.copyWith( + textTheme: theme.textTheme.copyWith( + labelLarge: const TextStyle( + fontSize: 20.0, + fontWeight: FontWeight.bold, + ) + ), + ); + 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); + }); + + testWidgets('CheckedPopupMenuItem.labelTextStyle resolve material states', (WidgetTester tester) async { + final Key popupMenuButtonKey = UniqueKey(); + final MaterialStateProperty labelTextStyle = MaterialStateProperty.resolveWith( + (Set states) { + if (states.contains(MaterialState.selected)) { + return const TextStyle(color: Colors.red, fontSize: 24.0); + } + + return const TextStyle(color: Colors.amber, fontSize: 20.0); + }); + + await tester.pumpWidget( + MaterialApp( + home: Scaffold( + appBar: AppBar( + actions: [ + PopupMenuButton( + key: popupMenuButtonKey, + itemBuilder: (BuildContext context) => >[ + CheckedPopupMenuItem( + labelTextStyle: labelTextStyle, + child: const Text('Item 1'), + ), + CheckedPopupMenuItem( + checked: true, + labelTextStyle: labelTextStyle, + child: const Text('Item 2'), + ), + ], + ), + ], + ), + ), + ), + ); + + // Show the menu. + await tester.tap(find.byKey(popupMenuButtonKey)); + await tester.pumpAndSettle(); + + expect( + _labelStyle(tester, 'Item 1'), + labelTextStyle.resolve({}) + ); + expect( + _labelStyle(tester, 'Item 2'), + labelTextStyle.resolve({MaterialState.selected}) + ); + }); } class TestApp extends StatelessWidget { @@ -3377,3 +3488,10 @@ class _ClosureNavigatorObserver extends NavigatorObserver { @override void didReplace({Route? newRoute, Route? oldRoute}) => onDidChange(newRoute!); } + +TextStyle? _labelStyle(WidgetTester tester, String label) { + return tester.widget(find.descendant( + of: find.text(label), + matching: find.byType(RichText), + )).text.style; +} diff --git a/packages/flutter/test/material/popup_menu_theme_test.dart b/packages/flutter/test/material/popup_menu_theme_test.dart index 8b61cbc39a..e2b5ba6444 100644 --- a/packages/flutter/test/material/popup_menu_theme_test.dart +++ b/packages/flutter/test/material/popup_menu_theme_test.dart @@ -149,6 +149,13 @@ void main() { enabled: false, child: const Text('Disabled PopupMenuItem'), ), + const CheckedPopupMenuItem( + child: Text('Unchecked item'), + ), + const CheckedPopupMenuItem( + checked: true, + child: Text('Checked item'), + ), ]; }, ), @@ -181,22 +188,23 @@ void main() { /// [PopupMenuItem] specified above, so by finding the last descendent of /// popupItemKey that is of type DefaultTextStyle, this code retrieves the /// built [PopupMenuItem]. - final DefaultTextStyle enabledText = tester.widget( + DefaultTextStyle popupMenuItemLabel = tester.widget( find.descendant( of: find.byKey(enabledPopupItemKey), matching: find.byType(DefaultTextStyle), ).last, ); - expect(enabledText.style.fontFamily, 'Roboto'); - expect(enabledText.style.color, theme.colorScheme.onSurface); + expect(popupMenuItemLabel.style.fontFamily, 'Roboto'); + expect(popupMenuItemLabel.style.color, theme.colorScheme.onSurface); + /// Test disabled text color - final DefaultTextStyle disabledText = tester.widget( + popupMenuItemLabel = tester.widget( find.descendant( of: find.byKey(disabledPopupItemKey), matching: find.byType(DefaultTextStyle), ).last, ); - expect(disabledText.style.color, theme.colorScheme.onSurface.withOpacity(0.38)); + expect(popupMenuItemLabel.style.color, theme.colorScheme.onSurface.withOpacity(0.38)); final Offset topLeftButton = tester.getTopLeft(find.byType(PopupMenuButton)); final Offset topLeftMenu = tester.getTopLeft(find.byWidget(button)); @@ -217,6 +225,14 @@ void main() { RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1), SystemMouseCursors.click, ); + + // Test unchecked CheckedPopupMenuItem label. + ListTile listTile = tester.widget(find.byType(ListTile).first); + expect(listTile.titleTextStyle?.color, theme.colorScheme.onSurface); + + // Test checked CheckedPopupMenuItem label. + listTile = tester.widget(find.byType(ListTile).last); + expect(listTile.titleTextStyle?.color, theme.colorScheme.onSurface); }); testWidgetsWithLeakTracking('Popup menu uses values from PopupMenuThemeData', (WidgetTester tester) async { @@ -251,6 +267,13 @@ void main() { onTap: () { }, child: const Text('enabled'), ), + const CheckedPopupMenuItem( + child: Text('Unchecked item'), + ), + const CheckedPopupMenuItem( + checked: true, + child: Text('Checked item'), + ), ]; }, ), @@ -278,25 +301,25 @@ void main() { expect(button.shape, const BeveledRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(12)))); expect(button.elevation, 12.0); - final DefaultTextStyle enabledText = tester.widget( + DefaultTextStyle popupMenuItemLabel = tester.widget( find.descendant( of: find.byKey(enabledPopupItemKey), matching: find.byType(DefaultTextStyle), ).last, ); expect( - enabledText.style, + popupMenuItemLabel.style, popupMenuTheme.labelTextStyle?.resolve(enabled), ); /// Test disabled text color - final DefaultTextStyle disabledText = tester.widget( + popupMenuItemLabel = tester.widget( find.descendant( of: find.byKey(disabledPopupItemKey), matching: find.byType(DefaultTextStyle), ).last, ); expect( - disabledText.style, + popupMenuItemLabel.style, popupMenuTheme.labelTextStyle?.resolve(disabled), ); @@ -315,6 +338,14 @@ void main() { RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1), popupMenuTheme.mouseCursor?.resolve(enabled), ); + + // Test unchecked CheckedPopupMenuItem label. + ListTile listTile = tester.widget(find.byType(ListTile).first); + expect(listTile.titleTextStyle, popupMenuTheme.labelTextStyle?.resolve(enabled)); + + // Test checked CheckedPopupMenuItem label. + listTile = tester.widget(find.byType(ListTile).last); + expect(listTile.titleTextStyle, popupMenuTheme.labelTextStyle?.resolve(enabled)); }); testWidgetsWithLeakTracking('Popup menu widget properties take priority over theme', (WidgetTester tester) async { @@ -354,6 +385,11 @@ void main() { mouseCursor: cursor, child: const Text('Example'), ), + CheckedPopupMenuItem( + checked: true, + labelTextStyle: MaterialStateProperty.all(textStyle), + child: const Text('Checked item'), + ) ]; }, ), @@ -399,6 +435,10 @@ void main() { await gesture.moveTo(tester.getCenter(find.byKey(popupItemKey))); await tester.pumpAndSettle(); expect(RendererBinding.instance.mouseTracker.debugDeviceActiveCursor(1), cursor); + + // Test CheckedPopupMenuItem label. + final ListTile listTile = tester.widget(find.byType(ListTile).first); + expect(listTile.titleTextStyle, textStyle); }); group('Material 2', () {