Added support for elevation based dark theme overlay color in the Material widget #35494

darrenaustin · 2019-07-02T23:26:06Z

Description

In order to support the Material Dark theme specification, this PR adds support for lightening the background color of a Material widget when it is in a dark theme based on its elevation.

If the widget is placed in a dark theme (i.e. Theme.of(context).brightness == Brightness.dark) a semi-transparent white will be composited over the Material.color for the background of the widget. The level of the transparency follows the values in the specification.

If this feature is not wanted, it can be turned off by the new darkThemeOverlay boolean property. By default this is turned on, which may lead to some visual changes to existing apps.

Related Issues

Tests

I added a group of three new tests to test these features.

Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Does your PR require Flutter developers to manually update their apps to accommodate your change?

Yes, this is a breaking change (Please read Handling breaking changes). Link to announcement email TBD

… Also added a `darkThemeOverlay` boolean to turn it off in cases where it not wanted.

rami-a

Looks pretty good but this will definitely have visually breaking changes for a lot of people. If anyone is using a dark theme they will have to then opt-out of this.

rami-a · 2019-07-03T13:24:56Z

packages/flutter/lib/src/material/material.dart

+  ///   * [Material.elevation]
+  ///   * [ThemeData.brightness]
+  ///   * <https://material.io/design/color/dark-theme.html>
+  final bool darkThemeOverlay;


I think a verb in the bool name is preferred, something like applyDarkThemeOverlay

Good point. I was also thinking maybe it should include elevation as well. But then applyDarkThemeElevationOverlay is really getting to be a mouthful :).

rami-a · 2019-07-03T13:27:30Z

packages/flutter/lib/src/material/material.dart

+        : elevation <= 12.0 ? 0.14
+        : elevation <= 16.0 ? 0.15
+        : 0.16;
+    final Color whiteOverlay = Color.fromRGBO(255, 255, 255, opacity);


This could be Colors.white.withOpacity(opacity)

Yeah, I thought of that, but didn't want to include 'colors.dart' just for white. Looking at it again, that does look more readable.

darrenaustin · 2019-07-03T20:20:40Z

Looks pretty good but this will definitely have visually breaking changes for a lot of people. If anyone is using a dark theme they will have to then opt-out of this.

Yeah, this concerns me as well. It does seem like it wouldn't be that drastic a visual change, as it only applies a slight white to elevated widgets in a dark theme. That said, I really hate breaking existing apps.

I have thought about making the default off and adding a property to ThemeData that would allow us to control it on at the theme level. That would allow a potential ThemeData.from(colorScheme) do the right thing here for new apps, but existing apps would be unchanged.

But that leads to more cruft and bloat in ThemeData for what may be a small breakage.

Thoughts?

rami-a · 2019-07-03T20:32:13Z

Hmm I actually do like the idea of adding this to ThemeData, despite how bloated it is becoming.

Otherwise you would have to opt-out for each and every widget you create individually. So even if it is a big breaking change, switching one flag in your ThemeData to go back to what you had is a better experience.

I'm still on the fence though about whether it should be opt-in vs opt-out. But I'm definitely in favor of adding the flag to ThemeData to give broader control as well as being able to specify on a per-Material level.

HansMuller · 2019-07-03T22:37:55Z

@rami-a I agree, it seems safest and least disruptive to make this an opt-in ThemeData flag.

* Use the new service protocol message names clearVMTimeline setVMTimelineFlags getVMTimeline getVMTimelineFlags * Fix clearTimeline at another spot.

