From 0711f8c03c2c4c743386abf490e6dddf243be5e6 Mon Sep 17 00:00:00 2001
From: Elizabeth Mitchell <lizmitchell@google.com>
Date: Thu, 24 Aug 2023 16:45:22 -0700
Subject: [PATCH] fix(radio): make host radio role to fix a11y

Why? role="radio" items need to be in the same DOM scope to properly announce "radio X of X" and not "radio 1 of 1"

PiperOrigin-RevId: 559908996
---
 docs/components/radio.md                      |  41 ++++--
 radio/demo/demo.ts                            |  12 +-
 radio/demo/stories.ts                         | 128 ++++++++++--------
 radio/harness.ts                              |   2 +-
 radio/internal/_radio.scss                    |   2 +-
 radio/internal/radio.ts                       |  89 +++++++-----
 radio/internal/single-selection-controller.ts |   7 +-
 radio/radio_test.ts                           |  17 +++
 8 files changed, 175 insertions(+), 123 deletions(-)

diff --git a/docs/components/radio.md b/docs/components/radio.md
index 6163c5a632..bbfc3c834b 100644
--- a/docs/components/radio.md
+++ b/docs/components/radio.md
@@ -60,7 +60,7 @@ Radios behave like
 elements and form a group with the same `name` attribute. Only one radio can be
 selected in a group.
 
-Radios can be pre-selected by adding a `selected` attribute.
+Radios can be pre-selected by adding a `checked` attribute.
 
 Add a `value` to identify which radio is selected in a form.
 
@@ -87,7 +87,7 @@ Add a `value` to identify which radio is selected in a form.
 <form>
   <md-radio name="animals" value="cats"></md-radio>
   <md-radio name="animals" value="dogs"></md-radio>
-  <md-radio name="animals" value="birds" selected></md-radio>
+  <md-radio name="animals" value="birds" checked></md-radio>
 </form>
 ```
 
@@ -115,15 +115,16 @@ Associate a label with a radio using the `<label>` element.
 <!-- catalog-only-end -->
 
 ```html
-<label>
-  <md-radio name="animals" value="cats"></md-radio>
-  Cats
-</label>
+<md-radio id="cats-radio" name="animals" value="cats"></md-radio>
+<label for="cats-radio">Cats</label>
 
 <md-radio id="dogs-radio" name="animals" value="dogs"></md-radio>
 <label for="dogs-radio">Dogs</label>
 ```
 
+> Note: do not wrap radios inside of a `<label>`, which stops screen readers
+> from correctly announcing the number of radios in a group.
+
 ## Accessibility
 
 Add an
@@ -131,15 +132,25 @@ Add an
 attribute to radios without labels or radios whose labels need to be more
 descriptive.
 
+Place radios inside a
+[`role="radiogroup"`](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Roles/radiogroup_role)<!-- {.external} -->.
+Radio groups must display a label, either with `aria-label` or
+`aria-labelledby`.
+
 ```html
-<label>
-  <md-radio name="group" value="1" aria-label="First"></md-radio>
-  1st
-</label>
-<label>
-  <md-radio name="group" value="2" aria-label="Second"></md-radio>
-  2nd
-</label>
+<div role="radiogroup" aria-labelledby="group-title">
+  <h3 id="group-title">Starting position</h3>
+  <div>
+    <md-radio id="first-radio" name="group" value="1"
+        aria-label="First"></md-radio>
+    <label for="first-radio">1st</label>
+  </div>
+  <div>
+    <md-radio id="second-radio" name="group" value="2"
+        aria-label="Second"></md-radio>
+    <label for="second-radio">2nd</label>
+  </div>
+</div>
 ```
 
 > Note: radios are not automatically labelled by `<label>` elements and always
@@ -191,6 +202,6 @@ Token                            | Default value
 </style>
 
 <md-radio name="group"></md-radio>
-<md-radio name="group" selected></md-radio>
+<md-radio name="group" checked></md-radio>
 <md-radio name="group"></md-radio>
 ```
diff --git a/radio/demo/demo.ts b/radio/demo/demo.ts
index 36764a9c52..d791ef5411 100644
--- a/radio/demo/demo.ts
+++ b/radio/demo/demo.ts
@@ -8,22 +8,12 @@ import './index.js';
 import './material-collection.js';
 
 import {KnobTypesToKnobs, MaterialCollection, materialInitsToStoryInits, setUpDemo} from './material-collection.js';
