Esri / calcite-colors

Esri's official color documentation repository that is leveraged by Calcite and all other Esri initiatives.
https://esri.github.io/calcite-colors/
Other
26 stars 6 forks source link

Charts: Add chart color set for teams to pull in colors for their charts. #54

Closed julio8a closed 3 years ago

julio8a commented 3 years ago

Background about this PR:

The section below focuses on two types of color sets: Quantitative (gradient ramps) and Qualitative (Categorical)

image image

It's important to note, that these color values are for charts only and should not be leveraged for UI design, which we will document

@driskull, I just copied over the dashboard file and I think I created the json file correctly 🀞 (designer here). I'm not sure if we need the .ts or scss or if anything needs to get restructured/reformatted. Really open to any feedback as this is really rough state PR.

Please let me know your thoughts and feedback.

CC'ing folks that were involved in a meeting back on July 8th. @mitc7862, @bstifle, @subgan82, @rmstinson, @pemberdom

53

kyle-03674 commented 3 years ago

@julio8a I see what you're saying and this makes sense to me.
One detail on the categorical ramps...If we use the vibrant set I think this one, "vibrant-rainbow," could get closer to the original. ... I'm guessing your screenshot was more a Proof of Concept. image

julio8a commented 3 years ago

@kyle-03674, I think I just had an old set of vibrant colors where the VR didn't exist? or I must have overlooked those. Yes, Those are much more fitting.

bstifle commented 3 years ago

Dig it! this sounds like a good compromise

driskull commented 3 years ago

@jcfranco could you review this and let me know what you think? I think it can be simplified a bit but wasn't sure exactly how.

I think in general we want the ability to get access to chart colors via JSON/TS/JS. Does that sound right?

driskull commented 3 years ago

@jcfranco can you review this?

jcfranco commented 3 years ago

What's the priority/timeframe for this? Also, can you update the PR title?

driskull commented 3 years ago

What's the priority/timeframe for this? Also, can you update the PR title?

I would say medium priority so we can begin using these colors via an NPM dependency in the JSAPI for 4.18.

The goal is for consistent chart colors throughout our apps and APIs.

PR title updated.

jcfranco commented 3 years ago

@driskull Thanks! I'll try to get the review done by end of today. πŸ‘€

driskull commented 3 years ago

Thanks. Let me know if you have ideas for how we can better automate or deliver this to different projects.

driskull commented 3 years ago

@julio8a this good to merge? anyone you think should review?

julio8a commented 3 years ago

@paulcpederson, if you have time, can you give this a quick review before the merge?

bstifle commented 3 years ago

@julio8a were you able to make the updates kyle suggested?

julio8a commented 3 years ago

Thanks for the reminder @bstifle. Just pushed up the changes for those two colors.

paulcpederson commented 3 years ago

Because you moved the build files to dist (if I'm reading this code correctly), you'll need to also update all the documentation to make sure it points to the correct paths.

A note in the readme for these color palettes would be good.

Also, since we're moving things around, I think that technically makes this a breaking change and we should go to 3.0.0

driskull commented 3 years ago

@paulcpederson let me know if that's good now.

paulcpederson commented 3 years ago

Thanks @driskull πŸ‘

driskull commented 3 years ago

Can someone do a release to npm for this?

paulcpederson commented 3 years ago

already did πŸ˜‰