[CP-stable]Fix buttons with icons ignore provided foregroundColor (#163201)

This pull request is created by [automatic cherry pick workflow](https://github.com/flutter/flutter/blob/main/docs/releases/Flutter-Cherrypick-Process.md#automatically-creates-a-cherry-pick-request)
Please fill in the form below, and a flutter domain expert will evaluate this cherry pick request.

### Issue Link:
What is the link to the issue this cherry-pick is addressing?

PR: https://github.com/flutter/flutter/pull/162880
Issue: https://github.com/flutter/flutter/issues/162839

### Changelog Description:
Explain this cherry pick in one line that is accessible to most Flutter developers. See [best practices](https://github.com/flutter/flutter/blob/main/docs/releases/Hotfix-Documentation-Best-Practices.md) for examples

Fixes buttons with icons ignore `foregroundColor`.

### Impact Description:
What is the impact (ex. visual jank on Samsung phones, app crash, cannot ship an iOS app)? Does it impact development (ex. flutter doctor crashes when Android Studio is installed), or the shipping production app (the app crashes on launch)

It impacts how buttons with icons render color when using `foregroundColor`.

### Workaround:
Is there a workaround for this issue?

Provide additional property such as `iconColor`  along with `foregroundColor` to match colors for button label and icon.

### Risk:
What is the risk level of this cherry-pick?

### Test Coverage:
Are you confident that your fix is well-tested by automated tests?

### Validation Steps:
What are the steps to validate that this fix works?

Testing and samples.
This commit is contained in:
flutteractionsbot 2025-02-19 20:49:14 -08:00 committed by GitHub
parent d73fc34a73
commit 9bc7df0177
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 283 additions and 7 deletions

View File

@ -389,6 +389,16 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
});
}
Color? effectiveIconColor() {
return widgetStyle?.iconColor?.resolve(statesController.value) ??
themeStyle?.iconColor?.resolve(statesController.value) ??
widgetStyle?.foregroundColor?.resolve(statesController.value) ??
themeStyle?.foregroundColor?.resolve(statesController.value) ??
defaultStyle.iconColor?.resolve(statesController.value) ??
// Fallback to foregroundColor if iconColor is null.
defaultStyle.foregroundColor?.resolve(statesController.value);
}
final double? resolvedElevation = resolve<double?>((ButtonStyle? style) => style?.elevation);
final TextStyle? resolvedTextStyle = resolve<TextStyle?>(
(ButtonStyle? style) => style?.textStyle,
@ -409,7 +419,7 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
final Size? resolvedMinimumSize = resolve<Size?>((ButtonStyle? style) => style?.minimumSize);
final Size? resolvedFixedSize = resolve<Size?>((ButtonStyle? style) => style?.fixedSize);
final Size? resolvedMaximumSize = resolve<Size?>((ButtonStyle? style) => style?.maximumSize);
final Color? resolvedIconColor = resolve<Color?>((ButtonStyle? style) => style?.iconColor);
final Color? resolvedIconColor = effectiveIconColor();
final double? resolvedIconSize = resolve<double?>((ButtonStyle? style) => style?.iconSize);
final BorderSide? resolvedSide = resolve<BorderSide?>((ButtonStyle? style) => style?.side);
final OutlinedBorder? resolvedShape = resolve<OutlinedBorder?>(
@ -551,10 +561,7 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
customBorder: resolvedShape!.copyWith(side: resolvedSide),
statesController: statesController,
child: IconTheme.merge(
data: IconThemeData(
color: resolvedIconColor ?? resolvedForegroundColor,
size: resolvedIconSize,
),
data: IconThemeData(color: resolvedIconColor, size: resolvedIconSize),
child: result,
),
);

View File

@ -161,6 +161,9 @@ class ElevatedButton extends ButtonStyleButton {
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
/// [ButtonStyle.iconSize].
///
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
/// null, the button icon will use the default icon color.
///
/// The button's elevations are defined relative to the [elevation]
/// parameter. The disabled elevation is the same as the parameter
/// value, [elevation] + 2 is used when the button is hovered

View File

@ -232,6 +232,9 @@ class FilledButton extends ButtonStyleButton {
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
/// [ButtonStyle.iconSize].
///
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
/// null, the button icon will use the default icon color.
///
/// The button's elevations are defined relative to the [elevation]
/// parameter. The disabled elevation is the same as the parameter
/// value, [elevation] + 2 is used when the button is hovered

View File

@ -160,6 +160,9 @@ class OutlinedButton extends ButtonStyleButton {
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
/// [ButtonStyle.iconSize].
///
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
/// null, the button icon will use the default icon color.
///
/// If [overlayColor] is specified and its value is [Colors.transparent]
/// then the pressed/focused/hovered highlights are effectively defeated.
/// Otherwise a [WidgetStateProperty] with the same opacities as the

View File

@ -169,6 +169,9 @@ class TextButton extends ButtonStyleButton {
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
/// [ButtonStyle.iconSize].
///
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
/// null, the button icon will use the default icon color.
///
/// If [overlayColor] is specified and its value is [Colors.transparent]
/// then the pressed/focused/hovered highlights are effectively defeated.
/// Otherwise a [WidgetStateProperty] with the same opacities as the

View File

@ -2510,4 +2510,27 @@ void main() {
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets('ElevatedButton icon uses provided foregroundColor over default icon color', (
WidgetTester tester,
) async {
const Color foregroundColor = Color(0xFFFF1234);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: ElevatedButton.icon(
style: ElevatedButton.styleFrom(foregroundColor: foregroundColor),
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
});
}

View File

@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
TextStyle iconStyle(WidgetTester tester, IconData icon) {
final RichText iconRichText = tester.widget<RichText>(
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
);
return iconRichText.text.style!;
}
test('ElevatedButtonThemeData lerp special cases', () {
expect(ElevatedButtonThemeData.lerp(null, null, 0), null);
const ElevatedButtonThemeData data = ElevatedButtonThemeData();
@ -263,7 +270,7 @@ void main() {
);
});
testWidgets('Material3 - ElevatedButton repsects Theme shadowColor', (WidgetTester tester) async {
testWidgets('Material3 - ElevatedButton respects Theme shadowColor', (WidgetTester tester) async {
const ColorScheme colorScheme = ColorScheme.light();
const Color shadowColor = Color(0xff000001);
const Color overriddenColor = Color(0xff000002);
@ -334,7 +341,7 @@ void main() {
expect(material.shadowColor, shadowColor);
});
testWidgets('Material2 - ElevatedButton repsects Theme shadowColor', (WidgetTester tester) async {
testWidgets('Material2 - ElevatedButton respects Theme shadowColor', (WidgetTester tester) async {
const ColorScheme colorScheme = ColorScheme.light();
const Color shadowColor = Color(0xff000001);
const Color overriddenColor = Color(0xff000002);
@ -442,4 +449,33 @@ void main() {
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets(
'ElevatedButton icon uses provided ElevatedButtonTheme foregroundColor over default icon color',
(WidgetTester tester) async {
const Color foregroundColor = Color(0xFFFFA500);
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(
elevatedButtonTheme: ElevatedButtonThemeData(
style: ElevatedButton.styleFrom(foregroundColor: foregroundColor),
),
),
home: Material(
child: Center(
child: ElevatedButton.icon(
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
},
);
}

View File

@ -2756,4 +2756,38 @@ void main() {
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets('FilledButton icon uses provided foregroundColor over default icon color', (
WidgetTester tester,
) async {
const Color foregroundColor = Color(0xFFFF1234);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: Column(
children: <Widget>[
FilledButton.icon(
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
FilledButton.tonalIcon(
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
onPressed: () {},
icon: const Icon(Icons.mail),
label: const Text('Button'),
),
],
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
expect(iconStyle(tester, Icons.mail).color, foregroundColor);
});
}

View File

@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
TextStyle iconStyle(WidgetTester tester, IconData icon) {
final RichText iconRichText = tester.widget<RichText>(
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
);
return iconRichText.text.style!;
}
test('FilledButtonThemeData lerp special cases', () {
expect(FilledButtonThemeData.lerp(null, null, 0), null);
const FilledButtonThemeData data = FilledButtonThemeData();
@ -360,4 +367,43 @@ void main() {
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets(
'FilledButton icon uses provided FilledButtonTheme foregroundColor over default icon color',
(WidgetTester tester) async {
const Color foregroundColor = Color(0xFFFFA500);
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(
filledButtonTheme: FilledButtonThemeData(
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
),
),
home: Material(
child: Center(
child: Column(
children: <Widget>[
FilledButton.icon(
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
FilledButton.icon(
onPressed: () {},
icon: const Icon(Icons.mail),
label: const Text('Button'),
),
],
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
expect(iconStyle(tester, Icons.mail).color, foregroundColor);
},
);
}

View File

@ -2854,4 +2854,27 @@ void main() {
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets('OutlinedButton icon uses provided foregroundColor over default icon color', (
WidgetTester tester,
) async {
const Color foregroundColor = Color(0xFFFF1234);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: OutlinedButton.icon(
style: OutlinedButton.styleFrom(foregroundColor: foregroundColor),
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
});
}

View File

@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
TextStyle iconStyle(WidgetTester tester, IconData icon) {
final RichText iconRichText = tester.widget<RichText>(
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
);
return iconRichText.text.style!;
}
test('OutlinedButtonThemeData lerp special cases', () {
expect(OutlinedButtonThemeData.lerp(null, null, 0), null);
const OutlinedButtonThemeData data = OutlinedButtonThemeData();
@ -446,4 +453,33 @@ void main() {
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
},
);
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets(
'OutlinedButton icon uses provided OutlinedButtonTheme foregroundColor over default icon color',
(WidgetTester tester) async {
const Color foregroundColor = Color(0xFFFFA500);
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(
outlinedButtonTheme: OutlinedButtonThemeData(
style: OutlinedButton.styleFrom(foregroundColor: foregroundColor),
),
),
home: Material(
child: Center(
child: OutlinedButton.icon(
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
},
);
}

View File

@ -2547,4 +2547,27 @@ void main() {
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
expect(buttonTopRight.dx, iconTopRight.dx + 16.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets('TextButton icon uses provided foregroundColor over default icon color', (
WidgetTester tester,
) async {
const Color foregroundColor = Color(0xFFFF1234);
await tester.pumpWidget(
MaterialApp(
home: Material(
child: Center(
child: TextButton.icon(
style: TextButton.styleFrom(foregroundColor: foregroundColor),
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
});
}

View File

@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';
void main() {
TextStyle iconStyle(WidgetTester tester, IconData icon) {
final RichText iconRichText = tester.widget<RichText>(
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
);
return iconRichText.text.style!;
}
test('TextButtonTheme lerp special cases', () {
expect(TextButtonThemeData.lerp(null, null, 0), null);
const TextButtonThemeData data = TextButtonThemeData();
@ -447,4 +454,33 @@ void main() {
expect(buttonTopRight.dx, iconTopRight.dx + 16.0);
});
// Regression test for https://github.com/flutter/flutter/issues/162839.
testWidgets(
'TextButton icon uses provided TextButtonThemeData foregroundColor over default icon color',
(WidgetTester tester) async {
const Color foregroundColor = Color(0xFFFFA500);
await tester.pumpWidget(
MaterialApp(
theme: ThemeData(
textButtonTheme: TextButtonThemeData(
style: TextButton.styleFrom(foregroundColor: foregroundColor),
),
),
home: Material(
child: Center(
child: TextButton.icon(
onPressed: () {},
icon: const Icon(Icons.add),
label: const Text('Button'),
),
),
),
),
);
expect(iconStyle(tester, Icons.add).color, foregroundColor);
},
);
}