fix(Radio): remove redundant aria-disabled (#2635)

* fix(Radio): remove redundant aria-disabled

* test(e2e): refactor toHaveNoViolations to use axe-core directly for custom rules

* test: update Radio test snapshots

* chore: add changeset

* test(vrt): update snapshots

* test: update snapshots

* chore: run prettier

Co-authored-by: joshblack <joshblack@users.noreply.github.com>

.changeset/seven-chefs-confess.md

@@ -0,0 +1,5 @@
+---
+'@primer/react': patch
+---
+
+Update Radio to only use disabled when provided and no longer set aria-disabled

e2e/components/RadioGroup.test.ts

@@ -0,0 +1,81 @@
+import {test, expect} from '@playwright/test'
+import {visit} from '../test-helpers/storybook'
+import {themes} from '../test-helpers/themes'
+
+test.describe('RadioGroup', () => {
+  test.describe('Default', () => {
+    for (const theme of themes) {
+      test.describe(theme, () => {
+        test('default @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-forms-radiogroup-examples--default',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+
+          // Default state
+          expect(await page.screenshot()).toMatchSnapshot(`RadioGroup.Default.${theme}.png`)
+        })
+
+        test('disabled @vrt', async ({page}) => {
+          await visit(page, {
+            id: 'components-forms-radiogroup-examples--default',
+            globals: {
+              colorScheme: theme,
+            },
+            args: {
+              disabled: true,
+            },
+          })
+
+          expect(await page.screenshot()).toMatchSnapshot(`RadioGroup.Default.disabled.${theme}.png`)
+
+          await expect(page).toHaveNoViolations({
+            rules: {
+              'color-contrast': {
+                enabled: theme !== 'dark_dimmed',
+              },
+            },
+          })
+        })
+
+        test('disabled @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-forms-radiogroup-examples--default',
+            globals: {
+              colorScheme: theme,
+            },
+            args: {
+              disabled: true,
+            },
+          })
+
+          await expect(page).toHaveNoViolations({
+            rules: {
+              'color-contrast': {
+                enabled: theme !== 'dark_dimmed',
+              },
+            },
+          })
+        })
+
+        test('axe @aat', async ({page}) => {
+          await visit(page, {
+            id: 'components-forms-radiogroup-examples--default',
+            globals: {
+              colorScheme: theme,
+            },
+          })
+          await expect(page).toHaveNoViolations({
+            rules: {
+              'color-contrast': {
+                enabled: theme !== 'dark_dimmed',
+              },
+            },
+          })
+        })
+      })
+    }
+  })
+})

e2e/matchers/toHaveNoViolations.ts

@@ -1,6 +1,5 @@
 import {Page, expect, test} from '@playwright/test'
-import AxeBuilder from '@axe-core/playwright'
-import {AxeResults} from 'axe-core'
+import {AxeResults, source} from 'axe-core'
 import path from 'node:path'
 import fs from 'node:fs'
 
