Skip to content
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

creates a docs-only <ColorComponent> to allow display of separate colors #377

Merged
merged 3 commits into from
Apr 17, 2019

Conversation

mdole
Copy link
Contributor

@mdole mdole commented Apr 16, 2019

Discussed with @owendodd and Design would like to have the option to not display every color in Palette in the same section, instead picking out a color or group of colors to display.

This is due to a change in the way we're dealing with colors - in Notion, you can see Primary, Secondary, and Deprecated colors, and these lists will likely grow and change over time.

The docs update in this PR allows greater flexibility by creating a mini-component that takes a color (e.g. black100) and outputting a color bar, hex code, and title in the same manner as the old Colors page.

After this is merged, @owendodd can create <ColorComponents /> using the docs admin to match our current colors.

Two things that could be improved about this implementation:

  1. hexes are not automatically generated and have to be input separately from the color code. I feel like this could be fixed, but not sure how.
  2. This will have to be manually updated every time we change or add to our colors. Would be great to figure out a way to automate that process.

Old page, which always displayed every color:
Screen Shot 2019-04-16 at 1 42 00 PM

New page, which allows display of select colors:
Screen Shot 2019-04-16 at 1 42 03 PM

@artsyit
Copy link
Contributor

artsyit commented Apr 16, 2019

Deploy preview for artsy-palette ready!

Built with commit 0820aa2

https://deploy-preview-377--artsy-palette.netlify.com

@mdole
Copy link
Contributor Author

mdole commented Apr 16, 2019

ok, updated! I'm a little unclear on how the GlobalStyles bit works/how to test it - I couldn't figure out where we're currently using those. If I guessed right on those, I think this should be good to go @zephraph if you want to give it a quick look

@zephraph
Copy link
Contributor

I think this is good for a first pass. We can always follow up with how we can improve the pattern going forward.

Great work @mdole and thanks @damassi and @owendodd for the valuable input 🙏

@zephraph zephraph merged commit e8109fd into artsy:master Apr 17, 2019
@artsyit
Copy link
Contributor

artsyit commented Apr 17, 2019

🚀 PR was released in v4.9.1 🚀

@mdole
Copy link
Contributor Author

mdole commented Apr 17, 2019

Thanks @zephraph - it does look like the MDX portion is bugged, and I'm not sure how to fix this. @ds300 or @damassi would one of you have a few minutes to pair on this today?
Screen Shot 2019-04-17 at 9 33 34 AM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants