Skip to content

Conversation

@darrenaustin
Copy link
Contributor

@darrenaustin darrenaustin commented Nov 10, 2021

As part of the migration to Material 3, this PR adds new colors to ColorScheme and deprecates two colors that are no longer being supported:

The added colors include a new ‘tertiary’ group and ‘container’ variants for several of the colors:

  • primaryContainer
  • secondaryContainer
  • tertiary
  • tertiaryContainer
  • surfaceVariant
  • errorContainer
  • onPrimaryContainer
  • onSecondaryContainer
  • onTertiary
  • onTertiaryContainer
  • onSurfaceVariant
  • onErrorContainer
  • outline
  • shadow
  • inverseSurface
  • onInverseSurface
  • inversePrimary

The two colors being deprecated:

  • primaryVariant
  • secondaryVariant

Includes flutter fix data to automatically remove references to the two deprecated colors and rename references to them with primaryContainer and secondaryContainer.

See the design doc for more discussion about this change.

Fixes: #89852

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

@flutter-dashboard flutter-dashboard bot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. f: integration_test The flutter/packages/integration_test plugin c: contributor-productivity Team-specific productivity, code health, technical debt. labels Nov 10, 2021
@google-cla google-cla bot added the cla: yes label Nov 10, 2021
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

Copy link

@jambestwick jambestwick left a comment

Choose a reason for hiding this comment

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

see

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.

LGTM

@darrenaustin
Copy link
Contributor Author

@Piinks if you get a chance, can you review my flutter fix for this to make sure I wasn't holding it wrong?

There are two deprecated colors for ColorScheme: primaryVariant and secondaryVariant. They need to be removed from the 5 constructors and the copyWith method. I then map references of these properties to primaryContainer and secondaryContainer. Does this look right:

f9e61c4

Thx.

@flutter-dashboard flutter-dashboard bot added the c: tech-debt Technical debt, code quality, testing, etc. label Nov 19, 2021
@skia-gold
Copy link

Gold has detected about 1 new digest(s) on patchset 9.
View them at https://flutter-gold.skia.org/cl/github/93427

@Piinks
Copy link
Contributor

Piinks commented Nov 19, 2021

Sure thing! Happy to check it out, thanks for writing a fix! :)

@InfiniteCoder06
Copy link

InfiniteCoder06 commented Dec 12, 2021

@darrenaustin @rami-a There is a wrong in color scheme

background: background ?? Color(palette.neutral.get(90)),

It must be 99 and not 90 according to Material Color design system for light theme.

@rami-a
Copy link
Contributor

rami-a commented Dec 13, 2021

Thanks for the catch @Hacker437 . It looks like light theme's onSurface is incorrect too, it should be 10 not 0. @darrenaustin can we double check these all against the source of truth?

@guidezpl
Copy link
Member

I've made Scheme public, so instead of keeping things in sync, we can just do background: scheme.background

@rami-a
Copy link
Contributor

rami-a commented Dec 13, 2021

Yeah I agree with that @guidezpl so I made #95172 to track the work

naturallymitchell pushed a commit to naturallymitchell/experiences that referenced this pull request May 4, 2022
This helps unblock the roller.

See flutter/flutter#93427

Change-Id: I4a75255c499ea7cdde40845a88c5441b344db67f
Reviewed-on: https://fuchsia-review.googlesource.com/c/experiences/+/613961
Fuchsia-Auto-Submit: Darren Chan <[email protected]>
Reviewed-by: Chase Latta <[email protected]>
Commit-Queue: Darren Chan <[email protected]>
naturallymitchell pushed a commit to naturallymitchell/experiences that referenced this pull request May 4, 2022
Move off of the deprecated fields.

See flutter/flutter#93427.

Change-Id: Ib45ef5ef6c343e591b080187a469ec26de054563
Reviewed-on: https://fuchsia-review.googlesource.com/c/experiences/+/613962
Reviewed-by: Chase Latta <[email protected]>
Reviewed-by: Sanjay Chouksey <[email protected]>
Commit-Queue: Darren Chan <[email protected]>
github-actions bot pushed a commit to gnoliyil/fuchsia that referenced this pull request Oct 11, 2022
This helps unblock the roller.

See flutter/flutter#93427

Change-Id: I4a75255c499ea7cdde40845a88c5441b344db67f
Reviewed-on: https://fuchsia-review.googlesource.com/c/experiences/+/613961
Fuchsia-Auto-Submit: Darren Chan <[email protected]>
Reviewed-by: Chase Latta <[email protected]>
Commit-Queue: Darren Chan <[email protected]>
github-actions bot pushed a commit to gnoliyil/fuchsia that referenced this pull request Oct 11, 2022
Move off of the deprecated fields.

See flutter/flutter#93427.

Change-Id: Ib45ef5ef6c343e591b080187a469ec26de054563
Reviewed-on: https://fuchsia-review.googlesource.com/c/experiences/+/613962
Reviewed-by: Chase Latta <[email protected]>
Reviewed-by: Sanjay Chouksey <[email protected]>
Commit-Queue: Darren Chan <[email protected]>
@github-worst-company
Copy link

Will the new surface colors be added? There are 7 new color roles which act as a replacement for the color tint.
Here are the docs https://m3.material.io/styles/color/the-color-system/color-roles#0abbf8b7-61e1-49ee-9f97-4967beb1e4fe

@guidezpl
Copy link
Member

Yes, the issue for that is #115912

auto-submit bot pushed a commit that referenced this pull request May 19, 2023
…Scheme` (#127124)

This PR is to remove deprecated `primaryVariant` and `secondaryVariant` from framework. These two apis are made obsolete in #93427

Part of #127042
CaseyHillers pushed a commit to CaseyHillers/flutter that referenced this pull request May 24, 2023
…Scheme` (flutter#127124)

This PR is to remove deprecated `primaryVariant` and `secondaryVariant` from framework. These two apis are made obsolete in flutter#93427

Part of flutter#127042
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. c: tech-debt Technical debt, code quality, testing, etc. f: integration_test The flutter/packages/integration_test plugin 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.

Update ColorScheme to support Material 3 changes (additional color scheme slots)

9 participants