-
Notifications
You must be signed in to change notification settings - Fork 210
docs(statuslight): docs to storybook migration #3152
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| import { Container } from "@spectrum-css/preview/decorators"; | ||
| import { html } from "lit"; | ||
| import { classMap } from "lit/directives/class-map.js"; | ||
| import { styleMap } from "lit/directives/style-map.js"; | ||
|
|
@@ -9,6 +10,7 @@ export const Template = ({ | |
| size = "m", | ||
| variant = "info", | ||
| label, | ||
| isDisabled, | ||
| customStyles = {}, | ||
| } = {}) => html` | ||
| <div | ||
|
|
@@ -17,9 +19,45 @@ export const Template = ({ | |
| [`${rootClass}--size${size?.toUpperCase()}`]: | ||
| typeof size !== "undefined", | ||
| [`${rootClass}--${variant}`]: typeof variant !== "undefined", | ||
| "is-disabled": isDisabled, | ||
| })} | ||
| style=${styleMap(customStyles)} | ||
| > | ||
| ${label} | ||
| </div> | ||
| `; | ||
|
|
||
| // TODO: the accent variant will be removed in S2. | ||
| export const SemanticGroup = (args, context) => Container({ | ||
| withBorder: false, | ||
| direction: "column", | ||
| content: html`${[ | ||
| "accent", | ||
| "neutral", | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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.
|
||
| "info", | ||
| "negative", | ||
| "notice", | ||
| "positive"].map(variant => Template({...args, variant: variant, label: `${variant.charAt(0).toUpperCase() + variant.slice(1)} status` }, context)) | ||
| }` | ||
| }); | ||
|
|
||
| export const NonsemanticGroup = (args, context) => Container({ | ||
| withBorder: false, | ||
| direction: "column", | ||
| content: html`${[ | ||
| "gray", | ||
| "red", | ||
| "orange", | ||
| "yellow", | ||
| "chartreuse", | ||
| "celery", | ||
| "green", | ||
| "seafoam", | ||
| "cyan", | ||
| "blue", | ||
| "indigo", | ||
| "purple", | ||
| "fuchsia", | ||
| "magenta",].map(variant => Template({...args, variant: variant, label: `${variant.charAt(0).toUpperCase() + variant.slice(1)}`}, context)) | ||
| }` | ||
| }); | ||
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That sounds reasonable to me. We can always add it in later if we want to incorporate an example to the docs.