Skip to content
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

Updated ColorScheme.dark() colors to match the Material Dark theme specification #36106

Merged
merged 3 commits into from
Jul 19, 2019

Conversation

darrenaustin
Copy link
Contributor

@darrenaustin darrenaustin commented Jul 12, 2019

Description

According the the Material Dark theme spec the default dark color scheme's primaryVariant, surface, background and error colors should be:

Primary Variant: #3700B3 
Surface: #121212
Background: #121212
Error: #CF6679

Which are different than the current implementation of ColorScheme.dark(). As this is supposed to be the default dark theme for applications to use, it should use the recommended colors.

This PR updates ColorScheme.dark() to the match the spec colors.

Related Issues

Fixes: #36021

Tests

Added a new color_scheme_test.dart that checks the light and dark scheme against the spec values.

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?

If you depended on ColorScheme.dark() to provide the current implementation's colors, then you can just use copyWith to set those colors back to the original non-spec colors using the following:

ColorScheme.dark().copyWith(
  primaryVariant: const Color(0xff4b01d0),
  surface: Colors.black,
  background: Colors.black,
  error: const Color(0xffb00020),
)

See the announcement email for more details.

@darrenaustin darrenaustin added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. c: API break Backwards-incompatible API changes labels Jul 12, 2019
@darrenaustin darrenaustin requested a review from HansMuller July 12, 2019 22:47
@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. While there are exceptions to this rule, if this patch modifies code it is probably not an exception.

Reviewers: Read the Tree Hygine page and make sure this patch meets those guidelines before LGTMing.

/cc @dnfield

@darrenaustin darrenaustin force-pushed the dark_color_scheme_update branch from 54fd73a to f2825e5 Compare July 13, 2019 00:24
@darrenaustin darrenaustin changed the title Updated ColorScheme.dark() surface and background color to match spec. Updated ColorScheme.dark() colors to match the Material Dark theme specification Jul 13, 2019
Copy link
Contributor

@HansMuller HansMuller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a pair of (trivial) tests that verify the default light and dark color scheme colors per the spec.

Copy link
Contributor

@rami-a rami-a left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@darrenaustin darrenaustin merged commit 252491f into flutter:master Jul 19, 2019
@darrenaustin darrenaustin deleted the dark_color_scheme_update branch July 19, 2019 18:31
johnsonmh pushed a commit to johnsonmh/flutter that referenced this pull request Jul 30, 2019
…ecification (flutter#36106)

Updated ColorScheme.dark() primaryVariant, surface, background and error
colors to match the spec.
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
c: API break Backwards-incompatible API changes f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ColorScheme.dark() scheme doesn't match the current Material Dark theme spec
5 participants