Add `CheckedPopupMenuItem‎.labelTextStyle` and update default text style for Material 3 #131060

TahaTesser · 2023-07-21T15:40:59Z

fixes Update CheckedPopupMenuItem‎ for Material 3

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

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: <Widget>[
          PopupMenuButton<String>(
            icon: const Icon(Icons.more_vert),
            itemBuilder: (BuildContext context) => <PopupMenuEntry<String>>[
              const CheckedPopupMenuItem<String>(
                // labelTextStyle: MaterialStateProperty.resolveWith(
                //     (Set<MaterialState> 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<String>(
                checked: true,
                // labelTextStyle: MaterialStateProperty.resolveWith(
                //     (Set<MaterialState> 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<String>(
                value: 'Close',
                child: Text('Close'),
              ),
            ],
          )
        ],
      ),
    );
  }
}

Customized `textTheme.labelLarge` text theme.

Before	After

New `CheckedPopupMenuItem‎.labelTextStyle` parameter with material states support

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

…yle for Material 3

HansMuller

LGTM

rydmike · 2023-07-28T20:03:30Z

@TahaTesser and @HansMuller, interesting find and fix with the labelLarge (M3 14dp) size versus bodyLarge(M3 16dp) on CheckedPopupMenuItem‎.

This got me to look closer at the M3 spec for the text in menus. I had not noticed it, but yes indeed M3 spec calls for labelLarge. I had just assumed it should be bodyLarge in M3, because in all except one place is bodyLarge used so far in M3 in Flutter in all the new menus.

All the new M3 menus, like MenuBar, MenuAnchor and DropdownMenu are using bodyLarge by default in Flutter, pretty much everywhere. Changing this default would be breaking.

The only place where new menus have actually used labelLarge is the TextField in the DropdownMenu where the input field uses labelLarge, but the menu that it open uses bodyLarge on its items, as do all other menu parts in Flutter that come from MenuItemButton used in e.g. MenuAnchor and MenuBar.

I had wrongly assumed that the labelLarge in the TextField in DropdownMenu might be wrong, since that was the deviant style sticking out, so I did not even doubt the other styles. The style bodyLarge in the menus all come from MenuItemButton here:

flutter/packages/flutter/lib/src/material/menu_anchor.dart

Line 3788 in 096b7c3

    
           return MaterialStatePropertyAll<TextStyle?>(Theme.of(context).textTheme.bodyLarge);

This is a bit of dilemma, on one side it is M3 spec, on the other side changing this wrong default now is also breaking. Not that I personally mind, I'm just stating that it will be breaking to update the default for many apps, to now follow the correct M3 spec.

This of course argues for continuing to use labelLarge as style for the TextField in DropdownMenu, since it is actually correct M3 spec that way. The only thing with it, is that it is poor default design, since that this text input style fits poorly with the default size of a TextField in M3, which is also bodyLarge. The DropdownMenu is typically used in forms where TextField are also used as inputs, but currently fits poorly by default with the TextStyle in DropdownMenu. One of the requires theming/styling to fit with the other one.

I discussed the deviation of the DropdownMenu concerning its built in TextField, and the actual TextField and the MenuItemButton text styles, as a side note in this issue:
#131350

I did however not raise an issue about it, but it does kind feel like it should be an issue of its own now.

Personally I like when the input field in the DropdownMenu" matches the used size in TextField` and menu items are the same too. These kind of default style deviations in the M3 spec defaults can always be fixed with own better/opinionated theming than M3 defaults. M3 defaults are not always consistent across components in its design specs. It is almost there, but not quite, this is an example of such a case.

One could of course also argue that TextField should instead be LabelLarge too, and not bodyLarge and that would then make it all fit with the menus too and the DropdownMenu:s current input field's default labelLarge and the M3 spec for the menuItems labelLarge and even buttons in general, that also use labelLarge as default.

EDIT:

If we look at Jetpack Compose and what a DropdownMenu looks like there. We can see that the input field is actually bodyLarge like in a TextField, but the items are using the labelLarge size like the M3 spec states for the items, so they are a bit smaller, but do follow the M3 spec for the items. The text input field are then just follows the spec for the TextField.

This means that Flutter basically has its TextStyles for input field and menu items (in all menus) reversed! Yes this should be its own Flutter M3 spec issue. I will open one later.

