Revert "Accessibility engineering review for Link component. (#3317)" (…

…#3459)

This reverts commit cc235fd.

docs/content/Link.mdx

@@ -10,6 +10,10 @@ import data from '../../src/Link/Link.docs.json'
 
 The Link component styles anchor tags with default hyperlink color cues and hover text decoration. `Link` is used for destinations, or moving from one page to another.
 
+In special cases where you'd like a `<button>` styled like a `Link`, use `<Link as='button'>`. Make sure to provide a click handler with `onClick`.
+
+**Important:** When using the `as` prop, be sure to always render an accessible element type, like `a`, `button`, `input`, or `summary`.
+
 ## Examples
 
 ```jsx live

generated/components.json

@@ -215,6 +215,11 @@
               "name": "ref",
               "type": "React.RefObject<HTMLAnchorElement>"
             },
+            {
+              "name": "as",
+              "type": "React.ElementType",
+              "defaultValue": "\"a\""
+            },
             {
               "name": "sx",
               "type": "SystemStyleObject"
@@ -895,7 +900,7 @@
         },
         {
           "name": "variant",
-          "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'\n| 'link'",
+          "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
           "defaultValue": "'default'",
           "description": "Change the visual style of the button."
         },
@@ -2057,7 +2062,7 @@
           "name": "href",
           "type": "string",
           "defaultValue": "",
-          "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element)."
+          "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element. If `as` is specified, the link behavior may need different props)."
         },
         {
           "name": "muted",
@@ -2081,6 +2086,11 @@
           "name": "ref",
           "type": "React.RefObject<HTMLAnchorElement>"
         },
+        {
+          "name": "as",
+          "type": "React.ElementType",
+          "defaultValue": "\"a\""
+        },
         {
           "name": "sx",
           "type": "SystemStyleObject"

src/ActionList/ActionList.docs.json

@@ -114,6 +114,11 @@
           "name": "ref",
           "type": "React.RefObject<HTMLAnchorElement>"
         },
+        {
+          "name": "as",
+          "type": "React.ElementType",
+          "defaultValue": "\"a\""
+        },
         {
           "name": "sx",
           "type": "SystemStyleObject"

src/ActionList/LinkItem.tsx

@@ -9,7 +9,6 @@ import {ActionListItemProps} from './shared'
 type LinkProps = {
   download?: string
   href?: string
-  to?: string
   hrefLang?: string
   media?: string
   ping?: string
@@ -22,7 +21,7 @@ type LinkProps = {
 // LinkItem does not support selected, variants, etc.
 export type ActionListLinkItemProps = Pick<ActionListItemProps, 'active' | 'children' | 'sx'> & LinkProps
 
-export const LinkItem = React.forwardRef(({sx = {}, active, ...props}, forwardedRef) => {
+export const LinkItem = React.forwardRef(({sx = {}, active, as: Component, ...props}, forwardedRef) => {
   const styles = {
     // occupy full size of Item
     paddingX: 2,
@@ -36,8 +35,6 @@ export const LinkItem = React.forwardRef(({sx = {}, active, ...props}, forwarded
     '&:hover': {color: 'inherit', textDecoration: 'none'},
   }
 
-  if (props.href === undefined && props.to !== undefined) props.href = props.to
-
   return (
     <Item
       active={active}
@@ -48,7 +45,14 @@ export const LinkItem = React.forwardRef(({sx = {}, active, ...props}, forwarded
           props.onClick && props.onClick(event as React.MouseEvent<HTMLAnchorElement>)
         }
         return (
-          <Link sx={merge(styles, sx as SxProp)} {...rest} {...props} onClick={clickHandler} ref={forwardedRef}>
+          <Link
+            as={Component}
+            sx={merge(styles, sx as SxProp)}
+            {...rest}
+            {...props}
+            onClick={clickHandler}
+            ref={forwardedRef}
+          >
             {children}
           </Link>
         )

src/Button/Button.docs.json

@@ -13,7 +13,7 @@
     },
     {
       "name": "variant",
-      "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'\n| 'link'",
+      "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
       "defaultValue": "'default'",
       "description": "Change the visual style of the button."
     },
@@ -67,4 +67,4 @@
       ]
     }
   ]
-}
+}

src/Button/Button.stories.tsx

@@ -22,7 +22,7 @@ export default {
       control: {
         type: 'radio',
       },
-      options: ['default', 'primary', 'danger', 'invisible', 'outline', 'link'],
+      options: ['default', 'primary', 'danger', 'invisible', 'outline'],
     },
     alignContent: {
       control: {

src/Button/styles.ts

@@ -186,25 +186,6 @@ export const getVariantStyles = (variant: VariantType = 'default', theme?: Theme
         borderColor: 'btn.outline.selectedBorder',
       },
     },
-    link: {
-      color: 'accent.fg',
-      backgroundColor: 'transparent',
-      display: 'inline-flex',
-      border: 'none',
-      height: 'unset',
-      padding: 0,
-      '&:hover:not(:disabled)': {
-        textDecoration: 'underline',
-      },
-      '&:focus-visible, &:focus': {
-        outlineOffset: '2px',
-      },
-      '&:disabled, &[aria-disabled=true]': {
-        color: 'primer.fg.disabled',
-        backgroundColor: 'transparent',
-        borderColor: 'transparent',
-      },
-    },
   }
   return style[variant]
 }

src/Button/types.ts

@@ -8,7 +8,7 @@ export const StyledButton = styled.button<SxProp>`
   ${sx};
 `
 
-export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outline' | 'link'
+export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outline'
 
 export type Size = 'small' | 'medium' | 'large'
 

src/Link/Link.docs.json

@@ -9,7 +9,7 @@
       "name": "href",
       "type": "string",
       "defaultValue": "",
-      "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element)."
+      "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element. If `as` is specified, the link behavior may need different props)."
     },
     {
       "name": "muted",
@@ -33,10 +33,15 @@
       "name": "ref",
       "type": "React.RefObject<HTMLAnchorElement>"
     },
+    {
+      "name": "as",
+      "type": "React.ElementType",
+      "defaultValue": "\"a\""
+    },
     {
       "name": "sx",
       "type": "SystemStyleObject"
     }
   ],
   "subcomponents": []
-}
+}

src/Link/Link.features.stories.tsx

@@ -1,6 +1,5 @@
 import Link from '../Link'
-import Box from '../Box'
-import {Meta, StoryFn} from '@storybook/react'
+import {Meta} from '@storybook/react'
 import React from 'react'
 import {ComponentProps} from '../utils/types'
 
@@ -20,20 +19,3 @@ export const Underline = () => (
     Link
   </Link>
 )
-
-export const WithinText: StoryFn<typeof Link> = args => (
-  <Box as="p">
-    This{' '}
-    <Link href="#" {...args}>
-      link
-    </Link>{' '}
-    is inside of other text.
-  </Box>
-)
-
-WithinText.args = {
-  muted: false,
-  underline: false,
-}
-
-WithinText.argTypes = {}

src/Link/Link.tsx

@@ -1,4 +1,4 @@
-import React, {forwardRef} from 'react'
+import React, {forwardRef, useEffect} from 'react'
 import styled from 'styled-components'
 import {system} from 'styled-system'
 import {get} from '../constants'
@@ -23,27 +23,55 @@ const hoverColor = system({
 const StyledLink = styled.a<StyledLinkProps>`
   color: ${props => (props.muted ? get('colors.fg.muted')(props) : get('colors.accent.fg')(props))};
   text-decoration: ${props => (props.underline ? 'underline' : 'none')};
-  &:hover,
-  &:focus {
+  &:hover {
     text-decoration: ${props => (props.muted ? 'none' : 'underline')};
     ${props => (props.hoverColor ? hoverColor : props.muted ? `color: ${get('colors.accent.fg')(props)}` : '')};
   }
+  &:is(button) {
+    display: inline-block;
+    padding: 0;
+    font-size: inherit;
+    white-space: nowrap;
+    cursor: pointer;
+    user-select: none;
+    background-color: transparent;
+    border: 0;
+    appearance: none;
+  }
   ${sx};
 `
 
-const Link = forwardRef(({as, ...props}, forwardedRef) => {
+const Link = forwardRef(({as: Component = 'a', ...props}, forwardedRef) => {
   const innerRef = React.useRef<HTMLAnchorElement>(null)
   useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
-  if (as !== undefined) {
-    // eslint-disable-next-line no-console
-    console.warn(
-      'Links no longer accept an as prop. If you need to style another tag as a link, you should use a different component and apply appropriate styling.',
-    )
+  if (__DEV__) {
+    /**
+     * The Linter yells because it thinks this conditionally calls an effect,
+     * but since this is a compile-time flag and not a runtime conditional
+     * this is safe, and ensures the entire effect is kept out of prod builds
+     * shaving precious bytes from the output, and avoiding mounting a noop effect
+     */
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    useEffect(() => {
+      if (
+        innerRef.current &&
+        !(innerRef.current instanceof HTMLButtonElement) &&
+        !(innerRef.current instanceof HTMLAnchorElement)
+      ) {
+        // eslint-disable-next-line no-console
+        console.error(
+          'Error: Found `Link` component that renders an inaccessible element',
+          innerRef.current,
+          'Please ensure `Link` always renders as <a> or <button>',
+        )
+      }
+    }, [innerRef])
   }
 
   return (
     <StyledLink
+      as={Component}
       {...props}
       // @ts-ignore shh
       ref={innerRef}

src/Link/__tests__/Link.test.tsx

@@ -5,7 +5,7 @@ import {render as HTMLRender} from '@testing-library/react'
 import {axe} from 'jest-axe'
 
 describe('Link', () => {
-  behavesAsComponent({Component: Link, options: {skipAs: true}})
+  behavesAsComponent({Component: Link})
 
   checkExports('Link', {
     default: Link,
@@ -29,11 +29,24 @@ describe('Link', () => {
     expect(render(<Link sx={{fontStyle: 'italic'}} />)).toHaveStyleRule('font-style', 'italic')
   })
 
+  it('applies button styles when rendering a button element', () => {
+    expect(render(<Link as="button" />)).toMatchSnapshot()
+  })
+
   it('respects the "muted" prop', () => {
     expect(render(<Link muted />)).toMatchSnapshot()
   })
 
   it('respects the  "sx" prop when "muted" prop is also passed', () => {
     expect(render(<Link muted sx={{color: 'fg.onEmphasis'}} />)).toMatchSnapshot()
   })
+
+  it('logs a warning when trying to render invalid "as" prop', () => {
+    const consoleSpy = jest.spyOn(global.console, 'error').mockImplementation()
+
+    HTMLRender(<Link as="i" />)
+    expect(consoleSpy).toHaveBeenCalled()
+
+    consoleSpy.mockRestore()
+  })
 })