@@ -21,23 +20,62 @@ const defaultOptions = {
     region: {
       enabled: false,
     },
+    // Custom rules
+    'avoid-both-disabled-and-aria-disabled': {
+      enabled: true,
+    },
   },
 }
 
 expect.extend({
   async toHaveNoViolations(page: Page, options = {rules: {}}) {
-    // @ts-ignore Page from @playwright/test should satisfy Page from
-    // playwright-core
-    const result = await new AxeBuilder({page})
-      .options({
-        ...defaultOptions,
-        ...options,
-        rules: {
-          ...defaultOptions.rules,
-          ...options.rules,
-        },
+    const runConfig = {
+      ...defaultOptions,
+      ...options,
+      rules: {
+        ...defaultOptions.rules,
+        ...options.rules,
+      },
+    }
+
+    await page.evaluate(source)
+
+    const result: AxeResults = await page.evaluate(runConfig => {
+      // @ts-ignore `axe` is a global variable defined by page.evaluate() above
+      const axe = window.axe
+
+      axe.configure({
+        rules: [
+          {
+            id: 'avoid-both-disabled-and-aria-disabled',
+            excludeHidden: true,
+            selector: 'button, fieldset, input, optgroup, option, select, textarea',
+            all: ['check-avoid-both-disabled-and-aria-disabled'],
+            any: [],
+            metadata: {
+              help: '[aria-disabled] may be used in place of native HTML [disabled] to allow tab-focus on an otherwise ignored element. Setting both attributes is contradictory.',
+              helpUrl: 'https://www.w3.org/TR/html-aria/#docconformance-attr',
+            },
+            tags: ['custom-github-rule'],
+          },
+        ],
+        checks: [
+          {
+            id: 'check-avoid-both-disabled-and-aria-disabled',
+            /**
+             * Check an element with native `disabled` support doesn't have both `disabled` and `aria-disabled` set.
+             */
+            evaluate: (el: Element) => !(el.hasAttribute('aria-disabled') && el.hasAttribute('disabled')),
+            metadata: {
+              impact: 'critical',
+            },
+          },
+        ],
       })
-      .analyze()
+
+      // @ts-ignore `axe` is a global variable defined by page.evaluate() above
+      return axe.run(runConfig)
+    }, runConfig)
 
     saveResult(result)
 

e2e/test-helpers/storybook.ts

@@ -3,19 +3,27 @@ import {waitForImages} from './waitForImages'
 
 interface Options {
   id: string
+  args?: Record<string, string | boolean>
   globals?: Record<string, string>
 }
 
 const {STORYBOOK_URL = 'http://localhost:6006'} = process.env
 
 export async function visit(page: Page, options: Options) {
-  const {id, globals} = options
+  const {id, args, globals} = options
   // In CI, the static server strips `.html` extensions
   const url = process.env.CI ? new URL(`${STORYBOOK_URL}/iframe`) : new URL(`${STORYBOOK_URL}/iframe.html`)
 
   url.searchParams.set('id', id)
   url.searchParams.set('viewMode', 'story')
 
+  if (args) {
+    const serialized = Object.entries(args)
+      .map(([key, value]) => `${key}:${value}`)
+      .join(',')
+    url.searchParams.set('args', serialized)
+  }
+
   if (globals) {
     let params = ''
     for (const [key, value] of Object.entries(globals)) {

package.json

@@ -107,7 +107,6 @@
     "styled-system": "^5.1.5"
   },
   "devDependencies": {
-    "@axe-core/playwright": "4.5.0",
     "@babel/cli": "7.19.3",
     "@babel/core": "7.14.8",
     "@babel/eslint-parser": "7.15.7",

script/generate-e2e-tests.js

@@ -228,6 +228,17 @@ const components = new Map([
       ],
     },
   ],
+  [
+    'RadioGroup',
+    {
+      stories: [
+        {
+          id: 'components-forms-radiogroup-examples--default',
+          name: 'Default',
+        },
+      ],
+    },
+  ],
   [
     'UnderlineNav',
     {

src/Radio.tsx

@@ -76,7 +76,6 @@ const Radio = React.forwardRef<HTMLInputElement, RadioProps>(
         name={name}
         ref={ref}
         disabled={disabled}
-        aria-disabled={disabled ? 'true' : 'false'}
         checked={checked}
         aria-checked={checked ? 'true' : 'false'}
         required={required}

src/__tests__/Radio.test.tsx

@@ -104,18 +104,14 @@ describe('Radio', () => {
 
     expect(radio.disabled).toEqual(true)
     expect(radio).not.toBeChecked()
-    expect(radio).toHaveAttribute('aria-disabled', 'true')
 
     fireEvent.change(radio)
 
     expect(radio.disabled).toEqual(true)
     expect(radio).not.toBeChecked()
-    expect(radio).toHaveAttribute('aria-disabled', 'true')
 
     // remove disabled attribute and retest
     rerender(<Radio {...defaultProps} onChange={handleChange} />)
-
-    expect(radio).toHaveAttribute('aria-disabled', 'false')
   })
 
   it('renders an uncontrolled component correctly', () => {

src/__tests__/__snapshots__/CheckboxOrRadioGroup.test.tsx.snap

@@ -114,7 +114,6 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"
@@ -146,7 +145,6 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"
@@ -178,7 +176,6 @@ exports[`CheckboxOrRadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"

src/__tests__/__snapshots__/Radio.test.tsx.snap

@@ -23,7 +23,6 @@ exports[`Radio renders consistently 1`] = `
 
 <input
   aria-checked="false"
-  aria-disabled="false"
   aria-invalid="false"
   aria-required="false"
   className="c0"

src/__tests__/__snapshots__/RadioGroup.test.tsx.snap

@@ -114,7 +114,6 @@ exports[`RadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"
@@ -146,7 +145,6 @@ exports[`RadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"
@@ -178,7 +176,6 @@ exports[`RadioGroup renders consistently 1`] = `
         >
           <input
             aria-checked="false"
-            aria-disabled="false"
             aria-invalid="false"
             aria-required="false"
             className="c6"