Accessibility engineering review for Link component.

.changeset/curly-otters-repeat.md

@@ -0,0 +1,7 @@
+---
+"@primer/react": minor
+---
+
+Link components no longer accept an `as` prop. If you need link-like styling, you should use a different component and style accordingly.
+
+Links that are rendered within `<p>` tags will always have underlines to distinguish from the surrounding text.

docs/content/Link.mdx

@@ -10,10 +10,6 @@ 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,11 +215,6 @@
               "name": "ref",
               "type": "React.RefObject<HTMLAnchorElement>"
             },
-            {
-              "name": "as",
-              "type": "React.ElementType",
-              "defaultValue": "\"a\""
-            },
             {
               "name": "sx",
               "type": "SystemStyleObject"
@@ -2048,7 +2043,7 @@
           "name": "href",
           "type": "string",
           "defaultValue": "",
-          "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)."
+          "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element)."
         },
         {
           "name": "muted",
@@ -2072,11 +2067,6 @@
           "name": "ref",
           "type": "React.RefObject<HTMLAnchorElement>"
         },
-        {
-          "name": "as",
-          "type": "React.ElementType",
-          "defaultValue": "\"a\""
-        },
         {
           "name": "sx",
           "type": "SystemStyleObject"

src/ActionList/ActionList.docs.json

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

src/ActionList/LinkItem.tsx

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

src/BaseStyles.tsx

@@ -26,6 +26,10 @@ const GlobalStyle = createGlobalStyle<{colorScheme?: 'light' | 'dark'}>`
   details-dialog:focus:not(:focus-visible):not(.focus-visible) {
     outline: none;
   }
+
+  p a[class*=StyledLink] {
+    text-decoration: underline;
+  }
 `
 
 const Base = styled.div<SystemTypographyProps & SystemCommonProps>`

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. If `as` is specified, the link behavior may need different props)."
+      "description": "URL to be used for the Link. (The `href` is passed to the underlying `<a>` element)."
     },
     {
       "name": "muted",
@@ -33,15 +33,10 @@
       "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,5 +1,6 @@
 import Link from '../Link'
-import {Meta} from '@storybook/react'
+import Box from '../Box'
+import {Meta, StoryFn} from '@storybook/react'
 import React from 'react'
 import {ComponentProps} from '../utils/types'
 
@@ -19,3 +20,20 @@ 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, useEffect} from 'react'
+import React, {forwardRef} from 'react'
 import styled from 'styled-components'
 import {system} from 'styled-system'
 import {get} from '../constants'
@@ -23,55 +23,27 @@ 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 {
+  &:hover,
+  &:focus {
     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: Component = 'a', ...props}, forwardedRef) => {
+const Link = forwardRef(({as, ...props}, forwardedRef) => {
   const innerRef = React.useRef<HTMLAnchorElement>(null)
   useRefObjectAsForwardedRef(forwardedRef, innerRef)
 
-  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])
+  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.',
+    )
   }
 
   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})
+  behavesAsComponent({Component: Link, options: {skipAs: true}})
 
   checkExports('Link', {
     default: Link,
@@ -29,24 +29,11 @@ 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()
-  })
 })