-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Support for elevation based dark theme overlay color in the Material widget #35560
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support for elevation based dark theme overlay color in the Material widget #35560
Conversation
… Also added a `darkThemeOverlay` boolean to turn it off in cases where it not wanted.
- Renamed Material.darkThemeOverlay -> Material.applyDarkThemeElevationOverlay - Added applyDarkThemeElevationOverlay to ThemeData - Modified Material to use the theme's overlay setting if none is provided to the widget. - Turned the overlay off by default
HansMuller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, mostly just some comments about the docs.
rami-a
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
- Removed applyDarkThemeElevationOverlay from Material as it is covered by ThemeData. - Updated documentation based on feedback. - Made just the overlay color animate to get around issues with OutlineButton.
D'oh. Old habits die hard. Thanks. |
- Only apply the overlays to Material widgets using the surface color. - Switched to using the elevation->opacity formula that Android is using. - Renamed applyDarkThemeElevationOverlay -> applyElevationOverlay. - No longer checking against ThemeData.brightness to dark as applyElevationOverlay should only be set to true on dark themes.
rami-a
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
| ), | ||
| child: buildMaterial(color: surfaceColor, elevation: 8.0)) | ||
| ); | ||
| final RenderPhysicalShape model = getShadow(tester); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason the helper function is called getShadow? I was a little confused by this
HansMuller
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| // indicate the level of elevation. | ||
| Color _elevationOverlayColor(BuildContext context, Color background, double elevation) { | ||
| final ThemeData theme = Theme.of(context); | ||
| if (elevation > 0.0 && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conventional formatting for this would be one-line or:
if (elevation > 0.0 &&
theme.applyElevationOverlay &&
background == theme.colorScheme.surface) {
| ChipThemeData chipTheme, | ||
| TargetPlatform platform, | ||
| MaterialTapTargetSize materialTapTargetSize, | ||
| bool applyElevationOverlay, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe applyElevationOverlayColor?
| /// | ||
| /// Material drop shadows can be difficult to see in a dark theme, so the | ||
| /// elevation of a surface should be portrayed with an "overlay" in addition | ||
| /// to the shadow. As the elevation of the component increases, the bright |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the bright => the white
| /// Note: this setting is here to maintain backwards compatibility with | ||
| /// apps that were built before the Material Dark theme specification | ||
| /// was published. New apps should set this to [true] for dark themes | ||
| /// (where [brightness] is [Brightness.dark]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(where => where
| /// | ||
| /// See also: | ||
| /// | ||
| /// * [Material.elevation], which effects how transparent the overlay is. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the overlay => the white overlay
| /// See also: | ||
| /// | ||
| /// * [Material.elevation], which effects how transparent the overlay is. | ||
| /// * [Material.color], which if it matches [colorScheme.surface] an overlay |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- [Material.color], the white color overlay will only be applied of the material's color is [colorScheme.surface].
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much clearer. Thx.
| ), | ||
| ) | ||
| ); | ||
| await tester |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be one line
- Minor formatting and doc changes - Renamed ThemeData.applyElevationOverlay -> ThemeData.applyElevationOverlayColor
…widget (flutter#35560) Added support for a semi-transparent white overlay color for `Material` widgets to indicate their elevation in a dart theme. A new `ThemeData.applyElevationOverlayColor` flag was added to control this behavior, which is off by default for backwards compatibility reasons.
Description
In dark themes, drop shadows used to indicate elevation in Material apps can be difficult to see. To help with this, the Material Dark theme specification, adds a new semi-transparent overlay on surfaces to lighten them according to their elevation. This PR adds support for this feature.
If a
Materialwidget is in a theme where the newThemeData.applyElevationOverlayflag is turned on, and theMaterial.coloris the theme's surface color (i.e.ThemeData.colorScheme.surface), then a semi-transparent white will be composited over theMaterial.colorfor the background of the widget. The level of the transparency follows the values in the specification.By default this feature is turned off to maintain backwards compatibility with apps built before the Dark theme spec was published.
To turn it on for your app, you can adjust your dark themes to have
ThemeData.applyElevationOverlayset totrue.Related Issues
#35494 - A previous attempt at this PR.
Tests
I added a group of new tests in material_test.dart to exercise this feature.
Checklist
///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?