flutter/engine@ffba2f6...7d3e722 git log ffba2f6..7d3e722 --no-merges --oneline 7d3e722 Roll buildroot to c5a493b. (flutter/engine#9649) 8deeb77 make EmbeddedViewParams a unique ptr (flutter/engine#9640) 7862af5 Roll src/third_party/skia bd3d8d39b3e7..215ff3325230 (4 commits) (flutter/engine#9648) a9ee687 iOS PlatformView clip path (flutter/engine#9478) c19b53c Roll src/third_party/skia effee2065796..bd3d8d39b3e7 (1 commits) (flutter/engine#9647) aeaa5ed Roll src/third_party/skia 2ef826576819..effee2065796 (1 commits) (flutter/engine#9646) 1cc1f04 Roll src/third_party/skia f2c52efce52b..2ef826576819 (2 commits) (flutter/engine#9645) 1c295b2 Roll src/third_party/skia 9fb7fa537d93..f2c52efce52b (1 commits) (flutter/engine#9644) 56a3f41 Android Embedding Refactor PR31: Integrate platform views with the new embedding and the plugin shim. (flutter/engine#9206) 8ac7cbd Fix warning about settings unavailable GN arg build_glfw_shell (flutter/engine#9642) f665717 Roll src/third_party/skia 21a486d04ae0..9fb7fa537d93 (21 commits) (flutter/engine#9639) fd24007 Roll Dart to 67ab3be10d35d994641da167cc806f20a7ffa679 (flutter/engine#9638) 628b174 Fixes a plugin overwrite bug in the plugin shim system. (flutter/engine#9589) 345d350 Added Doxyfile. (flutter/engine#9632) 64b9eef Revert "Roll Dart to 67ab3be10d35d994641da167cc806f20a7ffa679 (#9634)" (flutter/engine#9637) 45e1ad2 Roll Dart to 67ab3be10d35d994641da167cc806f20a7ffa679 (flutter/engine#9634) da6475e Roll fuchsia/sdk/core/mac-amd64 from TRwIGIJLuznLNQzJk17zfboJrkErpe1XvGr0njCwemoC to byM-kyxL4bemlTYNqhKUfJfZoIUrCSzS6XzsFr4n9-MC (flutter/engine#9629) bee12e6 Roll fuchsia/sdk/core/mac-amd64 from MBS_xxNZ_O32DxW1bhOeJisJqYG9JY_FAtSa3ZYupg4C to TRwIGIJLuznLNQzJk17zfboJrkErpe1XvGr0njCwemoC (flutter/engine#9626) fea8fa6 Roll fuchsia/sdk/core/linux-amd64 from Efv1uHDvhLYgT8mvQmdAiJv7HiLix2L_kDRkK6P9ER4C to I2Qe1zxgckzIzMBTztvzeWYsDgcb9Fw-idSI16oIlx8C (flutter/engine#9625) e7f8ca1 [all] add fuchsia.{net.NameLookup,posix.socket.Provider} (flutter/engine#9546) a72a999 Roll fuchsia/sdk/core/mac-amd64 from B-JzM_H7hG5kOIRFjmUqI3uWBAV7no8BnCTknbd2wQ4C to MBS_xxNZ_O32DxW1bhOeJisJqYG9JY_FAtSa3ZYupg4C (flutter/engine#9624) b3fe3e9 Fix a race in the embedder accessibility unit test (flutter/engine#9585) 55818f6 Roll fuchsia/sdk/core/linux-amd64 from xtoj1ola0unTQOetly-V77CgpT6g8L1JUKWDqS8SuAQC to Efv1uHDvhLYgT8mvQmdAiJv7HiLix2L_kDRkK6P9ER4C (flutter/engine#9623) 37a54f2 Roll fuchsia/sdk/core/linux-amd64 from O1niQGtIRghvjuMMCmxevRA1Y6seUn6onOao6Wii9hQC to xtoj1ola0unTQOetly-V77CgpT6g8L1JUKWDqS8SuAQC (flutter/engine#9622) 458a764 Roll fuchsia/sdk/core/mac-amd64 from neDu8hWotIrKCQkxz1ScSJC4NoBva1_YVszr6wMbcZQC to B-JzM_H7hG5kOIRFjmUqI3uWBAV7no8BnCTknbd2wQ4C (flutter/engine#9621) 6e7b4ca Roll fuchsia/sdk/core/linux-amd64 from gMVpYn1cxQ0LeU-TSryUCg2o3rNcf7JWvlOqY6G00MYC to O1niQGtIRghvjuMMCmxevRA1Y6seUn6onOao6Wii9hQC (flutter/engine#9620) e4d354d Roll src/third_party/skia 161f47dfbf6a..21a486d04ae0 (2 commits) (flutter/engine#9619) 2802875 Roll fuchsia/sdk/core/mac-amd64 from tPyg8rqV40gsoXEhDf7VskccnbJGCh4_bZp71YOUinEC to neDu8hWotIrKCQkxz1ScSJC4NoBva1_YVszr6wMbcZQC (flutter/engine#9618) 8e405c1 Roll fuchsia/sdk/core/mac-amd64 from UkCx2sMZsCM-w9nEuQC2TRfnJ7wjJCxsCxSDEx2uPegC to tPyg8rqV40gsoXEhDf7VskccnbJGCh4_bZp71YOUinEC (flutter/engine#9615) 609a980 Fix uninitialized variables and put tests in flutter namespace. (flutter/engine#9613) 9807894 Roll fuchsia/sdk/core/linux-amd64 from ur0ah3sh2atct83EqYX28SjG3fKt-7Driu48GbpdxmMC to gMVpYn1cxQ0LeU-TSryUCg2o3rNcf7JWvlOqY6G00MYC (flutter/engine#9612) 4e344e6 Wire up custom event loop interop for the GLFW embedder. (flutter/engine#9089) 54c6226 Roll fuchsia/sdk/core/mac-amd64 from gyWAjP3BPfhpvHOOwaTusfA8JaGcY_UzjpoIGQnA_W0C to UkCx2sMZsCM-w9nEuQC2TRfnJ7wjJCxsCxSDEx2uPegC (flutter/engine#9611) c3f8cab Roll fuchsia/sdk/core/linux-amd64 from xmxDtsnD0sfj7wxUaiMMhUwh72prBvMcYHY07lgTotcC to ur0ah3sh2atct83EqYX28SjG3fKt-7Driu48GbpdxmMC (flutter/engine#9610) 6f1a748 Document various classes in //flutter/shell/common. (flutter/engine#9591) 4d36530 Roll fuchsia/sdk/core/mac-amd64 from 4PD6FCl4NvKCavA0AVsdKtZPB3G5K72KprkEH0mr064C to gyWAjP3BPfhpvHOOwaTusfA8JaGcY_UzjpoIGQnA_W0C (flutter/engine#9609) cf084bd Roll fuchsia/sdk/core/linux-amd64 from XRYatTY5OvCnQ-5rGC8AnYltKa68CBxmnEK8QO0fpvQC to xmxDtsnD0sfj7wxUaiMMhUwh72prBvMcYHY07lgTotcC (flutter/engine#9607) 8d05400 disable mysterious failing tests (flutter/engine#9608) aa817a9 Roll fuchsia/sdk/core/mac-amd64 from DeTFBSaxMBfZpfK0c7CifGpJbJLOrs3WtuCOaINwmrwC to 4PD6FCl4NvKCavA0AVsdKtZPB3G5K72KprkEH0mr064C (flutter/engine#9606) 4e988e0 Roll fuchsia/sdk/core/linux-amd64 from jXpdljb7CHe8PEpUGGYqGvx6vFar6QRUh6HmpxMoS9sC to XRYatTY5OvCnQ-5rGC8AnYltKa68CBxmnEK8QO0fpvQC (flutter/engine#9605) 84ae36a Roll fuchsia/sdk/core/mac-amd64 from IMr36r3rRLs1G7T5OtCudVMoWjPcjRbYGgyDg7LSxPwC to DeTFBSaxMBfZpfK0c7CifGpJbJLOrs3WtuCOaINwmrwC (flutter/engine#9604) 7135c8d Roll fuchsia/sdk/core/mac-amd64 from qcwCYvuT0PeU97HpvMVmC114EOKwfrk8PdFdZz_m6CIC to IMr36r3rRLs1G7T5OtCudVMoWjPcjRbYGgyDg7LSxPwC (flutter/engine#9602) 8f4df03 Roll fuchsia/sdk/core/linux-amd64 from Y3kUPtfq2frI60zx7VssO-WG733jtODCmnESyz0UGdEC to jXpdljb7CHe8PEpUGGYqGvx6vFar6QRUh6HmpxMoS9sC (flutter/engine#9601) 6c6a0d7 [trace clients] Remove fuchsia.tracelink.Registry (flutter/engine#9593) f931539 Roll fuchsia/sdk/core/mac-amd64 from UsrGfX7Fj96MljgqUFc_A-o_ufsa_FX3eaG14ZSDWMAC to qcwCYvuT0PeU97HpvMVmC114EOKwfrk8PdFdZz_m6CIC (flutter/engine#9600) ...

Marking it as experimental was breaking tests and packaging scripts on stable branches.

CupertinoTextField now supports vertical alignment via the textAlignVertical parameter.

This is necessary for those who wish to comply with static analysis like `strict-raw-types: true`, which includes generated code within their package.

- 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

googlebot · 2019-07-03T23:57:24Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

ℹ️ Googlers: Go here for more info.

darrenaustin · 2019-07-04T00:24:30Z

I seemed to have borked this branch with some unfortunate git shenanigans. I have opened a new one that has a clean history: #35560.

Added support for dark theme overlay colors to the Material widget.…

c482e75

… Also added a `darkThemeOverlay` boolean to turn it off in cases where it not wanted.

googlebot added the cla: yes label Jul 2, 2019

darrenaustin added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. labels Jul 2, 2019

darrenaustin requested review from HansMuller and rami-a and removed request for rami-a July 2, 2019 23:28

rami-a approved these changes Jul 3, 2019

View reviewed changes

a-siva and others added 16 commits July 3, 2019 16:56

Use the new service protocol message names (#35482)

b6b310b

* Use the new service protocol message names clearVMTimeline setVMTimelineFlags getVMTimeline getVMTimelineFlags * Fix clearTimeline at another spot.

mark windows and macos chrome dev mode as flaky (#35495)

090c0c7

Update macrobenchmarks README and app name (#35477)

e7a616a

fix default artifacts to exclude ios and android (#35303)

a4e105d

Mark update-packages as non-experimental (#35467)

619cb30

Marking it as experimental was breaking tests and packaging scripts on stable branches.

CupertinoTextField vertical alignment (#34723)

9065d49

CupertinoTextField now supports vertical alignment via the textAlignVertical parameter.

Re-apply 'Add currentSystemFrameTimeStamp to SchedulerBinding' (#35492)

faf34fe

Include tags in SemanticsNode debug properties (#35491)

461956a

Move usage flutter create tests into memory filesystem. (#35160)

0adb590

fix Selection handles position is off (#34665)

884af76

add APK build time benchmarks (#35481)

a8500f0

Fix RenderFittedBox when child.size.isEmpty (#35487)

d827cdb

Add type to StreamChannel in generated test code. (#35367)

863e984

This is necessary for those who wish to comply with static analysis like `strict-raw-types: true`, which includes generated code within their package.

Do not use ideographic baseline for RenderPargraph baseline (#35493)

c42c796

googlebot removed the cla: yes label Jul 3, 2019

googlebot added the cla: no label Jul 3, 2019

darrenaustin mentioned this pull request Jul 4, 2019

Support for elevation based dark theme overlay color in the Material widget #35560

Merged

8 tasks

darrenaustin closed this Jul 4, 2019

HansMuller changed the title ~~[WIP] Added support for elevation based dark theme overlay color in the Material widget~~ Added support for elevation based dark theme overlay color in the Material widget Aug 15, 2019

darrenaustin deleted the material_dark_overlay branch January 28, 2020 23:18

github-actions bot locked as resolved and limited conversation to collaborators Aug 1, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Added support for elevation based dark theme overlay color in the Material widget #35494

Added support for elevation based dark theme overlay color in the Material widget #35494

Uh oh!

darrenaustin commented Jul 2, 2019

Uh oh!

rami-a left a comment

Uh oh!

rami-a Jul 3, 2019

Uh oh!

darrenaustin Jul 3, 2019

Uh oh!

rami-a Jul 3, 2019

Uh oh!

darrenaustin Jul 3, 2019

Uh oh!

darrenaustin commented Jul 3, 2019

Uh oh!

rami-a commented Jul 3, 2019

Uh oh!

HansMuller commented Jul 3, 2019

Uh oh!

googlebot commented Jul 3, 2019

Uh oh!

darrenaustin commented Jul 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants

Added support for elevation based dark theme overlay color in the Material widget #35494

Added support for elevation based dark theme overlay color in the Material widget #35494

Uh oh!

Conversation

darrenaustin commented Jul 2, 2019

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

rami-a left a comment

Choose a reason for hiding this comment

Uh oh!

rami-a Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

darrenaustin Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

rami-a Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

darrenaustin Jul 3, 2019

Choose a reason for hiding this comment

Uh oh!

darrenaustin commented Jul 3, 2019

Uh oh!

rami-a commented Jul 3, 2019

Uh oh!

HansMuller commented Jul 3, 2019

Uh oh!

googlebot commented Jul 3, 2019

Uh oh!

darrenaustin commented Jul 4, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

14 participants