From c7a3f0fed70d306ba24353e36934210ed1c72d6e Mon Sep 17 00:00:00 2001 From: Qun Cheng <36861262+QuncCccccc@users.noreply.github.com> Date: Thu, 12 Jan 2023 12:39:14 -0800 Subject: [PATCH] IconButtonTheme should be overridden by the AppBar/AppBarTheme's iconTheme and actionsIconTheme (#118216) --- .../flutter/lib/src/material/app_bar.dart | 73 ++++++++- .../flutter/test/material/app_bar_test.dart | 112 +++++++++++++ .../test/material/app_bar_theme_test.dart | 150 ++++++++++++++++++ 3 files changed, 327 insertions(+), 8 deletions(-) diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index dcb0597d8745..71e89aa0c7a8 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -11,12 +11,14 @@ import 'package:flutter/widgets.dart'; import 'app_bar_theme.dart'; import 'back_button.dart'; +import 'button_style.dart'; import 'color_scheme.dart'; import 'colors.dart'; import 'constants.dart'; import 'debug.dart'; import 'flexible_space_bar.dart'; import 'icon_button.dart'; +import 'icon_button_theme.dart'; import 'icons.dart'; import 'material.dart'; import 'material_localizations.dart'; @@ -909,6 +911,7 @@ class _AppBarState extends State { assert(!widget.primary || debugCheckHasMediaQuery(context)); assert(debugCheckHasMaterialLocalizations(context)); final ThemeData theme = Theme.of(context); + final IconButtonThemeData iconButtonTheme = IconButtonTheme.of(context); final AppBarTheme appBarTheme = AppBarTheme.of(context); final AppBarTheme defaults = theme.useMaterial3 ? _AppBarDefaultsM3(context) : _AppBarDefaultsM2(context); final ScaffoldState? scaffold = Scaffold.maybeOf(context); @@ -1019,13 +1022,46 @@ class _AppBarState extends State { } } if (leading != null) { - // Based on the Material Design 3 specs, the leading IconButton should have - // a size of 48x48, and a highlight size of 40x40. Users can also put other - // type of widgets on leading with the original config. if (theme.useMaterial3) { - leading = ConstrainedBox( + if (leading is IconButton) { + final IconButtonThemeData effectiveIconButtonTheme; + + // This comparison is to check if there is a custom [overallIconTheme]. If true, it means that no + // custom [overallIconTheme] is provided, so [iconButtonTheme] is applied. Otherwise, we generate + // a new [IconButtonThemeData] based on the values from [overallIconTheme]. If [iconButtonTheme] only + // has null values, the default [overallIconTheme] will be applied below by [IconTheme.merge] + if (overallIconTheme == defaults.iconTheme) { + effectiveIconButtonTheme = iconButtonTheme; + } else { + // The [IconButton.styleFrom] method is used to generate a correct [overlayColor] based on the [foregroundColor]. + final ButtonStyle leadingIconButtonStyle = IconButton.styleFrom( + foregroundColor: overallIconTheme.color, + iconSize: overallIconTheme.size, + ); + + effectiveIconButtonTheme = IconButtonThemeData( + style: iconButtonTheme.style?.copyWith( + foregroundColor: leadingIconButtonStyle.foregroundColor, + overlayColor: leadingIconButtonStyle.overlayColor, + iconSize: leadingIconButtonStyle.iconSize, + ) + ); + } + + leading = Center( + child: IconButtonTheme( + data: effectiveIconButtonTheme, + child: leading + ) + ); + } + + // Based on the Material Design 3 specs, the leading IconButton should have + // a size of 48x48, and a highlight size of 40x40. Users can also put other + // type of widgets on leading with the original config. + leading = ConstrainedBox( constraints: BoxConstraints.tightFor(width: widget.leadingWidth ?? _kLeadingWidth), - child: leading is IconButton ? Center(child: leading) : leading, + child: leading, ); } else { leading = ConstrainedBox( @@ -1101,9 +1137,30 @@ class _AppBarState extends State { // Allow the trailing actions to have their own theme if necessary. if (actions != null) { - actions = IconTheme.merge( - data: actionsIconTheme, - child: actions, + final IconButtonThemeData effectiveActionsIconButtonTheme; + if (actionsIconTheme == defaults.actionsIconTheme) { + effectiveActionsIconButtonTheme = iconButtonTheme; + } else { + final ButtonStyle actionsIconButtonStyle = IconButton.styleFrom( + foregroundColor: actionsIconTheme.color, + iconSize: actionsIconTheme.size, + ); + + effectiveActionsIconButtonTheme = IconButtonThemeData( + style: iconButtonTheme.style?.copyWith( + foregroundColor: actionsIconButtonStyle.foregroundColor, + overlayColor: actionsIconButtonStyle.overlayColor, + iconSize: actionsIconButtonStyle.iconSize, + ) + ); + } + + actions = IconButtonTheme( + data: effectiveActionsIconButtonTheme, + child: IconTheme.merge( + data: actionsIconTheme, + child: actions, + ), ); } diff --git a/packages/flutter/test/material/app_bar_test.dart b/packages/flutter/test/material/app_bar_test.dart index 7edf5c5acd44..73aa1d12f7b4 100644 --- a/packages/flutter/test/material/app_bar_test.dart +++ b/packages/flutter/test/material/app_bar_test.dart @@ -3195,6 +3195,118 @@ void main() { expect(actionIconColor(), actionsIconColor); expect(actionIconButtonColor(), actionsIconColor); }); + + testWidgets('AppBar.iconTheme should override any IconButtonTheme present in the theme - M3', (WidgetTester tester) async { + final ThemeData themeData = ThemeData( + iconButtonTheme: IconButtonThemeData( + style: IconButton.styleFrom( + foregroundColor: Colors.red, + iconSize: 32.0, + ), + ), + useMaterial3: true, + ); + + const IconThemeData overallIconTheme = IconThemeData(color: Colors.yellow, size: 30.0); + await tester.pumpWidget( + MaterialApp( + theme: themeData, + home: Scaffold( + appBar: AppBar( + iconTheme: overallIconTheme, + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}), + title: const Text('title'), + actions: [ + IconButton(icon: const Icon(Icons.add), onPressed: () {}), + ], + ), + ), + ), + ); + + Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; + double? leadingIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; + Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; + double? actionIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; + + expect(leadingIconButtonColor(), Colors.yellow); + expect(leadingIconButtonSize(), 30.0); + expect(actionIconButtonColor(), Colors.yellow); + expect(actionIconButtonSize(), 30.0); + }); + + testWidgets('AppBar.actionsIconTheme should override any IconButtonTheme present in the theme - M3', (WidgetTester tester) async { + final ThemeData themeData = ThemeData( + iconButtonTheme: IconButtonThemeData( + style: IconButton.styleFrom( + foregroundColor: Colors.red, + iconSize: 32.0, + ), + ), + useMaterial3: true, + ); + + const IconThemeData actionsIconTheme = IconThemeData(color: Colors.yellow, size: 30.0); + await tester.pumpWidget( + MaterialApp( + theme: themeData, + home: Scaffold( + appBar: AppBar( + actionsIconTheme: actionsIconTheme, + title: const Text('title'), + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}), + actions: [ + IconButton(icon: const Icon(Icons.add), onPressed: () {}), + ], + ), + ), + ), + ); + + Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; + double? leadingIconButtonSize() => iconStyle(tester, Icons.menu)?.fontSize; + Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; + double? actionIconButtonSize() => iconStyle(tester, Icons.add)?.fontSize; + + // The leading icon button uses the style in the IconButtonTheme because only actionsIconTheme is provided. + expect(leadingIconButtonColor(), Colors.red); + expect(leadingIconButtonSize(), 32.0); + expect(actionIconButtonColor(), Colors.yellow); + expect(actionIconButtonSize(), 30.0); + }); + + testWidgets('The foregroundColor property of the AppBar overrides any IconButtonTheme present in the theme - M3', (WidgetTester tester) async { + final ThemeData themeData = ThemeData( + iconButtonTheme: IconButtonThemeData( + style: IconButton.styleFrom( + foregroundColor: Colors.red, + ), + ), + useMaterial3: true, + ); + + await tester.pumpWidget( + MaterialApp( + theme: themeData, + home: Scaffold( + appBar: AppBar( + foregroundColor: Colors.purple, + title: const Text('title'), + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}), + actions: [ + IconButton(icon: const Icon(Icons.add), onPressed: () {}), + ], + ), + ), + ), + ); + + Color? leadingIconButtonColor() => iconStyle(tester, Icons.menu)?.color; + Color? actionIconButtonColor() => iconStyle(tester, Icons.add)?.color; + + expect(leadingIconButtonColor(), Colors.purple); + expect(actionIconButtonColor(), Colors.purple); + }); }); testWidgets('AppBarTheme.backwardsCompatibility', (WidgetTester tester) async { diff --git a/packages/flutter/test/material/app_bar_theme_test.dart b/packages/flutter/test/material/app_bar_theme_test.dart index 424dad713ec6..f006bb00e2a2 100644 --- a/packages/flutter/test/material/app_bar_theme_test.dart +++ b/packages/flutter/test/material/app_bar_theme_test.dart @@ -508,6 +508,149 @@ void main() { expect(appBar.surfaceTintColor, Colors.yellow); }); + testWidgets('AppBarTheme.iconTheme.color takes priority over IconButtonTheme.foregroundColor - M3', (WidgetTester tester) async { + const IconThemeData overallIconTheme = IconThemeData(color: Colors.yellow); + await tester.pumpWidget(MaterialApp( + theme: ThemeData( + iconButtonTheme: IconButtonThemeData( + style: IconButton.styleFrom(foregroundColor: Colors.red), + ), + appBarTheme: const AppBarTheme(iconTheme: overallIconTheme), + useMaterial3: true, + ), + home: Scaffold( + appBar: AppBar( + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {},), + actions: [ IconButton(icon: const Icon(Icons.add), onPressed: () {},) ], + title: const Text('Title'), + ), + ), + )); + + final Color? leadingIconButtonColor = _iconStyle(tester, Icons.menu)?.color; + final Color? actionIconButtonColor = _iconStyle(tester, Icons.add)?.color; + + expect(leadingIconButtonColor, overallIconTheme.color); + expect(actionIconButtonColor, overallIconTheme.color); + }); + + testWidgets('AppBarTheme.iconTheme.size takes priority over IconButtonTheme.iconSize - M3', (WidgetTester tester) async { + const IconThemeData overallIconTheme = IconThemeData(size: 30.0); + await tester.pumpWidget(MaterialApp( + theme: ThemeData( + iconButtonTheme: IconButtonThemeData( + style: IconButton.styleFrom(iconSize: 32.0), + ), + appBarTheme: const AppBarTheme(iconTheme: overallIconTheme), + useMaterial3: true, + ), + home: Scaffold( + appBar: AppBar( + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {},), + actions: [ IconButton(icon: const Icon(Icons.add), onPressed: () {},) ], + title: const Text('Title'), + ), + ), + )); + + final double? leadingIconButtonSize = _iconStyle(tester, Icons.menu)?.fontSize; + final double? actionIconButtonSize = _iconStyle(tester, Icons.add)?.fontSize; + + expect(leadingIconButtonSize, overallIconTheme.size); + expect(actionIconButtonSize, overallIconTheme.size); + }); + + + testWidgets('AppBarTheme.actionsIconTheme.color takes priority over IconButtonTheme.foregroundColor - M3', (WidgetTester tester) async { + const IconThemeData actionsIconTheme = IconThemeData(color: Colors.yellow); + final IconButtonThemeData iconButtonTheme = IconButtonThemeData( + style: IconButton.styleFrom(foregroundColor: Colors.red), + ); + + await tester.pumpWidget(MaterialApp( + theme: ThemeData( + iconButtonTheme: iconButtonTheme, + appBarTheme: const AppBarTheme(actionsIconTheme: actionsIconTheme), + useMaterial3: true, + ), + home: Scaffold( + appBar: AppBar( + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {},), + actions: [ IconButton(icon: const Icon(Icons.add), onPressed: () {},) ], + title: const Text('Title'), + ), + ), + )); + + final Color? leadingIconButtonColor = _iconStyle(tester, Icons.menu)?.color; + final Color? actionIconButtonColor = _iconStyle(tester, Icons.add)?.color; + + expect(leadingIconButtonColor, Colors.red); // leading color should come from iconButtonTheme + expect(actionIconButtonColor, actionsIconTheme.color); + }); + + testWidgets('AppBarTheme.actionsIconTheme.size takes priority over IconButtonTheme.iconSize - M3', (WidgetTester tester) async { + const IconThemeData actionsIconTheme = IconThemeData(size: 30.0); + final IconButtonThemeData iconButtonTheme = IconButtonThemeData( + style: IconButton.styleFrom(iconSize: 32.0), + ); + await tester.pumpWidget(MaterialApp( + theme: ThemeData( + iconButtonTheme: iconButtonTheme, + appBarTheme: const AppBarTheme(actionsIconTheme: actionsIconTheme), + useMaterial3: true, + ), + home: Scaffold( + appBar: AppBar( + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {},), + actions: [ IconButton(icon: const Icon(Icons.add), onPressed: () {},) ], + title: const Text('Title'), + ), + ), + )); + + final double? leadingIconButtonSize = _iconStyle(tester, Icons.menu)?.fontSize; + final double? actionIconButtonSize = _iconStyle(tester, Icons.add)?.fontSize; + + expect(leadingIconButtonSize, 32.0); // The size of leading icon button should come from iconButtonTheme + expect(actionIconButtonSize, actionsIconTheme.size); + }); + + testWidgets('AppBarTheme.foregroundColor takes priority over IconButtonTheme.foregroundColor - M3', (WidgetTester tester) async { + final IconButtonThemeData iconButtonTheme = IconButtonThemeData( + style: IconButton.styleFrom(foregroundColor: Colors.red), + ); + const AppBarTheme appBarTheme = AppBarTheme( + foregroundColor: Colors.green, + ); + final ThemeData themeData = ThemeData( + iconButtonTheme: iconButtonTheme, + appBarTheme: appBarTheme, + useMaterial3: true, + ); + + await tester.pumpWidget( + MaterialApp( + theme: themeData, + home: Scaffold( + appBar: AppBar( + title: const Text('title'), + leading: IconButton(icon: const Icon(Icons.menu), onPressed: () {}), + actions: [ + IconButton(icon: const Icon(Icons.add), onPressed: () {}), + ], + ), + ), + ), + ); + + final Color? leadingIconButtonColor = _iconStyle(tester, Icons.menu)?.color; + final Color? actionIconButtonColor = _iconStyle(tester, Icons.add)?.color; + + expect(leadingIconButtonColor, appBarTheme.foregroundColor); + expect(actionIconButtonColor, appBarTheme.foregroundColor); + }); + testWidgets('AppBar uses AppBarTheme.titleSpacing', (WidgetTester tester) async { const double kTitleSpacing = 10; await tester.pumpWidget(MaterialApp( @@ -760,3 +903,10 @@ DefaultTextStyle _getAppBarText(WidgetTester tester) { ).first, ); } + +TextStyle? _iconStyle(WidgetTester tester, IconData icon) { + final RichText iconRichText = tester.widget( + find.descendant(of: find.byIcon(icon).first, matching: find.byType(RichText)), + ); + return iconRichText.text.style; +}