-import {boolInput, Knob, selectDropdown} from './index.js';
+import {boolInput, Knob} from './index.js';
 
 import {stories, StoryKnobs} from './stories.js';
 
 const collection =
     new MaterialCollection<KnobTypesToKnobs<StoryKnobs>>('Radio', [
-      new Knob('checked', {
-        ui: selectDropdown({
-          options: [
-            {value: '0', label: 'First'},
-            {value: '1', label: 'Second'},
-            {value: '2', label: 'Third'},
-          ],
-        }),
-        defaultValue: '1'
-      }),
       new Knob('disabled', {ui: boolInput(), defaultValue: false}),
     ]);
 
diff --git a/radio/demo/stories.ts b/radio/demo/stories.ts
index a4445bcda4..530fa2a135 100644
--- a/radio/demo/stories.ts
+++ b/radio/demo/stories.ts
@@ -7,81 +7,93 @@
 import '@material/web/radio/radio.js';
 
 import {labelStyles, MaterialStoryInit} from './material-collection.js';
-import {html} from 'lit';
+import {css, html} from 'lit';
 
 /** Knob types for radio stories. */
 export interface StoryKnobs {
-  checked: '0'|'1'|'2'|undefined;
   disabled: boolean;
 }
 
-const standard: MaterialStoryInit<StoryKnobs> = {
-  name: '<md-radio>',
-  render({checked, disabled}) {
+const radio: MaterialStoryInit<StoryKnobs> = {
+  name: 'Radios',
+  render({disabled}) {
     return html`
-      <md-radio
-        name="story1"
-        value="radioA"
-        touch-target="wrapper"
-        .checked=${checked === '0'}
-        .disabled="${disabled || false}">
-      </md-radio>
-      <md-radio
-        name="story1"
-        value="radioB"
-        touch-target="wrapper"
-        .checked=${checked === '1'}
-        .disabled="${disabled || false}">
-      </md-radio>
-      <md-radio
-        name="story1"
-        value="radioC"
-        touch-target="wrapper"
-        .checked=${checked === '2'}
-        .disabled="${disabled || false}">
-      </md-radio>
-      `;
-  },
-};
-
-const labeled: MaterialStoryInit<StoryKnobs> = {
-  name: 'Labeled',
-  styles: labelStyles,
-  render({checked, disabled}) {
-    return html`
-      <label>
-        First Radio
+      <div role="radiogroup" aria-label="An example group of radio buttons">
         <md-radio
-          name="story2"
-          value="radioA"
+          aria-label="First radio"
+          name="group"
           touch-target="wrapper"
-          .checked=${checked === '0'}
-          .disabled="${disabled || false}">
+          ?disabled=${disabled}>
         </md-radio>
-      </label>
-      <label>
-        Second Radio
         <md-radio
-          name="story2"
-          value="radioB"
+          aria-label="Second radio"
+          name="group"
           touch-target="wrapper"
-          .checked=${checked === '1'}
-          .disabled="${disabled || false}">
+          ?disabled=${disabled}>
         </md-radio>
-      </label>
-      <label>
-        Third Radio
         <md-radio
-          name="story2"
-          value="radioC"
+          aria-label="Third radio"
+          name="group"
           touch-target="wrapper"
-          .checked=${checked === '2'}
-          .disabled="${disabled || false}">
+          ?disabled=${disabled}>
         </md-radio>
-      </label>
-      `;
+      </div>
+    `;
+  },
+};
+
+const withLabels: MaterialStoryInit<StoryKnobs> = {
+  name: 'With labels',
+  styles: [
+    labelStyles, css`
+      .column {
+        display: flex;
+        flex-direction: column;
+      }
+
+      .radio-label {
+        display: flex;
+        align-items: center;
+      }
+    `
+  ],
+  render({disabled}) {
+    return html`
+      <div class="column" role="radiogroup" aria-label="Animals">
+        <div class="radio-label">
+          <md-radio
+            aria-label="Birds"
+            id="birds-radio"
+            name="with-labels"
+            touch-target="wrapper"
+            ?disabled=${disabled}>
+          </md-radio>
+          <label for="birds-radio">Birds</label>
+        </div>
+        <div class="radio-label">
+          <md-radio
+            aria-label="Cats"
+            id="cats-radio"
+            name="with-labels"
+            touch-target="wrapper"
+            ?disabled=${disabled}>
+          </md-radio>
+          <label for="cats-radio">Cats</label>
+        </div>
+        <div class="radio-label">
+          <md-radio
+            aria-label="Dogs"
+            id="dogs-radio"
+            name="with-labels"
+            touch-target="wrapper"
+            ?disabled=${disabled}>
+          </md-radio>
+          <label for="dogs-radio">Dogs</label>
+        </div>
+      </div>
+    `;
   },
 };
 
 /** Radio stories. */
-export const stories = [standard, labeled];
+export const stories = [radio, withLabels];
diff --git a/radio/harness.ts b/radio/harness.ts
index 3e6f1b429b..54cef3f5ce 100644
--- a/radio/harness.ts
+++ b/radio/harness.ts
@@ -14,6 +14,6 @@ import {Radio} from './internal/radio.js';
 export class RadioHarness extends Harness<Radio> {
   override async getInteractiveElement() {
     await this.element.updateComplete;
-    return this.element.renderRoot.querySelector('input') as HTMLInputElement;
+    return this.element as HTMLElement;
   }
 }
diff --git a/radio/internal/_radio.scss b/radio/internal/_radio.scss
index 9823ff5d3c..1e68455aab 100644
--- a/radio/internal/_radio.scss
+++ b/radio/internal/_radio.scss
@@ -49,6 +49,7 @@ $_md-sys-motion: tokens.md-sys-motion-values();
 
     display: inline-flex;
     height: var(--_icon-size);
+    outline: none;
     position: relative;
     vertical-align: top; // Fix extra space when placed inside display: block
     width: var(--_icon-size);
@@ -89,7 +90,6 @@ $_md-sys-motion: tokens.md-sys-motion-values();
   input {
     appearance: none;
     height: 48px;
-    outline: none;
     margin: 0;
     position: absolute;
     width: 48px;
diff --git a/radio/internal/radio.ts b/radio/internal/radio.ts
index 1a6d989311..59b691746f 100644
--- a/radio/internal/radio.ts
+++ b/radio/internal/radio.ts
@@ -7,13 +7,11 @@
 import '../../focus/md-focus-ring.js';
 import '../../ripple/ripple.js';
 
-import {html, isServer, LitElement, nothing} from 'lit';
-import {property, query} from 'lit/decorators.js';
+import {html, isServer, LitElement} from 'lit';
+import {property} from 'lit/decorators.js';
 import {classMap} from 'lit/directives/class-map.js';
 
-import {ARIAMixinStrict} from '../../internal/aria/aria.js';
-import {requestUpdateOnAriaChange} from '../../internal/aria/delegate.js';
-import {dispatchActivationClick, isActivationClick, redispatchEvent} from '../../internal/controller/events.js';
+import {isActivationClick} from '../../internal/controller/events.js';
 
 import {SingleSelectionController} from './single-selection-controller.js';
 
@@ -24,14 +22,6 @@ let maskId = 0;
  * A radio component.
  */
 export class Radio extends LitElement {
-  static {
-    requestUpdateOnAriaChange(Radio);
-  }
-
-  /** @nocollapse */
-  static override shadowRootOptions:
-      ShadowRootInit = {...LitElement.shadowRootOptions, delegatesFocus: true};
-
   /** @nocollapse */
   static readonly formAssociated = true;
 
@@ -95,7 +85,6 @@ export class Radio extends LitElement {
     return this.internals.labels;
   }
 
-  @query('input') private readonly input!: HTMLInputElement|null;
   private readonly selectionController = new SingleSelectionController(this);
   private readonly internals =
       (this as HTMLElement /* needed for closure */).attachInternals();
@@ -104,28 +93,30 @@ export class Radio extends LitElement {
     super();
     this.addController(this.selectionController);
     if (!isServer) {
-      this.addEventListener('click', (event: Event) => {
-        if (!isActivationClick(event)) {
-          return;
-        }
-        this.focus();
-        dispatchActivationClick(this.input!);
-      });
+      this.addEventListener('click', this.handleClick.bind(this));
+      this.addEventListener('keydown', this.handleKeydown.bind(this));
     }
   }
 
-  override focus() {
-    this.input?.focus();
+  override connectedCallback() {
+    super.connectedCallback();
+    // Firefox does not support ElementInternals aria yet, so we need to hydrate
+    // an attribute.
+    if (!('role' in this.internals)) {
+      this.setAttribute('role', 'radio');
+      return;
+    }
+
+    this.internals.role = 'radio';
   }
 
   protected override render() {
     const classes = {checked: this.checked};
-    // Needed for closure conformance
-    const {ariaLabel} = this as ARIAMixinStrict;
     return html`
-      <div class="container ${classMap(classes)}">
-        <md-ripple for="input" ?disabled=${this.disabled}></md-ripple>
-        <md-focus-ring part="focus-ring" for="input"></md-focus-ring>
+      <div class="container ${classMap(classes)}" aria-hidden="true">
+        <md-ripple part="ripple" .control=${this}
+            ?disabled=${this.disabled}></md-ripple>
+        <md-focus-ring part="focus-ring" .control=${this}></md-focus-ring>
         <svg class="icon" viewBox="0 0 20 20">
           <mask id="${this.maskId}">
             <rect width="100%" height="100%" fill="white" />
@@ -135,28 +126,58 @@ export class Radio extends LitElement {
               mask="url(#${this.maskId})" />
           <circle class="inner circle" cx="10" cy="10" r="5" />
         </svg>
+
         <input
           id="input"
           type="radio"
-          name=${this.name}
-          aria-label=${ariaLabel || nothing}
+          tabindex="-1"
           .checked=${this.checked}
           .value=${this.value}
           ?disabled=${this.disabled}
-          @change=${this.handleChange}
         >
       </div>
     `;
   }
 
-  private handleChange(event: Event) {
+  protected override updated() {
+    // Firefox does not support ElementInternals aria yet, so we need to hydrate
+    // an attribute.
+    if (!('ariaChecked' in this.internals)) {
+      this.setAttribute('aria-checked', String(this.checked));
+      return;
+    }
+
+    this.internals.ariaChecked = String(this.checked);
+  }
+
+  private async handleClick(event: Event) {
     if (this.disabled) {
       return;
     }
 
-    // Per spec, the change event on a radio input always represents checked.
+    // allow event to propagate to user code after a microtask.
+    await 0;
+    if (event.defaultPrevented) {
+      return;
+    }
+
+    if (isActivationClick(event)) {
+      this.focus();
+    }
+
+    // Per spec, clicking on a radio input always selects it.
     this.checked = true;
-    redispatchEvent(this, event);
+    this.dispatchEvent(new Event('change', {bubbles: true}));
+  }
+
+  private async handleKeydown(event: KeyboardEvent) {
+    // allow event to propagate to user code after a microtask.
+    await 0;
+    if (event.key !== ' ' || event.defaultPrevented) {
+      return;
+    }
+
+    this.click();
   }
 
   /** @private */
diff --git a/radio/internal/single-selection-controller.ts b/radio/internal/single-selection-controller.ts
index c1c935ebc4..0af2e2b280 100644
--- a/radio/internal/single-selection-controller.ts
+++ b/radio/internal/single-selection-controller.ts
@@ -122,7 +122,7 @@ export class SingleSelectionController implements ReactiveController {
     // 2. If an element is focused, the others are no longer focusable.
     if (checkedSibling || this.focused) {
       const focusable = checkedSibling || this.host;
-      focusable.removeAttribute('tabindex');
+      focusable.tabIndex = 0;
 
       for (const sibling of siblings) {
         if (sibling !== focusable) {
@@ -134,7 +134,7 @@ export class SingleSelectionController implements ReactiveController {
 
     // 3. If none are checked or focused, all are focusable.
     for (const sibling of siblings) {
-      sibling.removeAttribute('tabindex');
+      sibling.tabIndex = 0;
     }
   }
 
@@ -212,12 +212,13 @@ export class SingleSelectionController implements ReactiveController {
         if (sibling !== nextSibling) {
           sibling.checked = false;
           sibling.tabIndex = -1;
+          sibling.blur();
         }
       }
 
       // The next sibling should be checked, focused and dispatch a change event
       nextSibling.checked = true;
-      nextSibling.removeAttribute('tabindex');
+      nextSibling.tabIndex = 0;
       nextSibling.focus();
       // Fire a change event since the change is triggered by a user action.
       // This matches native <input type="radio"> behavior.
diff --git a/radio/radio_test.ts b/radio/radio_test.ts
index 755e205e48..e6865957b3 100644
--- a/radio/radio_test.ts
+++ b/radio/radio_test.ts
@@ -77,6 +77,23 @@ describe('<md-radio>', () => {
           .toBeTrue();
     });
 
+    it('clicking a radio can be default prevented', async () => {
+      const {harnesses} = await setupTest(radioGroup);
+      const unselected = harnesses[1];
+      expect(unselected.element.checked)
+          .withContext('unselected checked')
+          .toBeFalse();
+
+      unselected.element.addEventListener('click', event => {
+        event.preventDefault();
+      });
+
+      await unselected.clickWithMouse();
+      expect(unselected.element.checked)
+          .withContext('after clicking checked')
+          .toBeFalse();
+    });
+
     it('clicking a radio should unselect other radio which is already selected',
        async () => {
          const {harnesses} = await setupTest(radioGroupPreSelected);