From f44c81dad2a1383faff202a74a9bdb1c41248295 Mon Sep 17 00:00:00 2001 From: Christian Schilling Date: Mon, 30 Sep 2024 12:58:05 +0200 Subject: [PATCH 1/2] Add ability to focus sl-radio-group dynamically --- .../radio-group/radio-group.component.ts | 19 +++- .../radio-group/radio-group.test.ts | 96 +++++++++++++++++++ 2 files changed, 111 insertions(+), 4 deletions(-) diff --git a/src/components/radio-group/radio-group.component.ts b/src/components/radio-group/radio-group.component.ts index aecf78b5df..01f952997d 100644 --- a/src/components/radio-group/radio-group.component.ts +++ b/src/components/radio-group/radio-group.component.ts @@ -191,17 +191,23 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor event.preventDefault(); } - private handleLabelClick() { + private handleFocus(options?: FocusOptions) { const radios = this.getAllRadios(); const checked = radios.find(radio => radio.checked); - const radioToFocus = checked || radios[0]; + const firstEnabledRadio = radios.find(radio => !radio.disabled); + const radioToFocus = checked || firstEnabledRadio; - // Move focus to the checked radio (or the first one if none are checked) when clicking the label + // Call focus for the checked radio + // If no radio is checked, focus the first one that is not disabled if (radioToFocus) { - radioToFocus.focus(); + radioToFocus.focus(options); } } + private handleLabelClick() { + this.handleFocus(); + } + private handleInvalid(event: Event) { this.formControlController.setValidity(false); this.formControlController.emitInvalidEvent(event); @@ -325,6 +331,11 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor this.formControlController.updateValidity(); } + /** Sets focus on the radio-group. */ + focus(options?: FocusOptions) { + this.handleFocus(options); + } + render() { const hasLabelSlot = this.hasSlotController.test('label'); const hasHelpTextSlot = this.hasSlotController.test('help-text'); diff --git a/src/components/radio-group/radio-group.test.ts b/src/components/radio-group/radio-group.test.ts index ba9c9a7e29..4fa2e532dc 100644 --- a/src/components/radio-group/radio-group.test.ts +++ b/src/components/radio-group/radio-group.test.ts @@ -300,6 +300,102 @@ describe('when a size is applied', () => { }); }); +describe('when handling focus', () => { + const doAction = async (instance: SlRadioGroup, type: string) => { + if (type === 'focus') { + instance.focus(); + await instance.updateComplete; + return; + } + + const label = instance.shadowRoot!.querySelector('#label')!; + label.click(); + await instance.updateComplete; + }; + + // Tests for focus and label actions with radio buttons + ['focus', 'label'].forEach(actionType => { + describe(`when using ${actionType}`, () => { + it('should do nothing if all elements are disabled', async () => { + const el = await fixture(html` + + + + + + + `); + + const validFocusHandler = sinon.spy(); + + Array.from(el.querySelectorAll('sl-radio')).forEach(radio => + radio.addEventListener('sl-focus', validFocusHandler) + ); + + expect(validFocusHandler).to.not.have.been.called; + await doAction(el, actionType); + expect(validFocusHandler).to.not.have.been.called; + }); + + it('should focus the first radio that is enabled when the group receives focus', async () => { + const el = await fixture(html` + + + + + + + `); + + const invalidFocusHandler = sinon.spy(); + const validFocusHandler = sinon.spy(); + + const disabledRadio = el.querySelector('#radio-0')!; + const validRadio = el.querySelector('#radio-1')!; + + disabledRadio.addEventListener('sl-focus', invalidFocusHandler); + validRadio.addEventListener('sl-focus', validFocusHandler); + + expect(invalidFocusHandler).to.not.have.been.called; + expect(validFocusHandler).to.not.have.been.called; + + await doAction(el, actionType); + + expect(invalidFocusHandler).to.not.have.been.called; + expect(validFocusHandler).to.have.been.called; + }); + + it('should focus the currently enabled radio when the group receives focus', async () => { + const el = await fixture(html` + + + + + + + `); + + const invalidFocusHandler = sinon.spy(); + const validFocusHandler = sinon.spy(); + + const disabledRadio = el.querySelector('#radio-0')!; + const validRadio = el.querySelector('#radio-2')!; + + disabledRadio.addEventListener('sl-focus', invalidFocusHandler); + validRadio.addEventListener('sl-focus', validFocusHandler); + + expect(invalidFocusHandler).to.not.have.been.called; + expect(validFocusHandler).to.not.have.been.called; + + await doAction(el, actionType); + + expect(invalidFocusHandler).to.not.have.been.called; + expect(validFocusHandler).to.have.been.called; + }); + }); + }); +}); + describe('when the value changes', () => { it('should emit sl-change when toggled with the arrow keys', async () => { const radioGroup = await fixture(html` From d0f57e335fe11d39b8eb032784271144b539b034 Mon Sep 17 00:00:00 2001 From: Christian Schilling Date: Wed, 2 Oct 2024 07:55:43 +0200 Subject: [PATCH 2/2] Adjusted to review findings --- .../radio-group/radio-group.component.ts | 28 ++++++++----------- 1 file changed, 12 insertions(+), 16 deletions(-) diff --git a/src/components/radio-group/radio-group.component.ts b/src/components/radio-group/radio-group.component.ts index 01f952997d..c1b4dc4bee 100644 --- a/src/components/radio-group/radio-group.component.ts +++ b/src/components/radio-group/radio-group.component.ts @@ -191,21 +191,8 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor event.preventDefault(); } - private handleFocus(options?: FocusOptions) { - const radios = this.getAllRadios(); - const checked = radios.find(radio => radio.checked); - const firstEnabledRadio = radios.find(radio => !radio.disabled); - const radioToFocus = checked || firstEnabledRadio; - - // Call focus for the checked radio - // If no radio is checked, focus the first one that is not disabled - if (radioToFocus) { - radioToFocus.focus(options); - } - } - private handleLabelClick() { - this.handleFocus(); + this.focus(); } private handleInvalid(event: Event) { @@ -332,8 +319,17 @@ export default class SlRadioGroup extends ShoelaceElement implements ShoelaceFor } /** Sets focus on the radio-group. */ - focus(options?: FocusOptions) { - this.handleFocus(options); + public focus(options?: FocusOptions) { + const radios = this.getAllRadios(); + const checked = radios.find(radio => radio.checked); + const firstEnabledRadio = radios.find(radio => !radio.disabled); + const radioToFocus = checked || firstEnabledRadio; + + // Call focus for the checked radio + // If no radio is checked, focus the first one that is not disabled + if (radioToFocus) { + radioToFocus.focus(options); + } } render() {