-
Notifications
You must be signed in to change notification settings - Fork 26.8k
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
Dark mode elevation overlay color is only applied to Material of type canvas, when surface and background colors are equal #90353
Comments
Author's observations can be reproduced with samples provided on flutter doctor -v[✓] Flutter (Channel master, 2.6.0-6.0.pre.147, on macOS 11.5.1 20G80 darwin-arm, locale en-GB)
• Flutter version 2.6.0-6.0.pre.147 at /Users/nexus/dev/sdks/flutters
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision 83dfb2237a (2 days ago), 2021-09-17 23:48:04 -0400
• Engine revision 31792e0340
• Dart version 2.15.0 (build 2.15.0-120.0.dev)
[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
• Android SDK at /Users/nexus/Library/Android/sdk
• Platform android-31, build-tools 31.0.0
• Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)
• All Android licenses accepted.
[✓] Xcode - develop for iOS and macOS (Xcode 12.5.1)
• Xcode at /Applications/Xcode.app/Contents/Developer
• CocoaPods version 1.11.0
[✓] Chrome - develop for the web
• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome
[✓] Android Studio (version 2020.3)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)
[✓] VS Code (version 1.60.1)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 3.26.0
[✓] Connected device (3 available)
• M2007J20CG (mobile) • 5dd3be00 • android-arm64 • Android 11 (API 30)
• macOS (desktop) • macos • darwin-arm64 • macOS 11.5.1 20G80 darwin-arm
• Chrome (web) • chrome • web-javascript • Google Chrome 93.0.4577.82
• No issues found! [✓] Flutter (Channel stable, 2.5.1, on macOS 11.5.1 20G80 darwin-arm, locale en-GB)
• Flutter version 2.5.1 at /Users/nexus/dev/sdks/flutter
• Upstream repository https://github.com/flutter/flutter.git
• Framework revision ffb2ecea52 (3 days ago), 2021-09-17 15:26:33 -0400
• Engine revision b3af521a05
• Dart version 2.14.2
[✓] Android toolchain - develop for Android devices (Android SDK version 31.0.0)
• Android SDK at /Users/nexus/Library/Android/sdk
• Platform android-31, build-tools 31.0.0
• Java binary at: /Applications/Android Studio.app/Contents/jre/Contents/Home/bin/java
• Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)
• All Android licenses accepted.
[✓] Xcode - develop for iOS and macOS
• Xcode at /Applications/Xcode.app/Contents/Developer
• Xcode 12.5.1, Build version 12E507
• CocoaPods version 1.11.0
[✓] Chrome - develop for the web
• Chrome at /Applications/Google Chrome.app/Contents/MacOS/Google Chrome
[✓] Android Studio (version 2020.3)
• Android Studio at /Applications/Android Studio.app/Contents
• Flutter plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/9212-flutter
• Dart plugin can be installed from:
🔨 https://plugins.jetbrains.com/plugin/6351-dart
• Java version OpenJDK Runtime Environment (build 11.0.10+0-b96-7249189)
[✓] VS Code (version 1.60.1)
• VS Code at /Applications/Visual Studio Code.app/Contents
• Flutter extension version 3.26.0
[✓] Connected device (3 available)
• M2007J20CG (mobile) • 5dd3be00 • android-arm64 • Android 11 (API 30)
• macOS (desktop) • macos • darwin-arm64 • macOS 11.5.1 20G80 darwin-arm
• Chrome (web) • chrome • web-javascript • Google Chrome 93.0.4577.82
• No issues found! |
Thanks @danagbemava-nc for the triage. Now let's see what the Flutter Material design experts thinks. Personally I think it is just an oversight that the line with the or comparison with the background color: color.withOpacity(1.0) == theme.colorScheme.background.withOpacity(1.0) is missing from the It is not a common use cases on mobile devices, maybe a bit with tablets. However with the arrival of Material V3 (You) I think this kind of usage and need will increase on mobile devices too. This type of usage, and the demonstrated issue, only becomes visible when using color branded I have no idea how the Flutter Material team intends to support the beautiful color branded themes we have seen in Material V3 (You) examples, but this fix will enable more nuanced support for it, using already existing features in Flutter. |
Thanks @danagbemava-nc for the typo corrections in the issue description. I was looking for some tests for I wrote the passing and failing tests for the The tests show how If a dark theme is created this way, it means that elevated This is because with this theme creation method:
and in a default theme, both these The result is that in dark mode Widgets that use The tests that fail in this test suite, are the last Expect in F) and last Expect in G). In failing part of test F) it is tested what happens when we use Currently In failing part of test G) the same situation is demonstrated by using the older default ThemeData dark = ThemeData(
brightness: Brightness.dark,
applyElevationOverlayColor: true,
); We request theme.colorScheme.background = Colors.grey[700]
theme.colorScheme.surface = Colors.grey[800]
theme.canvasColor = Colors.grey[850]
theme.cardColor = Colors.grey[800];
theme.backgroundColor = Colors.grey[700];
theme.dialogBackgroundColor = Colors.grey[800]; Creating a situation where very few surfaces that are elevated, will actually get any It is correct that the Material Design Guide states that the semi-transparent color should use the
These are still all Material surfaces that can be elevated all needs to get That these elevated Material surfaces in Flutter, refer to their themed surface background colors with some other name than "surface", is an implementation detail in Flutter. For correct
This would ensure correct dark mode elevation behavior when using Tests for
|
SummaryWhy is the current implementation problematic? It prevents creation of more nuanced themed dark modes, where Flutter For a more correct Material Design based dark mode, when using such nuanced dark themes, the dark surfaces with slightly differing primary color blended surface colors, should also get elevation overlay color applied. If current built-in Flutter Widgets use a default themed color, that is not equal to This creates challenges and limitations when using more nuanced dark themes, that use slight variations for different They must all be set to the same color value, as the one used on A more complete
|
Description
In dark mode, when using themes that specify
applyElevationOverlayColor: true
with themes that specify slightly differentbackground
andsurface
colors, the Material dark mode overlay color only gets applied correctly toMaterial
of typecard
, it is not applied toMaterial
of typecanvas
.If we are using the same themed color for
background
andsurface
in a dark theme, then elevation overlay color is applied to bothMaterial
of typecanvas
and of typecard
. This behavior is inconsistent and not expected.Expected behavior would be to get the elevation overlay color also on Material of of type
canvas
, even if its themed color differs slighlt fromsurface
color.This issue applies to
All Flutter channels and all platforms.
Steps to Reproduce
In dark mode, when using theme's that specify
applyElevationOverlayColor: true
themes with defaultbackground
andsurface
colors get elevation overlay color applied correctly, as shown here:OK - Default or Equal background & surface colors => Elevation overlay is OK
OK - Default background and surface - Example code
Issue: Using surface and background colors that are not equal
If we define colors for
background
andsurface
in dark theme mode that are different from each other, we no longer get any elevation color overlay onMaterial
using the background color, thatMaterial
of typecanvas
uses for its color. We still do get it onMaterial
using thesurface
colored material, whichMaterial
of typecard
uses.This is inconsistent with the default built in background colors behavior and also not consistent with expected behavior when using
Material
typecanvas
withelevation
> 0.Typically one gets into to this situation when using
background
andsurface
colors that for example blend in a bit of primary color into the surfaces, but do so at different strength forbackground
andsurface
, for more a nuanced primary color branded designed.Fail example
In this example we create such a theme to demonstrate the issue. With this example we can see that we no longer get any elevation color applied to
Material
of typecanvas
.FAIL - No background overlay elevation color - Example code
Here we see that we get a flat uniform color, despite different elevation, with
Material
of typecanvas
:The example is using slightly different color for
surface
andbackground
in its dark theme:If we change the
background
color to use the same color definition as thesurface
color:then we get overlay color on them too, as seen here:
We expected overlay color also when
background
was using the slightly darker color branded background colorColor.alphaBlend(kDarkPrimary.withAlpha(0x25), const Color(0xff121212)),
it should actually have looked like this:Expected result:
Cause of issue
The lack of overlay color on the
Material
of typecanvas
in the issue case, is caused by theapplyOverlay
color function.The
applyOverlay
function is used by the framework to apply the overlay color if theme specified it should be used, and if we are in dark mode. It also checks if the used color without opacity, is thesurface
color, and only then applies it.However, since
Material
, when it is of typecanvas
, is actually using thecolorScheme.background
as its themed color, it will not get any elevation overlay color if the theme specifiesbackground
andsurface
colors that differs slightly from each other.This situation typically occurs when making color branded surfaces and backgrounds, and one uses slightly different color branded strengths for these colors. This type of color branding is more common on desktop and web designs, but might find more uses on devices with Material V3 as well.
The current behavior in Flutter prevents us from using more nuanced color branded surfaces, where
surface
andbackground
colors use slightly different dark color branded strengths, while retaining the nice elevation overlay onMaterial
of typecanvas
.The current behavior also creates a situation where sometimes themed
Material
of typecanvas
, gets overlay color applied when so defined and as expected, but sometimes it does not. Either it should never get it, or always, whenMaterial
of typecard
gets it as well. Not as now, only "sometimes", ie when thebackground
color happens to be using a themed color, that happens to be equal to themedsurface
color.It is worth noticing that if for some reason Material design spec would call for that
Material
of typecanvas
should never get overlay color applied in dark mode for elevations, then widgets likeDrawer
andBottomNavigationBar
would not get it either. I don't think that is the intent.Solution proposal
Consider adding a check to see if we are using the surface or background color to the
applyOverlay
function:The above modification makes the
applyOverlay
color function work correctly withMaterial
, also when using color branded themes where the themed colorscolorScheme.surface
andcolorScheme.background
differ slightly from each other.The above code change was used to produce the earlier visual presentation of the expected outcome above.
Pull request
I'm willing to submit a pull request with this fix, if this solution is considered acceptable.
Flutter doctor
The text was updated successfully, but these errors were encountered: