patternfly / patternfly-design

Use this repo to file all new feature or design change requests for the PatternFly project
114 stars 104 forks source link

Update Sketch chart color family #1218

Closed heyethankim closed 1 year ago

heyethankim commented 1 year ago

Currently there is discrepancy between Sketch and PatternFly.com. The Sketch chart color family should be updated according to the PatternFly chart color family definition.

image
heyethankim commented 1 year ago

FYI. @mceledonia, @mcarrano, @KevinFCormier, @Randy424

mcarrano commented 1 year ago

@lboehling can you take a look?

lboehling commented 1 year ago

I'll open a design kit issue for this! Thanks @heyethankim @mcarrano

lboehling commented 1 year ago

@mcoker can i get your insight on this issue? We have 6 shades for our gray chart colors shown in the design kit, and only 5 gray shades for the charts on pf.org. It looks like the additional lighter gray is being used in donut utilization and bullet charts.

Looking at the charts on PF.org, the donut utilization and bullet charts also use a a different lighter value gray than the #EDEDED --pf-chart-color-black-100 used in our design kit. These are the lightest values on those charts: --pf-chart-bullet--qualitative-range--ColorScale--100: #f0f0f0 and --pf-chart-donut--threshold--first--Color: #f0f0f0

Should we add in another step to the gray family on patternfly.com and match the values of the lightest color so that both sketch and pf are using #F0F0F0 for pf-chart-color-black-100? Any other ideas on how to align on these values?

FYI @mcarrano @mceledonia @heyethankim @jhiadlov

mcoker commented 1 year ago

Here is the current black palette that we use, which you can see in https://github.com/patternfly/patternfly/blob/main/src/patternfly/base/_chart-globals.scss.

I included all of the $pf-color-black[...] and $pf-chart-color-black[...] vars, and under each of them are the vars that reference them directly. Some of the $pf-chart-[not "color"] vars listed here may be used as the values for other vars, too.

We have 6 $pf-chart-color-black-[...] vars, and each is set to the value of a global $pf-color-black-[...] var from the default PF palette. It's worth noting that we don't have #ededed (referenced in the sketch screenshot above) or #bbbbbb, #72767B, or #4D5258 (referenced from the .org screenshot above) in the PF global palette, either.

It seems like $pf-chart-color-black-[1-5]00 are what are used for scales. As far as I can tell, I don't see how $pf-chart-color-black-600 is ever used. And $pf-color-black-900 is just used for text I believe, or a solid black (it's the same as our $pf-global--Color--100 primary text color).

So if we're happy with how the charts present, I think we might just reference $pf-chart-color-black-[1-5]00 in sketch and .org (we're already doing that) and correct the hex values so they reflect what I have in the list above?

lboehling commented 1 year ago

That approach sounds good to me @mcoker. thank you!! I can open a website issue to correct those hex values, and fix them in the sketch file.