docs(statuslight): docs to storybook migration#3152
Conversation
|
File metricsSummaryTotal size: 4.31 MB* 🎉 No changes detected in any packages * Size determined by adding together the size of the main file for all packages in the library.* Results are not gzipped or minified. * An ASCII character in UTF-8 is 8 bits or 1 byte. |
|
🚀 Deployed on https://pr-3152--spectrum-css.netlify.app |
bafef35 to
599c210
Compare
| export const Disabled = Template.bind({}); | ||
| Disabled.args = { | ||
| isDisabled: true, | ||
| }; | ||
| Disabled.tags = ["!dev"]; | ||
| Disabled.parameters = { | ||
| chromatic: { disabledSnapshot: true }, | ||
| }; |
There was a problem hiding this comment.
We specifically removed the disabled styles and the disabled variant from the docs page in #1495. There's also no disabled status lights in S2. The only reason I added this is because the guidelines page references a disabled status light state. Perhaps that's out of date?
I have a separate commit for the disabled story and testing for easy removal. If we decide to keep the disabled stuff, I have a draft PR here: #3153 to add the disabled styles back into our CSS.
599c210 to
acb656f
Compare
rise-erpelding
left a comment
There was a problem hiding this comment.
This is great! I added a couple of my non-blocking thoughts/questions below and we'll need to follow up one way or the other with Disabled, but clearly you've done the work on that and will apply whichever version is needed! ⭐
| * Status lights should always include a label with text that clearly communicates the kind of status being shown. Color alone is not enough to communicate the status. Do not change the text color to match the dot. | ||
| * | ||
| * When the text is too long for the horizontal space available, it wraps to form another line. | ||
| */ |
There was a problem hiding this comment.
May I ask what your reasoning was for not showing the text wrapping to form another line? I think I made the opposite choice in some work I just did for switch, but I'm on the fence on whether I think that's the best way or whether we specifically need to demonstrate text wrapping.
I notice we're testing status light wrapping in the testing preview, which is definitely good, but I'm not sure that necessarily means we need to show it on the Docs page 🤔
There was a problem hiding this comment.
I feel like I had a similar conversation with Cassondra about this when I moved the documentation over for another component, but now I'm not sure where that conversation is. In-line alert is like this, where the truncation/wrapping option is only in the test view, and I know I migrated those docs. Maybe that's what I was thinking of? Badge is like this too I see, but then checkbox and radio show wrapping on their docs pages.
I figure at least a call out is good on the docs page. And then people can also test with a long label themselves? I know that's not a very convincing reason/answer!
There was a problem hiding this comment.
That sounds reasonable to me. We can always add it in later if we want to incorporate an example to the docs.
| direction: "column", | ||
| content: html`${[ | ||
| "accent", | ||
| "neutral", |
There was a problem hiding this comment.
I had never noticed before (or maybe I forgot) that the neutral variant is italicized. Clearly that's an intentional design decision, but it's nowhere in the guidelines. Have you ever picked up any historical knowledge about that? I'd love to see some sort of explanation on the Docs page, although I definitely don't think it's a requirement here.
There was a problem hiding this comment.
I don't have much historical knowledge as to why it was italics, but I do know that, for S2, the italicization is no longer supported.
Note: The neutral variant of the status light label uses default-font-style rather than italics as in S1.
cf70e87 to
531b993
Compare
castastrophe
left a comment
There was a problem hiding this comment.
Nice additions and updates.
| * Status lights should always include a label with text that clearly communicates the kind of status being shown. Color alone is not enough to communicate the status. Do not change the text color to match the dot. | ||
| * | ||
| * When the text is too long for the horizontal space available, it wraps to form another line. | ||
| */ |
There was a problem hiding this comment.
That sounds reasonable to me. We can always add it in later if we want to incorporate an example to the docs.
| }, context); | ||
| Sizing.tags = ["!dev"]; | ||
| Sizing.parameters = { | ||
| chromatic: { disabledSnapshot: true }, |
There was a problem hiding this comment.
| chromatic: { disabledSnapshot: true }, | |
| chromatic: { disableSnapshot: true }, |
Oh I noticed these snapshots were being added! Can you update this and then I'll re-approve?
There was a problem hiding this comment.
gah! Good catch, thank you! d3cf45e
I left you a message in Slack about this, but what are your thoughts on me just dropping the disabled status light commit for now?
531b993 to
b31dceb
Compare
Description
This PR continues migrating documentation from the static docs site to Storybook, with a focus on the status light component. When possible, documentation and/or stories were brought over from the S2 status light migration.
CHANGELOGJira/Specs
CSS-939
This PR has no CSS changes, so no changeset is needed.
How and where has this been tested?
Please tag yourself on the tests you've marked complete to confirm the tests have been run by someone other than the author.
Validation steps
Regression testing
Validate:
Screenshots
To-do list