-
-
Notifications
You must be signed in to change notification settings - Fork 29
Open
Labels
status: discussingUnder discussion threads. Closed as stale after 60 days of inactivity.Under discussion threads. Closed as stale after 60 days of inactivity.
Description
Description
The Button component can render both a button
or an a
depending on the href
prop:
Lines 175 to 188 in 884b6ca
if (href && !disabled) { | |
return ( | |
<Link | |
className={className} | |
href={href} | |
download={download} | |
target={target} | |
ref={ref as React.Ref<HTMLAnchorElement>} | |
onClick={onClick} | |
{...rest} | |
> | |
{children} | |
</Link> | |
); |
We designed the component this way to mimic React Bootstrap's Button to help ease the migration process. However, this implementation has caused some confusion and even code smell (code - the Button receives an href
, which makes it a link, but the onClick
handler has some conditional that can make the link behave like a button).
I think we should update Button to just render a button
element. For links that look like a button, we can either:
- Create a ButtonLink component; or
- Add a
style
orvariant
prop ourLink
component (variant
could imply functionality differences, so I'm not sure if this prop is appropriate)interface LinkProps { style?: 'link' | 'button' // 'link' is the default }
Others
The button / link split was also mentioned in #23.
Metadata
Metadata
Assignees
Labels
status: discussingUnder discussion threads. Closed as stale after 60 days of inactivity.Under discussion threads. Closed as stale after 60 days of inactivity.