-
Notifications
You must be signed in to change notification settings - Fork 27.5k
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
Add support for generating a full ColorScheme
from a single color
#93463
Conversation
2dfe964
to
478229b
Compare
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
4ad5ca3
to
87689e4
Compare
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 all looks generally good to me. We're going to need a really good migration guide.
/// If any of the optional color parameters are non-null they will be | ||
/// used in place of the generated colors for that field in the resulting | ||
/// color scheme. | ||
/// |
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.
Hopefully a fun interactive color-scheme generator example appears here...
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.
It is on my list, but it would be a lot easier if we had a built in color picker... 😄
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.
You could always use 3 or 4 sliders for a rough and ready color picker. Adding an example in a separate PR would be fine.
brightness: brightness, | ||
); | ||
} | ||
} | ||
|
||
/// Create a ColorScheme based on a purple primary color that matches the | ||
/// [baseline Material color scheme](https://material.io/design/color/the-color-system.html#color-theme-creation). | ||
const ColorScheme.light({ |
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.
As with the basic constructor, best to refer to ColorScheme.fromSeed() here.
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.
Not sure I get why here. If they are looking for a pre-built 'light' color scheme, why would they want to use fromSeed
(other than to make sure they get a full color scheme), and what should they use for the seed color? This kind of points out the problem with not having a M3 version of this and 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.
I was thinking that it would be helpful to get apps off of ColorScheme.light(), since there isn't a perfect migration path that preserves the existing colors and adds new ones are comparable to what you'd get from ColorScheme.fromSeed(). Hopefully we can migrate developers that have used ColorScheme.light() to something like ColorScheme.fromSeed(seedColor: const Color(just the right purple seed color here). So for now we just remind them that the fromSeed factory is almost as trivial to write and so much more useful.
_onInverseSurface = onInverseSurface, | ||
_inversePrimary = inversePrimary, | ||
_primaryVariant = primaryVariant, | ||
_secondaryVariant = secondaryVariant; | ||
|
||
/// Create the recommended dark color scheme that matches the | ||
/// [baseline Material color scheme](https://material.io/design/color/dark-theme.html#ui-application). | ||
const ColorScheme.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.
As with the basic constructor, best to refer to ColorScheme.fromSeed() here.
|
||
/// Generate a [ColorScheme] based around the given `seedColor`. | ||
/// | ||
/// Using the seedColor as a starting point, a set of tonal palettes are |
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 way to generate a high-contrast color scheme from a seed color? It would help to provide a path from all of the existing factories, to ColorScheme.fromSeed().
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.
I didn't see an obvious way to do this with material_color_utilities
, but @guidezpl would know.
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.
Sorry for the late reply.. There isn't
87689e4
to
0b2d93f
Compare
Thanks for the feedback. I have addressed most of your comments with 86c971b, let me know what you think. A migration guide is next on my list, so I took notes from your comments here. Thx. |
0b2d93f
to
86c971b
Compare
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
You proposed breaking this into the package dependency and then the ColorScheme changes. That sounds like a good idea.
/// If any of the optional color parameters are non-null they will be | ||
/// used in place of the generated colors for that field in the resulting | ||
/// color scheme. | ||
/// |
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.
You could always use 3 or 4 sliders for a rough and ready color picker. Adding an example in a separate PR would be fine.
brightness: brightness, | ||
); | ||
} | ||
} | ||
|
||
/// Create a ColorScheme based on a purple primary color that matches the | ||
/// [baseline Material color scheme](https://material.io/design/color/the-color-system.html#color-theme-creation). | ||
const ColorScheme.light({ |
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.
I was thinking that it would be helpful to get apps off of ColorScheme.light(), since there isn't a perfect migration path that preserves the existing colors and adds new ones are comparable to what you'd get from ColorScheme.fromSeed(). Hopefully we can migrate developers that have used ColorScheme.light() to something like ColorScheme.fromSeed(seedColor: const Color(just the right purple seed color here). So for now we just remind them that the fromSeed factory is almost as trivial to write and so much more useful.
…orScheme.fromSeed API.
86c971b
to
954447b
Compare
ColorScheme
from a single color
As part of the migration to Material 3, this PR adds a new APIs to make it easy to generate new Material 3 ColorSchemes from just a single color.
The design doc discusses this in more detail.
The new APIs are as follows:
ColorScheme.fromSeed()
You can construct a complete Material 3 ColorScheme derived from the tones of a single seed color with:
ThemeData(Color colorSchemeSeed, ...)
There is a new
colorSchemeSeed
parameter to theThemeData
factory constructor that will allow the app to generate the theme's color scheme from a seed color:Pre-launch Checklist
///
).