Unverified Commit b77b149d authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Add `PopupMenuButton.iconColor`, `PopupMenuTheme.iconSize` and fix button icon...

Add `PopupMenuButton.iconColor`, `PopupMenuTheme.iconSize` and fix button icon using unexpected color propert (#132054)

fixes [PopupMenuButton uses color property for icon color](https://github.com/flutter/flutter/issues/127802) 
fixes [`popup_menu_test.dart` lacks default icon color tests.](https://github.com/flutter/flutter/issues/132050) 

### Description
- Add  `PopupMenuButton..iconColor` and fix the PopupMenu button icon using an unexpected color property.
- Add the missing `PopupMenuTheme.iconSize`.
- Clean up some tests and minor improvements.

### Code sample

<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(
        popupMenuTheme: PopupMenuThemeData(
          // iconSize: 75,
          // iconColor: Colors.amber,
          color: Colors.deepPurple[100],
        ),
      ),
      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(
      appBar: AppBar(title: const Text('PopupMenuButton')),
      body: Center(
        child: PopupMenuButton<SampleItem>(
          iconSize: 75,
          // iconColor: Colors.amber,
          color: Colors.deepPurple[100],
          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('Item 1'),
            ),
            const PopupMenuItem<SampleItem>(
              value: SampleItem.itemTwo,
              child: Text('Item 2'),
            ),
            const PopupMenuDivider(),
            const CheckedPopupMenuItem<SampleItem>(
              value: SampleItem.itemThree,
              checked: true,
              child: Text('Item 3'),
            ),
          ],
        ),
      ),
    );
  }
}

