feat: add refactored getColor utility
#1763
Conversation
| } | ||
| }; | ||
|
|
||
| type ColorParameters = { |
There was a problem hiding this comment.
💡 Wondering if there is consumer value in externalizing this type.
There was a problem hiding this comment.
💡 Wondering if there is consumer value in externalizing this type.
Yeah, I was wondering the same thing. Let's hold off for now, as we don't export types for the other utilities. If a consumer wants it, we can release the exported type.
| } else { | ||
| const _colors = scale([PALETTE.white, _hue, PALETTE.black]) | ||
| .correctLightness() | ||
| .colors(PALETTE_SIZE + 2); |
There was a problem hiding this comment.
It might be good to explain that this generates a 14-shade color palette that includes white and black. Further down white and black is excluded to create a standardized 12-shade palette.
| * found at http://www.apache.org/licenses/LICENSE-2.0. | ||
| */ | ||
|
|
||
| import { scale, valid } from 'chroma-js'; |
There was a problem hiding this comment.
While chroma-js is a wonderful library, it does add 15.7k to the bundle size. And because it's not tree-shakeable, even pulling in a couple methods will import the whole package.
In the future, we could attempt to reproduce something close to scale to reduce the JS footprint.
As for valid, we could potentially leverage CSS.support (browser only). Ex:
CSS.support('color', 'red') // true
CSS.support('color', '#fff') //true
CSS.support('color', 'bad') // false
There was a problem hiding this comment.
Great callout @ze-flo. Can I request that we allow this "brand backfill" feature of getColor through for now? I'd like to lean into the RFC process for evaluating efficacy of other approaches (let's take a closer look at color2k, d3-color, etc). Specifically, let's add a scale research task to our v9 dev sequence. That way, we can get this into the hands of early adopters while continuing forward with the v9 effort.
| if (shade === undefined) { | ||
| retVal = _hue; | ||
| } else { | ||
| const _colors = scale([PALETTE.white, _hue, PALETTE.black]) |
There was a problem hiding this comment.
Chaining .mode('lab') could result in more vibrant colors in some cases.
Here's a sandbox highlighting the differences.
There was a problem hiding this comment.
Awesome. Similar to the previous comment, I'd like to continue forward and get some of this comparative analysis into our designer's hands for evaluation. To my eyes, the LAB side has a tendency to yellow/brown-out the colors. We know the designers are getting good leverage out of Leonardo, which defaults to RGB and then adds some OKLCH sugar on top of chroma.js. But we also know that standard CSS advancements are happening fast, and Leonardo only just graduated from alpha after having been dormant for nearly 2 years. Anyway, more to research in this space.
There was a problem hiding this comment.
To my eyes, the LAB side has a tendency to yellow/brown-out the colors.
Good observation! 👁️
defaults to RGB and then adds some OKLCH sugar on top of chroma.js
Very interesting. Thanks for sharing this.
I agree. We can iteratively fine-tune this as we see fit. Great work!
| - `npm start` to launch Storybook with live reload. Use `PACKAGE=dirname npm start` | ||
| (where `dirname` is a package directory name) to limit Storybook launch to the | ||
| given Garden package. |
There was a problem hiding this comment.
Slipped this efficiency tip in since I keep forgetting about it 😅
Description
Provides a fully refactored
getColorutility that will support v9 color redesigns (incl. dark mode) along with supporting tests and Storybook demo.Checklist
design updates will be Garden Designer approved (add the designer as a reviewer)npm start)renders as expected with reversed (RTL) directionrenders as expected with Bedrock CSS (?bedrock)tested for WCAG 2.1 AA accessibility compliancetested in Chrome, Firefox, Safari, and Edge