Accessibility engineering review for Link component.

.changeset/curly-otters-repeat.md

@@ -0,0 +1,6 @@
+---
+"@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.
+Button has a `link` variant which can be used for this purpose.

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"
@@ -900,7 +895,7 @@
  },
  {
  "name": "variant",
- "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
+ "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'\n| 'link'",
  "defaultValue": "'default'",
  "description": "Change the visual style of the button."
  },
@@ -2062,7 +2057,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",
@@ -2086,11 +2081,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/Button/Button.docs.json

@@ -13,7 +13,7 @@
  },
  {
  "name": "variant",
- "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'",
+ "type": "| 'default'\n| 'primary'\n| 'danger'\n| 'outline'\n| 'invisible'\n| 'link'",
  "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'],
+ options: ['default', 'primary', 'danger', 'invisible', 'outline', 'link'],
  },
  alignContent: {
  control: {

src/Button/styles.ts

@@ -186,6 +186,25 @@ 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'
+export type VariantType = 'default' | 'primary' | 'invisible' | 'danger' | 'outline' | 'link'
 
 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. 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()
- })
 })