Token components #1488
Token components #1488
Conversation
🦋 Changeset detectedLatest commit: 83a7d40 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
size-limit report 📦
|
|
|
||
| export interface TokenBaseProps | ||
| extends Omit<React.HTMLProps<HTMLSpanElement | HTMLButtonElement | HTMLAnchorElement>, 'size' | 'id'> { | ||
| as?: 'button' | 'a' | 'span' |
There was a problem hiding this comment.
Should we support ReactRouter-like link components? Similar to ActionList.Item: https://primer.style/react/ActionList#example-with-custom-item-renderer
There was a problem hiding this comment.
Just realized something...
We change the way we render the "x" button if the token is a link or a button. For example, we don't let the "x" button get focused with the keyboard, and we don't give the "x" button an aria-label.
| variants: { | ||
| sm: { | ||
| fontSize: 0, | ||
| gap: 1, |
There was a problem hiding this comment.
Looks like the flexbox gap property still doesn't have support in Safari <14.1. Do we need to support Safari <14.1? https://caniuse.com/flexbox-gap
There was a problem hiding this comment.
I'm not sure if we need to support Safari <14.1. Do we have some guidelines on Primer's browser support?
Co-authored-by: Cole Bemis <colebemis@github.com>
Co-authored-by: Cole Bemis <colebemis@github.com>
|
@colebemis - I think we should rename |
Agreed. I was actually going to suggest that 👍 |
src/stories/Token.stories.tsx
Outdated
| export const profileToken = () => ( | ||
| <ExampleCollectionContainer> | ||
| <SingleExampleContainer label="Resting"> | ||
| <TokenProfile size="lg" avatarSrc="https://avatars.githubusercontent.com/mperrotti" text="Mike Perrotti" /> |
There was a problem hiding this comment.
Should we use children instead of text here?
<TokenProfile size="lg" avatarSrc="https://avatars.githubusercontent.com/mperrotti">
Mike Perrotti
</TokenProfile>
<LabelToken fillColor="#FFF06C">Bug</LabelToken>We can still define what we accept there by adding types for children
There was a problem hiding this comment.
When I use children, I expect to be able to pass a ReactNode, so it feels a little strange to limit it to string. Also, we use text instead of children in the ActionList.Item component.
This could just be a personal preference though, so I'm open to using children if other reviewers feel like this makes more sense.
There was a problem hiding this comment.
I'm okay with either, children feels like the way to put contents inside that already exists.
When I use children, I expect to be able to pass a ReactNode, so it feels a little strange to limit it to string.
That makes sense! I guess it's contextual. In the case of a label, you would pass a string in children. The type check is to warn us if it is stretched/misused beyond that.
Also, we use text instead of children in the ActionList.Item component.
That's another one I'd like to change, proposal coming soon 😅
There was a problem hiding this comment.
That's another one I'd like to change, proposal coming soon 😅
Good to know!
Co-authored-by: Cole Bemis <colebemis@github.com>
I'm not sure 🤔 - it could be the |
|
I've solved the size mystery! I’m importing something from @primer/primitives, so that’s being added to the bundle. I’ll hunt it it down and remove it.
|
|
The Investigating now... Edit: This issue only happens when the cell is in edit mode |
There was a problem hiding this comment.
Excellent work, @mperrotti! After you add a changeset, I think this is a ready to go 
Since the discussion thread is very long, I put together a quick summary of the next steps for this PR:
Must do (in this PR)
- Add a changeset
Should do (in follow up PRs)
- Find Safari-compatible (<14.1) flexbox gap property alternative (comment)
- Find alternatives for fade.* variables (comment)
- Fix token button font (comment)
Could do
- Use children prop instead of text prop (comment)
Co-authored-by: Cole Bemis <colebemis@github.com>
Components have been added to support different kinds of "Tokens". A Token represents a piece of data. They are typically used to show a collection of related attributes.
Screenshots
Default
Profile
Issue Label
Merge checklist
Take a look at the What we look for in reviews section of the contributing guidelines for more information on how we review PRs.