From 1c903f4d26e964fe3b3e7b068375e78037bd5113 Mon Sep 17 00:00:00 2001 From: Cory LaViska Date: Mon, 14 Mar 2022 17:47:02 -0400 Subject: [PATCH] refactor radio logic --- docs/resources/changelog.md | 1 + src/components/radio-group/radio-group.ts | 80 ++++++++++++++++++++--- src/components/radio/radio.test.ts | 38 ++++++++--- src/components/radio/radio.ts | 74 ++++----------------- 4 files changed, 114 insertions(+), 79 deletions(-) diff --git a/docs/resources/changelog.md b/docs/resources/changelog.md index 1bb36c3a..88e1a722 100644 --- a/docs/resources/changelog.md +++ b/docs/resources/changelog.md @@ -11,6 +11,7 @@ _During the beta period, these restrictions may be relaxed in the event of a mis - Fixed a bug that prevented form submission from working as expected in some cases - Fixed a bug that prevented `` from toggling `vertical` properly [#703](https://github.com/shoelace-style/shoelace/issues/703) - Fixed a bug that prevented `` from rendering a color initially [#704](https://github.com/shoelace-style/shoelace/issues/704) +- Refactored `` to move selection logic into `` - Upgraded the status of `` from experimental to stable ## 2.0.0-beta.71 diff --git a/src/components/radio-group/radio-group.ts b/src/components/radio-group/radio-group.ts index fc6e2291..021be275 100644 --- a/src/components/radio-group/radio-group.ts +++ b/src/components/radio-group/radio-group.ts @@ -2,6 +2,7 @@ import { html, LitElement } from 'lit'; import { customElement, property, query } from 'lit/decorators.js'; import { classMap } from 'lit/directives/class-map.js'; import type SlRadio from '~/components/radio/radio'; +import { emit } from '~/internal/event'; import styles from './radio-group.styles'; /** @@ -26,15 +27,72 @@ export default class SlRadioGroup extends LitElement { /** Shows the fieldset and legend that surrounds the radio group. */ @property({ type: Boolean, attribute: 'fieldset' }) fieldset = false; - handleFocusIn() { - // When tabbing into the fieldset, make sure it lands on the checked radio - requestAnimationFrame(() => { - const checkedRadio = [...(this.defaultSlot.assignedElements({ flatten: true }) as SlRadio[])].find( - el => el.tagName.toLowerCase() === 'sl-radio' && el.checked - ); + connectedCallback() { + super.connectedCallback(); + this.setAttribute('role', 'radiogroup'); + } - checkedRadio?.focus(); + getAllRadios() { + return this.defaultSlot + .assignedElements({ flatten: true }) + .filter(el => el.tagName.toLowerCase() === 'sl-radio') as SlRadio[]; + } + + handleRadioClick(event: MouseEvent) { + const target = event.target as HTMLElement; + const checkedRadio = target.closest('sl-radio'); + + if (checkedRadio) { + const radios = this.getAllRadios(); + radios.forEach(radio => { + radio.checked = radio === checkedRadio; + radio.input.tabIndex = radio === checkedRadio ? 0 : -1; + }); + } + } + + handleKeyDown(event: KeyboardEvent) { + if (['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight'].includes(event.key)) { + const radios = this.getAllRadios().filter(radio => !radio.disabled); + const checkedRadio = radios.find(radio => radio.checked) ?? radios[0]; + const incr = ['ArrowUp', 'ArrowLeft'].includes(event.key) ? -1 : 1; + let index = radios.indexOf(checkedRadio) + incr; + if (index < 0) { + index = radios.length - 1; + } + if (index > radios.length - 1) { + index = 0; + } + + this.getAllRadios().forEach(radio => { + radio.checked = false; + radio.input.tabIndex = -1; + }); + + radios[index].focus(); + radios[index].checked = true; + radios[index].input.tabIndex = 0; + + emit(radios[index], 'sl-change'); + + event.preventDefault(); + } + } + + handleSlotChange() { + const radios = this.getAllRadios(); + const checkedRadio = radios.find(radio => radio.checked); + + radios.forEach(radio => { + radio.setAttribute('role', 'radio'); + radio.input.tabIndex = -1; }); + + if (checkedRadio) { + checkedRadio.input.tabIndex = 0; + } else if (radios.length > 0) { + radios[0].input.tabIndex = 0; + } } render() { @@ -45,13 +103,15 @@ export default class SlRadioGroup extends LitElement { 'radio-group': true, 'radio-group--has-fieldset': this.fieldset })} - role="radiogroup" - @focusin=${this.handleFocusIn} > ${this.label} - + `; } diff --git a/src/components/radio/radio.test.ts b/src/components/radio/radio.test.ts index 0da3d096..e05f4a36 100644 --- a/src/components/radio/radio.test.ts +++ b/src/components/radio/radio.test.ts @@ -1,5 +1,6 @@ -import { expect, fixture, html, oneEvent } from '@open-wc/testing'; +import { expect, fixture, html, oneEvent, waitUntil } from '@open-wc/testing'; import { sendKeys } from '@web/test-runner-commands'; +import sinon from 'sinon'; import type SlRadioGroup from '~/components/radio-group/radio-group'; import type SlRadio from './radio'; @@ -52,12 +53,33 @@ describe('', () => { expect(radio2.checked).to.be.true; }); - it('should not fire sl-change when checked is set by javascript', async () => { - const el = await fixture(html` `); - el.addEventListener('sl-change', () => expect.fail('event fired')); - el.checked = true; - await el.updateComplete; - el.checked = false; - await el.updateComplete; + 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')[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'); + }); }); }); diff --git a/src/components/radio/radio.ts b/src/components/radio/radio.ts index a2247108..ac0d952b 100644 --- a/src/components/radio/radio.ts +++ b/src/components/radio/radio.ts @@ -54,20 +54,9 @@ export default class SlRadio extends LitElement { */ @property({ type: Boolean, reflect: true }) invalid = false; - firstUpdated() { - this.updateComplete.then(() => { - const radios = this.getAllRadios(); - const checkedRadio = radios.find(radio => radio.checked); - radios.forEach(radio => { - radio.input.tabIndex = -1; - }); - - if (checkedRadio) { - checkedRadio.input.tabIndex = 0; - } else if (radios.length > 0) { - radios[0].input.tabIndex = 0; - } - }); + connectedCallback(): void { + super.connectedCallback(); + this.setAttribute('role', 'radio'); } /** Simulates a click on the radio. */ @@ -107,37 +96,33 @@ export default class SlRadio extends LitElement { return [...radioGroup.querySelectorAll('sl-radio')].filter((radio: this) => radio.name === this.name); } - getSiblingRadios() { - return this.getAllRadios().filter(radio => radio !== this); - } - handleBlur() { this.hasFocus = false; emit(this, 'sl-blur'); } - @watch('checked', { waitUntilFirstUpdate: true }) + @watch('checked') handleCheckedChange() { - if (this.checked) { - this.input.tabIndex = 0; + this.setAttribute('aria-checked', this.checked ? 'true' : 'false'); - this.getSiblingRadios().forEach(radio => { - radio.input.tabIndex = -1; - radio.checked = false; - }); + if (this.hasUpdated) { + emit(this, 'sl-change'); } } handleClick() { this.checked = true; - 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 - this.input.disabled = this.disabled; - this.invalid = !this.input.checkValidity(); + if (this.hasUpdated) { + this.input.disabled = this.disabled; + this.invalid = !this.input.checkValidity(); + } } handleFocus() { @@ -145,38 +130,7 @@ export default class SlRadio extends LitElement { emit(this, 'sl-focus'); } - handleKeyDown(event: KeyboardEvent) { - if (['ArrowUp', 'ArrowDown', 'ArrowLeft', 'ArrowRight'].includes(event.key)) { - const radios = this.getAllRadios().filter(radio => !radio.disabled); - const incr = ['ArrowUp', 'ArrowLeft'].includes(event.key) ? -1 : 1; - let index = radios.indexOf(this) + incr; - if (index < 0) { - index = radios.length - 1; - } - if (index > radios.length - 1) { - index = 0; - } - - this.getAllRadios().forEach(radio => { - radio.checked = false; - radio.input.tabIndex = -1; - }); - - radios[index].focus(); - radios[index].checked = true; - radios[index].input.tabIndex = 0; - - emit(radios[index], 'sl-change'); - - event.preventDefault(); - } - } - render() { - this.setAttribute('role', 'radio'); - this.setAttribute('aria-checked', this.checked ? 'true' : 'false'); - this.setAttribute('aria-disabled', this.disabled ? 'true' : 'false'); - return html`