Radio, Checkbox: Fix group label (#652)

* docs: add @sirrah-tam as a contributor * fix: update radio/checkbox labeling Remove concatenation for label reference as well as the aria-labelledby attribute so thelegend element is used to calculate accName for group. Added the aria-describedby attribute with the ID ref message to look for the associated hint/description text. * chore: add changeset * fix: use messageID value for aria-describedby ref Co-authored-by: Brent Swisher <[email protected]> * fix(radio/checkbox): add ifDefined directive for aria-describedby * test(radio/checkbox group): add tests for accDesc * fix: use template literal for groupID expression Co-authored-by: Dane Hillard <[email protected]> * fix: use template literal for groupID expression Co-authored-by: Dane Hillard <[email protected]> --------- Co-authored-by: Brent Swisher <[email protected]> Co-authored-by: Dane Hillard <[email protected]>
ithaka · Nov 30, 2023 · 3ad3fd6 · 3ad3fd6
1 parent 9cf3860
commit 3ad3fd6
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 12 deletions.
diff --git a/.changeset/twenty-buckets-sneeze.md b/.changeset/twenty-buckets-sneeze.md
@@ -0,0 +1,5 @@
+---
+'@ithaka/pharos': patch
+---
+
+Update radio/checkbox group labelling to include aria-describedby and remove aria-labelledby
diff --git a/packages/pharos/src/components/checkbox-group/pharos-checkbox-group.test.ts b/packages/pharos/src/components/checkbox-group/pharos-checkbox-group.test.ts
@@ -135,6 +135,28 @@ describe('pharos-checkbox-group', () => {
     expect(message?.textContent).to.equal(text);
   });
 
+  it('renders the provided message as the groups accessible description', async () => {
+    const text = 'Please make a selection';
+    component = await fixture(
+      html`
+        <test-pharos-checkbox-group message="${text}">
+          <span slot="legend">Checkbox Group Header</span>
+          <test-pharos-checkbox value="1"
+            ><span slot="label">Checkbox 1</span></test-pharos-checkbox
+          >
+          <test-pharos-checkbox value="2"
+            ><span slot="label">Checkbox 2</span></test-pharos-checkbox
+          >
+        </test-pharos-checkbox-group>
+      `
+    );
+    const groupDescID = component.renderRoot
+      .querySelector('fieldset')
+      ?.getAttribute('aria-describedby');
+
+    expect(component.renderRoot.querySelector(`#${groupDescID}`)?.textContent)?.to.equal(text);
+  });
+
   it('updates the state of its children', async () => {
     const boxes = component.querySelectorAll('test-pharos-checkbox') as NodeListOf<PharosCheckbox>;
     component.disabled = true;

diff --git a/packages/pharos/src/components/checkbox-group/pharos-checkbox-group.ts b/packages/pharos/src/components/checkbox-group/pharos-checkbox-group.ts
@@ -1,6 +1,7 @@
 import { html } from 'lit';
 import { property, queryAssignedElements } from 'lit/decorators.js';
 import type { PropertyValues, TemplateResult, CSSResultArray } from 'lit';
+import { ifDefined } from 'lit/directives/if-defined.js';
 import { checkboxGroupStyles } from './pharos-checkbox-group.css';
 import type { PharosCheckbox } from '../checkbox/pharos-checkbox';
 import { FormElement } from '../base/form-element';
@@ -81,16 +82,11 @@ export class PharosCheckboxGroup extends FormElement {
   }
 
   protected override render(): TemplateResult {
-    const labels = ['legend'];
-    if (this.messageId) {
-      labels.push(this.messageId);
-    }
-
     return html`
       <fieldset
         class="checkbox-group checkbox-group--${this.horizontal ? 'horizontal' : 'vertical'}"
         aria-invalid="${this.invalidated}"
-        aria-labelledby="${labels.join(' ')}"
+        aria-describedby=${ifDefined(this.messageId)}
       >
         <legend id="legend" class="checkbox-group__legend">
           <slot name="legend"></slot>

diff --git a/packages/pharos/src/components/radio-group/pharos-radio-group.test.ts b/packages/pharos/src/components/radio-group/pharos-radio-group.test.ts
@@ -328,6 +328,28 @@ describe('pharos-radio-group', () => {
     expect(message?.textContent).to.equal(text);
   });
 
+  it('renders the provided message as the groups accessible description', async () => {
+    const text = 'Please make a selection';
+    component = await fixture(
+      html`
+        <test-pharos-radio-group message="${text}">
+          <span slot="legend">Radio Group Header</span>
+          <test-pharos-radio-button value="1"
+            ><span slot="label">Radio 1</span></test-pharos-radio-button
+          >
+          <test-pharos-radio-button value="2"
+            ><span slot="label">Radio 2</span></test-pharos-radio-button
+          >
+        </test-pharos-radio-group>
+      `
+    );
+    const groupDescID = component.renderRoot
+      .querySelector('fieldset')
+      ?.getAttribute('aria-describedby');
+
+    expect(component.renderRoot.querySelector(`#${groupDescID}`)?.textContent)?.to.equal(text);
+  });
+
   it("stops propagation of its child's change event", async () => {
     const event = new Event('change');
     const changeSpy: SinonSpy = sinon.spy(event, 'stopPropagation');

diff --git a/packages/pharos/src/components/radio-group/pharos-radio-group.ts b/packages/pharos/src/components/radio-group/pharos-radio-group.ts
@@ -1,6 +1,7 @@
 import { html } from 'lit';
 import { property, queryAssignedElements } from 'lit/decorators.js';
 import type { PropertyValues, TemplateResult, CSSResultArray } from 'lit';
+import { ifDefined } from 'lit/directives/if-defined.js';
 import { radioGroupStyles } from './pharos-radio-group.css';
 import type { PharosRadioButton } from '../radio-button/pharos-radio-button';
 
@@ -167,17 +168,12 @@ export class PharosRadioGroup extends FormElement {
   }
 
   protected override render(): TemplateResult {
-    const labels = ['legend'];
-    if (this.messageId) {
-      labels.push(this.messageId);
-    }
-
     return html`
       <fieldset
         class="radio-group radio-group--${this.horizontal ? 'horizontal' : 'vertical'}"
         aria-required="${this.required}"
         aria-invalid="${this.invalidated}"
-        aria-labelledby="${labels.join(' ')}"
+        aria-describedby=${ifDefined(this.messageId)}
         role="radiogroup"
       >
         <legend id="legend" class="radio-group__legend">