Convert StateComponent to support functional colors internally #269
Conversation
closes primer#232 Instead of green, red, purple we now support open, closed, merged. We still support the old colors until functional colors have fully landed into Primer CSS.
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/primer/view-components/HKZtYr7rNnSx3Z6W4dfRHGnd4RQH |
Co-authored-by: Manuel Puyol <manuelpuyol@github.com>
Co-authored-by: Manuel Puyol <manuelpuyol@github.com>
Co-authored-by: Manuel Puyol <manuelpuyol@github.com>
Co-authored-by: Manuel Puyol <manuelpuyol@github.com>
| @@ -43,7 +43,7 @@ Component for rendering the status of an item. | |||
| | Name | Type | Default | Description | | |||
| | :- | :- | :- | :- | | |||
| | `title` | `String` | N/A | `title` HTML attribute. | | |||
| | `color` | `Symbol` | `:default` | Background color. One of `:default`, `:green`, `:red`, or `:purple`. | | |||
| | `color` | `Symbol` | `:default` | Background color. One of `:open`, `:closed`, `:merged`, `:default`, `:green`, `:red`, or `:purple`. | | |||
There was a problem hiding this comment.
It's a little odd to have this argument named color, as that's also a system argument. But perhaps that's a concern for another time 😄
There was a problem hiding this comment.
I think this is part of the conversation about splitting system_arguments into different things
srt32
changed the title
|
This PR has me thinking..... given we don't bundle primer/css with this library, if consumers of this gem are consuming the old version of primer css, then this change would be breaking since they don't have the new functional classes defined yet. How do we want to approach this? |
@srt32 good point. There are only 4 other public users of the library: https://github.com/primer/view_components/network/dependents?package_id=UGFja2FnZS0xMzY1MzYwNjkz, so I'm tempted not to worry about it too much. I do think we should point this out in the changelog for the release, as the first entry. It might also be worth adding a note about the implicit dependency to the |
|
Is css-next as a private repo a typical workflow we do? Ideally we could point at a version of primer/css that anyone could consume. |
|
We also do call out the dependency already ;) |
supports #232
Instead of green, red, purple we now support open, closed, merged.
We still support the old colors until functional colors have fully
landed into Primer CSS.