Update AppBar and AppBar Theme to new Theme conventions and latest Material spec #71184
Conversation
flutter-dashboard
bot
added
f: material design
|
Gold has detected about 4 untriaged digest(s) on patchset 7. |
|
Gold has detected about 4 untriaged digest(s) on patchset 8. |
|
What's the difference/priority of |
|
(just a small typo "ApBarTheme Changes" on the last topic) |
|
@HansMuller Thanks a lot! The behavior before this PR causes the only point of hacks singularity in my code. |
|
@HansMuller based on the summary above and quick look at code updates, it looks like it should solve multiple reported issues and challenges people have been having with the AppBar theming. Nice work! 👍🏻 For my use case, a simple test will be if it is now easier (=possible) to create a working MaterialApp ThemeData object that uses a white app bar background color, when the light theme's primary color is of brightness dark. The white or light app bar background color then requires dark/black text and icon theme, which goes against typography color for the primaryTextTheme and the "on" primary color. Or wise versa the case (but much less common and not really often used) in a dark app theme with a colored app bar with a background color of brightness light, that requires dark text/icon color, which goes against the color of the textTheme's typography color in dark themes. Basically these two cases go against needed text and icon colors we get via the overall default light and dark themes as default for the app bar. Due to this conflict, those two cases were a bit complicated to setup via just the standard ThemeData factory before. If this is now possible with these new features, well then this is really golden, based on the specs and the code it certainly looks like it should be. Testing the above use cases will just be a simple case of redoing the things I discussed here #50606 (comment), using these new features to verify that it now works without resorting to creating a new text theme, outside Flutter's text themes that does not get correct localizations applied. I also appreciate that these new theming features brings the app bar theming in line with color scheme based themes and how they behave, which of course is according to the theme consistency roadmap. I will test these new features as soon as they land in master. Generally it looks good and is a very welcome improvement. |
If iconTheme is specified then its color is used, even if foregroundColor is also specified. If iconTheme is not specified, then foregroundColor is the default iconTheme's color. IconThemeData.color can be null so, if iconTheme specified and its color is null, the AppBar would inherit the enclosing IconTheme's color. That's all the same as it ever was. I will make sure that there's a test which confirms as much. |
|
Perfect! |
|
I believe both of the use cases you brought up should be straightforward. In both of the cases that follow, I'm assuming that the AppBar descendant was created with
ThemeData.from(colorScheme: ColorScheme.dark()).copyWith(
appBarTheme: AppBarTheme(
backgroundColor: Colors.white,
foregroundColor: Colors.black,
),
)
ThemeData.from(colorScheme: ColorScheme.dark()).copyWith(
appBarTheme: AppBarTheme(
backgroundColor: Colors.amber,
foregroundColor: Colors.black,
),
) |
|
@HansMuller That's what gathered from looking at the changes too. |
|
It still needs a review however it's substantially similar to an earlier PR, so hopefully the review will not take too long. |
HansMuller
force-pushed
the
appbar_color_scheme_integration
branch
2 times, most recently
from
ab43801 to
e06099b
Compare
darrenaustin
left a comment
darrenaustin
left a comment
There was a problem hiding this comment.
Overall, looks awesome with a lot of cleaned up documentation and more consistent property usage. Just had a few minor questions and caught a typo, but otherwise LGTM!
There was a problem hiding this comment.
Perhaps mention this a temporary setting to manage the migration and that it will be deprecated and removed eventually?
There was a problem hiding this comment.
'is built within' is that referring to the surrounding MaterialApp or the Scaffold? Should we mention where this comes from here?
There was a problem hiding this comment.
What I meant was that the widgets that the AppBar builds are created within an AnnotatedRegion. Will try and make that clearer.
There was a problem hiding this comment.
Maybe break this long line up, or pull the brightness value into its own variable?
There was a problem hiding this comment.
OK, added a variable.
There was a problem hiding this comment.
Should we mark this as deprecated, or are you waiting to get things migrated first?
There was a problem hiding this comment.
Same question about deprecation here.
There was a problem hiding this comment.
I've tried to improve the documentation a little. for the obsolete properties, including this one.
HansMuller
force-pushed
the
appbar_color_scheme_integration
branch
from
e06099b to
0f957e9
Compare
HansMuller
force-pushed
the
appbar_color_scheme_integration
branch
from
0f957e9 to
800a134
Compare
6 participants
AppBar and AppBarTheme are changing to bring them into alignment with the Material spec and to make configuring AppBars relatively straightforward.
This pull request is the first step in a sequence of changes and is not intended to be a "breaking change" https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes. The temporary
backwardsCompatibilityAppBar flag can be set to false (default is true) to enable the changes.AppBar Changes
All of the properties listed here can be overridden with an AppBar widget parameter or by an AppBarTheme parameter with the same name. For example the default AppBar background color is the theme's ColorScheme.primary color https://api.flutter.dev/flutter/material/ColorScheme/primary.html. If AppBar.backgroundColor is non-null, then it's used instead of the default. If AppBar.backgroundColor is null and AppBarTheme.backgroundColor is non-null then AppBarTheme.backgroundColor is used. By default both AppBar.backgroundColor and AppBarThemme.backgroundColor are null.
To keep the following descriptions brief and clear we use "or" to mean
??, "theme" for the value ofTheme.of(context), "colorScheme" for the value ofTheme.of(context).colorScheme, "appbarTheme" for the value ofAppBarTheme.of(context), and brightness for the value ofTheme.of(context).colorScheme.brightness.The app bar's "toolbar" is contains the app bar's leading, title, and actions widgets. The AppBar uses a NavigationToolbar to manage the toolbar's layout.
The following AppBar properties are either new, obsolete, or have changed.
backwardsCompatibility(new temporary property)Tests and apps can opt into the new properties and defaults by setting this property to false.
This property will exist until the Flutter repo and all of the existing tests have been migrated. After that it will be deprecated and eventually removed.
backgroundColor(existing property, updated defaults)The fill color to use for the app bar's [Material].
The default is appBarTheme.backgroundColor or colorScheme.surface if the theme's brightness is dark, colorScheme.primary if the theme's brightness is light.
If backwardsCompatible is true then the default is AppBarTheme.color or theme.primaryColor. By default, theme.primaryColor is darkColors.grey[900] if the theme's brightness dark, theme.colorScheme.primary otherwise.
The new default values bring this property into line with the Material spec. Using AppBarTheme.backgroundColor instead of AppBarTheme.color for the theme's version of the property is for the sake of consistency with all of the other AppBar properties and their AppBarTheme counterparts. This default value's change also avoids the use of theme.primaryColor which confusingly is only the theme's primary color if the overall theme's brightness is light.
foregroundColor(new property)The color of [Text] and [Icon]s within the app bar's toolbar.
The default is appBarTheme.foregroundColor or colorScheme.onSurface if the theme's brightness is dark, colorScheme.onPrimary if the theme's brightness is light.
This property was added to simplify specifying the default text and icon color for the AppBar's toolbar. It is used to configure the default values of textStyle, titleTextStyle, iconTheme, and actionsIconTheme.
toolbarTextStyle(new property)The TextStyle for the leading and actions widgets.
The default is appBarTheme.toolbarTextStyle or theme.textTheme.bodyText2.copyWith(foregroundColor).
If backwardsCompatible is true then the default is appBarTheme.textTheme.bodyText2 or theme.primaryTextTheme.bodyText2.
This property was added to simplify setting the text style of the leading and actions widgets by eliminating the need to create a TextTheme. This change also avoids the use of the confusing theme.primaryTextTheme value whose color is neither the theme's primary ColorScheme color nor the theme's primaryColor.
titleTextStyle(new property)The TextStyle for the title widget.
The default is appBarTheme.titleTextStyle or theme.textTheme?.headline6.copyWith(foregroundColor).
If backwardsCompatible is true then the default is appBarTheme.textTheme.headline6 or theme.primaryTextTheme.headline6.
This property was added to simplify setting the AppBar title's text style by eliminating the need to create a TextTheme. This change also avoids the use of the confusing theme.primaryTextTheme value whose color is neither the theme's primary ColorScheme color nor the theme's primaryColor.
iconTheme(existing property, updated default)The color, opacity, and size for toolbar icons.
The default is theme.iconTheme.copyWith(color: foregroundColor).
If backwards compatible is true then the default value is theme.primaryIconTheme.
This property's default value was changed to simplify the common case of specifying the AppBar's overall text/icon foreground color. This change also avoids the use of the confusing theme.primaryIconTheme value whose color is neither the theme's primary ColorScheme color nor the theme's primaryColor.
actionsIconTheme(existing property, updated defaults)The color, opacity, and size to use for the icons that appear in the app bar's actions.
Default is the iconTheme.
This property's effective default value was changed to simplify the common case of specifying the AppBar's overall text/icon foreground color. This change also avoids the use of the confusing theme.primaryIconTheme value whose color is neither the theme's primary ColorScheme color nor the theme's primaryColor.
brightness(obsolete property)This property is no longer used.
For reasons lost to history the AppBarTheme's
brightnessparameter does not override the color scheme's brightness for the other AppBar properties. It's only ever been used to select one of two values for systemOverlayStyle. Specifying a systemOverlayStyle value is more easily and clearly specified by just providing an explicit value for the new overlayStyle property.systemOverlayStyle(new property)The style to use for the system overlays that overlap the AppBar.
The default is appBarTheme.systemOverlayStyle or SystemUiOverlayStyle.light for dark themes, SystemUiOverlayStyle.dark otherwise.
This property was added because setting this value indirectly, via the (now obsolete) brightness property, was needlessly complicated and less flexible.
AppBarTheme Changes
For more information, see the descriptions of the corresponding AppBar properties above.
backgroundColor(new property)The default value for the app bar's backgroundColor.
brightness(obsolete property)No longer used to select the app bar's systemOverlayStyle.
systemOverlayStyle(new property)The default value for the app bar's systemOverlayStyle.
Fixes
Fixes #70645
Fixes #67921
Fixes #67497
Fixes #61618
Fixes #51820
Fixes #50606