… text style for Material 3 (flutter/flutter#131060)

flutter/flutter@e81907e...c2b40ca 2023-07-29 [email protected] Roll Flutter Engine from 24ccd8fffbcb to 5184062f7543 (2 revisions) (flutter/flutter#131554) 2023-07-29 [email protected] Roll Flutter Engine from 90bc483524fe to 24ccd8fffbcb (1 revision) (flutter/flutter#131540) 2023-07-29 [email protected] Roll Flutter Engine from 27128f29a066 to 90bc483524fe (2 revisions) (flutter/flutter#131535) 2023-07-29 [email protected] Roll Flutter Engine from 69b5b77edaf7 to 27128f29a066 (1 revision) (flutter/flutter#131533) 2023-07-29 [email protected] Roll Flutter Engine from b854fdea2715 to 69b5b77edaf7 (1 revision) (flutter/flutter#131532) 2023-07-29 [email protected] Roll Flutter Engine from 0a5b2f521539 to b854fdea2715 (1 revision) (flutter/flutter#131530) 2023-07-29 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.21.0 to 2.21.2 (flutter/flutter#131512) 2023-07-29 [email protected] Roll Flutter Engine from f5c17f0e2406 to 0a5b2f521539 (2 revisions) (flutter/flutter#131529) 2023-07-29 [email protected] Roll Flutter Engine from b4bf592279d6 to f5c17f0e2406 (1 revision) (flutter/flutter#131525) 2023-07-28 [email protected] Roll Flutter Engine from da3721adba65 to b4bf592279d6 (2 revisions) (flutter/flutter#131519) 2023-07-28 [email protected] Roll Flutter Engine from aa1278eb7b84 to da3721adba65 (5 revisions) (flutter/flutter#131514) 2023-07-28 [email protected] Reland - Fix floating SnackBar throws when FAB is on the top (flutter/flutter#131475) 2023-07-28 [email protected] Device discovery output cleanup (flutter/flutter#131223) 2023-07-28 [email protected] Roll Flutter Engine from 182e1189873b to aa1278eb7b84 (2 revisions) (flutter/flutter#131500) 2023-07-28 [email protected] Update `BottomSheet.enableDrag` & `BottomSheet.showDragHandle` docs for animation controller (flutter/flutter#131484) 2023-07-28 [email protected] Deprecate `useMaterial3` parameter in `ThemeData.copyWith()` (flutter/flutter#131455) 2023-07-28 [email protected] Roll Flutter Engine from 73615d66b2fe to 182e1189873b (2 revisions) (flutter/flutter#131492) 2023-07-28 [email protected] Add `CheckedPopupMenuItemâ��.labelTextStyle` and update default text style for Material 3 (flutter/flutter#131060) 2023-07-28 [email protected] Remove obsolete `complex_layout_ios__compile.dart` test (flutter/flutter#131487) 2023-07-28 [email protected] Roll Flutter Engine from e97014c71014 to 73615d66b2fe (3 revisions) (flutter/flutter#131485) 2023-07-28 [email protected] Shared state to support multi screen inspection (flutter/flutter#129452) 2023-07-28 [email protected] Roll Packages from f4ae933 to 10aab44 (4 revisions) (flutter/flutter#131483) 2023-07-28 [email protected] Move ios_app_with_extensions_test to host only (flutter/flutter#131441) 2023-07-28 [email protected] Update `Card.color` documentation for Material 3 (flutter/flutter#131468) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md

@OverRide

…yle for Material 3 (flutter#131060) fixes [Update `CheckedPopupMenuItemâ��` for Material 3](flutter#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 <details> <summary>expand to view the code sample</summary> ```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: <Widget>[ PopupMenuButton<String>( icon: const Icon(Icons.more_vert), itemBuilder: (BuildContext context) => <PopupMenuEntry<String>>[ const CheckedPopupMenuItem<String>( // labelTextStyle: MaterialStateProperty.resolveWith( // (Set<MaterialState> 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<String>( checked: true, // labelTextStyle: MaterialStateProperty.resolveWith( // (Set<MaterialState> 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<String>( value: 'Close', child: Text('Close'), ), ], ) ], ), ); } } ``` </details> ### Customized `textTheme.labelLarge` text theme. | Before | After | | --------------- | --------------- | | <img src="https://github.com/flutter/flutter/assets/48603081/2672438d-b2da-479b-a5d3-d239ef646365" /> | <img src="https://github.com/flutter/flutter/assets/48603081/b9f83719-dede-4c2f-8247-18f74e63eb29" /> | ### New `CheckedPopupMenuItemâ��.labelTextStyle` parameter with material states support <img src="https://github.com/flutter/flutter/assets/48603081/ef0a88aa-9811-42b1-a3aa-53b90c8d43fb" height="450" />

@OverRide

…yle for Material 3 (flutter#131060) fixes [Update `CheckedPopupMenuItemâ��` for Material 3](flutter#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 <details> <summary>expand to view the code sample</summary> ```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: <Widget>[ PopupMenuButton<String>( icon: const Icon(Icons.more_vert), itemBuilder: (BuildContext context) => <PopupMenuEntry<String>>[ const CheckedPopupMenuItem<String>( // labelTextStyle: MaterialStateProperty.resolveWith( // (Set<MaterialState> 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<String>( checked: true, // labelTextStyle: MaterialStateProperty.resolveWith( // (Set<MaterialState> 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<String>( value: 'Close', child: Text('Close'), ), ], ) ], ), ); } } ``` </details> ### Customized `textTheme.labelLarge` text theme. | Before | After | | --------------- | --------------- | | <img src="https://github.com/flutter/flutter/assets/48603081/2672438d-b2da-479b-a5d3-d239ef646365" /> | <img src="https://github.com/flutter/flutter/assets/48603081/b9f83719-dede-4c2f-8247-18f74e63eb29" /> | ### New `CheckedPopupMenuItemâ��.labelTextStyle` parameter with material states support <img src="https://github.com/flutter/flutter/assets/48603081/ef0a88aa-9811-42b1-a3aa-53b90c8d43fb" height="450" />

Add CheckedPopupMenuItem‎.labelTextStyle and update default text st…

71aac48

…yle for Material 3

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Jul 21, 2023

TahaTesser requested a review from HansMuller July 21, 2023 15:42

HansMuller requested a review from QuncCccccc July 21, 2023 15:45

HansMuller approved these changes Jul 28, 2023

View reviewed changes

HansMuller added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 28, 2023

auto-submit bot merged commit 038ec62 into flutter:master Jul 28, 2023

TahaTesser deleted the checked_popupmenu_item_label branch July 28, 2023 18:07

HansMuller mentioned this pull request Jul 28, 2023

Flutter Framework Team: Notable Commits #121415

Closed

rydmike mentioned this pull request Jul 28, 2023

DropdownMenu leading icon usage layout issue #131350

Closed

16 tasks

engine-flutter-autoroll mentioned this pull request Jul 29, 2023

Roll Flutter from e81907e07a38 to e94e185c364c (23 revisions) flutter/packages#4585

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 29, 2023

038ec62 Add CheckedPopupMenuItem‎.labelTextStyle and update default…

bb4e4d1

… text style for Material 3 (flutter/flutter#131060)

engine-flutter-autoroll mentioned this pull request Jul 29, 2023

Roll Flutter from e81907e07a38 to c2b40ca7013b (24 revisions) flutter/packages#4588

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 29, 2023

038ec62 Add CheckedPopupMenuItem‎.labelTextStyle and update default…

c845329

… text style for Material 3 (flutter/flutter#131060)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add `CheckedPopupMenuItem‎.labelTextStyle` and update default text style for Material 3 #131060

Add `CheckedPopupMenuItem‎.labelTextStyle` and update default text style for Material 3 #131060

Uh oh!

TahaTesser commented Jul 21, 2023

Uh oh!

HansMuller left a comment

Uh oh!

rydmike commented Jul 28, 2023 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add CheckedPopupMenuItem‎.labelTextStyle and update default text style for Material 3 #131060

Add CheckedPopupMenuItem‎.labelTextStyle and update default text style for Material 3 #131060

Uh oh!

Conversation

TahaTesser commented Jul 21, 2023

Description

Code sample

Customized textTheme.labelLarge text theme.

New CheckedPopupMenuItem‎.labelTextStyle parameter with material states support

Pre-launch Checklist

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

rydmike commented Jul 28, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Add `CheckedPopupMenuItem‎.labelTextStyle` and update default text style for Material 3 #131060

Add `CheckedPopupMenuItem‎.labelTextStyle` and update default text style for Material 3 #131060

Customized `textTheme.labelLarge` text theme.

New `CheckedPopupMenuItem‎.labelTextStyle` parameter with material states support

rydmike commented Jul 28, 2023 •

edited

Loading