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

Fix `iconTheme` in `AppBar` doesn't apply custom `Colors.white` in the dark mode for M3 (#130574)

fixes [[Material3] AppBar does not respect `foregroundColor` or `iconTheme` for leading and actions in some cases](https://github.com/flutter/flutter/issues/130485)

### Description

- Fix `Colors.white` not applied in dark mode
- Add regression tests
- make `iconStyle` private for consistency

### Before
![Screenshot 2023-07-14 at 18 40 58](https://github.com/flutter/flutter/assets/48603081/a6caffd6-d9a1-407a-aea7-c30047bfe7c7)

### After
![Screenshot 2023-07-14 at 18 41 04](https://github.com/flutter/flutter/assets/48603081/f864da7a-2ff8-46a4-8927-591e50050502)
parent b552a0b3
...@@ -4,8 +4,6 @@ ...@@ -4,8 +4,6 @@
import 'package:flutter/painting.dart'; import 'package:flutter/painting.dart';
import 'colors.dart';
/// The minimum dimension of any interactive region according to Material /// The minimum dimension of any interactive region according to Material
/// guidelines. /// guidelines.
/// ///
...@@ -53,9 +51,11 @@ const EdgeInsets kMaterialListPadding = EdgeInsets.symmetric(vertical: 8.0); ...@@ -53,9 +51,11 @@ const EdgeInsets kMaterialListPadding = EdgeInsets.symmetric(vertical: 8.0);
/// The default color for [ThemeData.iconTheme] when [ThemeData.brightness] is /// The default color for [ThemeData.iconTheme] when [ThemeData.brightness] is
/// [Brightness.dark]. This color is used in [IconButton] to detect whether /// [Brightness.dark]. This color is used in [IconButton] to detect whether
/// [IconTheme.of(context).color] is the same as the default color of [ThemeData.iconTheme]. /// [IconTheme.of(context).color] is the same as the default color of [ThemeData.iconTheme].
const Color kDefaultIconLightColor = Colors.white; // ignore: prefer_const_constructors
final Color kDefaultIconLightColor = Color(0xFFFFFFFF);
/// The default color for [ThemeData.iconTheme] when [ThemeData.brightness] is /// The default color for [ThemeData.iconTheme] when [ThemeData.brightness] is
/// [Brightness.light]. This color is used in [IconButton] to detect whether /// [Brightness.light]. This color is used in [IconButton] to detect whether
/// [IconTheme.of(context).color] is the same as the default color of [ThemeData.iconTheme]. /// [IconTheme.of(context).color] is the same as the default color of [ThemeData.iconTheme].
const Color kDefaultIconDarkColor = Colors.black87; // ignore: prefer_const_constructors
final Color kDefaultIconDarkColor = Color(0xDD000000);
...@@ -963,9 +963,9 @@ class _IconButtonM3 extends ButtonStyleButton { ...@@ -963,9 +963,9 @@ class _IconButtonM3 extends ButtonStyleButton {
bool isIconThemeDefault(Color? color) { bool isIconThemeDefault(Color? color) {
if (isDark) { if (isDark) {
return color == kDefaultIconLightColor; return identical(color, kDefaultIconLightColor);
} }
return color == kDefaultIconDarkColor; return identical(color, kDefaultIconDarkColor);
} }
final bool isDefaultColor = isIconThemeDefault(iconTheme.color); final bool isDefaultColor = isIconThemeDefault(iconTheme.color);
final bool isDefaultSize = iconTheme.size == const IconThemeData.fallback().size; final bool isDefaultSize = iconTheme.size == const IconThemeData.fallback().size;
......
...@@ -543,7 +543,7 @@ class ThemeData with Diagnosticable { ...@@ -543,7 +543,7 @@ class ThemeData with Diagnosticable {
} }
textTheme = defaultTextTheme.merge(textTheme); textTheme = defaultTextTheme.merge(textTheme);
primaryTextTheme = defaultPrimaryTextTheme.merge(primaryTextTheme); primaryTextTheme = defaultPrimaryTextTheme.merge(primaryTextTheme);
iconTheme ??= isDark ? const IconThemeData(color: kDefaultIconLightColor) : const IconThemeData(color: kDefaultIconDarkColor); iconTheme ??= isDark ? IconThemeData(color: kDefaultIconLightColor) : IconThemeData(color: kDefaultIconDarkColor);
primaryIconTheme ??= primaryIsDark ? const IconThemeData(color: Colors.white) : const IconThemeData(color: Colors.black); primaryIconTheme ??= primaryIsDark ? const IconThemeData(color: Colors.white) : const IconThemeData(color: Colors.black);
// COMPONENT THEMES // COMPONENT THEMES
......
...@@ -55,7 +55,7 @@ ScrollController primaryScrollController(WidgetTester tester) { ...@@ -55,7 +55,7 @@ ScrollController primaryScrollController(WidgetTester tester) {
return PrimaryScrollController.of(tester.element(find.byType(CustomScrollView))); return PrimaryScrollController.of(tester.element(find.byType(CustomScrollView)));
} }
TextStyle? iconStyle(WidgetTester tester, IconData icon) { TextStyle? _iconStyle(WidgetTester tester, IconData icon) {
final RichText iconRichText = tester.widget<RichText>( final RichText iconRichText = tester.widget<RichText>(
find.descendant(of: find.byIcon(icon).first, matching: find.byType(RichText)), find.descendant(of: find.byIcon(icon).first, matching: find.byType(RichText)),
); );
...@@ -575,7 +575,7 @@ void main() { ...@@ -575,7 +575,7 @@ void main() {
), ),
); );
Color? iconColor() => iconStyle(tester, Icons.menu)?.color; Color? iconColor() => _iconStyle(tester, Icons.menu)?.color;
final Color iconColorM2 = themeData.colorScheme.onPrimary; final Color iconColorM2 = themeData.colorScheme.onPrimary;
final Color iconColorM3 = themeData.colorScheme.onSurfaceVariant; final Color iconColorM3 = themeData.colorScheme.onSurfaceVariant;
expect(iconColor(), useMaterial3 ? iconColorM3 : iconColorM2); expect(iconColor(), useMaterial3 ? iconColorM3 : iconColorM2);
...@@ -616,7 +616,7 @@ void main() { ...@@ -616,7 +616,7 @@ void main() {
), ),
); );
Color? iconColor() => iconStyle(tester, Icons.menu)?.color; Color? iconColor() => _iconStyle(tester, Icons.menu)?.color;
expect(iconColor(), color); expect(iconColor(), color);
}); });
...@@ -655,7 +655,7 @@ void main() { ...@@ -655,7 +655,7 @@ void main() {
), ),
); );
Color? iconColor() => iconStyle(tester, Icons.menu)?.color; Color? iconColor() => _iconStyle(tester, Icons.menu)?.color;
final Color iconColorM2 = themeData.colorScheme.onPrimary; final Color iconColorM2 = themeData.colorScheme.onPrimary;
final Color iconColorM3 = themeData.colorScheme.onSurfaceVariant; final Color iconColorM3 = themeData.colorScheme.onSurfaceVariant;
expect(iconColor(), useMaterial3 ? iconColorM3 : iconColorM2); expect(iconColor(), useMaterial3 ? iconColorM3 : iconColorM2);
...@@ -696,7 +696,7 @@ void main() { ...@@ -696,7 +696,7 @@ void main() {
), ),
); );
Color? iconColor() => iconStyle(tester, Icons.menu)?.color; Color? iconColor() => _iconStyle(tester, Icons.menu)?.color;
expect(iconColor(), color); expect(iconColor(), color);
}); });
...@@ -3123,8 +3123,8 @@ void main() { ...@@ -3123,8 +3123,8 @@ void main() {
expect(actionIconTheme.color, foregroundColor); expect(actionIconTheme.color, foregroundColor);
// Test icon color // Test icon color
Color? leadingIconColor() => iconStyle(tester, Icons.add_circle)?.color; Color? leadingIconColor() => _iconStyle(tester, Icons.add_circle)?.color;
Color? actionIconColor() => iconStyle(tester, Icons.ac_unit)?.color; Color? actionIconColor() => _iconStyle(tester, Icons.ac_unit)?.color;
expect(leadingIconColor(), foregroundColor); expect(leadingIconColor(), foregroundColor);
expect(actionIconColor(), foregroundColor); expect(actionIconColor(), foregroundColor);
...@@ -3156,8 +3156,8 @@ void main() { ...@@ -3156,8 +3156,8 @@ void main() {
Color textColor() { Color textColor() {
return tester.renderObject<RenderParagraph>(find.text('title')).text.style!.color!; return tester.renderObject<RenderParagraph>(find.text('title')).text.style!.color!;
} }
Color? leadingIconColor() => iconStyle(tester, Icons.add_circle)?.color; Color? leadingIconColor() => _iconStyle(tester, Icons.add_circle)?.color;
Color? actionIconColor() => iconStyle(tester, Icons.ac_unit)?.color; Color? actionIconColor() => _iconStyle(tester, Icons.ac_unit)?.color;
// M2 default color are onPrimary, and M3 has onSurface for leading and title, // M2 default color are onPrimary, and M3 has onSurface for leading and title,
// onSurfaceVariant for actions. // onSurfaceVariant for actions.
...@@ -3191,9 +3191,9 @@ void main() { ...@@ -3191,9 +3191,9 @@ void main() {
), ),
); );
Color? leadingIconColor() => iconStyle(tester, Icons.add_circle)?.color; Color? leadingIconColor() => _iconStyle(tester, Icons.add_circle)?.color;
Color? actionIconColor() => iconStyle(tester, Icons.ac_unit)?.color; Color? actionIconColor() => _iconStyle(tester, Icons.ac_unit)?.color;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconColor(), iconColor); expect(leadingIconColor(), iconColor);
expect(actionIconColor(), iconColor); expect(actionIconColor(), iconColor);
...@@ -3227,9 +3227,9 @@ void main() { ...@@ -3227,9 +3227,9 @@ void main() {
), ),
); );
Color? leadingIconColor() => iconStyle(tester, Icons.add_circle)?.color; Color? leadingIconColor() => _iconStyle(tester, Icons.add_circle)?.color;
Color? actionIconColor() => iconStyle(tester, Icons.ac_unit)?.color; Color? actionIconColor() => _iconStyle(tester, Icons.ac_unit)?.color;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconColor(), themeData.colorScheme.onSurface); expect(leadingIconColor(), themeData.colorScheme.onSurface);
expect(actionIconColor(), actionsIconColor); expect(actionIconColor(), actionsIconColor);
...@@ -3265,9 +3265,9 @@ void main() { ...@@ -3265,9 +3265,9 @@ void main() {
), ),
); );
Color? leadingIconColor() => iconStyle(tester, Icons.add_circle)?.color; Color? leadingIconColor() => _iconStyle(tester, Icons.add_circle)?.color;
Color? actionIconColor() => iconStyle(tester, Icons.ac_unit)?.color; Color? actionIconColor() => _iconStyle(tester, Icons.ac_unit)?.color;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconColor(), overallIconColor); expect(leadingIconColor(), overallIconColor);
expect(actionIconColor(), actionsIconColor); expect(actionIconColor(), actionsIconColor);
...@@ -3302,10 +3302,10 @@ void main() { ...@@ -3302,10 +3302,10 @@ void main() {
), ),
); );
Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
double? leadingIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; double? leadingIconButtonSize() => _iconStyle(tester, Icons.menu)?.fontSize;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
double? actionIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; double? actionIconButtonSize() => _iconStyle(tester, Icons.menu)?.fontSize;
expect(leadingIconButtonColor(), Colors.yellow); expect(leadingIconButtonColor(), Colors.yellow);
expect(leadingIconButtonSize(), 30.0); expect(leadingIconButtonSize(), 30.0);
...@@ -3338,8 +3338,8 @@ void main() { ...@@ -3338,8 +3338,8 @@ void main() {
), ),
); );
Color? leadingIconButtonColor() => iconStyle(tester, Icons.arrow_back)?.color; Color? leadingIconButtonColor() => _iconStyle(tester, Icons.arrow_back)?.color;
double? leadingIconButtonSize() => iconStyle(tester, Icons.arrow_back)?.fontSize; double? leadingIconButtonSize() => _iconStyle(tester, Icons.arrow_back)?.fontSize;
expect(leadingIconButtonColor(), Colors.yellow); expect(leadingIconButtonColor(), Colors.yellow);
expect(leadingIconButtonSize(), 30.0); expect(leadingIconButtonSize(), 30.0);
...@@ -3374,10 +3374,10 @@ void main() { ...@@ -3374,10 +3374,10 @@ void main() {
), ),
); );
Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
double? leadingIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; double? leadingIconButtonSize() => _iconStyle(tester, Icons.menu)?.fontSize;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
double? actionIconButtonSize() => iconStyle(tester, Icons.add)?.fontSize; double? actionIconButtonSize() => _iconStyle(tester, Icons.add)?.fontSize;
// The leading icon button uses the style in the IconButtonTheme because only actionsIconTheme is provided. // The leading icon button uses the style in the IconButtonTheme because only actionsIconTheme is provided.
expect(leadingIconButtonColor(), Colors.red); expect(leadingIconButtonColor(), Colors.red);
...@@ -3413,8 +3413,8 @@ void main() { ...@@ -3413,8 +3413,8 @@ void main() {
), ),
); );
Color? actionIconButtonColor() => iconStyle(tester, Icons.arrow_back)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.arrow_back)?.color;
double? actionIconButtonSize() => iconStyle(tester, Icons.arrow_back)?.fontSize; double? actionIconButtonSize() => _iconStyle(tester, Icons.arrow_back)?.fontSize;
expect(actionIconButtonColor(), Colors.yellow); expect(actionIconButtonColor(), Colors.yellow);
expect(actionIconButtonSize(), 30.0); expect(actionIconButtonSize(), 30.0);
...@@ -3446,12 +3446,124 @@ void main() { ...@@ -3446,12 +3446,124 @@ void main() {
), ),
); );
Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.purple); expect(leadingIconButtonColor(), Colors.purple);
expect(actionIconButtonColor(), Colors.purple); expect(actionIconButtonColor(), Colors.purple);
}); });
// This is a regression test for https://github.com/flutter/flutter/issues/130485.
testWidgets('Material3 - AppBar.iconTheme is correctly applied in dark mode', (WidgetTester tester) async {
final ThemeData themeData = ThemeData(
colorScheme: const ColorScheme.dark().copyWith(onSurfaceVariant: Colors.red),
useMaterial3: true,
);
await tester.pumpWidget(
MaterialApp(
theme: themeData,
home: Scaffold(
appBar: AppBar(
iconTheme: const IconThemeData(color: Colors.white),
leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}),
actions: <Widget>[
IconButton(icon: const Icon(Icons.add), onPressed: () {}),
],
),
),
),
);
Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.white);
expect(actionIconButtonColor(), Colors.white);
});
// This is a regression test for https://github.com/flutter/flutter/issues/130485.
testWidgets('Material3 - AppBar.foregroundColor is correctly applied in dark mode', (WidgetTester tester) async {
final ThemeData themeData = ThemeData(
colorScheme: const ColorScheme.dark().copyWith(onSurfaceVariant: Colors.red),
useMaterial3: true,
);
await tester.pumpWidget(
MaterialApp(
theme: themeData,
home: Scaffold(
appBar: AppBar(
foregroundColor: Colors.white,
leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}),
actions: <Widget>[
IconButton(icon: const Icon(Icons.add), onPressed: () {}),
],
),
),
),
);
Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.white);
expect(actionIconButtonColor(), Colors.white);
});
// This is a regression test for https://github.com/flutter/flutter/issues/130485.
testWidgets('Material3 - AppBar.iconTheme is correctly applied in light mode', (WidgetTester tester) async {
final ThemeData themeData = ThemeData(
colorScheme: const ColorScheme.light().copyWith(onSurfaceVariant: Colors.red),
useMaterial3: true,
);
await tester.pumpWidget(
MaterialApp(
theme: themeData,
home: Scaffold(
appBar: AppBar(
iconTheme: const IconThemeData(color: Colors.black87),
leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}),
actions: <Widget>[
IconButton(icon: const Icon(Icons.add), onPressed: () {}),
],
),
),
),
);
Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.black87);
expect(actionIconButtonColor(), Colors.black87);
});
// This is a regression test for https://github.com/flutter/flutter/issues/130485.
testWidgets('Material3 - AppBar.foregroundColor is correctly applied in light mode', (WidgetTester tester) async {
final ThemeData themeData = ThemeData(
colorScheme: const ColorScheme.light().copyWith(onSurfaceVariant: Colors.red),
useMaterial3: true,
);
await tester.pumpWidget(
MaterialApp(
theme: themeData,
home: Scaffold(
appBar: AppBar(
foregroundColor: Colors.black87,
leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}),
actions: <Widget>[
IconButton(icon: const Icon(Icons.add), onPressed: () {}),
],
),
),
),
);
Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.black87);
expect(actionIconButtonColor(), Colors.black87);
});
}); });
group('MaterialStateColor scrolledUnder', () { group('MaterialStateColor scrolledUnder', () {
......
...@@ -1055,6 +1055,34 @@ void main() { ...@@ -1055,6 +1055,34 @@ void main() {
// one is used. This results in a difference for doubles in debugFillProperties between // one is used. This results in a difference for doubles in debugFillProperties between
// the web and the rest of Flutter's target platforms. // the web and the rest of Flutter's target platforms.
}, skip: kIsWeb); // https://github.com/flutter/flutter/issues/87364 }, skip: kIsWeb); // https://github.com/flutter/flutter/issues/87364
// This is a regression test for https://github.com/flutter/flutter/issues/130485.
testWidgets('Material3 - AppBarTheme.iconTheme correctly applies custom white color in dark mode', (WidgetTester tester) async {
final ThemeData themeData = ThemeData(
useMaterial3: true,
brightness: Brightness.dark,
appBarTheme: const AppBarTheme(iconTheme: IconThemeData(color: Colors.white)),
);
await tester.pumpWidget(
MaterialApp(
theme: themeData,
home: Scaffold(
appBar: AppBar(
leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}),
actions: <Widget>[
IconButton(icon: const Icon(Icons.add), onPressed: () {}),
],
),
),
),
);
Color? leadingIconButtonColor() => _iconStyle(tester, Icons.menu)?.color;
Color? actionIconButtonColor() => _iconStyle(tester, Icons.add)?.color;
expect(leadingIconButtonColor(), Colors.white);
expect(actionIconButtonColor(), Colors.white);
});
} }
AppBarTheme _appBarTheme() { AppBarTheme _appBarTheme() {
......
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