-
Notifications
You must be signed in to change notification settings - Fork 179
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
feat(components): create new Tag component #15441
Conversation
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.
looks good with one nit!
{iconName != null && iconPosition === 'left' ? ( | ||
<Icon name={iconName} aria-label={`icon_${text}`} css={ICON_STYLE} /> | ||
) : null} | ||
<StyledText css={TEXT_STYLE}>{text}</StyledText> |
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.
nit: can we use as='h3'
to avoid redundancy?
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 think that the h3
StyledText
tag doesn't translate correctly for the ODD so i'd have to override it still with the css prop
Could you add tests for Tag component? I think need to add Tag to |
@jerader |
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.
once you update focus-within
style and Storybook title, this pr is good to go on dev side.
closes RSQ-84 https://opentrons.atlassian.net/browse/RSQ-84
Overview
This PR creates the Tag component that was added to the design system. Additionally, it deprecates the usage of the
basic
type inChip
componentTest Plan
Search for
Tag
in Storybook and look through the different props. Make sure it matches the Figma designsChangelog
basic
type from Chip component, story and testNOTE: i did not make a test for this component. I think atoms in the components directory like this are not as important to have a test since we can check how it looks/works via the story
Review requests
see test plan
Risk assessment
low