feat(button): size enum on Button, add deprecationWarning util #229
Conversation
ahobson
requested review from
gidjin,
sarboc,
suzubara and
tinyels
as code owners
ahobson
force-pushed
the
adh-button-enums-187
branch
from
3fe37e2
to
589ec5e
Compare
| } | ||
| if (size === 'small') { | ||
| isSmall = true | ||
| } |
There was a problem hiding this comment.
I think another way to do this logic is:
const isBig = size === 'big' || (!size && big)
const isSmall = size === 'small' || (!size && small)
There was a problem hiding this comment.
That's assuming we want to ignore the deprecated props if they pass down size. @suzubara I'm blanking on what we decided here... I know we said we would NOT fix if they passed down big small together ...but do we want to guard against size='big' small ?
There was a problem hiding this comment.
I vote we don't try to fix that; that's why we are moving to enums, right?
There was a problem hiding this comment.
Also, I very much like your suggestion, but I have a counter suggestion. This is slightly into code golf territory, but how would you feel about
const isBig = big ? big : size === 'big'
const isSmall = small ? small : size === 'small'It changes things to prefer the old big and small in case of both e.g. big and size, but that feels appropriate to me
There was a problem hiding this comment.
Or to prefer size
const isBig = size ? size === 'big' : big
const isSmall = size ? size === 'small' : small
There was a problem hiding this comment.
I like the last option (that prefers the size prop). I don't think we need to deal with guarding against both big small, that is their choice and will be deprecated 🤷♀️
There was a problem hiding this comment.
Let's also update all of Button.stories.tsx to make sure we only show examples with size (and none with deprecated props).
🤔 Is it worth also adding tests for "ignore deprecated big/small props if size exist"? In other words if someone uses size that's what we follow first (even if big/small passed down).
I've added a commit that does both of these |
ahobson
force-pushed
the
adh-button-enums-187
branch
from
5d91226
to
8b2d208
Compare
haworku
changed the title
|
updated PR title (since it generates our changelog) to note that this work also adds |
haworku
changed the title
ahobson
force-pushed
the
adh-button-enums-187
branch
from
b535e48
to
5b0348c
Compare
Provide a size enum to eventually replace the small and big boolean props on Button re #187
* Log deprecation warnings * Prefer size over big/small; test that interaction
ahobson
force-pushed
the
adh-button-enums-187
branch
from
744cfdb
to
d859f81
Compare
…works#229) * feat(button): size enum on Button component Provide a size enum to eventually replace the small and big boolean props on Button re trussworks#187 * feat(Button): Add deprecation warnings, size preferred * Log deprecation warnings * Prefer size over big/small; test that interaction * Comment deprecationWarning in production * Button prop deprecation starts in v1.6.0
* feat(button): size enum on Button component Provide a size enum to eventually replace the small and big boolean props on Button re #187 * feat(Button): Add deprecation warnings, size preferred * Log deprecation warnings * Prefer size over big/small; test that interaction * Comment deprecationWarning in production * Button prop deprecation starts in v1.6.0
Provide a size enum to eventually replace the small and big boolean props on Button
re #187