DSD-1688: Updates StatusBadge styles and Storybook docs #1528
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
oliviawongnyc
changed the title
oliviawongnyc
added
the
Needs Review
| You may render an icon within the `StatusBadge` by passing it as a child to the component. | ||
| If you are using an icon, you should still pass text to the badge so that the user can | ||
| understand its meaning. | ||
|
|
There was a problem hiding this comment.
Can you add a visual code snippet here to show the pattern for adding an icon and text inside of the StatusBadge?
|
Never mind my previous comment about pointing this to the react 18 branch. I forgot we were making a new 2.1.6 (right @bigfishdesign13 ?) release so this PR is fine. Just add the code snippet in the mdx file. |
| <StatusBadge level="low"> | ||
| <Icon | ||
| color="ui.black" | ||
| mr="2" |
There was a problem hiding this comment.
Adding an icon will require some spacing between the icon and the text, which is what you have done here. However, there are a few things I would like to consider as we decide how to handle this.
The Icon component includes an align prop which adds 4px spacing to the left or right based on the prop value. I never liked this prop, but, that being said, I've never done anything about it. Ultimately, I don't think the prop would work here because the space value is less than what we need (4px vs. 8px).
Another option would be to apply styles to the StatusBadge component that would handle the spacing in a flex element. You just added display: "flex", so this option is viable. With that, we would need to also add gap: "xs" and that woudl enforce a standard spacing in the component. While I do like this idea, this runs counter what we have done in other components that can accept an icon (ex. Button and Link).
Another option would be to ask consuming apps to pass in the appropriate spacing value, like you've done here. If this will be our recommendation, please update the example to use mr="xs" instead of mr="2".
What do you think?
Cc @EdwinGuzman
There was a problem hiding this comment.
Right. Let's stick to being consistent and let the dev/consuming app add the appropriate styles in the Icon when it is passed. This means we may need more code snippets if the icon is added at the end rather than at the beginning.
Do add that we highly recommend the styles when adding the example and that the "xs" value is consistent with NYPL designs.
There was a problem hiding this comment.
Okay! Take a look at what I pushed up and lmk if that is aligned with what you are thinking.
bigfishdesign13
added
Changes Requested
EdwinGuzman
added
Ship It
Fixes JIRA ticket DSD-1688
This PR does the following:
StatusBadgestyles so that if an icon is passed, the icon and text line up with one another (notably, changed display to flex and added an alignItems: center)How has this been tested?
Accessibility concerns or updates
StatusBadge, it should always be accompanied by text so the user knows the meaning of the badge. I am not sure if this i true, but it seemed logical. Interested to hear your thoughts.Checklist:
Front End Review: