Unverified Commit 038ec62b authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

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

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

<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" />
parent 79033ed8
......@@ -475,6 +475,7 @@ class CheckedPopupMenuItem<T> extends PopupMenuItem<T> {
super.enabled,
super.padding,
super.height,
super.labelTextStyle,
super.mouseCursor,
super.child,
});
......@@ -529,9 +530,19 @@ class _CheckedPopupMenuItemState<T> extends PopupMenuItemState<T, CheckedPopupMe
@override
Widget buildChild() {
final ThemeData theme = Theme.of(context);
final PopupMenuThemeData popupMenuTheme = PopupMenuTheme.of(context);
final PopupMenuThemeData defaults = theme.useMaterial3 ? _PopupMenuDefaultsM3(context) : _PopupMenuDefaultsM2(context);
final Set<MaterialState> states = <MaterialState>{
if (widget.checked) MaterialState.selected,
};
final MaterialStateProperty<TextStyle?>? 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),
......
......@@ -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: <Widget>[
PopupMenuButton<void>(
key: popupMenuButtonKey,
itemBuilder: (BuildContext context) => <PopupMenuItem<void>>[
const CheckedPopupMenuItem<void>(
child: Text('Item 1'),
),
const CheckedPopupMenuItem<int>(
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<TextStyle?> labelTextStyle = MaterialStateProperty.resolveWith(
(Set<MaterialState> 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: <Widget>[
PopupMenuButton<void>(
key: popupMenuButtonKey,
itemBuilder: (BuildContext context) => <PopupMenuItem<void>>[
CheckedPopupMenuItem<void>(
labelTextStyle: labelTextStyle,
child: const Text('Item 1'),
),
CheckedPopupMenuItem<int>(
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(<MaterialState>{})
);
expect(
_labelStyle(tester, 'Item 2'),
labelTextStyle.resolve(<MaterialState>{MaterialState.selected})
);
});
}
class TestApp extends StatelessWidget {
......@@ -3377,3 +3488,10 @@ class _ClosureNavigatorObserver extends NavigatorObserver {
@override
void didReplace({Route<dynamic>? newRoute, Route<dynamic>? oldRoute}) => onDidChange(newRoute!);
}
TextStyle? _labelStyle(WidgetTester tester, String label) {
return tester.widget<RichText>(find.descendant(
of: find.text(label),
matching: find.byType(RichText),
)).text.style;
}
......@@ -149,6 +149,13 @@ void main() {
enabled: false,
child: const Text('Disabled PopupMenuItem'),
),
const CheckedPopupMenuItem<void>(
child: Text('Unchecked item'),
),
const CheckedPopupMenuItem<void>(
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>(
DefaultTextStyle popupMenuItemLabel = tester.widget<DefaultTextStyle>(
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<DefaultTextStyle>(
popupMenuItemLabel = tester.widget<DefaultTextStyle>(
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<void>));
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<ListTile>(find.byType(ListTile).first);
expect(listTile.titleTextStyle?.color, theme.colorScheme.onSurface);
// Test checked CheckedPopupMenuItem label.
listTile = tester.widget<ListTile>(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<Object>(
child: Text('Unchecked item'),
),
const CheckedPopupMenuItem<Object>(
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>(
DefaultTextStyle popupMenuItemLabel = tester.widget<DefaultTextStyle>(
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<DefaultTextStyle>(
popupMenuItemLabel = tester.widget<DefaultTextStyle>(
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<ListTile>(find.byType(ListTile).first);
expect(listTile.titleTextStyle, popupMenuTheme.labelTextStyle?.resolve(enabled));
// Test checked CheckedPopupMenuItem label.
listTile = tester.widget<ListTile>(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<void>(
checked: true,
labelTextStyle: MaterialStateProperty.all<TextStyle>(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<ListTile>(find.byType(ListTile).first);
expect(listTile.titleTextStyle, textStyle);
});
group('Material 2', () {
......
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