fix(foundation): remove readonly from radio button (#6438)

* fix(foundation): remove readonly from radio button readonly on radio button violates a WCAG rule. see #6437 there's no evidence readonly applies on radio button. it does on radio group however - [aria-readonly associated roles on MDN](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Attributes/aria-readonly#associated_roles) doesn’t list radio - https://stackoverflow.com/questions/1953017/why-cant-radio-buttons-be-readonly - https://www.w3schools.com/jsref/dom_obj_radio.asp * remove any evidence of readonly from class * completely remove any readonly * Change files * group slotted radio readonly handling * docs update * Update change/@microsoft-fast-foundation-b9885563-e3e8-4c85-a55d-bac04c627fec.json Co-authored-by: Nicholas Rice <[email protected]> * Update change/@microsoft-fast-foundation-b9885563-e3e8-4c85-a55d-bac04c627fec.json Co-authored-by: Nicholas Rice <[email protected]> * prevent check of radio buttons * Update packages/web-components/fast-foundation/src/radio/radio.ts Co-authored-by: Nicholas Rice <[email protected]> * revert * remove unused * FASTRadioGroup to be used as type only * fixup prettier and API report --------- Co-authored-by: Nicholas Rice <[email protected]> Co-authored-by: Rob Eisenberg <[email protected]> Co-authored-by: Chris Holt <[email protected]>
microsoft · Jun 10, 2024 · fda881b · fda881b
1 parent f124e58
commit fda881b
Show file tree

Hide file tree

Showing 12 changed files with 29 additions and 122 deletions.
diff --git a/change/@microsoft-fast-foundation-b9885563-e3e8-4c85-a55d-bac04c627fec.json b/change/@microsoft-fast-foundation-b9885563-e3e8-4c85-a55d-bac04c627fec.json
@@ -0,0 +1,7 @@
+{
+  "type": "prerelease",
+  "comment": "remove readonly support from fast radio",
+  "packageName": "@microsoft/fast-foundation",
+  "email": "[email protected]",
+  "dependentChangeType": "prerelease"
+}
diff --git a/packages/web-components/fast-foundation/docs/api-report.md b/packages/web-components/fast-foundation/docs/api-report.md
@@ -1638,9 +1638,6 @@ export class FASTRadio extends FormAssociatedRadio implements RadioControl {
     // @beta
     keypressHandler(e: KeyboardEvent): boolean | void;
     name: string;
-    readOnly: boolean;
-    // (undocumented)
-    protected readOnlyChanged(): void;
 }
 
 // @public
@@ -1667,8 +1664,6 @@ export class FASTRadioGroup extends FASTElement {
     protected nameChanged(): void;
     orientation: RadioGroupOrientation;
     readOnly: boolean;
-    // (undocumented)
-    protected readOnlyChanged(): void;
     // @internal (undocumented)
     slottedRadioButtons: HTMLElement[];
     // (undocumented)
@@ -2532,7 +2527,7 @@ export interface PropertyTarget {
 export type ProxyElement = HTMLSelectElement | HTMLTextAreaElement | HTMLInputElement;
 
 // @public
-export type RadioControl = Pick<HTMLInputElement, "checked" | "disabled" | "readOnly" | "focus" | "setAttribute" | "getAttribute">;
+export type RadioControl = Pick<HTMLInputElement, "checked" | "disabled" | "focus" | "setAttribute" | "getAttribute">;
 
 // @public
 export const RadioGroupOrientation: {

diff --git a/packages/web-components/fast-foundation/src/radio-group/README.md b/packages/web-components/fast-foundation/src/radio-group/README.md
@@ -88,7 +88,6 @@ export const myRadioGroup = RadioGroup.compose({
 
 | Name                         | Privacy   | Description | Parameters                                   | Return | Inherited From |
 | ---------------------------- | --------- | ----------- | -------------------------------------------- | ------ | -------------- |
-| `readOnlyChanged`            | protected |             |                                              | `void` |                |
 | `disabledChanged`            | protected |             |                                              | `void` |                |
 | `nameChanged`                | protected |             |                                              | `void` |                |
 | `valueChanged`               | protected |             |                                              | `void` |                |

diff --git a/packages/web-components/fast-foundation/src/radio-group/radio-group.pw.spec.ts b/packages/web-components/fast-foundation/src/radio-group/radio-group.pw.spec.ts
@@ -317,32 +317,6 @@ test.describe("Radio Group", () => {
         }
     });
 
-    test("should set all child radio elements to readonly when the `readonly` property is true", async () => {
-        await root.evaluate(node => {
-            node.innerHTML = /* html */ `
-                <fast-radio-group readonly>
-                    <fast-radio></fast-radio>
-                    <fast-radio></fast-radio>
-                    <fast-radio></fast-radio>
-                </fast-radio-group>
-            `;
-        });
-
-        await expect(element).toHaveBooleanAttribute("readonly");
-
-        expect(
-            await radios.evaluateAll(radios =>
-                radios.every(radio => radio.hasAttribute("readonly"))
-            )
-        ).toBeTruthy();
-
-        expect(
-            await radios.evaluateAll(radios =>
-                radios.every(radio => radio.getAttribute("aria-readonly") === "true")
-            )
-        ).toBeTruthy();
-    });
-
     test("should set tabindex of 0 to a child radio with a matching `value`", async () => {
         await root.evaluate(node => {
             node.innerHTML = /* html */ `

diff --git a/packages/web-components/fast-foundation/src/radio-group/radio-group.ts b/packages/web-components/fast-foundation/src/radio-group/radio-group.ts
@@ -32,17 +32,6 @@ export class FASTRadioGroup extends FASTElement {
      */
     @attr({ attribute: "readonly", mode: "boolean" })
     public readOnly: boolean;
-    protected readOnlyChanged(): void {
-        if (this.slottedRadioButtons !== undefined) {
-            this.slottedRadioButtons.forEach((radio: FASTRadio) => {
-                if (this.readOnly) {
-                    radio.readOnly = true;
-                } else {
-                    radio.readOnly = false;
-                }
-            });
-        }
-    }
 
     /**
      * Disables the radio group and child radios.
@@ -173,10 +162,6 @@ export class FASTRadioGroup extends FASTElement {
                 radio.setAttribute("name", this.name);
             }
 
-            if (this.readOnly) {
-                radio.readOnly = true;
-            }
-
             if (this.value && this.value === radio.value) {
                 this.selectedRadio = radio;
                 this.focusedRadio = radio;
@@ -239,16 +224,8 @@ export class FASTRadioGroup extends FASTElement {
         const radio: FASTRadio = group[index] as FASTRadio;
         if (!this.isInsideToolbar) {
             radio.setAttribute("tabindex", "0");
-            if (radio.readOnly) {
-                this.slottedRadioButtons.forEach((nextRadio: FASTRadio) => {
-                    if (nextRadio !== radio) {
-                        nextRadio.setAttribute("tabindex", "-1");
-                    }
-                });
-            } else {
-                radio.checked = true;
-                this.selectedRadio = radio;
-            }
+            radio.checked = true;
+            this.selectedRadio = radio;
         }
         this.focusedRadio = radio;
         radio.focus();
@@ -350,11 +327,7 @@ export class FASTRadioGroup extends FASTElement {
     };
 
     private checkFocusedRadio = (): void => {
-        if (
-            this.focusedRadio !== null &&
-            !this.focusedRadio.readOnly &&
-            !this.focusedRadio.checked
-        ) {
+        if (this.focusedRadio !== null && !this.focusedRadio.checked) {
             this.focusedRadio.checked = true;
             this.focusedRadio.setAttribute("tabindex", "0");
             this.focusedRadio.focus();

diff --git a/packages/web-components/fast-foundation/src/radio/README.md b/packages/web-components/fast-foundation/src/radio/README.md
@@ -113,31 +113,23 @@ export const myRadio = Radio.compose<RadioOptions>({
 
 #### Fields
 
-| Name       | Privacy | Type      | Default | Description                                                                                                                                                                                 | Inherited From      |
-| ---------- | ------- | --------- | ------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- |
-| `readOnly` | public  | `boolean` |         | When true, the control will be immutable by user interaction. See [readonly HTML attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly) for more information. |                     |
-| `name`     | public  | `string`  |         | The name of the radio. See [name attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#htmlattrdefname) for more info.                                              |                     |
-| `proxy`    |         |           |         |                                                                                                                                                                                             | FormAssociatedRadio |
+| Name    | Privacy | Type     | Default | Description                                                                                                                                    | Inherited From      |
+| ------- | ------- | -------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------- | ------------------- |
+| `name`  | public  | `string` |         | The name of the radio. See [name attribute](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#htmlattrdefname) for more info. |                     |
+| `proxy` |         |          |         |                                                                                                                                                | FormAssociatedRadio |
 
 #### Methods
 
-| Name              | Privacy   | Description                       | Parameters         | Return            | Inherited From |
-| ----------------- | --------- | --------------------------------- | ------------------ | ----------------- | -------------- |
-| `readOnlyChanged` | protected |                                   |                    | `void`            |                |
-| `keypressHandler` | public    | Handles key presses on the radio. | `e: KeyboardEvent` | `boolean or void` |                |
+| Name              | Privacy | Description                       | Parameters         | Return            | Inherited From |
+| ----------------- | ------- | --------------------------------- | ------------------ | ----------------- | -------------- |
+| `keypressHandler` | public  | Handles key presses on the radio. | `e: KeyboardEvent` | `boolean or void` |                |
 
 #### Events
 
 | Name     | Type | Description                                                | Inherited From |
 | -------- | ---- | ---------------------------------------------------------- | -------------- |
 | `change` |      | Emits a custom change event when the checked state changes |                |
 
-#### Attributes
-
-| Name       | Field    | Inherited From |
-| ---------- | -------- | -------------- |
-| `readonly` | readOnly |                |
-
 #### CSS Parts
 
 | Name      | Description                                       |

diff --git a/packages/web-components/fast-foundation/src/radio/radio.pw.spec.ts b/packages/web-components/fast-foundation/src/radio/radio.pw.spec.ts
@@ -71,17 +71,6 @@ test.describe("Radio", () => {
         await element.evaluate((node: FASTRadio) => (node.disabled = false));
 
         await expect(element).toHaveAttribute("aria-disabled", "false");
-
-        // Readonly
-        await expect(element).not.hasAttribute("aria-readonly");
-
-        await element.evaluate((node: FASTRadio) => (node.readOnly = true));
-
-        await expect(element).toHaveAttribute("aria-readonly", "true");
-
-        await element.evaluate((node: FASTRadio) => (node.readOnly = false));
-
-        await expect(element).toHaveAttribute("aria-readonly", "false");
     });
 
     test("should set a tabindex of 0 on the element", async () => {

diff --git a/packages/web-components/fast-foundation/src/radio/radio.spec.md b/packages/web-components/fast-foundation/src/radio/radio.spec.md
@@ -35,8 +35,6 @@ Extends [form associated custom element](../form-associated/form-associated-cust
   - The current checked state of the radio
 
 *Content attributes*
-- `readonly`
-  - The radio should be submitted with the form but should not be editable.
 - `disabled`
   - The radio should be disabled from user interaction and will not be submitted with the form data.
 - `value` - Not visible to the user, it's used for form data and to distinguish between other radio buttons of the same name attribute value.
@@ -72,7 +70,6 @@ Extends [form associated custom element](../form-associated/form-associated-cust
 - checked
 - disabled
 - required
-- readonly
 
 *Slotted Content/Slotted Classes*
 *CSS Parts*
@@ -89,9 +86,6 @@ The checked state can be toggled by:
 **disabled**: `true` or `false`
 When disabled, the value will not be changeable through user interaction. It should also not expose it's value to a form submission.
 
-**readonly**: `true` or `false`
-When readonly, the value will not be changeable through user interaction. The value will still be exposed to forms on submission.
-
 ### Accessibility
 The root element inside the shadow-dom of the radio will be a focusable element with the following accessibility content attributes:
 [MDN Web docs](https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/ARIA_Techniques/Using_the_radio_role)

diff --git a/packages/web-components/fast-foundation/src/radio/radio.template.ts b/packages/web-components/fast-foundation/src/radio/radio.template.ts
@@ -17,7 +17,6 @@ export function radioTemplate<T extends FASTRadio>(
             aria-checked="${x => x.checked}"
             aria-required="${x => x.required}"
             aria-disabled="${x => x.disabled}"
-            aria-readonly="${x => x.readOnly}"
             @keypress="${(x, c) => x.keypressHandler(c.event as KeyboardEvent)}"
         >
             <div part="control" class="control">

diff --git a/packages/web-components/fast-foundation/src/radio/radio.ts b/packages/web-components/fast-foundation/src/radio/radio.ts
@@ -1,6 +1,6 @@
-import { attr, observable } from "@microsoft/fast-element";
+import { observable } from "@microsoft/fast-element";
 import { keySpace } from "@microsoft/fast-web-utilities";
-import { FASTRadioGroup } from "../radio-group/index.js";
+import type { FASTRadioGroup } from "../radio-group/index.js";
 import type { StaticallyComposableHTML } from "../utilities/template-helpers.js";
 import { FormAssociatedRadio } from "./radio.form-associated.js";
 
@@ -10,7 +10,7 @@ import { FormAssociatedRadio } from "./radio.form-associated.js";
  */
 export type RadioControl = Pick<
     HTMLInputElement,
-    "checked" | "disabled" | "readOnly" | "focus" | "setAttribute" | "getAttribute"
+    "checked" | "disabled" | "focus" | "setAttribute" | "getAttribute"
 >;
 
 /**
@@ -34,20 +34,6 @@ export type RadioOptions = {
  * @public
  */
 export class FASTRadio extends FormAssociatedRadio implements RadioControl {
-    /**
-     * When true, the control will be immutable by user interaction. See {@link https://developer.mozilla.org/en-US/docs/Web/HTML/Attributes/readonly | readonly HTML attribute} for more information.
-     * @public
-     * @remarks
-     * HTML Attribute: readonly
-     */
-    @attr({ attribute: "readonly", mode: "boolean" })
-    public readOnly: boolean; // Map to proxy element
-    protected readOnlyChanged(): void {
-        if (this.proxy instanceof HTMLInputElement) {
-            this.proxy.readOnly = this.readOnly;
-        }
-    }
-
     /**
      * The name of the radio. See {@link https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#htmlattrdefname | name attribute} for more info.
      */
@@ -68,6 +54,12 @@ export class FASTRadio extends FormAssociatedRadio implements RadioControl {
     @observable
     public defaultSlottedNodes: Node[];
 
+    private get radioGroup() {
+        return (this as HTMLElement).closest(
+            "[role=radiogroup]"
+        ) as FASTRadioGroup | null;
+    }
+
     /**
      * @internal
      */
@@ -118,10 +110,7 @@ export class FASTRadio extends FormAssociatedRadio implements RadioControl {
     }
 
     private isInsideRadioGroup(): boolean {
-        const parent: HTMLElement | null = (this as HTMLElement).closest(
-            "[role=radiogroup]"
-        );
-        return parent !== null;
+        return this.radioGroup !== null;
     }
 
     /**
@@ -131,7 +120,7 @@ export class FASTRadio extends FormAssociatedRadio implements RadioControl {
     public keypressHandler(e: KeyboardEvent): boolean | void {
         switch (e.key) {
             case keySpace:
-                if (!this.checked && !this.readOnly) {
+                if (!this.checked && !this.radioGroup?.readOnly) {
                     this.checked = true;
                 }
                 return;

diff --git a/packages/web-components/fast-foundation/src/radio/stories/radio.register.ts b/packages/web-components/fast-foundation/src/radio/stories/radio.register.ts
@@ -119,8 +119,6 @@ const styles = css`
     }
 
     :host([disabled]) .label,
-    :host([readonly]) .label,
-    :host([readonly]) .control,
     :host([disabled]) .control {
         cursor: not-allowed;
     }

diff --git a/packages/web-components/fast-foundation/src/radio/stories/radio.stories.ts b/packages/web-components/fast-foundation/src/radio/stories/radio.stories.ts
@@ -8,7 +8,6 @@ export const storyTemplate = html<StoryArgs<FASTRadio>>`
         ?checked="${x => x.checked}"
         ?disabled="${x => x.disabled}"
         ?required="${x => x.required}"
-        ?readonly="${x => x.readOnly}"
         name="${x => x.name}"
         value="${x => x.value}"
     >
@@ -23,7 +22,6 @@ export default {
         checked: false,
         disabled: false,
         required: false,
-        readOnly: false,
         storyContent: "Label",
     },
     argTypes: {