-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Storybook: Add stub doc on existing colors. #65982
Conversation
This is neat :) I wonder if it would be beneficial to group the grays together like you'd done with the alert colors. This would condense the page a lot. Notes on accessibility pairings can appear in a section below with more details. While they're not part of gutenberg, components do reference accent/theme color(s), so we should probably include those too? |
Thank you!
These two together, make a lot of sense.
I'm tempted to do that as a followup. More than actually documenting the colors, the most important part of this PR is to get a start on organizing some of these bits. Once some of the missing pages are built out, I'm hoping there can be a slew of tiny PRs that are easy to make, that can be inspired by these first tiny steps. What do you think? |
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.
Given the scope of this PR (kickstarting an effort to add more docs), this is LGTM
We can focus on information hierarchy, contents and format later down the line.
We also need to get more knowledgeable with the tools offered natively by Storybook , such as the ColorPalette
and ColorItem
components used in this PR
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.
While this document (and following ones) are a WIP, should we make less discoverable? Maybe we could change the file path to something like storybook/stories/_system/tokens/color.mdx
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.
Good question. I would prefer not to, because even if the document, and followups of the same type, are meant to be stubs and starting points, they should nevertheless be immediately useful, and document what's already shipping. In this case, these colors are just extracted from base styles, but are more easily digestible here.
However I'm happy to defer to opinions otherwise, and move it.
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.
I don't have a strong opinion either. Maybe @mattrwalker can help, as he recently worked on the Storybook's site map?
In any case, not a blocking comment. Any changes that we discuss can be applied in follow-up PRs.
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Hmm yeah. Maybe black and white are their own set? |
I had the same thought. I have to step away for a bit, can do tomorrow, otherwise feel free to push. |
* Storybook: Add stub doc on existing colors. * Update 4.6 to 4.5 for contrast. * Update based on feedback. * Use SCSS also for the alerts. * Separate white and black. --- Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: mattrwalker <mattryanwalker@git.wordpress.org>
* Storybook: Add stub doc on existing colors. * Update 4.6 to 4.5 for contrast. * Update based on feedback. * Use SCSS also for the alerts. * Separate white and black. --- Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: mattrwalker <mattryanwalker@git.wordpress.org>
What?
Adds a first stub file for a new Storybook document on the colors that exist in base styles.
Why?
You shouldn't have to look in an SCSS file to find the existing colors documented,
Testing Instructions
Check out the branch, and run storybook:
npm run storybook:dev
, then browse to Tokens > Color