Use <Slot /> to merge props and render link buttons as <Link /> (#24

)

* fix: render `Link` for link buttons

* docs: correct "Link" section in "Button" docs

* fix: make socials `newTab` and disable correctly

* fix: remove underline

Co-authored-by: Shun Kakinoki <[email protected]>

components/pages/home/header.tsx

@@ -44,10 +44,15 @@ const FiveoutofnineHeader: FC = () => {
 
       {/* Links (desktop) */}
       <div className="hidden space-x-2 md:flex">
-        <Button intent="primary" href="https://twitter.com/fiveoutofnine" leftIcon={<Twitter />}>
+        <Button
+          intent="primary"
+          href="https://twitter.com/fiveoutofnine"
+          leftIcon={<Twitter />}
+          newTab
+        >
           Twitter
         </Button>
-        <Button href="https://github.com/fiveoutofnine" leftIcon={<Github />}>
+        <Button href="https://github.com/fiveoutofnine" leftIcon={<Github />} newTab>
           GitHub
         </Button>
       </div>

components/ui/button/index.tsx

@@ -3,6 +3,7 @@ import { type ForwardedRef, forwardRef } from 'react';
 
 import { buttonIconVariants, buttonVariants } from './styles';
 import type { ButtonProps } from './types';
+import { Slot } from '@radix-ui/react-slot';
 import { cx } from 'class-variance-authority';
 import { twMerge } from 'tailwind-merge';
 
@@ -18,8 +19,6 @@ const Button = forwardRef(
       leftIcon,
       rightIcon,
       newTab,
-      title,
-      onClick,
       children,
       ...rest
     }: ButtonProps,
@@ -32,29 +31,33 @@ const Button = forwardRef(
           className,
         ),
       ),
-      title: title || href || undefined,
       'data-variant': variant,
       'data-disabled': disabled,
       'aria-disabled': disabled,
       disabled,
       ref,
-      onClick: newTab && href ? () => window.open(href, '_blank') : onClick,
       ...rest,
     };
 
-    if (href && !newTab) {
+    // Destructure `ref` from `props: JSX.IntrinsicElements['button']`, so the
+    // remaining props are type-compatible with `<Link />` for the `<Slot />`
+    // component to merge in.
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const { ref: _, ...restWithoutRef } = props;
+
+    if (href && !disabled) {
       return (
-        <Link href={href} passHref legacyBehavior>
-          <button {...props}>
+        <Slot ref={ref} {...restWithoutRef}>
+          <Link href={href} {...(newTab ? { target: '_blank', rel: 'noopener noreferrer' } : {})}>
             {leftIcon && variant !== 'text' ? (
               <span className={buttonIconVariants({ size })}>{leftIcon}</span>
             ) : null}
             <span>{children}</span>
             {rightIcon && variant !== 'text' ? (
               <span className={buttonIconVariants({ size })}>{rightIcon}</span>
             ) : null}
-          </button>
-        </Link>
+          </Link>
+        </Slot>
       );
     }
 

components/ui/button/styles.tsx

@@ -19,6 +19,7 @@ export const buttonVariants = cva(
     'justify-center',
     'items-center',
     'font-medium',
+    'no-underline',
     'transition-colors',
     'focus-visible:outline-none',
     'focus-visible:ring-2',

components/ui/icon-button/index.tsx

@@ -3,6 +3,7 @@ import { type ForwardedRef, forwardRef } from 'react';
 
 import { iconButtonIconVariants, iconButtonVariants } from './styles';
 import type { IconButtonProps } from './types';
+import { Slot } from '@radix-ui/react-slot';
 import { cx } from 'class-variance-authority';
 import { twMerge } from 'tailwind-merge';
 
@@ -16,8 +17,6 @@ const IconButton = forwardRef(
       disabled = false,
       href,
       newTab,
-      title,
-      onClick,
       children,
       ...rest
     }: IconButtonProps,
@@ -30,23 +29,27 @@ const IconButton = forwardRef(
           className,
         ),
       ),
-      title: title || href || undefined,
       'data-variant': variant,
       'data-disabled': disabled,
       'aria-disabled': disabled,
       disabled,
       ref,
-      onClick: newTab && href ? () => window.open(href, '_blank') : onClick,
       ...rest,
     };
 
-    if (href && !newTab) {
+    // Destructure `ref` from `props: JSX.IntrinsicElements['button']`, so the
+    // remaining props are type-compatible with `<Link />` for the `<Slot />`
+    // component to merge in.
+    // eslint-disable-next-line @typescript-eslint/no-unused-vars
+    const { ref: _, ...restWithoutRef } = props;
+
+    if (href && !disabled) {
       return (
-        <Link href={href} passHref legacyBehavior>
-          <button {...props}>
+        <Slot ref={ref} {...restWithoutRef}>
+          <Link href={href} {...(newTab ? { target: '_blank', rel: 'noopener noreferrer' } : {})}>
             <span className={iconButtonIconVariants({ size })}>{children}</span>
-          </button>
-        </Link>
+          </Link>
+        </Slot>
       );
     }
 

package.json

@@ -18,6 +18,7 @@
     "@radix-ui/react-accordion": "^1.1.2",
     "@radix-ui/react-dialog": "^1.0.4",
     "@radix-ui/react-hover-card": "^1.0.6",
+    "@radix-ui/react-slot": "^1.0.2",
     "@radix-ui/react-tabs": "^1.0.3",
     "@radix-ui/react-toast": "^1.1.3",
     "@radix-ui/react-tooltip": "^1.0.5",

pages/design/component/button.mdx

@@ -107,7 +107,7 @@ To use icon-only buttons, use `IconButton`. It is identical to `Button`, except
 
 ## Link
 
-To have the button act as a link, pass in a URL to the `href` prop. To open the link in a new tab, use the `newTab` prop. Note that the final component is rendered as a `<button>` element, not `<a>`.
+To have the button act as a link, pass in a URL to the `href` prop. To open the link in a new tab, use the `newTab` prop. Note that the final component will be rendered as a `<a>` element, so browser features like right-clicking to open in a new tab will work.
 
 <DesignComponentsDisplay
   className="grid-flow-col grid-rows-1"