From fc6079a24f42809607b1bfe713cacd975cb2ceaa Mon Sep 17 00:00:00 2001 From: rami-a <2364772+rami-a@users.noreply.github.com> Date: Mon, 25 Feb 2019 13:47:05 -0500 Subject: [PATCH] [Material] Add the ability to theme trailing app bar actions independently from leading (#28214) * Allow app bar actions to be themed independently if needed * Update SliverAppBar docs that were out of date * Clarify fallback behavior in a comment * Analyzer * Address PR feedback * Address PR feedback * Put back checks on actions icon theme * Address PR feedback * Retrigger CI --- .../flutter/lib/src/material/app_bar.dart | 88 ++++++++++++----- .../lib/src/material/app_bar_theme.dart | 12 +++ .../test/material/app_bar_theme_test.dart | 99 +++++++++++++++++-- 3 files changed, 169 insertions(+), 30 deletions(-) diff --git a/packages/flutter/lib/src/material/app_bar.dart b/packages/flutter/lib/src/material/app_bar.dart index 5d4c5e541e..209753a4bd 100644 --- a/packages/flutter/lib/src/material/app_bar.dart +++ b/packages/flutter/lib/src/material/app_bar.dart @@ -135,10 +135,10 @@ class AppBar extends StatefulWidget implements PreferredSizeWidget { /// and [automaticallyImplyLeading] must not be null. Additionally, if /// [elevation] is specified, it must be non-negative. /// - /// If [backgroundColor], [elevation], [brightness], [iconTheme], or - /// [textTheme] are null, their [AppBarTheme] values will be used. If the - /// corresponding [AppBarTheme] property is null, then the default specified - /// in the property's documentation will be used. + /// If [backgroundColor], [elevation], [brightness], [iconTheme], + /// [actionsIconTheme], or [textTheme] are null, then their [AppBarTheme] + /// values will be used. If the corresponding [AppBarTheme] property is null, + /// then the default specified in the property's documentation will be used. /// /// Typically used in the [Scaffold.appBar] property. AppBar({ @@ -153,6 +153,7 @@ class AppBar extends StatefulWidget implements PreferredSizeWidget { this.backgroundColor, this.brightness, this.iconTheme, + this.actionsIconTheme, this.textTheme, this.primary = true, this.centerTitle, @@ -280,7 +281,7 @@ class AppBar extends StatefulWidget implements PreferredSizeWidget { /// /// The value is non-negative. /// - /// If this property is null then [ThemeData.appBarTheme.elevation] is used, + /// If this property is null, then [ThemeData.appBarTheme.elevation] is used, /// if that is also null, the default value is 4, the appropriate elevation /// for app bars. final double elevation; @@ -288,29 +289,38 @@ class AppBar extends StatefulWidget implements PreferredSizeWidget { /// The color to use for the app bar's material. Typically this should be set /// along with [brightness], [iconTheme], [textTheme]. /// - /// If this property is null then [ThemeData.appBarTheme.color] is used, - /// if that is also null then [ThemeData.primaryColor] is used. + /// If this property is null, then [ThemeData.appBarTheme.color] is used, + /// if that is also null, then [ThemeData.primaryColor] is used. final Color backgroundColor; /// The brightness of the app bar's material. Typically this is set along /// with [backgroundColor], [iconTheme], [textTheme]. /// - /// If this property is null then [ThemeData.appBarTheme.brightness] is used, - /// if that is also null then [ThemeData.primaryColorBrightness] is used. + /// If this property is null, then [ThemeData.appBarTheme.brightness] is used, + /// if that is also null, then [ThemeData.primaryColorBrightness] is used. final Brightness brightness; /// The color, opacity, and size to use for app bar icons. Typically this /// is set along with [backgroundColor], [brightness], [textTheme]. /// - /// If this property is null then [ThemeData.appBarTheme.iconTheme] is used, - /// if that is also null then [ThemeData.primaryIconTheme] is used. + /// If this property is null, then [ThemeData.appBarTheme.iconTheme] is used, + /// if that is also null, then [ThemeData.primaryIconTheme] is used. final IconThemeData iconTheme; + /// The color, opacity, and size to use for the icons that appear in the app + /// bar's [actions]. This should only be used when the [actions] should be + /// themed differently than the icon that appears in the app bar's [leading] + /// widget. + /// + /// If this property is null, then [ThemeData.appBarTheme.actionsIconTheme] is + /// used, if that is also null, then this falls back to [iconTheme]. + final IconThemeData actionsIconTheme; + /// The typographic styles to use for text in the app bar. Typically this is /// set along with [brightness] [backgroundColor], [iconTheme]. /// - /// If this property is null then [ThemeData.appBarTheme.textTheme] is used, - /// if that is also null then [ThemeData.primaryTextTheme] is used. + /// If this property is null, then [ThemeData.appBarTheme.textTheme] is used, + /// if that is also null, then [ThemeData.primaryTextTheme] is used. final TextTheme textTheme; /// Whether this app bar is being displayed at the top of the screen. @@ -400,9 +410,12 @@ class _AppBarState extends State { final bool canPop = parentRoute?.canPop ?? false; final bool useCloseButton = parentRoute is PageRoute && parentRoute.fullscreenDialog; - IconThemeData appBarIconTheme = widget.iconTheme + IconThemeData overallIconTheme = widget.iconTheme ?? appBarTheme.iconTheme ?? themeData.primaryIconTheme; + IconThemeData actionsIconTheme = widget.actionsIconTheme + ?? appBarTheme.actionsIconTheme + ?? overallIconTheme; TextStyle centerStyle = widget.textTheme?.title ?? appBarTheme.textTheme?.title ?? themeData.primaryTextTheme.title; @@ -416,8 +429,11 @@ class _AppBarState extends State { centerStyle = centerStyle.copyWith(color: centerStyle.color.withOpacity(opacity)); if (sideStyle?.color != null) sideStyle = sideStyle.copyWith(color: sideStyle.color.withOpacity(opacity)); - appBarIconTheme = appBarIconTheme.copyWith( - opacity: opacity * (appBarIconTheme.opacity ?? 1.0) + overallIconTheme = overallIconTheme.copyWith( + opacity: opacity * (overallIconTheme.opacity ?? 1.0) + ); + actionsIconTheme = actionsIconTheme.copyWith( + opacity: opacity * (actionsIconTheme.opacity ?? 1.0) ); } @@ -479,6 +495,14 @@ 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 Widget toolbar = NavigationToolbar( leading: leading, middle: title, @@ -493,7 +517,7 @@ class _AppBarState extends State { child: CustomSingleChildLayout( delegate: const _ToolbarContainerLayout(), child: IconTheme.merge( - data: appBarIconTheme, + data: overallIconTheme, child: DefaultTextStyle( style: sideStyle, child: toolbar, @@ -634,6 +658,7 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate { @required this.backgroundColor, @required this.brightness, @required this.iconTheme, + @required this.actionsIconTheme, @required this.textTheme, @required this.primary, @required this.centerTitle, @@ -658,6 +683,7 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate { final Color backgroundColor; final Brightness brightness; final IconThemeData iconTheme; + final IconThemeData actionsIconTheme; final TextTheme textTheme; final bool primary; final bool centerTitle; @@ -716,6 +742,7 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate { backgroundColor: backgroundColor, brightness: brightness, iconTheme: iconTheme, + actionsIconTheme: actionsIconTheme, textTheme: textTheme, primary: primary, centerTitle: centerTitle, @@ -740,6 +767,7 @@ class _SliverAppBarDelegate extends SliverPersistentHeaderDelegate { || backgroundColor != oldDelegate.backgroundColor || brightness != oldDelegate.brightness || iconTheme != oldDelegate.iconTheme + || actionsIconTheme != oldDelegate.actionsIconTheme || textTheme != oldDelegate.textTheme || primary != oldDelegate.primary || centerTitle != oldDelegate.centerTitle @@ -851,6 +879,7 @@ class SliverAppBar extends StatefulWidget { this.backgroundColor, this.brightness, this.iconTheme, + this.actionsIconTheme, this.textTheme, this.primary = true, this.centerTitle, @@ -944,7 +973,9 @@ class SliverAppBar extends StatefulWidget { /// The z-coordinate at which to place this app bar when it is above other /// content. This controls the size of the shadow below the app bar. /// - /// Defaults to 4, the appropriate elevation for app bars. + /// If this property is null, then [ThemeData.appBarTheme.elevation] is used, + /// if that is also null, the default value is 4, the appropriate elevation + /// for app bars. /// /// If [forceElevated] is false, the elevation is ignored when the app bar has /// no content underneath it. For example, if the app bar is [pinned] but no @@ -966,25 +997,37 @@ class SliverAppBar extends StatefulWidget { /// The color to use for the app bar's material. Typically this should be set /// along with [brightness], [iconTheme], [textTheme]. /// - /// Defaults to [ThemeData.primaryColor]. + /// If this property is null, then [ThemeData.appBarTheme.color] is used, + /// if that is also null, then [ThemeData.primaryColor] is used. final Color backgroundColor; /// The brightness of the app bar's material. Typically this is set along /// with [backgroundColor], [iconTheme], [textTheme]. /// - /// Defaults to [ThemeData.primaryColorBrightness]. + /// If this property is null, then [ThemeData.appBarTheme.brightness] is used, + /// if that is also null, then [ThemeData.primaryColorBrightness] is used. final Brightness brightness; /// The color, opacity, and size to use for app bar icons. Typically this /// is set along with [backgroundColor], [brightness], [textTheme]. /// - /// Defaults to [ThemeData.primaryIconTheme]. + /// If this property is null, then [ThemeData.appBarTheme.iconTheme] is used, + /// if that is also null, then [ThemeData.primaryIconTheme] is used. final IconThemeData iconTheme; + /// The color, opacity, and size to use for trailing app bar icons. This + /// should only be used when the trailing icons should be themed differently + /// than the leading icons. + /// + /// If this property is null, then [ThemeData.appBarTheme.actionsIconTheme] is + /// used, if that is also null, then this falls back to [iconTheme]. + final IconThemeData actionsIconTheme; + /// The typographic styles to use for text in the app bar. Typically this is /// set along with [brightness] [backgroundColor], [iconTheme]. /// - /// Defaults to [ThemeData.primaryTextTheme]. + /// If this property is null, then [ThemeData.appBarTheme.textTheme] is used, + /// if that is also null, then [ThemeData.primaryTextTheme] is used. final TextTheme textTheme; /// Whether this app bar is being displayed at the top of the screen. @@ -1148,6 +1191,7 @@ class _SliverAppBarState extends State with TickerProviderStateMix backgroundColor: widget.backgroundColor, brightness: widget.brightness, iconTheme: widget.iconTheme, + actionsIconTheme: widget.actionsIconTheme, textTheme: widget.textTheme, primary: widget.primary, centerTitle: widget.centerTitle, diff --git a/packages/flutter/lib/src/material/app_bar_theme.dart b/packages/flutter/lib/src/material/app_bar_theme.dart index 68ce5d372e..43d5f3c36e 100644 --- a/packages/flutter/lib/src/material/app_bar_theme.dart +++ b/packages/flutter/lib/src/material/app_bar_theme.dart @@ -34,6 +34,7 @@ class AppBarTheme extends Diagnosticable { this.color, this.elevation, this.iconTheme, + this.actionsIconTheme, this.textTheme, }); @@ -57,6 +58,11 @@ class AppBarTheme extends Diagnosticable { /// If null, [AppBar] uses [ThemeData.primaryIconTheme]. final IconThemeData iconTheme; + /// Default value for [AppBar.actionsIconTheme]. + /// + /// If null, [AppBar] uses [ThemeData.primaryIconTheme]. + final IconThemeData actionsIconTheme; + /// Default value for [AppBar.textTheme]. /// /// If null, [AppBar] uses [ThemeData.primaryTextTheme]. @@ -65,6 +71,7 @@ class AppBarTheme extends Diagnosticable { /// Creates a copy of this object with the given fields replaced with the /// new values. AppBarTheme copyWith({ + IconThemeData actionsIconTheme, Brightness brightness, Color color, double elevation, @@ -76,6 +83,7 @@ class AppBarTheme extends Diagnosticable { color: color ?? this.color, elevation: elevation ?? this.elevation, iconTheme: iconTheme ?? this.iconTheme, + actionsIconTheme: actionsIconTheme ?? this.actionsIconTheme, textTheme: textTheme ?? this.textTheme, ); } @@ -97,6 +105,7 @@ class AppBarTheme extends Diagnosticable { color: Color.lerp(a?.color, b?.color, t), elevation: lerpDouble(a?.elevation, b?.elevation, t), iconTheme: IconThemeData.lerp(a?.iconTheme, b?.iconTheme, t), + actionsIconTheme: IconThemeData.lerp(a?.actionsIconTheme, b?.actionsIconTheme, t), textTheme: TextTheme.lerp(a?.textTheme, b?.textTheme, t), ); } @@ -108,6 +117,7 @@ class AppBarTheme extends Diagnosticable { color, elevation, iconTheme, + actionsIconTheme, textTheme, ); } @@ -123,6 +133,7 @@ class AppBarTheme extends Diagnosticable { && typedOther.color == color && typedOther.elevation == elevation && typedOther.iconTheme == iconTheme + && typedOther.actionsIconTheme == actionsIconTheme && typedOther.textTheme == textTheme; } @@ -133,6 +144,7 @@ class AppBarTheme extends Diagnosticable { properties.add(DiagnosticsProperty('color', color, defaultValue: null)); properties.add(DiagnosticsProperty('elevation', elevation, defaultValue: null)); properties.add(DiagnosticsProperty('iconTheme', iconTheme, defaultValue: null)); + properties.add(DiagnosticsProperty('actionsIconTheme', actionsIconTheme, defaultValue: null)); properties.add(DiagnosticsProperty('textTheme', textTheme, defaultValue: null)); } } diff --git a/packages/flutter/test/material/app_bar_theme_test.dart b/packages/flutter/test/material/app_bar_theme_test.dart index 9ff392868f..57b4001b25 100644 --- a/packages/flutter/test/material/app_bar_theme_test.dart +++ b/packages/flutter/test/material/app_bar_theme_test.dart @@ -15,17 +15,25 @@ void main() { testWidgets('Passing no AppBarTheme returns defaults', (WidgetTester tester) async { await tester.pumpWidget(MaterialApp( - home: Scaffold(appBar: AppBar()), + home: Scaffold(appBar: AppBar( + actions: [ + IconButton(icon: const Icon(Icons.share), onPressed: () {}) + ], + )), )); final Material widget = _getAppBarMaterial(tester); final IconTheme iconTheme = _getAppBarIconTheme(tester); + final IconTheme actionsIconTheme = _getAppBarActionsIconTheme(tester); + final RichText actionIconText = _getAppBarIconRichText(tester); final DefaultTextStyle text = _getAppBarText(tester); expect(SystemChrome.latestStyle.statusBarBrightness, Brightness.dark); expect(widget.color, Colors.blue); expect(widget.elevation, 4.0); expect(iconTheme.data, const IconThemeData(color: Colors.white)); + expect(actionsIconTheme.data, const IconThemeData(color: Colors.white)); + expect(actionIconText.text.style.color, Colors.white); expect(text.style, Typography().englishLike.body1.merge(Typography().white.body1)); }); @@ -34,17 +42,26 @@ void main() { await tester.pumpWidget(MaterialApp( theme: ThemeData(appBarTheme: appBarTheme), - home: Scaffold(appBar: AppBar(title: const Text('App Bar Title'),)), + home: Scaffold(appBar: AppBar( + title: const Text('App Bar Title'), + actions: [ + IconButton(icon: const Icon(Icons.share), onPressed: () {}) + ], + )), )); final Material widget = _getAppBarMaterial(tester); final IconTheme iconTheme = _getAppBarIconTheme(tester); + final IconTheme actionsIconTheme = _getAppBarActionsIconTheme(tester); + final RichText actionIconText = _getAppBarIconRichText(tester); final DefaultTextStyle text = _getAppBarText(tester); expect(SystemChrome.latestStyle.statusBarBrightness, appBarTheme.brightness); expect(widget.color, appBarTheme.color); expect(widget.elevation, appBarTheme.elevation); expect(iconTheme.data, appBarTheme.iconTheme); + expect(actionsIconTheme.data, appBarTheme.actionsIconTheme); + expect(actionIconText.text.style.color, appBarTheme.actionsIconTheme.color); expect(text.style, appBarTheme.textTheme.body1); }); @@ -53,6 +70,7 @@ void main() { const Color color = Colors.orange; const double elevation = 3.0; const IconThemeData iconThemeData = IconThemeData(color: Colors.green); + const IconThemeData actionsIconThemeData = IconThemeData(color: Colors.lightBlue); const TextTheme textTheme = TextTheme(title: TextStyle(color: Colors.orange), body1: TextStyle(color: Colors.pink)); final ThemeData themeData = _themeData().copyWith(appBarTheme: _appBarTheme()); @@ -64,38 +82,76 @@ void main() { brightness: brightness, elevation: elevation, iconTheme: iconThemeData, - textTheme: textTheme,) - ), + actionsIconTheme: actionsIconThemeData, + textTheme: textTheme, + actions: [ + IconButton(icon: const Icon(Icons.share), onPressed: () {}) + ], + )), )); final Material widget = _getAppBarMaterial(tester); final IconTheme iconTheme = _getAppBarIconTheme(tester); + final IconTheme actionsIconTheme = _getAppBarActionsIconTheme(tester); + final RichText actionIconText = _getAppBarIconRichText(tester); final DefaultTextStyle text = _getAppBarText(tester); expect(SystemChrome.latestStyle.statusBarBrightness, brightness); expect(widget.color, color); expect(widget.elevation, elevation); expect(iconTheme.data, iconThemeData); + expect(actionsIconTheme.data, actionsIconThemeData); + expect(actionIconText.text.style.color, actionsIconThemeData.color); expect(text.style, textTheme.body1); }); + testWidgets('AppBar icon color takes priority over everything', (WidgetTester tester) async { + const Color color = Colors.lime; + const IconThemeData iconThemeData = IconThemeData(color: Colors.green); + const IconThemeData actionsIconThemeData = IconThemeData(color: Colors.lightBlue); + + final ThemeData themeData = _themeData().copyWith(appBarTheme: _appBarTheme()); + + await tester.pumpWidget(MaterialApp( + theme: themeData, + home: Scaffold(appBar: AppBar( + iconTheme: iconThemeData, + actionsIconTheme: actionsIconThemeData, + actions: [ + IconButton(icon: const Icon(Icons.share), color: color, onPressed: () {}) + ], + )), + )); + + final RichText actionIconText = _getAppBarIconRichText(tester); + expect(actionIconText.text.style.color, color); + }); + testWidgets('AppBarTheme properties take priority over ThemeData properties', (WidgetTester tester) async { final AppBarTheme appBarTheme = _appBarTheme(); final ThemeData themeData = _themeData().copyWith(appBarTheme: _appBarTheme()); await tester.pumpWidget(MaterialApp( theme: themeData, - home: Scaffold(appBar: AppBar()), + home: Scaffold(appBar: AppBar( + actions: [ + IconButton(icon: const Icon(Icons.share), onPressed: () {}) + ], + )), )); final Material widget = _getAppBarMaterial(tester); final IconTheme iconTheme = _getAppBarIconTheme(tester); + final IconTheme actionsIconTheme = _getAppBarActionsIconTheme(tester); + final RichText actionIconText = _getAppBarIconRichText(tester); final DefaultTextStyle text = _getAppBarText(tester); expect(SystemChrome.latestStyle.statusBarBrightness, appBarTheme.brightness); expect(widget.color, appBarTheme.color); expect(widget.elevation, appBarTheme.elevation); expect(iconTheme.data, appBarTheme.iconTheme); + expect(actionsIconTheme.data, appBarTheme.actionsIconTheme); + expect(actionIconText.text.style.color, appBarTheme.actionsIconTheme.color); expect(text.style, appBarTheme.textTheme.body1); }); @@ -104,17 +160,25 @@ void main() { await tester.pumpWidget(MaterialApp( theme: themeData, - home: Scaffold(appBar: AppBar()), + home: Scaffold(appBar: AppBar( + actions: [ + IconButton(icon: const Icon(Icons.share), onPressed: () {}) + ], + )), )); final Material widget = _getAppBarMaterial(tester); final IconTheme iconTheme = _getAppBarIconTheme(tester); + final IconTheme actionsIconTheme = _getAppBarActionsIconTheme(tester); + final RichText actionIconText = _getAppBarIconRichText(tester); final DefaultTextStyle text = _getAppBarText(tester); expect(SystemChrome.latestStyle.statusBarBrightness, themeData.brightness); expect(widget.color, themeData.primaryColor); expect(widget.elevation, 4.0); expect(iconTheme.data, themeData.primaryIconTheme); + expect(actionsIconTheme.data, themeData.primaryIconTheme); + expect(actionIconText.text.style.color, themeData.primaryIconTheme.color); expect(text.style, Typography().englishLike.body1.merge(Typography().white.body1).merge(themeData.primaryTextTheme.body1)); }); } @@ -124,13 +188,15 @@ AppBarTheme _appBarTheme() { const Color color = Colors.lightBlue; const double elevation = 6.0; const IconThemeData iconThemeData = IconThemeData(color: Colors.black); + const IconThemeData actionsIconThemeData = IconThemeData(color: Colors.pink); const TextTheme textTheme = TextTheme(body1: TextStyle(color: Colors.yellow)); return const AppBarTheme( + actionsIconTheme: actionsIconThemeData, brightness: brightness, color: color, elevation: elevation, iconTheme: iconThemeData, - textTheme: textTheme + textTheme: textTheme, ); } @@ -157,10 +223,27 @@ IconTheme _getAppBarIconTheme(WidgetTester tester) { find.descendant( of: find.byType(AppBar), matching: find.byType(IconTheme), - ), + ).first, ); } +IconTheme _getAppBarActionsIconTheme(WidgetTester tester) { + return tester.widget( + find.descendant( + of: find.byType(NavigationToolbar), + matching: find.byType(IconTheme), + ).first, + ); +} + +RichText _getAppBarIconRichText(WidgetTester tester) { + return tester.widget( + find.descendant( + of: find.byType(Icon), + matching: find.byType(RichText), + ).first, + ); +} DefaultTextStyle _getAppBarText(WidgetTester tester) { return tester.widget( find.descendant(