firka/flutter
firka/flutter
5
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix buttons with icons ignore provided foregroundColor (#162880)

Browse Source 
Fixes [Flutter 3.27 and later breaks past styling and theming of icon
color on buttons with
icons](https://github.com/flutter/flutter/issues/162839)

### Description

This PR fixes how the icon color is resolved in `ButtonStyleButton`.
This was regressed in https://github.com/flutter/flutter/pull/143501.

### Code Sample (taken from issue)

<details>
<summary>expand to view the code sample</summary> 

```dart
import 'package:flutter/material.dart';

void main() => runApp(const MyApp());

final ButtonStyle filledButtonStyle = FilledButton.styleFrom(
  foregroundColor: Colors.red,
  backgroundColor: Colors.grey,
);

final ButtonStyle elevatedButtonStyle = ElevatedButton.styleFrom(
  foregroundColor: Colors.orange.shade600,
  backgroundColor: Colors.blueGrey,
);

final ButtonStyle outlinedButtonStyle = OutlinedButton.styleFrom(
  foregroundColor: Colors.lightBlue,
);

final ButtonStyle textButtonStyle = TextButton.styleFrom(
  foregroundColor: Colors.green,
);

final ButtonStyle segmentedButtonStyle = SegmentedButton.styleFrom(
  selectedForegroundColor: Colors.tealAccent.shade700,
);

class MyApp extends StatelessWidget {
  const MyApp({super.key});

  @override
  Widget build(BuildContext context) {
    return MaterialApp(
      debugShowCheckedModeBanner: false,
      home: const HomePage(),
      theme: ThemeData(
        filledButtonTheme: FilledButtonThemeData(
          style: filledButtonStyle,
        ),
        elevatedButtonTheme: ElevatedButtonThemeData(
          style: elevatedButtonStyle,
        ),
        outlinedButtonTheme: OutlinedButtonThemeData(
          style: outlinedButtonStyle,
        ),
        textButtonTheme: TextButtonThemeData(
          style: textButtonStyle,
        ),
        segmentedButtonTheme: SegmentedButtonThemeData(
          style: segmentedButtonStyle,
        ),
      ),
    );
  }
}

class HomePage extends StatelessWidget {
  const HomePage({super.key});

  @override
  Widget build(BuildContext context) {
    return Scaffold(
      appBar: AppBar(title: const Text('Button Icon Color Issue')),
      body: Center(
        child: Column(
          children: [
            Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: [
                FilledButton.icon(
                  label: const Text('Filled Themed'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
                const SizedBox(width: 8),
                FilledButton.icon(
                  style: filledButtonStyle.copyWith(
                    foregroundColor: WidgetStateProperty.all(Colors.yellow),
                  ),
                  label: const Text('Filled Styled'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
              ],
            ),
            const SizedBox(height: 8),
            Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: [
                ElevatedButton.icon(
                  label: const Text('Elevated Themed'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
                const SizedBox(width: 8),
                ElevatedButton.icon(
                  style: elevatedButtonStyle.copyWith(
                    foregroundColor: WidgetStateProperty.all(Colors.lime),
                  ),
                  label: const Text('Elevated Styled'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
              ],
            ),
            const SizedBox(height: 8),
            Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: [
                OutlinedButton.icon(
                  label: const Text('Outlined Themed'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
                const SizedBox(width: 8),
                OutlinedButton.icon(
                  style: outlinedButtonStyle.copyWith(
                    foregroundColor: WidgetStateProperty.all(Colors.deepOrange),
                  ),
                  label: const Text('Outlined Styled'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
              ],
            ),
            const SizedBox(height: 8),
            Row(
              mainAxisAlignment: MainAxisAlignment.center,
              children: [
                TextButton.icon(
                  label: const Text('Text Themed'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
                const SizedBox(width: 8),
                TextButton.icon(
                  style: textButtonStyle.copyWith(
                    foregroundColor: WidgetStateProperty.all(Colors.pink),
                  ),
                  label: const Text('Text Styled'),
                  icon: const Icon(Icons.add),
                  onPressed: () {},
                ),
              ],
            ),
            const SizedBox(height: 8),
            const SegmentedButtonShowcase(),
          ],
        ),
      ),
    );
  }
}

class SegmentedButtonShowcase extends StatefulWidget {
  const SegmentedButtonShowcase({this.showOutlinedButton, super.key});
  final bool? showOutlinedButton;

  @override
  State<SegmentedButtonShowcase> createState() =>
      _SegmentedButtonShowcaseState();
}

enum Calendar { day, week, month, year }

class _SegmentedButtonShowcaseState extends State<SegmentedButtonShowcase> {
  Calendar _selected = Calendar.day;

  @override
  Widget build(BuildContext context) {
    return SegmentedButton<Calendar>(
      segments: const <ButtonSegment<Calendar>>[
        ButtonSegment<Calendar>(
          value: Calendar.day,
          label: Text('Day'),
          icon: Icon(Icons.calendar_view_day),
        ),
        ButtonSegment<Calendar>(
          value: Calendar.week,
          icon: Icon(Icons.calendar_view_week),
          label: Text('Week'),
        ),
        ButtonSegment<Calendar>(
          value: Calendar.month,
          icon: Icon(Icons.calendar_view_month),
          label: Text('Mont'),
        ),
        ButtonSegment<Calendar>(
          value: Calendar.year,
          icon: Icon(Icons.calendar_today),
          label: Text('Year'),
        ),
      ],
      selected: <Calendar>{_selected},
      onSelectionChanged: (Set<Calendar> selected) {
        setState(() {
          _selected = selected.first;
        });
      },
    );
  }
}

```

</details>

### Before

<img width="631" alt="Screenshot 2025-02-07 at 17 45 46"
src="https://github.com/user-attachments/assets/d40b1c4b-9952-4e11-8295-8a04bbaa7d74"
/>

### After


<img width="631" alt="Screenshot 2025-02-07 at 17 45 37"
src="https://github.com/user-attachments/assets/d308756e-83f2-42da-bc8d-e958d9f4bec5"
/>

## Pre-launch Checklist

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

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

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Taha Tesser 2025-02-13 10:33:12 +02:00 committed by GitHub
parent bb1ea3fb68
commit 60d0bfcca4
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 283 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

17
packages/flutter/lib/src/material/button_style_button.dart
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,
),
);

3
packages/flutter/lib/src/material/elevated_button.dart
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

3
packages/flutter/lib/src/material/filled_button.dart
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

3
packages/flutter/lib/src/material/outlined_button.dart
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

3
packages/flutter/lib/src/material/text_button.dart
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

23
packages/flutter/test/material/elevated_button_test.dart
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);
});
}

40
packages/flutter/test/material/elevated_button_theme_test.dart
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);
},
);
}

34
packages/flutter/test/material/filled_button_test.dart
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);
});
}

46
packages/flutter/test/material/filled_button_theme_test.dart
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);
},
);
}

23
packages/flutter/test/material/outlined_button_test.dart
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);
});
}

36
packages/flutter/test/material/outlined_button_theme_test.dart
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);
},
);
}

23
packages/flutter/test/material/text_button_test.dart
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);
});
}

36
packages/flutter/test/material/text_button_theme_test.dart
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);
},
);
}