From afcc46e1cc4d7f6e9d1a50f8b367da4b1d0c3143 Mon Sep 17 00:00:00 2001 From: Brandy Carney Date: Tue, 24 Nov 2020 11:43:59 -0500 Subject: [PATCH] fix(radio): properly announce radios on screen readers and resolve axe errors (#22507) --- core/src/components.d.ts | 2 +- core/src/components/alert/alert.tsx | 4 +- .../components/radio-group/radio-group.tsx | 30 +++- .../radio-group/test/basic/index.html | 141 +++++++++++---- .../radio-group/test/standalone/index.html | 30 ++-- core/src/components/radio/radio.scss | 23 ++- core/src/components/radio/radio.tsx | 48 ++--- core/src/components/radio/test/a11y/e2e.ts | 19 ++ .../src/components/radio/test/a11y/index.html | 166 ++++++++++++++++++ .../radio/test/standalone/index.html | 56 +++--- .../components/select/test/a11y/index.html | 12 +- core/src/utils/helpers.ts | 2 +- 12 files changed, 409 insertions(+), 124 deletions(-) create mode 100644 core/src/components/radio/test/a11y/e2e.ts create mode 100644 core/src/components/radio/test/a11y/index.html diff --git a/core/src/components.d.ts b/core/src/components.d.ts index 655125b8ef..d6c06fa6be 100644 --- a/core/src/components.d.ts +++ b/core/src/components.d.ts @@ -1708,7 +1708,7 @@ export namespace Components { */ "name": string; "setButtonTabindex": (value: number) => Promise; - "setFocus": () => Promise; + "setFocus": (ev: any) => Promise; /** * the value of the radio. */ diff --git a/core/src/components/alert/alert.tsx b/core/src/components/alert/alert.tsx index bfce34f037..bcb4e0e46a 100644 --- a/core/src/components/alert/alert.tsx +++ b/core/src/components/alert/alert.tsx @@ -158,7 +158,7 @@ export class Alert implements ComponentInterface, OverlayInterface { // If hitting arrow down or arrow right, move to the next radio // If we're on the last radio, move to the first radio - if (['ArrowDown', 'ArrowRight'].includes(ev.key)) { + if (['ArrowDown', 'ArrowRight'].includes(ev.code)) { nextEl = (index === radios.length - 1) ? radios[0] : radios[index + 1]; @@ -166,7 +166,7 @@ export class Alert implements ComponentInterface, OverlayInterface { // If hitting arrow up or arrow left, move to the previous radio // If we're on the first radio, move to the last radio - if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) { + if (['ArrowUp', 'ArrowLeft'].includes(ev.code)) { nextEl = (index === 0) ? radios[radios.length - 1] : radios[index - 1]; diff --git a/core/src/components/radio-group/radio-group.tsx b/core/src/components/radio-group/radio-group.tsx index 33ee100074..8a8731ce10 100644 --- a/core/src/components/radio-group/radio-group.tsx +++ b/core/src/components/radio-group/radio-group.tsx @@ -10,6 +10,7 @@ export class RadioGroup implements ComponentInterface { private inputId = `ion-rg-${radioGroupIds++}`; private labelId = `${this.inputId}-lbl`; + private label?: HTMLIonLabelElement | null; @Element() el!: HTMLElement; @@ -68,10 +69,9 @@ export class RadioGroup implements ComponentInterface { async connectedCallback() { // Get the list header if it exists and set the id // this is used to set aria-labelledby - const el = this.el; - const header = el.querySelector('ion-list-header') || el.querySelector('ion-item-divider'); + const header = this.el.querySelector('ion-list-header') || this.el.querySelector('ion-item-divider'); if (header) { - const label = header.querySelector('ion-label'); + const label = this.label = header.querySelector('ion-label'); if (label) { this.labelId = label.id = this.name + '-lbl'; } @@ -83,6 +83,9 @@ export class RadioGroup implements ComponentInterface { } private onClick = (ev: Event) => { + ev.preventDefault(); + ev.stopPropagation(); + const selectedRadio = ev.target && (ev.target as HTMLElement).closest('ion-radio'); if (selectedRadio) { const currentValue = this.value; @@ -110,12 +113,13 @@ export class RadioGroup implements ComponentInterface { // Only move the radio if the current focus is in the radio group if (ev.target && radios.includes(ev.target)) { const index = radios.findIndex(radio => radio === ev.target); + const current = radios[index]; let next; // If hitting arrow down or arrow right, move to the next radio // If we're on the last radio, move to the first radio - if (['ArrowDown', 'ArrowRight'].includes(ev.key)) { + if (['ArrowDown', 'ArrowRight'].includes(ev.code)) { next = (index === radios.length - 1) ? radios[0] : radios[index + 1]; @@ -123,29 +127,39 @@ export class RadioGroup implements ComponentInterface { // If hitting arrow up or arrow left, move to the previous radio // If we're on the first radio, move to the last radio - if (['ArrowUp', 'ArrowLeft'].includes(ev.key)) { + if (['ArrowUp', 'ArrowLeft'].includes(ev.code)) { next = (index === 0) ? radios[radios.length - 1] : radios[index - 1]; } if (next && radios.includes(next)) { - next.setFocus(); + next.setFocus(ev); if (!inSelectPopover) { this.value = next.value; } } + + // Update the radio group value when a user presses the + // space bar on top of a selected radio (only applies + // to radios in a select popover) + if (['Space'].includes(ev.code)) { + this.value = current.value; + } } } render() { + const { label, labelId } = this; + const mode = getIonMode(this); + return ( ); diff --git a/core/src/components/radio-group/test/basic/index.html b/core/src/components/radio-group/test/basic/index.html index e89dc9162a..c2f123977f 100644 --- a/core/src/components/radio-group/test/basic/index.html +++ b/core/src/components/radio-group/test/basic/index.html @@ -20,85 +20,156 @@ - - - Luckiest Man On Earth - +

+ Add Radio + Add Checked + Remove Radio +

- - Biff - - - - + + + + Luckiest Man On Earth + - - Griff - - - - + + Biff + + + + - - Buford - - - - + + Griff + + + + - - George - - + + Buford + + + + - + + George + + +
+ - Add Select - Add Checked Select - Remove Select + + + + Huey + + + + Dewey + + + + + Louie + + + + + + + + + + Maintenance Drone + + + + + Huey + + + + + Dewey + + + + + Louie + + + +
+ diff --git a/core/src/components/radio-group/test/standalone/index.html b/core/src/components/radio-group/test/standalone/index.html index 4f14aec170..0e5db1f98d 100644 --- a/core/src/components/radio-group/test/standalone/index.html +++ b/core/src/components/radio-group/test/standalone/index.html @@ -13,28 +13,28 @@ - + - - - - - - - - - + + + + + + + + + - - + +

allow-empty-selection="true":  - - - + + +

diff --git a/core/src/components/radio/radio.scss b/core/src/components/radio/radio.scss index 210201aab4..b552149837 100644 --- a/core/src/components/radio/radio.scss +++ b/core/src/components/radio/radio.scss @@ -25,7 +25,6 @@ pointer-events: none; } - .radio-icon { display: flex; @@ -38,11 +37,25 @@ contain: layout size style; } -button { - @include input-cover(); -} - .radio-icon, .radio-inner { box-sizing: border-box; } + +label { + @include input-cover(); + + display: flex; + + align-items: center; + + opacity: 0; +} + +input { + @include visually-hidden(); +} + +:host(:focus) { + outline: none; +} diff --git a/core/src/components/radio/radio.tsx b/core/src/components/radio/radio.tsx index 31ceab6267..9380c8d35a 100644 --- a/core/src/components/radio/radio.tsx +++ b/core/src/components/radio/radio.tsx @@ -2,7 +2,7 @@ import { Component, ComponentInterface, Element, Event, EventEmitter, Host, Meth import { getIonMode } from '../../global/ionic-global'; import { Color, StyleEventDetail } from '../../interface'; -import { addEventListener, findItemLabel, removeEventListener } from '../../utils/helpers'; +import { addEventListener, getAriaLabel, removeEventListener } from '../../utils/helpers'; import { createColorClasses, hostContext } from '../../utils/theme'; /** @@ -20,11 +20,10 @@ import { createColorClasses, hostContext } from '../../utils/theme'; shadow: true }) export class Radio implements ComponentInterface { - private buttonEl?: HTMLButtonElement; private inputId = `ion-rb-${radioButtonIds++}`; private radioGroup: HTMLIonRadioGroupElement | null = null; - @Element() el!: HTMLElement; + @Element() el!: HTMLIonRadioElement; /** * If `true`, the radio is selected. @@ -77,10 +76,11 @@ export class Radio implements ComponentInterface { /** @internal */ @Method() - async setFocus() { - if (this.buttonEl) { - this.buttonEl.focus(); - } + async setFocus(ev: any) { + ev.stopPropagation(); + ev.preventDefault(); + + this.el.focus(); } /** @internal */ @@ -139,17 +139,17 @@ export class Radio implements ComponentInterface { render() { const { inputId, disabled, checked, color, el, buttonTabindex } = this; const mode = getIonMode(this); - const labelId = inputId + '-lbl'; - const label = findItemLabel(el); - if (label) { - label.id = labelId; - } + const { label, labelId, labelText } = getAriaLabel(el, inputId); + return (
+
- + tabindex="-1" + id={inputId} + /> ); } diff --git a/core/src/components/radio/test/a11y/e2e.ts b/core/src/components/radio/test/a11y/e2e.ts new file mode 100644 index 0000000000..46cd82f25c --- /dev/null +++ b/core/src/components/radio/test/a11y/e2e.ts @@ -0,0 +1,19 @@ +import { newE2EPage } from '@stencil/core/testing'; + +test('radio: a11y', async () => { + const page = await newE2EPage({ + url: '/src/components/radio/test/a11y?ionic:_testing=true' + }); + + const compare = await page.compareScreenshot(); + expect(compare).toMatchScreenshot(); +}); + +test('radio:rtl: a11y', async () => { + const page = await newE2EPage({ + url: '/src/components/radio/test/a11y?ionic:_testing=true&rtl=true' + }); + + const compare = await page.compareScreenshot(); + expect(compare).toMatchScreenshot(); +}); diff --git a/core/src/components/radio/test/a11y/index.html b/core/src/components/radio/test/a11y/index.html new file mode 100644 index 0000000000..10ed629b03 --- /dev/null +++ b/core/src/components/radio/test/a11y/index.html @@ -0,0 +1,166 @@ + + + + + + Radio - a11y + + + + + + + + + + + + Radio - a11y + + + + +
+

Select a maintenance drone (native):

+ +
+ + +
+ +
+ + +
+ +
+ + +
+ +
+ + +
+
+ + + + + Select a maintenance drone: + + + + + Huey + + + + + Dewey + + + + + Fooey + + + + + Louie + + + + + + + + + Huey + + + + + Dewey + + + + + Fooey + + + + + Louie + + + + + +
+ +
Custom Labels
+
+ + +
+
+ + +
+
+ + +
+
+ + +
+
+
+
+ +
+ + + + + + + diff --git a/core/src/components/radio/test/standalone/index.html b/core/src/components/radio/test/standalone/index.html index 4f3feacf3a..aea1e2ce00 100644 --- a/core/src/components/radio/test/standalone/index.html +++ b/core/src/components/radio/test/standalone/index.html @@ -14,49 +14,49 @@

Default

- - + +

Colors: Unchecked

- - - - - - - - - + + + + + + + + +

Colors: Checked

- - - - - - - - - + + + + + + + + +

Disabled

- - - - + + + +

Custom

- - - - + + + +