Skip to content
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

[Chip] Add <Chip /> component, test, and story #27

Merged
merged 6 commits into from
Dec 7, 2018
Merged

[Chip] Add <Chip /> component, test, and story #27

merged 6 commits into from
Dec 7, 2018

Conversation

snags88
Copy link
Contributor

@snags88 snags88 commented Nov 30, 2018

This adds the Chip component to Radiance

Storybook

@snags88 snags88 self-assigned this Nov 30, 2018
@snags88 snags88 temporarily deployed to curology-radiance-pr-27 November 30, 2018 18:21 Inactive
@snags88 snags88 had a problem deploying to curology-radiance-pr-27 November 30, 2018 18:27 Failure
@snags88 snags88 temporarily deployed to curology-radiance-pr-27 November 30, 2018 18:31 Inactive
Copy link
Contributor

@smmullen90 smmullen90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. I personally dont like the word Tag because that means a lot of things for me, I wonder if we could update to be Badge? Thoughts?

const closedStyle = css`
background-color: ${COLORS.statusGreyBackground};
border-color: ${COLORS.statusGreyBorder};
span {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[note] i think if you rearrange, you can actually interpolate TagText here to make this more specific

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup that makes sense. Will update 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@snags88 @smmullen90 I believe we have badge already.

It's this:
image

It is similar though, wonder if we could have it as two separate types of badges.

I'll throw it up in the slack channel for people to discuss!

@snags88
Copy link
Contributor Author

snags88 commented Nov 30, 2018

@smmullen90 I can chat with ben regarding the naming!

@snags88
Copy link
Contributor Author

snags88 commented Dec 4, 2018

Consensus was to go with chip

@snags88 snags88 changed the title [Tag] Add <Tag /> component, test, and story [Chip] Add <Chip /> component, test, and story Dec 4, 2018
@snags88 snags88 temporarily deployed to curology-radiance-pr-27 December 4, 2018 02:07 Inactive
@snags88 snags88 temporarily deployed to curology-radiance-pr-27 December 4, 2018 02:08 Inactive
@snags88
Copy link
Contributor Author

snags88 commented Dec 4, 2018

Convert Chip component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants