-
Notifications
You must be signed in to change notification settings - Fork 2.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
Improved support for design system neutral color #16899
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 5fe109b:
|
Asset size changesSize Auditor did not detect a change in bundle size for any component! Baseline commit: 4e3e43703b6315081f68aa8b61e73c7cc349c4aa (build) |
Perf AnalysisNo significant results to display. All results
Perf Analysis (Fluent)Perf comparison
Perf tests with no regressions
|
}) | ||
public neutralBaseColor: string; | ||
protected neutralBaseColorChanged(oldValue: string, newValue: string): void { | ||
this.neutralPalette = createColorPalette(parseColorHexRGB(newValue)); |
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.
parseColorHexRGB can return null
so you might want to null check the result prior to providing it to createColorPalette
unless you want this to throw
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.
What's our standard here? I thought parseColor was the function that threw if the format of the color is not supported, but it looks now like it just runs a regex. What would be expected behavior if someone passed this something other than a valid hex color? Seems maybe preferable to throw instead of default to grey or something.
e8bc9a9
to
f144c53
Compare
- Added a neutralBaseColor property in the design system - Update neutralPalette and accentPalette when respective baseColor changes - Updated Card to base background color on local neutralPalette - Updated Card stories to illustrate use cases
614d650
to
5fe109b
Compare
🎉 Handy links: |
* Improved support for design system neutral color - Added a neutralBaseColor property in the design system - Update neutralPalette and accentPalette when respective baseColor changes - Updated Card to base background color on local neutralPalette - Updated Card stories to illustrate use cases * Change files * Update api report * Added null checking on parsed color Co-authored-by: Brian Heston <brheston@microsoft.com>
Pull request checklist
$ yarn change
Description of changes
Many use cases involve adjusting the neutral palette color, most commonly on cards or regions that have a special color. Currently this is difficult to do through properties alone, and is confusing even with code. This change makes adjusting the neutral color design system attributes easier as well as updates the Card component to adjust its background color with the neutral tint while respecting the relationship with its container.
Focus areas to test
(optional)