From 26c7b474889255e1e25665703c596f212eef15ad Mon Sep 17 00:00:00 2001 From: Jeri Peier Date: Fri, 20 Dec 2024 13:18:19 +0100 Subject: [PATCH] fix(sbb-radio-button-group): sync radios synchronously (#3323) --- .../core/mixins/form-associated-mixin.ts | 6 +- .../form-associated-radio-button-mixin.ts | 76 ++++++++++--------- .../common/radio-button-common.ts | 40 +--------- .../radio-button-group.spec.ts | 20 ++++- .../radio-button-group/radio-button-group.ts | 2 +- .../radio-button-panel/radio-button-panel.ts | 34 ++++++++- .../radio-button/radio-button.spec.ts | 26 +++---- 7 files changed, 107 insertions(+), 97 deletions(-) diff --git a/src/elements/core/mixins/form-associated-mixin.ts b/src/elements/core/mixins/form-associated-mixin.ts index 1dd4c97fc0..c6993210ac 100644 --- a/src/elements/core/mixins/form-associated-mixin.ts +++ b/src/elements/core/mixins/form-associated-mixin.ts @@ -5,11 +5,9 @@ import type { AbstractConstructor } from './constructor.js'; export declare abstract class SbbFormAssociatedMixinType { public get form(): HTMLFormElement | null; - public get name(): string; - public set name(value: string); + public accessor name: string; public get type(): string; - public get value(): V | null; - public set value(value: V | null); + public accessor value: V | null; public get validity(): ValidityState; public get validationMessage(): string; diff --git a/src/elements/core/mixins/form-associated-radio-button-mixin.ts b/src/elements/core/mixins/form-associated-radio-button-mixin.ts index e230832767..2f938e9916 100644 --- a/src/elements/core/mixins/form-associated-radio-button-mixin.ts +++ b/src/elements/core/mixins/form-associated-radio-button-mixin.ts @@ -1,9 +1,8 @@ -import type { LitElement, PropertyValues } from 'lit'; +import { isServer, type LitElement, type PropertyValues } from 'lit'; import { property } from 'lit/decorators.js'; import { getNextElementIndex, interactivityChecker, isArrowKeyPressed } from '../a11y.js'; import { SbbConnectedAbortController } from '../controllers.js'; -import { forceType } from '../decorators.js'; import type { Constructor } from './constructor.js'; import { SbbDisabledMixin, type SbbDisabledMixinType } from './disabled-mixin.js'; @@ -31,9 +30,9 @@ export declare class SbbFormAssociatedRadioButtonMixinType extends SbbFormAssociatedMixinType implements Partial, Partial { - public checked: boolean; - public disabled: boolean; - public required: boolean; + public accessor checked: boolean; + public accessor disabled: boolean; + public accessor required: boolean; protected associatedRadioButtons?: Set; /** @deprecated No longer used internally. */ @@ -65,9 +64,30 @@ export const SbbFormAssociatedRadioButtonMixin = ): void { super.willUpdate(changedProperties); - // On 'name' change, move 'this' to the new registry - if (changedProperties.has('name')) { - this._disconnectFromRegistry(); - this._connectToRegistry(); - if (this.checked) { - this._deselectGroupedRadios(); - } - this.updateFocusableRadios(); - } - - if (changedProperties.has('checked')) { - this.toggleAttribute('data-checked', this.checked); - this.internals.ariaChecked = this.checked.toString(); - this.updateFormValueOnCheckedChange(); - if (this.checked) { - this._deselectGroupedRadios(); - } - this.updateFocusableRadios(); - } - if (changedProperties.has('disabled')) { this.updateFocusableRadios(); } @@ -168,18 +169,12 @@ export const SbbFormAssociatedRadioButtonMixin = ; - export declare class SbbRadioButtonCommonElementMixinType extends SbbFormAssociatedRadioButtonMixinType { public get allowEmptySelection(): boolean; public set allowEmptySelection(boolean); @@ -41,7 +30,6 @@ export const SbbRadioButtonCommonElementMixin = = new EventEmitter( - this, - SbbRadioButtonCommonElement.events.stateChange, - { bubbles: true }, - ); - public constructor() { super(); this.addEventListener?.('click', (e) => this._handleClick(e)); @@ -107,21 +84,6 @@ export const SbbRadioButtonCommonElementMixin = ): void { - super.willUpdate(changedProperties); - - if (changedProperties.has('checked')) { - if (this.checked !== changedProperties.get('checked')!) { - this._stateChange.emit({ type: 'checked', checked: this.checked }); - } - } - if (changedProperties.has('disabled')) { - if (this.disabled !== changedProperties.get('disabled')!) { - this._stateChange.emit({ type: 'disabled', disabled: this.disabled }); - } - } - } - protected override isDisabledExternally(): boolean { return this.group?.disabled ?? false; } diff --git a/src/elements/radio-button/radio-button-group/radio-button-group.spec.ts b/src/elements/radio-button/radio-button-group/radio-button-group.spec.ts index 1b56ef0898..c943cfa02e 100644 --- a/src/elements/radio-button/radio-button-group/radio-button-group.spec.ts +++ b/src/elements/radio-button/radio-button-group/radio-button-group.spec.ts @@ -127,9 +127,25 @@ import '../radio-button-panel.js'; describe('events', () => { it('dispatches event on radio change', async () => { + radios[0].checked = true; const radio = radios[1]; - const changeSpy = new EventSpy('change'); - const inputSpy = new EventSpy('input'); + const changeSpy = new EventSpy('change', element); + const inputSpy = new EventSpy('input', element); + + element.addEventListener( + 'change', + () => { + expect(element.value).to.be.equal('Value two'); + expect( + Array.from( + element.querySelectorAll( + selector, + ), + ).filter((radio) => radio.checked).length, + ).to.be.equal(1); + }, + { once: true }, + ); radio.click(); await waitForLitRender(element); diff --git a/src/elements/radio-button/radio-button-group/radio-button-group.ts b/src/elements/radio-button/radio-button-group/radio-button-group.ts index 633fc02c94..84ea4bd729 100644 --- a/src/elements/radio-button/radio-button-group/radio-button-group.ts +++ b/src/elements/radio-button/radio-button-group/radio-button-group.ts @@ -59,7 +59,7 @@ class SbbRadioButtonGroupElement extends SbbDisabledMixin(LitElement) { if (!this._didLoad) { return; } - if (!val) { + if (val == null) { this.radioButtons.forEach((r) => (r.checked = false)); return; } diff --git a/src/elements/radio-button/radio-button-panel/radio-button-panel.ts b/src/elements/radio-button/radio-button-panel/radio-button-panel.ts index 1f0337a1a1..3ea1d0015f 100644 --- a/src/elements/radio-button/radio-button-panel/radio-button-panel.ts +++ b/src/elements/radio-button/radio-button-panel/radio-button-panel.ts @@ -10,6 +10,12 @@ import { customElement, property } from 'lit/decorators.js'; import { getOverride, slotState } from '../../core/decorators.js'; import { isLean } from '../../core/dom.js'; +import { EventEmitter } from '../../core/eventing.js'; +import type { + SbbCheckedStateChange, + SbbDisabledStateChange, + SbbStateChange, +} from '../../core/interfaces.js'; import { panelCommonStyle, SbbPanelMixin, @@ -20,6 +26,11 @@ import { radioButtonCommonStyle, SbbRadioButtonCommonElementMixin } from '../com import '../../screen-reader-only.js'; +export type SbbRadioButtonStateChange = Extract< + SbbStateChange, + SbbDisabledStateChange | SbbCheckedStateChange +>; + /** /** * It displays a radio button enhanced with the panel design. @@ -42,10 +53,10 @@ class SbbRadioButtonPanelElement extends SbbPanelMixin( // TODO: fix using ...super.events requires: https://github.com/sbb-design-systems/lyne-components/issues/2600 public static readonly events = { - stateChange: 'stateChange', change: 'change', input: 'input', panelConnected: 'panelConnected', + stateChange: 'stateChange', } as const; /** @@ -58,6 +69,17 @@ class SbbRadioButtonPanelElement extends SbbPanelMixin( private _hasSelectionExpansionPanelElement: boolean = false; + /** + * @internal + * Internal event that emits whenever the state of the radio option + * in relation to the parent selection panel changes. + */ + private _stateChange: EventEmitter = new EventEmitter( + this, + SbbRadioButtonPanelElement.events.stateChange, + { bubbles: true }, + ); + public override connectedCallback(): void { super.connectedCallback(); this._hasSelectionExpansionPanelElement = !!this.closest?.('sbb-selection-expansion-panel'); @@ -68,6 +90,16 @@ class SbbRadioButtonPanelElement extends SbbPanelMixin( if (changedProperties.has('checked')) { this.toggleAttribute('data-checked', this.checked); + + if (this.checked !== changedProperties.get('checked')!) { + this._stateChange.emit({ type: 'checked', checked: this.checked }); + } + } + + if (changedProperties.has('disabled')) { + if (this.disabled !== changedProperties.get('disabled')!) { + this._stateChange.emit({ type: 'disabled', disabled: this.disabled }); + } } } diff --git a/src/elements/radio-button/radio-button/radio-button.spec.ts b/src/elements/radio-button/radio-button/radio-button.spec.ts index eeae612eb3..a34f9269ab 100644 --- a/src/elements/radio-button/radio-button/radio-button.spec.ts +++ b/src/elements/radio-button/radio-button/radio-button.spec.ts @@ -22,48 +22,48 @@ describe(`sbb-radio-button`, () => { }); it('selects radio on click', async () => { - const stateChange = new EventSpy(SbbRadioButtonElement.events.stateChange); + const change = new EventSpy(SbbRadioButtonElement.events.change); element.click(); await waitForLitRender(element); expect(element).to.have.attribute('data-checked'); expect(element.checked).to.be.true; - await stateChange.calledOnce(); - expect(stateChange.count).to.be.equal(1); + await change.calledOnce(); + expect(change.count).to.be.equal(1); }); it('does not deselect radio if already checked', async () => { - const stateChange = new EventSpy(SbbRadioButtonElement.events.stateChange); + const change = new EventSpy(SbbRadioButtonElement.events.change); element.click(); await waitForLitRender(element); expect(element.checked).to.be.true; - await stateChange.calledOnce(); - expect(stateChange.count).to.be.equal(1); + await change.calledOnce(); + expect(change.count).to.be.equal(1); element.click(); await waitForLitRender(element); expect(element.checked).to.be.true; - await stateChange.calledOnce(); - expect(stateChange.count).to.be.equal(1); + await change.calledOnce(); + expect(change.count).to.be.equal(1); }); it('allows empty selection', async () => { - const stateChange = new EventSpy(SbbRadioButtonElement.events.stateChange); + const change = new EventSpy(SbbRadioButtonElement.events.change); element.allowEmptySelection = true; element.click(); await waitForLitRender(element); expect(element.checked).to.be.true; - await stateChange.calledOnce(); - expect(stateChange.count).to.be.equal(1); + await change.calledOnce(); + expect(change.count).to.be.equal(1); element.click(); await waitForLitRender(element); expect(element.checked).to.be.false; - await stateChange.calledTimes(2); - expect(stateChange.count).to.be.equal(2); + await change.calledTimes(2); + expect(change.count).to.be.equal(2); }); it('should convert falsy to false', async () => {