fix: icon button sizes & update button docs #1738
Conversation
joshuaellis
added
pr: fix
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🦋 Changeset detectedLatest commit: f623e78 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 Change: -782 B (-0.27%) Total Size: 288 kB
ℹ️ View Unchanged
|
joshuaellis
force-pushed
the
chore/buttons
branch
from
5a79a5c
to
e3129b9
Compare
joshuaellis
force-pushed
the
chore/buttons
branch
from
e3129b9
to
a5dafb0
Compare
packages/design-system/src/components/IconButton/IconButton.tsx
Outdated
Show resolved
Hide resolved
Co-authored-by: Rémi de Juvigny <8087692+remidej@users.noreply.github.com>
did you click visit preview? i've tweaked some settings and using incognito i can still see the preview, so might work now? |
My bad, I was following the link from the CI checks, but the direct link in the comment works perfectly |
| startIcon?: React.ReactNode; | ||
| variant?: ButtonVariant; | ||
| }; | ||
| type LinkButtonProps<C extends React.ElementType = typeof BaseLink> = ButtonProps<C>; |
There was a problem hiding this comment.
this is not clear,
why this is not type LinkButtonProps<C extends React.ElementType = "button"> = ButtonProps<C>;
how this LinkButton wrapped exactly? even this syntax is confusing for me, <Button<typeof BaseLink>. If you could please elaborate or discuss on call.
There was a problem hiding this comment.
Because the tag component used in the Button component below is BaseLink. In our polymorphic components, the C generic denotes the element which corresponds directly to the tag prop.
The <Button<typeof BaseLink> is because all components are just functions, and the Button component is a generic function that takes it's element type. So in this scenario because we're working with two layers of polymoprhism we add the component as the generic that our props correspond too to avoid type errors.
There was a problem hiding this comment.
ohk, I will check how this polymorphism works here, thanks
There was a problem hiding this comment.
we can go through it on thursday if it's still not clear 😄
| <Flex gap={2}> | ||
| {BUTTON_VARIANTS.map((variant) => ( | ||
| <IconButton key={variant} {...args} variant={variant}> | ||
| <Icon /> | ||
| </IconButton> | ||
| ))} | ||
| </Flex> | ||
| ); | ||
| }, | ||
| name: 'all variants', | ||
| } satisfies Story; | ||
|
|
||
| export const Group = { | ||
| render: () => ( | ||
| <IconButtonGroup> | ||
| <IconButton label="Edit"> | ||
| <Icons.Pencil /> | ||
| </IconButton> | ||
| <IconButton label="Clone"> | ||
| <Icons.Duplicate /> | ||
| </IconButton> | ||
| <IconButton label="Delete" variant="danger"> | ||
| <Icons.Trash /> | ||
| </IconButton> | ||
| </IconButtonGroup> |
There was a problem hiding this comment.
You will need to add source code as its incorrect for all variants and group. Also it would have been great if we also show withTooltip example in one of the example, so it replaces the default label?
What does it do?
Related issue(s)/PR(s)