Skip to content

Commit 9bc7df0

Browse files
flutteractionsbotflutteractionsbot
authored
[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: flutter/flutter#162880 Issue: flutter/flutter#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.
1 parent d73fc34 commit 9bc7df0

13 files changed

Lines changed: 283 additions & 7 deletions

‎packages/flutter/lib/src/material/button_style_button.dart‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,16 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
389389
});
390390
}
391391

392+
Color? effectiveIconColor() {
393+
return widgetStyle?.iconColor?.resolve(statesController.value) ??
394+
themeStyle?.iconColor?.resolve(statesController.value) ??
395+
widgetStyle?.foregroundColor?.resolve(statesController.value) ??
396+
themeStyle?.foregroundColor?.resolve(statesController.value) ??
397+
defaultStyle.iconColor?.resolve(statesController.value) ??
398+
// Fallback to foregroundColor if iconColor is null.
399+
defaultStyle.foregroundColor?.resolve(statesController.value);
400+
}
401+
392402
final double? resolvedElevation = resolve<double?>((ButtonStyle? style) => style?.elevation);
393403
final TextStyle? resolvedTextStyle = resolve<TextStyle?>(
394404
(ButtonStyle? style) => style?.textStyle,
@@ -409,7 +419,7 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
409419
final Size? resolvedMinimumSize = resolve<Size?>((ButtonStyle? style) => style?.minimumSize);
410420
final Size? resolvedFixedSize = resolve<Size?>((ButtonStyle? style) => style?.fixedSize);
411421
final Size? resolvedMaximumSize = resolve<Size?>((ButtonStyle? style) => style?.maximumSize);
412-
final Color? resolvedIconColor = resolve<Color?>((ButtonStyle? style) => style?.iconColor);
422+
final Color? resolvedIconColor = effectiveIconColor();
413423
final double? resolvedIconSize = resolve<double?>((ButtonStyle? style) => style?.iconSize);
414424
final BorderSide? resolvedSide = resolve<BorderSide?>((ButtonStyle? style) => style?.side);
415425
final OutlinedBorder? resolvedShape = resolve<OutlinedBorder?>(
@@ -551,10 +561,7 @@ class _ButtonStyleState extends State<ButtonStyleButton> with TickerProviderStat
551561
customBorder: resolvedShape!.copyWith(side: resolvedSide),
552562
statesController: statesController,
553563
child: IconTheme.merge(
554-
data: IconThemeData(
555-
color: resolvedIconColor ?? resolvedForegroundColor,
556-
size: resolvedIconSize,
557-
),
564+
data: IconThemeData(color: resolvedIconColor, size: resolvedIconSize),
558565
child: result,
559566
),
560567
);

‎packages/flutter/lib/src/material/elevated_button.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ class ElevatedButton extends ButtonStyleButton {
161161
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
162162
/// [ButtonStyle.iconSize].
163163
///
164+
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
165+
/// null, the button icon will use the default icon color.
166+
///
164167
/// The button's elevations are defined relative to the [elevation]
165168
/// parameter. The disabled elevation is the same as the parameter
166169
/// value, [elevation] + 2 is used when the button is hovered

‎packages/flutter/lib/src/material/filled_button.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,9 @@ class FilledButton extends ButtonStyleButton {
232232
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
233233
/// [ButtonStyle.iconSize].
234234
///
235+
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
236+
/// null, the button icon will use the default icon color.
237+
///
235238
/// The button's elevations are defined relative to the [elevation]
236239
/// parameter. The disabled elevation is the same as the parameter
237240
/// value, [elevation] + 2 is used when the button is hovered

‎packages/flutter/lib/src/material/outlined_button.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,9 @@ class OutlinedButton extends ButtonStyleButton {
160160
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
161161
/// [ButtonStyle.iconSize].
162162
///
163+
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
164+
/// null, the button icon will use the default icon color.
165+
///
163166
/// If [overlayColor] is specified and its value is [Colors.transparent]
164167
/// then the pressed/focused/hovered highlights are effectively defeated.
165168
/// Otherwise a [WidgetStateProperty] with the same opacities as the

‎packages/flutter/lib/src/material/text_button.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ class TextButton extends ButtonStyleButton {
169169
/// [ButtonStyle.iconColor] and [iconSize] is used to construct
170170
/// [ButtonStyle.iconSize].
171171
///
172+
/// If [iconColor] is null, the button icon will use [foregroundColor]. If [foregroundColor] is also
173+
/// null, the button icon will use the default icon color.
174+
///
172175
/// If [overlayColor] is specified and its value is [Colors.transparent]
173176
/// then the pressed/focused/hovered highlights are effectively defeated.
174177
/// Otherwise a [WidgetStateProperty] with the same opacities as the

‎packages/flutter/test/material/elevated_button_test.dart‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2510,4 +2510,27 @@ void main() {
25102510
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
25112511
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
25122512
});
2513+
2514+
// Regression test for https://github.com/flutter/flutter/issues/162839.
2515+
testWidgets('ElevatedButton icon uses provided foregroundColor over default icon color', (
2516+
WidgetTester tester,
2517+
) async {
2518+
const Color foregroundColor = Color(0xFFFF1234);
2519+
2520+
await tester.pumpWidget(
2521+
MaterialApp(
2522+
home: Material(
2523+
child: Center(
2524+
child: ElevatedButton.icon(
2525+
style: ElevatedButton.styleFrom(foregroundColor: foregroundColor),
2526+
onPressed: () {},
2527+
icon: const Icon(Icons.add),
2528+
label: const Text('Button'),
2529+
),
2530+
),
2531+
),
2532+
),
2533+
);
2534+
expect(iconStyle(tester, Icons.add).color, foregroundColor);
2535+
});
25132536
}

‎packages/flutter/test/material/elevated_button_theme_test.dart‎

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
66
import 'package:flutter_test/flutter_test.dart';
77

88
void main() {
9+
TextStyle iconStyle(WidgetTester tester, IconData icon) {
10+
final RichText iconRichText = tester.widget<RichText>(
11+
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
12+
);
13+
return iconRichText.text.style!;
14+
}
15+
916
test('ElevatedButtonThemeData lerp special cases', () {
1017
expect(ElevatedButtonThemeData.lerp(null, null, 0), null);
1118
const ElevatedButtonThemeData data = ElevatedButtonThemeData();
@@ -263,7 +270,7 @@ void main() {
263270
);
264271
});
265272

266-
testWidgets('Material3 - ElevatedButton repsects Theme shadowColor', (WidgetTester tester) async {
273+
testWidgets('Material3 - ElevatedButton respects Theme shadowColor', (WidgetTester tester) async {
267274
const ColorScheme colorScheme = ColorScheme.light();
268275
const Color shadowColor = Color(0xff000001);
269276
const Color overriddenColor = Color(0xff000002);
@@ -334,7 +341,7 @@ void main() {
334341
expect(material.shadowColor, shadowColor);
335342
});
336343

337-
testWidgets('Material2 - ElevatedButton repsects Theme shadowColor', (WidgetTester tester) async {
344+
testWidgets('Material2 - ElevatedButton respects Theme shadowColor', (WidgetTester tester) async {
338345
const ColorScheme colorScheme = ColorScheme.light();
339346
const Color shadowColor = Color(0xff000001);
340347
const Color overriddenColor = Color(0xff000002);
@@ -442,4 +449,33 @@ void main() {
442449

443450
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
444451
});
452+
453+
// Regression test for https://github.com/flutter/flutter/issues/162839.
454+
testWidgets(
455+
'ElevatedButton icon uses provided ElevatedButtonTheme foregroundColor over default icon color',
456+
(WidgetTester tester) async {
457+
const Color foregroundColor = Color(0xFFFFA500);
458+
459+
await tester.pumpWidget(
460+
MaterialApp(
461+
theme: ThemeData(
462+
elevatedButtonTheme: ElevatedButtonThemeData(
463+
style: ElevatedButton.styleFrom(foregroundColor: foregroundColor),
464+
),
465+
),
466+
home: Material(
467+
child: Center(
468+
child: ElevatedButton.icon(
469+
onPressed: () {},
470+
icon: const Icon(Icons.add),
471+
label: const Text('Button'),
472+
),
473+
),
474+
),
475+
),
476+
);
477+
478+
expect(iconStyle(tester, Icons.add).color, foregroundColor);
479+
},
480+
);
445481
}

‎packages/flutter/test/material/filled_button_test.dart‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2756,4 +2756,38 @@ void main() {
27562756
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
27572757
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
27582758
});
2759+
2760+
// Regression test for https://github.com/flutter/flutter/issues/162839.
2761+
testWidgets('FilledButton icon uses provided foregroundColor over default icon color', (
2762+
WidgetTester tester,
2763+
) async {
2764+
const Color foregroundColor = Color(0xFFFF1234);
2765+
2766+
await tester.pumpWidget(
2767+
MaterialApp(
2768+
home: Material(
2769+
child: Center(
2770+
child: Column(
2771+
children: <Widget>[
2772+
FilledButton.icon(
2773+
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
2774+
onPressed: () {},
2775+
icon: const Icon(Icons.add),
2776+
label: const Text('Button'),
2777+
),
2778+
FilledButton.tonalIcon(
2779+
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
2780+
onPressed: () {},
2781+
icon: const Icon(Icons.mail),
2782+
label: const Text('Button'),
2783+
),
2784+
],
2785+
),
2786+
),
2787+
),
2788+
),
2789+
);
2790+
expect(iconStyle(tester, Icons.add).color, foregroundColor);
2791+
expect(iconStyle(tester, Icons.mail).color, foregroundColor);
2792+
});
27592793
}

‎packages/flutter/test/material/filled_button_theme_test.dart‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@ import 'package:flutter/material.dart';
66
import 'package:flutter_test/flutter_test.dart';
77

88
void main() {
9+
TextStyle iconStyle(WidgetTester tester, IconData icon) {
10+
final RichText iconRichText = tester.widget<RichText>(
11+
find.descendant(of: find.byIcon(icon), matching: find.byType(RichText)),
12+
);
13+
return iconRichText.text.style!;
14+
}
15+
916
test('FilledButtonThemeData lerp special cases', () {
1017
expect(FilledButtonThemeData.lerp(null, null, 0), null);
1118
const FilledButtonThemeData data = FilledButtonThemeData();
@@ -360,4 +367,43 @@ void main() {
360367

361368
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
362369
});
370+
371+
// Regression test for https://github.com/flutter/flutter/issues/162839.
372+
testWidgets(
373+
'FilledButton icon uses provided FilledButtonTheme foregroundColor over default icon color',
374+
(WidgetTester tester) async {
375+
const Color foregroundColor = Color(0xFFFFA500);
376+
377+
await tester.pumpWidget(
378+
MaterialApp(
379+
theme: ThemeData(
380+
filledButtonTheme: FilledButtonThemeData(
381+
style: FilledButton.styleFrom(foregroundColor: foregroundColor),
382+
),
383+
),
384+
home: Material(
385+
child: Center(
386+
child: Column(
387+
children: <Widget>[
388+
FilledButton.icon(
389+
onPressed: () {},
390+
icon: const Icon(Icons.add),
391+
label: const Text('Button'),
392+
),
393+
FilledButton.icon(
394+
onPressed: () {},
395+
icon: const Icon(Icons.mail),
396+
label: const Text('Button'),
397+
),
398+
],
399+
),
400+
),
401+
),
402+
),
403+
);
404+
405+
expect(iconStyle(tester, Icons.add).color, foregroundColor);
406+
expect(iconStyle(tester, Icons.mail).color, foregroundColor);
407+
},
408+
);
363409
}

‎packages/flutter/test/material/outlined_button_test.dart‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2854,4 +2854,27 @@ void main() {
28542854
final Offset iconTopRight = tester.getTopRight(find.byIcon(Icons.add));
28552855
expect(buttonTopRight.dx, iconTopRight.dx + 24.0);
28562856
});
2857+
2858+
// Regression test for https://github.com/flutter/flutter/issues/162839.
2859+
testWidgets('OutlinedButton icon uses provided foregroundColor over default icon color', (
2860+
WidgetTester tester,
2861+
) async {
2862+
const Color foregroundColor = Color(0xFFFF1234);
2863+
2864+
await tester.pumpWidget(
2865+
MaterialApp(
2866+
home: Material(
2867+
child: Center(
2868+
child: OutlinedButton.icon(
2869+
style: OutlinedButton.styleFrom(foregroundColor: foregroundColor),
2870+
onPressed: () {},
2871+
icon: const Icon(Icons.add),
2872+
label: const Text('Button'),
2873+
),
2874+
),
2875+
),
2876+
),
2877+
);
2878+
expect(iconStyle(tester, Icons.add).color, foregroundColor);
2879+
});
28572880
}

0 commit comments

Comments
 (0)