``` 
	
</details>

![Group 2](https://github.com/flutter/flutter/assets/48603081/eb5404ae-2a07-4374-9821-66a0bbea041e)

![Group 1](https://github.com/flutter/flutter/assets/48603081/464e3957-1afb-4118-abcc-aad12591dc51)
parent e65c37ee
......@@ -1111,6 +1111,7 @@ class PopupMenuButton<T> extends StatefulWidget {
this.enabled = true,
this.shape,
this.color,
this.iconColor,
this.enableFeedback,
this.constraints,
this.position,
......@@ -1221,6 +1222,13 @@ class PopupMenuButton<T> extends StatefulWidget {
/// Theme.of(context).cardColor is used.
final Color? color;
/// If provided, this color is used for the button icon.
///
/// If this property is null, then [PopupMenuThemeData.iconColor] is used.
/// If [PopupMenuThemeData.iconColor] is also null then defaults to
/// [IconThemeData.color].
final Color? iconColor;
/// Whether detected gestures should provide acoustic and/or haptic feedback.
///
/// For example, on Android a tap will produce a clicking sound and a
......@@ -1355,6 +1363,7 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
@override
Widget build(BuildContext context) {
final IconThemeData iconTheme = IconTheme.of(context);
final PopupMenuThemeData popupMenuTheme = PopupMenuTheme.of(context);
final bool enableFeedback = widget.enableFeedback
?? PopupMenuTheme.of(context).enableFeedback
?? true;
......@@ -1378,8 +1387,8 @@ class PopupMenuButtonState<T> extends State<PopupMenuButton<T>> {
icon: widget.icon ?? Icon(Icons.adaptive.more),
padding: widget.padding,
splashRadius: widget.splashRadius,
iconSize: widget.iconSize ?? iconTheme.size,
color: widget.color ?? iconTheme.color,
iconSize: widget.iconSize ?? popupMenuTheme.iconSize ?? iconTheme.size,
color: widget.iconColor ?? popupMenuTheme.iconColor ?? iconTheme.color,
tooltip: widget.tooltip ?? MaterialLocalizations.of(context).showMenuTooltip,
onPressed: widget.enabled ? showButtonMenu : null,
enableFeedback: enableFeedback,
......
......@@ -55,6 +55,8 @@ class PopupMenuThemeData with Diagnosticable {
this.enableFeedback,
this.mouseCursor,
this.position,
this.iconColor,
this.iconSize,
});
/// The background color of the popup menu.
......@@ -95,6 +97,12 @@ class PopupMenuThemeData with Diagnosticable {
/// popup menu appear directly over the button that was used to create it.
final PopupMenuPosition? position;
/// The color of the icon in the popup menu button.
final Color? iconColor;
/// The size of the icon in the popup menu button.
final double? iconSize;
/// Creates a copy of this object with the given fields replaced with the
/// new values.
PopupMenuThemeData copyWith({
......@@ -108,6 +116,8 @@ class PopupMenuThemeData with Diagnosticable {
bool? enableFeedback,
MaterialStateProperty<MouseCursor?>? mouseCursor,
PopupMenuPosition? position,
Color? iconColor,
double? iconSize,
}) {
return PopupMenuThemeData(
color: color ?? this.color,
......@@ -120,6 +130,8 @@ class PopupMenuThemeData with Diagnosticable {
enableFeedback: enableFeedback ?? this.enableFeedback,
mouseCursor: mouseCursor ?? this.mouseCursor,
position: position ?? this.position,
iconColor: iconColor ?? this.iconColor,
iconSize: iconSize ?? this.iconSize,
);
}
......@@ -143,6 +155,8 @@ class PopupMenuThemeData with Diagnosticable {
enableFeedback: t < 0.5 ? a?.enableFeedback : b?.enableFeedback,
mouseCursor: t < 0.5 ? a?.mouseCursor : b?.mouseCursor,
position: t < 0.5 ? a?.position : b?.position,
iconColor: Color.lerp(a?.iconColor, b?.iconColor, t),
iconSize: lerpDouble(a?.iconSize, b?.iconSize, t),
);
}
......@@ -158,6 +172,8 @@ class PopupMenuThemeData with Diagnosticable {
enableFeedback,
mouseCursor,
position,
iconColor,
iconSize,
);
@override
......@@ -178,7 +194,9 @@ class PopupMenuThemeData with Diagnosticable {
&& other.labelTextStyle == labelTextStyle
&& other.enableFeedback == enableFeedback
&& other.mouseCursor == mouseCursor
&& other.position == position;
&& other.position == position
&& other.iconColor == iconColor
&& other.iconSize == iconSize;
}
@override
......@@ -194,6 +212,8 @@ class PopupMenuThemeData with Diagnosticable {
properties.add(DiagnosticsProperty<bool>('enableFeedback', enableFeedback, defaultValue: null));
properties.add(DiagnosticsProperty<MaterialStateProperty<MouseCursor?>>('mouseCursor', mouseCursor, defaultValue: null));
properties.add(EnumProperty<PopupMenuPosition?>('position', position, defaultValue: null));
properties.add(ColorProperty('iconColor', iconColor, defaultValue: null));
properties.add(DoubleProperty('iconSize', iconSize, defaultValue: null));
}
}
......
......@@ -199,7 +199,7 @@ void main() {
expect(cancels, equals(2));
});
testWidgets('disabled PopupMenuButton will not call itemBuilder, onOpened, onSelected or onCanceled', (WidgetTester tester) async {
testWidgets('Disabled PopupMenuButton will not call itemBuilder, onOpened, onSelected or onCanceled', (WidgetTester tester) async {
final GlobalKey popupButtonKey = GlobalKey();
bool itemBuilderCalled = false;
bool onOpenedCalled = false;
......@@ -326,7 +326,7 @@ void main() {
expect(onSelectedCalled, isFalse);
});
testWidgets('disabled PopupMenuButton is focusable with directional navigation', (WidgetTester tester) async {
testWidgets('Disabled PopupMenuButton is focusable with directional navigation', (WidgetTester tester) async {
final Key popupButtonKey = UniqueKey();
final GlobalKey childKey = GlobalKey();
......@@ -1084,7 +1084,7 @@ void main() {
expect(tester.getTopLeft(find.byWidgetPredicate((Widget w) => '${w.runtimeType}' == '_PopupMenu<int?>')), const Offset(50.0, 50.0));
});
testWidgets('open PopupMenu has correct semantics', (WidgetTester tester) async {
testWidgets('Opened PopupMenu has correct semantics', (WidgetTester tester) async {
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
MaterialApp(
......@@ -1296,7 +1296,7 @@ void main() {
semantics.dispose();
});
testWidgets('disabled PopupMenuItem has correct semantics', (WidgetTester tester) async {
testWidgets('Disabled PopupMenuItem has correct semantics', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/45044.
final SemanticsTester semantics = SemanticsTester(tester);
await tester.pumpWidget(
......@@ -2576,32 +2576,30 @@ void main() {
});
});
testWidgets('iconSize parameter tests', (WidgetTester tester) async {
Future<void> buildFrame({double? iconSize}) {
return tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
iconSize: iconSize,
itemBuilder: (_) => <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: 'value',
child: Text('child'),
),
],
),
testWidgets('Can customize PopupMenuButton icon', (WidgetTester tester) async {
const Color iconColor = Color(0xffffff00);
const double iconSize = 29.5;
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
body: Center(
child: PopupMenuButton<String>(
iconColor: iconColor,
iconSize: iconSize,
itemBuilder: (_) => <PopupMenuEntry<String>>[
const PopupMenuItem<String>(
value: 'value',
child: Text('child'),
),
],
),
),
),
);
}
await buildFrame();
expect(tester.getSize(find.byIcon(Icons.adaptive.more)), const Size(24, 24));
),
);
await buildFrame(iconSize: 50);
expect(tester.getSize(find.byIcon(Icons.adaptive.more)), const Size(50, 50));
expect(_iconStyle(tester, Icons.adaptive.more)?.color, iconColor);
expect(tester.getSize(find.byIcon(Icons.adaptive.more)), const Size(iconSize, iconSize));
});
testWidgets('does not crash in small overlay', (WidgetTester tester) async {
......@@ -3265,7 +3263,7 @@ void main() {
expect(childBottomLeft, menuTopLeft);
});
testWidgets('PopupmenuItem onTap should be calling after Navigator.pop', (WidgetTester tester) async {
testWidgets('PopupMenuItem onTap should be calling after Navigator.pop', (WidgetTester tester) async {
await tester.pumpWidget(
MaterialApp(
home: Scaffold(
......@@ -3494,3 +3492,10 @@ TextStyle? _labelStyle(WidgetTester tester, String label) {
matching: find.byType(RichText),
)).text.style;
}
TextStyle? _iconStyle(WidgetTester tester, IconData icon) {
return tester.widget<RichText>(find.descendant(
of: find.byIcon(icon),
matching: find.byType(RichText),
)).text.style;
}
......@@ -43,6 +43,8 @@ PopupMenuThemeData _popupMenuThemeM3() {
}
return SystemMouseCursors.alias;
}),
iconColor: const Color(0xfff12099),
iconSize: 17.0,
);
}
......@@ -86,19 +88,23 @@ void main() {
testWidgetsWithLeakTracking('PopupMenuThemeData implements debugFillProperties', (WidgetTester tester) async {
final DiagnosticPropertiesBuilder builder = DiagnosticPropertiesBuilder();
PopupMenuThemeData(
color: const Color(0xFFFFFFFF),
color: const Color(0xfffffff1),
shape: const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(2.0))),
elevation: 2.0,
shadowColor: const Color(0xff00ff00),
surfaceTintColor: const Color(0xff00ff00),
textStyle: const TextStyle(color: Color(0xffffffff)),
shadowColor: const Color(0xfffffff2),
surfaceTintColor: const Color(0xfffffff3),
textStyle: const TextStyle(color: Color(0xfffffff4)),
labelTextStyle: MaterialStateProperty.resolveWith((Set<MaterialState> states) {
if (states.contains(MaterialState.disabled)) {
return const TextStyle(color: Color(0xfff99ff0), fontSize: 12.0);
return const TextStyle(color: Color(0xfffffff5), fontSize: 12.0);
}
return const TextStyle(color: Color(0xfff12099), fontSize: 17.0);
return const TextStyle(color: Color(0xfffffff6), fontSize: 17.0);
}),
enableFeedback: false,
mouseCursor: MaterialStateMouseCursor.clickable,
position: PopupMenuPosition.over,
iconColor: const Color(0xfffffff8),
iconSize: 31.0,
).debugFillProperties(builder);
final List<String> description = builder.properties
......@@ -107,14 +113,18 @@ void main() {
.toList();
expect(description, <String>[
'color: Color(0xffffffff)',
'color: Color(0xfffffff1)',
'shape: RoundedRectangleBorder(BorderSide(width: 0.0, style: none), BorderRadius.circular(2.0))',
'elevation: 2.0',
'shadowColor: Color(0xff00ff00)',
'surfaceTintColor: Color(0xff00ff00)',
'text style: TextStyle(inherit: true, color: Color(0xffffffff))',
'shadowColor: Color(0xfffffff2)',
'surfaceTintColor: Color(0xfffffff3)',
'text style: TextStyle(inherit: true, color: Color(0xfffffff4))',
"labelTextStyle: Instance of '_MaterialStatePropertyWith<TextStyle?>'",
'enableFeedback: false',
'mouseCursor: MaterialStateMouseCursor(clickable)',
'position: over',
'iconColor: Color(0xfffffff8)',
'iconSize: 31.0'
]);
});
......@@ -165,6 +175,9 @@ void main() {
),
));
// Test default button icon color.
expect(_iconStyle(tester, Icons.adaptive.more)?.color, theme.iconTheme.color);
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
......@@ -282,6 +295,9 @@ void main() {
),
));
expect(_iconStyle(tester, Icons.adaptive.more)?.color, popupMenuTheme.iconColor);
expect(tester.getSize(find.byIcon(Icons.adaptive.more)), Size(popupMenuTheme.iconSize!, popupMenuTheme.iconSize!));
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
......@@ -354,15 +370,17 @@ void main() {
final Key popupButtonApp = UniqueKey();
final Key popupItemKey = UniqueKey();
const Color color = Colors.purple;
const Color surfaceTintColor = Colors.amber;
const Color shadowColor = Colors.green;
const Color color = Color(0xfff11fff);
const Color surfaceTintColor = Color(0xfff12fff);
const Color shadowColor = Color(0xfff13fff);
const ShapeBorder shape = RoundedRectangleBorder(
borderRadius: BorderRadius.all(Radius.circular(9.0)),
);
const double elevation = 7.0;
const TextStyle textStyle = TextStyle(color: Color(0xffffffef), fontSize: 19.0);
const TextStyle textStyle = TextStyle(color: Color(0xfff14fff), fontSize: 19.0);
const MouseCursor cursor = SystemMouseCursors.forbidden;
const Color iconColor = Color(0xfff15fff);
const double iconSize = 21.5;
await tester.pumpWidget(MaterialApp(
theme: ThemeData(useMaterial3: true, popupMenuTheme: popupMenuTheme),
......@@ -377,6 +395,8 @@ void main() {
surfaceTintColor: surfaceTintColor,
color: color,
shape: shape,
iconColor: iconColor,
iconSize: iconSize,
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<void>>[
PopupMenuItem<void>(
......@@ -398,6 +418,9 @@ void main() {
),
));
expect(_iconStyle(tester, Icons.adaptive.more)?.color, iconColor);
expect(tester.getSize(find.byIcon(Icons.adaptive.more)), const Size(iconSize, iconSize));
await tester.tap(find.byKey(popupButtonKey));
await tester.pumpAndSettle();
......@@ -719,3 +742,10 @@ void main() {
Set<MaterialState> enabled = <MaterialState>{};
Set<MaterialState> disabled = <MaterialState>{MaterialState.disabled};
TextStyle? _iconStyle(WidgetTester tester, IconData icon) {
return tester.widget<RichText>(find.descendant(
of: find.byIcon(icon),
matching: find.byType(RichText),
)).text.style;
}
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