From 90f4d77ed6767a56022a5b238adae2a26c44e149 Mon Sep 17 00:00:00 2001 From: Burton Smith Date: Tue, 2 Aug 2022 12:59:00 -0400 Subject: [PATCH 1/5] fix screen reader issues for radios and group --- .../radio-button/radio-button.test.ts | 145 +---------- src/components/radio-button/radio-button.ts | 98 ++------ .../radio-group/radio-group.styles.ts | 12 + .../radio-group/radio-group.test.ts | 152 ++++++++++-- src/components/radio-group/radio-group.ts | 227 +++++++++++++----- src/components/radio/radio.styles.ts | 6 +- src/components/radio/radio.test.ts | 172 +------------ src/components/radio/radio.ts | 153 +++--------- 8 files changed, 396 insertions(+), 569 deletions(-) diff --git a/src/components/radio-button/radio-button.test.ts b/src/components/radio-button/radio-button.test.ts index d5947183b..af59461b3 100644 --- a/src/components/radio-button/radio-button.test.ts +++ b/src/components/radio-button/radio-button.test.ts @@ -1,155 +1,22 @@ -import { aTimeout, expect, fixture, html, oneEvent, waitUntil } from '@open-wc/testing'; -import { sendKeys } from '@web/test-runner-commands'; -import sinon from 'sinon'; +import { expect, fixture, html } from '@open-wc/testing'; import type SlRadioGroup from '../../components/radio-group/radio-group'; import type SlRadioButton from './radio-button'; describe('', () => { - it('should be disabled with the disabled attribute', async () => { - const el = await fixture(html` `); - - expect(el.input.disabled).to.be.true; - }); - - it('should be valid by default', async () => { - const el = await fixture(html` `); - - expect(el.invalid).to.be.false; - }); - - it('should fire sl-change when clicked', async () => { - const el = await fixture(html` `); - setTimeout(() => el.input.click()); - const event = (await oneEvent(el, 'sl-change')) as CustomEvent; - expect(event.target).to.equal(el); - expect(el.checked).to.be.true; - }); - - it('should fire sl-change when toggled via keyboard - space', async () => { - const el = await fixture(html` `); - el.input.focus(); - setTimeout(() => sendKeys({ press: ' ' })); - const event = (await oneEvent(el, 'sl-change')) as CustomEvent; - expect(event.target).to.equal(el); - expect(el.checked).to.be.true; - }); - - it('should fire sl-change when toggled via keyboard - arrow key', async () => { + it('should not get checked when disabled', async () => { const radioGroup = await fixture(html` - - - + + + `); const radio1 = radioGroup.querySelector('#radio-1')!; const radio2 = radioGroup.querySelector('#radio-2')!; - radio1.input.focus(); - setTimeout(() => sendKeys({ press: 'ArrowRight' })); - const event = (await oneEvent(radio2, 'sl-change')) as CustomEvent; - expect(event.target).to.equal(radio2); - expect(radio2.checked).to.be.true; - }); - - it('should not get checked when disabled', async () => { - const radioGroup = await fixture(html` - - - - - `); - const radio1 = radioGroup.querySelector('sl-radio-button[checked]')!; - const radio2 = radioGroup.querySelector('sl-radio-button[disabled]')!; radio2.click(); await Promise.all([radio1.updateComplete, radio2.updateComplete]); - + expect(radio1.checked).to.be.true; expect(radio2.checked).to.be.false; }); - - describe('when submitting a form', () => { - it('should submit the correct value', async () => { - const form = await fixture(html` -
- - - - - - Submit -
- `); - const button = form.querySelector('sl-button')!; - const radio = form.querySelectorAll('sl-radio-button')[1]!; - const submitHandler = sinon.spy((event: SubmitEvent) => { - formData = new FormData(form); - event.preventDefault(); - }); - let formData: FormData; - - form.addEventListener('submit', submitHandler); - radio.click(); - button.click(); - - await waitUntil(() => submitHandler.calledOnce); - - expect(formData!.get('a')).to.equal('2'); - }); - }); - - describe('when resetting a form', () => { - it('should reset the element to its initial value', async () => { - const form = await fixture(html` -
- - - - - - Reset -
- `); - const button = form.querySelector('sl-button')!; - const radio1: SlRadioButton = form.querySelector('#radio-1')!; - const radio2: SlRadioButton = form.querySelector('#radio-2')!; - - radio2.click(); - await radio2.updateComplete; - - expect(radio2.checked).to.be.true; - expect(radio1.checked).to.be.false; - - setTimeout(() => button.click()); - - await oneEvent(form, 'reset'); - await radio1.updateComplete; - - expect(radio1.checked).to.true; - expect(radio2.checked).to.false; - }); - }); - - it('should show a constraint validation error when setCustomValidity() is called', async () => { - const form = await fixture(html` -
- - - - - Submit -
- `); - const button = form.querySelector('sl-button')!; - const radio = form.querySelectorAll('sl-radio-button')[1]!; - const submitHandler = sinon.spy((event: SubmitEvent) => event.preventDefault()); - - // Submitting the form after setting custom validity should not trigger the handler - radio.setCustomValidity('Invalid selection'); - form.addEventListener('submit', submitHandler); - button.click(); - - await aTimeout(100); - - expect(submitHandler).to.not.have.been.called; - }); }); diff --git a/src/components/radio-button/radio-button.ts b/src/components/radio-button/radio-button.ts index 7c918b295..f91833d91 100644 --- a/src/components/radio-button/radio-button.ts +++ b/src/components/radio-button/radio-button.ts @@ -3,9 +3,7 @@ import { customElement, property, query, state } from 'lit/decorators.js'; import { classMap } from 'lit/directives/class-map.js'; import { ifDefined } from 'lit/directives/if-defined.js'; import { html } from 'lit/static-html.js'; -import { defaultValue } from '../../internal/default-value'; import { emit } from '../../internal/event'; -import { FormSubmitController } from '../../internal/form'; import { HasSlotController } from '../../internal/slot'; import { watch } from '../../internal/watch'; import styles from './radio-button.styles'; @@ -18,7 +16,6 @@ import type { CSSResultGroup } from 'lit'; * @slot - The radio's label. * * @event sl-blur - Emitted when the button loses focus. - * @event sl-change - Emitted when the button's checked state changes. * @event sl-focus - Emitted when the button gains focus. * * @slot - The button's label. @@ -38,17 +35,13 @@ export default class SlRadioButton extends LitElement { @query('.button') input: HTMLInputElement; @query('.hidden-input') hiddenInput: HTMLInputElement; - protected readonly formSubmitController = new FormSubmitController(this, { - value: (control: SlRadioButton) => (control.checked ? control.value : undefined), - defaultValue: (control: SlRadioButton) => control.defaultChecked, - setValue: (control: SlRadioButton, checked: boolean) => (control.checked = checked) - }); private readonly hasSlotController = new HasSlotController(this, '[default]', 'prefix', 'suffix'); @state() protected hasFocus = false; + @state() checked = false; /** The radio's name attribute. */ - @property() name: string; + @property({ reflect: true }) name: string; /** The radio's value attribute. */ @property() value: string; @@ -56,47 +49,20 @@ export default class SlRadioButton extends LitElement { /** Disables the radio. */ @property({ type: Boolean, reflect: true }) disabled = false; - /** Draws the radio in a checked state. */ - @property({ type: Boolean, reflect: true }) checked = false; + /** The button's size. */ + @property({ reflect: true }) size: 'small' | 'medium' | 'large' = 'medium'; - /** - * This will be true when the control is in an invalid state. Validity in radios is determined by the message provided - * by the `setCustomValidity` method. - */ - @property({ type: Boolean, reflect: true }) invalid = false; - - /** Gets or sets the default value used to reset this element. The initial value corresponds to the one originally specified in the HTML that created this element. */ - @defaultValue('checked') - defaultChecked = false; + /** Draws a pill-style button with rounded edges. */ + @property({ type: Boolean, reflect: true }) pill = false; connectedCallback(): void { super.connectedCallback(); - this.setAttribute('role', 'radio'); + this.setAttribute('role', 'presentation'); } - /** Simulates a click on the radio. */ - click() { - this.input.click(); - } - - /** Sets focus on the radio. */ - focus(options?: FocusOptions) { - this.input.focus(options); - } - - /** Removes focus from the radio. */ - blur() { - this.input.blur(); - } - - /** Checks for validity and shows the browser's validation message if the control is invalid. */ - reportValidity() { - return this.hiddenInput.reportValidity(); - } - - /** Sets a custom validation message. If `message` is not empty, the field will be considered invalid. */ - setCustomValidity(message: string) { - this.hiddenInput.setCustomValidity(message); + @watch('disabled', { waitUntilFirstUpdate: true }) + handleDisabledChange() { + this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false'); } handleBlur() { @@ -104,10 +70,14 @@ export default class SlRadioButton extends LitElement { emit(this, 'sl-blur'); } - handleClick() { - if (!this.disabled) { - this.checked = true; + handleClick(e: MouseEvent) { + if (this.disabled) { + e.preventDefault(); + e.stopPropagation(); + return; } + + this.checked = true; } handleFocus() { @@ -115,38 +85,13 @@ export default class SlRadioButton extends LitElement { emit(this, 'sl-focus'); } - @watch('checked') - handleCheckedChange() { - this.setAttribute('aria-checked', this.checked ? 'true' : 'false'); - - if (this.hasUpdated) { - emit(this, 'sl-change'); - } - } - - @watch('disabled', { waitUntilFirstUpdate: true }) - handleDisabledChange() { - this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false'); - - // Disabled form controls are always valid, so we need to recheck validity when the state changes - if (this.hasUpdated) { - this.input.disabled = this.disabled; - this.invalid = !this.input.checkValidity(); - } - } - - /** The button's size. */ - @property({ reflect: true }) size: 'small' | 'medium' | 'large' = 'medium'; - - /** Draws a pill-style button with rounded edges. */ - @property({ type: Boolean, reflect: true }) pill = false; - render() { return html` -
- +