Update token components' APIs #1998
Conversation
src/Token/TokenBase.tsx
Outdated
| export type TokenSizeKeys = 'small' | 'medium' | 'large' | 'extralarge' | ||
| // TODO: remove invalid "extralarge" size name in next breaking change | ||
| // ADR: https://github.com/github/primer/blob/main/adrs/2022-02-09-size-naming-guidelines.md | ||
| export type TokenSizeKeys = 'small' | 'medium' | 'large' | 'extralarge' | 'xlarge' |
There was a problem hiding this comment.
What do you think about adding a @deprecated flag to the extralarge value here?
We'd need to bump to minor instead, but feels like we can give early notice that we'll remove it more formally later.
There was a problem hiding this comment.
Is it possible to add the @deprecated flag to specific values?
There was a problem hiding this comment.
It's not supported end-to-end yet, but you can get it working for anyone browsing the codebase by hoisting it into a separate type and then reapplying it in the union or as an intersection. Maybe someone will find it useful 🤷
E.g. something like:
| export type TokenSizeKeys = 'small' | 'medium' | 'large' | 'extralarge' | 'xlarge' | |
| /** @deprecated to be removed in v36.. probably **/ | |
| type ExtraLarge = 'extralarge' | |
| export type TokenSizeKeys = | |
| | 'small' | |
| | 'medium' | |
| | 'large' | |
| | 'xlarge' | |
| | ExtraLarge |
will show this..
| @@ -239,7 +240,8 @@ function TextInputWithTokensInnerComponent<TokenComponentType extends AnyReactCo | |||
| small: 'small', | |||
| medium: 'small', | |||
| large: 'medium', | |||
| extralarge: 'medium' | |||
| extralarge: 'medium', | |||
| xlarge: 'medium' // will eventually replace "extralarge" per this ADR: https://github.com/github/primer/blob/main/adrs/2022-02-09-size-naming-guidelines.md | |||
There was a problem hiding this comment.
I think this adr is on a private repo, can we remove the url?
There was a problem hiding this comment.
I think this link is helpful for maintainers. I think it's okay that's internal-only
🦋 Changeset detectedLatest commit: 931d73b 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 📦
|
Summary of changes:
'xlarge'size option to eventually replace'extralarge'to align with our size naming ADRleadingVisualin small tokens because it's rendered <16pxhideRemoveButtonprop to AvatarTokenMerge 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.