Skip to content

Update Button to only render <button> #211

@huyenltnguyen

Description

@huyenltnguyen

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    status: discussingUnder discussion threads. Closed as stale after 60 days of inactivity.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions