From 6ad135e510ac2498018f536007d9dcd7ff37a076 Mon Sep 17 00:00:00 2001 From: Davide Mininni <101575400+DavideMininni-Fincons@users.noreply.github.com> Date: Wed, 8 Jan 2025 15:31:11 +0100 Subject: [PATCH] fix(sbb-select): handle properties change (#3334) --- src/elements/accordion/accordion.ts | 7 ++- src/elements/core/mixins/disabled-mixin.ts | 2 +- src/elements/select/select.spec.ts | 63 +++++++++++++++++++ src/elements/select/select.ts | 45 ++++++++++++- src/elements/toggle/toggle/toggle.ts | 2 +- src/elements/train/train-wagon/train-wagon.ts | 2 +- 6 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/elements/accordion/accordion.ts b/src/elements/accordion/accordion.ts index 75eead73c0..6022f0cd7d 100644 --- a/src/elements/accordion/accordion.ts +++ b/src/elements/accordion/accordion.ts @@ -32,13 +32,16 @@ class SbbAccordionElement extends SbbHydrationMixin(LitElement) { * The heading level for the sbb-expansion-panel-headers within the component. * @controls SbbExpansionPanelElement.titleLevel */ - @handleDistinctChange((e) => e._setTitleLevelOnChildren()) + @handleDistinctChange((e: SbbAccordionElement) => e._setTitleLevelOnChildren()) @property({ attribute: 'title-level' }) public accessor titleLevel: SbbTitleLevel | null = null; /** Whether more than one sbb-expansion-panel can be open at the same time. */ @forceType() - @handleDistinctChange((e, newValue, oldValue) => e._resetExpansionPanels(newValue, !!oldValue)) + @handleDistinctChange( + (e: SbbAccordionElement, newValue: boolean, oldValue: boolean | undefined) => + e._resetExpansionPanels(newValue, !!oldValue), + ) @property({ type: Boolean }) public accessor multi: boolean = false; diff --git a/src/elements/core/mixins/disabled-mixin.ts b/src/elements/core/mixins/disabled-mixin.ts index 045fe5c492..16e261e175 100644 --- a/src/elements/core/mixins/disabled-mixin.ts +++ b/src/elements/core/mixins/disabled-mixin.ts @@ -26,7 +26,7 @@ export const SbbDisabledMixin = >( /** Whether the component is disabled. */ @forceType() @property({ reflect: true, type: Boolean }) - @getOverride((e, v) => v || e.isDisabledExternally()) + @getOverride((e: SbbDisabledElement, v: boolean): boolean => v || e.isDisabledExternally()) public accessor disabled: boolean = false; /** diff --git a/src/elements/select/select.spec.ts b/src/elements/select/select.spec.ts index 2e3603a902..23143b251d 100644 --- a/src/elements/select/select.spec.ts +++ b/src/elements/select/select.spec.ts @@ -303,6 +303,69 @@ describe(`sbb-select`, () => { expect(comboBoxElement).to.have.attribute('aria-expanded', 'true'); }); + it('update multiple attribute', async () => { + const didOpen = new EventSpy(SbbSelectElement.events.didOpen, element); + element.dispatchEvent(new CustomEvent('click')); + await didOpen.calledOnce(); + expect(didOpen.count).to.be.equal(1); + await waitForLitRender(element); + + expect(element.value).to.be.equal(null); + element.toggleAttribute('multiple', true); + await waitForLitRender(element); + expect(element.value).to.be.eql([]); + element.toggleAttribute('multiple', false); + await waitForLitRender(element); + expect(element.value).to.be.equal(null); + + firstOption.dispatchEvent(new CustomEvent('click')); + await waitForLitRender(element); + expect(element.value).to.be.eql('1'); + element.toggleAttribute('multiple', true); + await waitForLitRender(element); + expect(element.value).to.be.eql(['1']); + + firstOption.dispatchEvent(new CustomEvent('click')); + thirdOption.dispatchEvent(new CustomEvent('click')); + secondOption.dispatchEvent(new CustomEvent('click')); + await waitForLitRender(element); + + expect(element.value).to.be.eql(['3', '2']); + element.toggleAttribute('multiple', false); + await waitForLitRender(element); + expect(element.value).to.be.eql('3'); + }); + + it('close the panel if disabled', async () => { + const didOpen = new EventSpy(SbbSelectElement.events.didOpen, element); + const didClose = new EventSpy(SbbSelectElement.events.didClose, element); + element.dispatchEvent(new CustomEvent('click')); + await didOpen.calledOnce(); + expect(didOpen.count).to.be.equal(1); + await waitForLitRender(element); + + element.toggleAttribute('disabled', true); + await waitForLitRender(element); + + await didClose.calledOnce(); + expect(didClose.count).to.be.equal(1); + }); + + it('close the panel if readonly', async () => { + const didOpen = new EventSpy(SbbSelectElement.events.didOpen, element); + const didClose = new EventSpy(SbbSelectElement.events.didClose, element); + element.dispatchEvent(new CustomEvent('click')); + await didOpen.calledOnce(); + expect(didOpen.count).to.be.equal(1); + await waitForLitRender(element); + + element.toggleAttribute('readonly', true); + await waitForLitRender(element); + + await didClose.calledOnce(); + expect(didClose.count).to.be.equal(1); + }); + it('handles keypress on host', async () => { const didOpen = new EventSpy(SbbSelectElement.events.didOpen, element); const didClose = new EventSpy(SbbSelectElement.events.didClose, element); diff --git a/src/elements/select/select.ts b/src/elements/select/select.ts index 16ff346825..5b3cf3c53b 100644 --- a/src/elements/select/select.ts +++ b/src/elements/select/select.ts @@ -8,7 +8,12 @@ import { until } from 'lit/directives/until.js'; import { getNextElementIndex } from '../core/a11y.js'; import { SbbOpenCloseBaseElement } from '../core/base-elements.js'; import { SbbLanguageController } from '../core/controllers.js'; -import { forceType, hostAttributes } from '../core/decorators.js'; +import { + forceType, + getOverride, + handleDistinctChange, + hostAttributes, +} from '../core/decorators.js'; import { isNextjs, isSafari, isZeroAnimationDuration, setOrRemoveAttribute } from '../core/dom.js'; import { EventEmitter } from '../core/eventing.js'; import { @@ -92,11 +97,23 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin( /** Whether the select allows for multiple selection. */ @forceType() + @handleDistinctChange((e: SbbSelectElement, newValue: boolean) => e._onMultipleChanged(newValue)) @property({ reflect: true, type: Boolean }) public accessor multiple: boolean = false; + @forceType() + @handleDistinctChange((e: SbbSelectElement, newValue: boolean) => + e._closeOnDisabledReadonlyChanged(newValue), + ) + @property({ reflect: true, type: Boolean }) + @getOverride((e: SbbSelectElement, v: boolean): boolean => v || e.isDisabledExternally()) + public override accessor disabled: boolean = false; + /** Whether the select is readonly. */ @forceType() + @handleDistinctChange((e: SbbSelectElement, newValue: boolean) => + e._closeOnDisabledReadonlyChanged(newValue), + ) @property({ type: Boolean }) public accessor readonly: boolean = false; @@ -305,6 +322,28 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin( } } + /** + * The `value` property should be adapted when the `multiple` property changes: + * - if it changes to true, the 'value' is set to an array; + * - if it changes to false, the first available option is set as 'value' otherwise it's set to null. + */ + private _onMultipleChanged(newValue: boolean): void { + if (newValue) { + this.value = this.value !== null ? [this.value as string] : []; + } else { + this.value = (this.value as string[]).length ? (this.value as string[])[0] : null; + } + } + + /** + * If the `disabled` or the `readonly` properties are set, and the panel is open, close it. + */ + private _closeOnDisabledReadonlyChanged(newValue: boolean): void { + if (this.isOpen && newValue) { + this.close(); + } + } + /** Sets the _displayValue by checking the internal sbb-options and setting the correct `selected` value on them. */ private _onValueChanged(newValue: string | string[]): void { const options = this._filteredOptions; @@ -453,6 +492,10 @@ class SbbSelectElement extends SbbUpdateSchedulerMixin( element.toggleAttribute('data-negative', this.negative); element.toggleAttribute('data-multiple', this.multiple); }); + + this.querySelectorAll?.( + 'sbb-option, sbb-optgroup', + ).forEach((e) => e.requestUpdate?.()); } private _setupSelect(): void { diff --git a/src/elements/toggle/toggle/toggle.ts b/src/elements/toggle/toggle/toggle.ts index 511b2dccb5..5edaf85870 100644 --- a/src/elements/toggle/toggle/toggle.ts +++ b/src/elements/toggle/toggle/toggle.ts @@ -36,7 +36,7 @@ class SbbToggleElement extends LitElement { /** Whether the toggle is disabled. */ @forceType() - @handleDistinctChange((e) => e._updateDisabled()) + @handleDistinctChange((e: SbbToggleElement) => e._updateDisabled()) @property({ reflect: true, type: Boolean }) public accessor disabled: boolean = false; diff --git a/src/elements/train/train-wagon/train-wagon.ts b/src/elements/train/train-wagon/train-wagon.ts index bd5797b3f5..4630925aed 100644 --- a/src/elements/train/train-wagon/train-wagon.ts +++ b/src/elements/train/train-wagon/train-wagon.ts @@ -70,7 +70,7 @@ class SbbTrainWagonElement extends SbbNamedSlotListMixin e._sectorChanged()) + @handleDistinctChange((e: SbbTrainWagonElement) => e._sectorChanged()) @property({ reflect: true, converter: omitEmptyConverter }) public accessor sector: string = '';