Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Button to only render <button> #211

Open
huyenltnguyen opened this issue Jun 28, 2024 · 0 comments
Open

Update Button to only render <button> #211

huyenltnguyen opened this issue Jun 28, 2024 · 0 comments
Labels
status: discussing Under discussion threads. Closed as stale after 60 days of inactivity.

Comments

@huyenltnguyen
Copy link
Member

huyenltnguyen commented Jun 28, 2024

Description

The Button component can render both a button or an a depending on the href prop:

ui/src/button/button.tsx

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 or variant prop our Link 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.

@huyenltnguyen huyenltnguyen added the status: discussing Under discussion threads. Closed as stale after 60 days of inactivity. label Jun 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: discussing Under discussion threads. Closed as stale after 60 days of inactivity.
Projects
None yet
Development

No branches or pull requests

1 participant