From df10a395f7d1cab30fa99a5da139333d352c79bc Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Wed, 20 Nov 2024 14:27:50 +0100 Subject: [PATCH 1/3] refactor(sbb-icon): complete update cycle after image loading --- src/elements/datepicker/datepicker/datepicker.ts | 1 - src/elements/icon/icon.snapshot.spec.ts | 10 ++-------- src/elements/icon/icon.ts | 9 ++++++--- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/elements/datepicker/datepicker/datepicker.ts b/src/elements/datepicker/datepicker/datepicker.ts index 0a9ddf23eb..3cfff34bbd 100644 --- a/src/elements/datepicker/datepicker/datepicker.ts +++ b/src/elements/datepicker/datepicker/datepicker.ts @@ -148,7 +148,6 @@ export function isDateAvailable( min: string | number | null | undefined, max: string | number | null | undefined, ): boolean { - // TODO: Get date adapter from config const dateAdapter: DateAdapter = readConfig().datetime?.dateAdapter ?? defaultDateAdapter; const dateMin = dateAdapter.deserialize(min); const dateMax = dateAdapter.deserialize(max); diff --git a/src/elements/icon/icon.snapshot.spec.ts b/src/elements/icon/icon.snapshot.spec.ts index 3ea25c1cf0..8fdb04c70b 100644 --- a/src/elements/icon/icon.snapshot.spec.ts +++ b/src/elements/icon/icon.snapshot.spec.ts @@ -1,8 +1,7 @@ -import { aTimeout, expect } from '@open-wc/testing'; +import { expect } from '@open-wc/testing'; import { html } from 'lit/static-html.js'; -import { mergeConfig, type SbbIconConfig } from '../core/config.js'; -import { readConfig } from '../core/config.js'; +import { mergeConfig, readConfig, type SbbIconConfig } from '../core/config.js'; import { fixture, testA11yTreeSnapshot } from '../core/testing/private.js'; import { waitForLitRender } from '../core/testing.js'; @@ -107,11 +106,6 @@ describe(`sbb-icon`, () => { `); icon.setAttribute('name', 'pie-medium'); - // TODO: Optimize with https://lit.dev/docs/elements/lifecycle/#getUpdateComplete - // The update of the internal state happens a tick after the updateComplete down below completes. - // We could change this by implementing a getUpdateComplete which starts with a name change - // and completes with the new icon loaded. - await aTimeout(0); await waitForLitRender(icon); expect(icon).dom.to.be.equal(` diff --git a/src/elements/icon/icon.ts b/src/elements/icon/icon.ts index 4d048a78e1..3529d76c23 100644 --- a/src/elements/icon/icon.ts +++ b/src/elements/icon/icon.ts @@ -2,6 +2,7 @@ import { html, type PropertyValues, type TemplateResult } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import { forceType, omitEmptyConverter } from '../core/decorators.js'; +import { SbbUpdateSchedulerMixin } from '../core/mixins.js'; import { SbbIconBase } from './icon-base.js'; @@ -10,7 +11,7 @@ import { SbbIconBase } from './icon-base.js'; */ export @customElement('sbb-icon') -class SbbIconElement extends SbbIconBase { +class SbbIconElement extends SbbUpdateSchedulerMixin(SbbIconBase) { /** * We need to additionally observe the svgicon attribute * for sbb-angular compatibility. @@ -55,11 +56,13 @@ class SbbIconElement extends SbbIconBase { return super.fetchSvgIcon(namespace, name); } - protected override willUpdate(changedProperties: PropertyValues): void { + protected override async willUpdate(changedProperties: PropertyValues): Promise { super.willUpdate(changedProperties); if (changedProperties.has('name') && this.name) { - this.loadSvgIcon(this.name); + this.startUpdate(); + await this.loadSvgIcon(this.name); + this.completeUpdate(); } } From 37a411b81b7c85f56721b780f37451ef0901c722 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 21 Nov 2024 08:17:40 +0100 Subject: [PATCH 2/3] fix: remove unused async modifier on willUpdate --- src/elements/checkbox/checkbox-panel/checkbox-panel.ts | 2 +- src/elements/checkbox/common/checkbox-common.ts | 2 +- src/elements/core/mixins/required-mixin.ts | 2 +- .../radio-button/radio-button-panel/radio-button-panel.ts | 2 +- src/elements/toggle-check/toggle-check.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/elements/checkbox/checkbox-panel/checkbox-panel.ts b/src/elements/checkbox/checkbox-panel/checkbox-panel.ts index 43abf00d6d..c59c514fde 100644 --- a/src/elements/checkbox/checkbox-panel/checkbox-panel.ts +++ b/src/elements/checkbox/checkbox-panel/checkbox-panel.ts @@ -73,7 +73,7 @@ class SbbCheckboxPanelElement extends SbbPanelMixin( { bubbles: true }, ); - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('checked')) { diff --git a/src/elements/checkbox/common/checkbox-common.ts b/src/elements/checkbox/common/checkbox-common.ts index 4cff2b464c..8f1c3e5fb2 100644 --- a/src/elements/checkbox/common/checkbox-common.ts +++ b/src/elements/checkbox/common/checkbox-common.ts @@ -49,7 +49,7 @@ export const SbbCheckboxCommonElementMixin = > ['disabled', 'required', 'size'].forEach((p) => this.requestUpdate(p)); } - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('checked') || changedProperties.has('indeterminate')) { diff --git a/src/elements/core/mixins/required-mixin.ts b/src/elements/core/mixins/required-mixin.ts index 3d82b98f0c..2cbb687bd6 100644 --- a/src/elements/core/mixins/required-mixin.ts +++ b/src/elements/core/mixins/required-mixin.ts @@ -32,7 +32,7 @@ export const SbbRequiredMixin = < } private _required: boolean = false; - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('required')) { 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 a68c565acf..bd6ec1b224 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 @@ -60,7 +60,7 @@ class SbbRadioButtonPanelElement extends SbbPanelMixin( this._hasSelectionExpansionPanelElement = !!this.closest?.('sbb-selection-expansion-panel'); } - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('checked')) { diff --git a/src/elements/toggle-check/toggle-check.ts b/src/elements/toggle-check/toggle-check.ts index 21382b758e..85b0e81c12 100644 --- a/src/elements/toggle-check/toggle-check.ts +++ b/src/elements/toggle-check/toggle-check.ts @@ -38,7 +38,7 @@ class SbbToggleCheckElement extends SbbFormAssociatedCheckboxMixin(SbbIconNameMi @property({ attribute: 'label-position', reflect: true }) public accessor labelPosition: 'before' | 'after' = 'after'; - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('checked')) { From 9137fc636619af05adef6a407be58d2ec2319448 Mon Sep 17 00:00:00 2001 From: Jeremias Peier Date: Thu, 21 Nov 2024 10:48:36 +0100 Subject: [PATCH 3/3] fix: review --- src/elements/clock/clock.ts | 4 ++-- .../flip-card/flip-card-summary/flip-card-summary.ts | 6 +++--- src/elements/icon/icon.ts | 11 ++++------- src/elements/table/table-wrapper/table-wrapper.ts | 2 +- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/elements/clock/clock.ts b/src/elements/clock/clock.ts index 004e40cc82..080f16d0aa 100644 --- a/src/elements/clock/clock.ts +++ b/src/elements/clock/clock.ts @@ -87,11 +87,11 @@ class SbbClockElement extends LitElement { /** Callback function for minutes hand. */ private _moveMinutesHandFn = (): void => this._moveMinutesHand(); - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (!isServer && changedProperties.has('now')) { - await this._startOrConfigureClock(); + this._startOrConfigureClock(); } } diff --git a/src/elements/flip-card/flip-card-summary/flip-card-summary.ts b/src/elements/flip-card/flip-card-summary/flip-card-summary.ts index 25539dcb2b..af5c8ac6fb 100644 --- a/src/elements/flip-card/flip-card-summary/flip-card-summary.ts +++ b/src/elements/flip-card/flip-card-summary/flip-card-summary.ts @@ -26,10 +26,10 @@ class SbbFlipCardSummaryElement extends LitElement { @property({ attribute: 'image-alignment', reflect: true }) public accessor imageAlignment: SbbFlipCardImageAlignment = 'after'; - protected override willUpdate(_changedProperties: PropertyValues): void { - super.willUpdate(_changedProperties); + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); - if (_changedProperties.has('imageAlignment')) { + if (changedProperties.has('imageAlignment')) { this.closest?.('sbb-flip-card')?.setAttribute('data-image-alignment', this.imageAlignment); } } diff --git a/src/elements/icon/icon.ts b/src/elements/icon/icon.ts index 3529d76c23..887e542af5 100644 --- a/src/elements/icon/icon.ts +++ b/src/elements/icon/icon.ts @@ -2,16 +2,15 @@ import { html, type PropertyValues, type TemplateResult } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import { forceType, omitEmptyConverter } from '../core/decorators.js'; -import { SbbUpdateSchedulerMixin } from '../core/mixins.js'; import { SbbIconBase } from './icon-base.js'; /** - * It displays an icon loaded from a registered namespace. + * Displays an icon loaded from a registered namespace. */ export @customElement('sbb-icon') -class SbbIconElement extends SbbUpdateSchedulerMixin(SbbIconBase) { +class SbbIconElement extends SbbIconBase { /** * We need to additionally observe the svgicon attribute * for sbb-angular compatibility. @@ -56,13 +55,11 @@ class SbbIconElement extends SbbUpdateSchedulerMixin(SbbIconBase) { return super.fetchSvgIcon(namespace, name); } - protected override async willUpdate(changedProperties: PropertyValues): Promise { + protected override willUpdate(changedProperties: PropertyValues): void { super.willUpdate(changedProperties); if (changedProperties.has('name') && this.name) { - this.startUpdate(); - await this.loadSvgIcon(this.name); - this.completeUpdate(); + this.loadSvgIcon(this.name); } } diff --git a/src/elements/table/table-wrapper/table-wrapper.ts b/src/elements/table/table-wrapper/table-wrapper.ts index 9fae8faab4..addf2859b8 100644 --- a/src/elements/table/table-wrapper/table-wrapper.ts +++ b/src/elements/table/table-wrapper/table-wrapper.ts @@ -29,7 +29,7 @@ class SbbTableWrapperElement extends SbbNegativeMixin(LitElement) { }); private _tableWrapper!: HTMLElement; - protected override firstUpdated(changedProperties: PropertyValues): void { + protected override firstUpdated(changedProperties: PropertyValues): void { super.firstUpdated(changedProperties); this._tableWrapper = this.shadowRoot!.querySelector('.sbb-table-wrapper')!; this._resizeObserver.observe(this._tableWrapper);