-
-
Notifications
You must be signed in to change notification settings - Fork 30
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
styleorvariantprop ourLinkcomponent (variantcould 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.