From b61053ab4c2d8cebd9e47a0695370f523720ad05 Mon Sep 17 00:00:00 2001 From: Davide Mininni Date: Mon, 16 Dec 2024 13:17:25 +0100 Subject: [PATCH 1/4] fix: improve of null/undefined values for Angular wrapper --- src/elements/calendar/calendar.ts | 6 +++--- .../datepicker-toggle/datepicker-toggle.ts | 12 ++++++------ .../common/navigation-action-common.ts | 2 +- src/elements/slider/slider.ts | 4 ++-- src/elements/stepper/stepper/stepper.ts | 16 ++++++++-------- src/elements/tag/tag-group/tag-group.ts | 4 ++-- src/elements/time-input/time-input.ts | 2 +- 7 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/elements/calendar/calendar.ts b/src/elements/calendar/calendar.ts index 5da8b91918..0937ef12ba 100644 --- a/src/elements/calendar/calendar.ts +++ b/src/elements/calendar/calendar.ts @@ -123,7 +123,7 @@ class SbbCalendarElement extends SbbHydrationMixin(LitElement) { /** The maximum valid date. Takes T Object, ISOString, and Unix Timestamp (number of seconds since Jan 1, 1970). */ @property() - public set max(value: SbbDateLike | undefined) { + public set max(value: SbbDateLike | null) { this._max = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value)); } public get max(): T | null { @@ -133,7 +133,7 @@ class SbbCalendarElement extends SbbHydrationMixin(LitElement) { /** A configured date which acts as the current date instead of the real current date. Recommended for testing purposes. */ @property() - public set now(value: SbbDateLike | undefined) { + public set now(value: SbbDateLike | null) { this._now = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value)); } public get now(): T { @@ -143,7 +143,7 @@ class SbbCalendarElement extends SbbHydrationMixin(LitElement) { /** The selected date. Takes T Object, ISOString, and Unix Timestamp (number of seconds since Jan 1, 1970). */ @property() - public set selected(value: SbbDateLike | undefined) { + public set selected(value: SbbDateLike | null) { this._selectedDate = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value)); if ( !!this._selectedDate && diff --git a/src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts b/src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts index 71197d28a1..92d6706f49 100644 --- a/src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts +++ b/src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts @@ -157,14 +157,14 @@ class SbbDatepickerToggleElement extends SbbNegativeMixin(SbbHydration return; } calendar.wide = datepicker.wide; - calendar.now = this._nowOrUndefined(); + calendar.now = this._nowOrNull(); calendar.dateFilter = datepicker.dateFilter; } private _datePickerChanged(event: Event): void { this._datePickerElement = event.target as SbbDatepickerElement; if (this._calendarElement) { - this._calendarElement.selected = this._datePickerElement.valueAsDate || undefined; + this._calendarElement.selected = this._datePickerElement.valueAsDate ?? null; } } @@ -179,7 +179,7 @@ class SbbDatepickerToggleElement extends SbbNegativeMixin(SbbHydration ) { return; } - this._calendarElement.selected = this._datePickerElement!.valueAsDate ?? undefined; + this._calendarElement.selected = this._datePickerElement!.valueAsDate ?? null; this._configureCalendar(this._calendarElement, this._datePickerElement!); this._calendarElement.resetPosition(); } @@ -190,8 +190,8 @@ class SbbDatepickerToggleElement extends SbbNegativeMixin(SbbHydration this._popoverElement.trigger = this._triggerElement; } - private _nowOrUndefined(): T | undefined { - return this._datePickerElement?.hasCustomNow() ? this._datePickerElement.now : undefined; + private _nowOrNull(): T | null { + return this._datePickerElement?.hasCustomNow() ? this._datePickerElement.now : null; } protected override render(): TemplateResult { @@ -220,7 +220,7 @@ class SbbDatepickerToggleElement extends SbbNegativeMixin(SbbHydration .view=${this.view} .min=${this._min} .max=${this._max} - .now=${this._nowOrUndefined()} + .now=${this._nowOrNull()} ?wide=${this._datePickerElement?.wide} .dateFilter=${this._datePickerElement?.dateFilter} @dateSelected=${(d: CustomEvent) => { diff --git a/src/elements/navigation/common/navigation-action-common.ts b/src/elements/navigation/common/navigation-action-common.ts index e6bbd4dd7d..7f2347df56 100644 --- a/src/elements/navigation/common/navigation-action-common.ts +++ b/src/elements/navigation/common/navigation-action-common.ts @@ -21,7 +21,7 @@ export declare class SbbNavigationActionCommonElementMixinType { public accessor size: SbbNavigationActionSize; public get marker(): SbbNavigationMarkerElement | null; public get section(): SbbNavigationSectionElement | null; - public connectedSection: SbbNavigationSectionElement | null; + public connectedSection: SbbNavigationSectionElement | undefined; } // eslint-disable-next-line @typescript-eslint/naming-convention diff --git a/src/elements/slider/slider.ts b/src/elements/slider/slider.ts index e3a9288a45..1a55b530c6 100644 --- a/src/elements/slider/slider.ts +++ b/src/elements/slider/slider.ts @@ -56,8 +56,8 @@ class SbbSliderElement extends SbbDisabledMixin(SbbFormAssociatedMixin(LitElemen /** Numeric value for the inner HTMLInputElement. */ @property({ attribute: 'value-as-number', type: Number }) - public set valueAsNumber(value: number) { - this.value = value?.toString(); + public set valueAsNumber(value: number | null) { + this.value = value?.toString() || null; } public get valueAsNumber(): number | null { return Number(this.value); diff --git a/src/elements/stepper/stepper/stepper.ts b/src/elements/stepper/stepper/stepper.ts index 1c67f89cf2..375ff93d14 100644 --- a/src/elements/stepper/stepper/stepper.ts +++ b/src/elements/stepper/stepper/stepper.ts @@ -37,8 +37,8 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** Overrides the behaviour of `orientation` property. */ @property({ attribute: 'horizontal-from', reflect: true }) - public set horizontalFrom(value: SbbHorizontalFrom) { - this._horizontalFrom = breakpoints.includes(value) ? value : undefined; + public set horizontalFrom(value: SbbHorizontalFrom | undefined) { + this._horizontalFrom = value && breakpoints.includes(value) ? value : undefined; if (this._horizontalFrom && this._loaded) { this._checkOrientation(); } @@ -60,7 +60,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** The currently selected step. */ @property({ attribute: false }) - public set selected(step: SbbStepElement) { + public set selected(step: SbbStepElement | undefined) { if (this._loaded) { this._select(step); } @@ -71,8 +71,8 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** The currently selected step index. */ @property({ attribute: 'selected-index', type: Number }) - public set selectedIndex(index: number) { - if (this._loaded) { + public set selectedIndex(index: number | undefined) { + if (this._loaded && index !== undefined) { this._select(this.steps[index]); } } @@ -122,7 +122,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { } } - private _isValidStep(step: SbbStepElement): boolean { + private _isValidStep(step: SbbStepElement | undefined): boolean { if (!step || (!this.linear && step.label?.hasAttribute('disabled'))) { return false; } @@ -139,7 +139,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { return true; } - private _select(step: SbbStepElement): void { + private _select(step: SbbStepElement | undefined): void { if (!this._isValidStep(step)) { return; } @@ -154,7 +154,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { } const current = this.selected; current?.deselect(); - step.select(); + step!.select(); this._setMarkerSize(); this._configureLinearMode(); // In case the focus is currently inside the stepper, we focus the selected step label. diff --git a/src/elements/tag/tag-group/tag-group.ts b/src/elements/tag/tag-group/tag-group.ts index 5211bb0762..0cd4c3d202 100644 --- a/src/elements/tag/tag-group/tag-group.ts +++ b/src/elements/tag/tag-group/tag-group.ts @@ -58,7 +58,7 @@ class SbbTagGroupElement extends SbbNamedSlotListMixin t.checked).map((t) => t.value) : (this.tags.find((t) => t.checked)?.value ?? null); } - private _value: string | string[] | null = null; + private _value: string | (string | null)[] | null = null; /** The child instances of sbb-tag as an array. */ public get tags(): SbbTagElement[] { diff --git a/src/elements/time-input/time-input.ts b/src/elements/time-input/time-input.ts index 4ec0a839e0..cc8e0fec38 100644 --- a/src/elements/time-input/time-input.ts +++ b/src/elements/time-input/time-input.ts @@ -37,7 +37,7 @@ class SbbTimeInputElement extends LitElement { /** Reference of the native input connected to the datepicker. */ @property() - public set input(value: string | HTMLElement) { + public set input(value: string | HTMLElement | null) { this._input = value; this._findInputElement(); } From 59ebdc0fdc9a105d2725caa419d1a7cfbe772dac Mon Sep 17 00:00:00 2001 From: Davide Mininni Date: Mon, 16 Dec 2024 15:00:45 +0100 Subject: [PATCH 2/4] fix: tests --- src/elements/calendar/calendar.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elements/calendar/calendar.spec.ts b/src/elements/calendar/calendar.spec.ts index 642bf653e6..79793cb3c0 100644 --- a/src/elements/calendar/calendar.spec.ts +++ b/src/elements/calendar/calendar.spec.ts @@ -256,7 +256,7 @@ describe(`sbb-calendar`, () => { }); it('opens month view with current date', async () => { - element.selected = undefined; + element.selected = null; element.now = '2022-08-15'; element.view = 'month'; await waitForLitRender(element); From ca15fd3a5d03616b7ae4ab1b48f0c7d0e6585135 Mon Sep 17 00:00:00 2001 From: Davide Mininni Date: Mon, 16 Dec 2024 16:03:29 +0100 Subject: [PATCH 3/4] fix: review Jeri pt.1 --- src/elements/slider/slider.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/elements/slider/slider.ts b/src/elements/slider/slider.ts index 1a55b530c6..cd13d25753 100644 --- a/src/elements/slider/slider.ts +++ b/src/elements/slider/slider.ts @@ -57,7 +57,7 @@ class SbbSliderElement extends SbbDisabledMixin(SbbFormAssociatedMixin(LitElemen /** Numeric value for the inner HTMLInputElement. */ @property({ attribute: 'value-as-number', type: Number }) public set valueAsNumber(value: number | null) { - this.value = value?.toString() || null; + this.value = value?.toString() ?? null; } public get valueAsNumber(): number | null { return Number(this.value); From cc89daea6f2edc2152c4672688b02c55e9ab4677 Mon Sep 17 00:00:00 2001 From: Davide Mininni Date: Tue, 17 Dec 2024 10:00:22 +0100 Subject: [PATCH 4/4] fix: review Jeri/Lukas --- src/elements/stepper/step/step.ts | 8 +++--- src/elements/stepper/stepper/readme.md | 18 ++++++------ src/elements/stepper/stepper/stepper.ts | 38 ++++++++++++------------- 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/src/elements/stepper/step/step.ts b/src/elements/stepper/step/step.ts index bd5413b0cd..022793d554 100644 --- a/src/elements/stepper/step/step.ts +++ b/src/elements/stepper/step/step.ts @@ -19,10 +19,10 @@ import style from './step.scss?lit&inline'; let nextId = 0; export type SbbStepValidateEventDetails = { - currentIndex?: number; - currentStep?: SbbStepElement; - nextIndex?: number; - nextStep?: SbbStepElement; + currentIndex: number | null; + currentStep: SbbStepElement | null; + nextIndex: number | null; + nextStep: SbbStepElement | null; }; /** diff --git a/src/elements/stepper/stepper/readme.md b/src/elements/stepper/stepper/readme.md index 79e723bd5f..46f52b261c 100644 --- a/src/elements/stepper/stepper/readme.md +++ b/src/elements/stepper/stepper/readme.md @@ -103,15 +103,15 @@ Use an `aria-label` attribute to describe the purpose of the stepper. The `sbb-s ## Properties -| Name | Attribute | Privacy | Type | Default | Description | -| ---------------- | ----------------- | ------- | -------------------------------- | ------------------ | --------------------------------------------------------------------------------- | -| `horizontalFrom` | `horizontal-from` | public | `SbbHorizontalFrom \| undefined` | | Overrides the behaviour of `orientation` property. | -| `linear` | `linear` | public | `boolean` | `false` | If set to true, only the current and previous labels can be clicked and selected. | -| `orientation` | `orientation` | public | `SbbOrientation` | `'horizontal'` | Steps orientation, either horizontal or vertical. | -| `selected` | - | public | `SbbStepElement \| undefined` | | The currently selected step. | -| `selectedIndex` | `selected-index` | public | `number \| undefined` | | The currently selected step index. | -| `size` | `size` | public | `'s' \| 'm'` | `'m' / 's' (lean)` | Size variant, either s or m. | -| `steps` | - | public | `SbbStepElement[]` | | The steps of the stepper. | +| Name | Attribute | Privacy | Type | Default | Description | +| ---------------- | ----------------- | ------- | --------------------------- | ------------------ | --------------------------------------------------------------------------------- | +| `horizontalFrom` | `horizontal-from` | public | `SbbHorizontalFrom \| null` | `null` | Overrides the behaviour of `orientation` property. | +| `linear` | `linear` | public | `boolean` | `false` | If set to true, only the current and previous labels can be clicked and selected. | +| `orientation` | `orientation` | public | `SbbOrientation` | `'horizontal'` | Steps orientation, either horizontal or vertical. | +| `selected` | - | public | `SbbStepElement \| null` | | The currently selected step. | +| `selectedIndex` | `selected-index` | public | `number \| null` | | The currently selected step index. | +| `size` | `size` | public | `'s' \| 'm'` | `'m' / 's' (lean)` | Size variant, either s or m. | +| `steps` | - | public | `SbbStepElement[]` | | The steps of the stepper. | ## Methods diff --git a/src/elements/stepper/stepper/stepper.ts b/src/elements/stepper/stepper/stepper.ts index 375ff93d14..011474ad07 100644 --- a/src/elements/stepper/stepper/stepper.ts +++ b/src/elements/stepper/stepper/stepper.ts @@ -37,16 +37,16 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** Overrides the behaviour of `orientation` property. */ @property({ attribute: 'horizontal-from', reflect: true }) - public set horizontalFrom(value: SbbHorizontalFrom | undefined) { - this._horizontalFrom = value && breakpoints.includes(value) ? value : undefined; + public set horizontalFrom(value: SbbHorizontalFrom | null) { + this._horizontalFrom = value && breakpoints.includes(value) ? value : null; if (this._horizontalFrom && this._loaded) { this._checkOrientation(); } } - public get horizontalFrom(): SbbHorizontalFrom | undefined { + public get horizontalFrom(): SbbHorizontalFrom | null { return this._horizontalFrom; } - private _horizontalFrom?: SbbHorizontalFrom | undefined; + private _horizontalFrom: SbbHorizontalFrom | null = null; /** Steps orientation, either horizontal or vertical. */ @property({ reflect: true }) @@ -60,24 +60,24 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** The currently selected step. */ @property({ attribute: false }) - public set selected(step: SbbStepElement | undefined) { + public set selected(step: SbbStepElement | null) { if (this._loaded) { this._select(step); } } - public get selected(): SbbStepElement | undefined { - return this.querySelector?.('sbb-step[data-selected]') ?? undefined; + public get selected(): SbbStepElement | null { + return this.querySelector?.('sbb-step[data-selected]') ?? null; } /** The currently selected step index. */ @property({ attribute: 'selected-index', type: Number }) - public set selectedIndex(index: number | undefined) { - if (this._loaded && index !== undefined) { + public set selectedIndex(index: number | null) { + if (this._loaded && index !== null) { this._select(this.steps[index]); } } - public get selectedIndex(): number | undefined { - return this.selected ? this.steps.indexOf(this.selected) : undefined; + public get selectedIndex(): number | null { + return this.selected ? this.steps.indexOf(this.selected) : null; } /** The steps of the stepper. */ @@ -95,14 +95,14 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { /** Selects the next step. */ public next(): void { - if (this.selectedIndex !== undefined) { + if (this.selectedIndex !== null) { this._select(this.steps[this.selectedIndex + 1]); } } /** Selects the previous step. */ public previous(): void { - if (this.selectedIndex !== undefined) { + if (this.selectedIndex !== null) { this._select(this.steps[this.selectedIndex - 1]); } } @@ -122,7 +122,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { } } - private _isValidStep(step: SbbStepElement | undefined): boolean { + private _isValidStep(step: SbbStepElement | null): boolean { if (!step || (!this.linear && step.label?.hasAttribute('disabled'))) { return false; } @@ -131,7 +131,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { return step === this.steps[0]; } - if (this.linear && this.selectedIndex !== undefined) { + if (this.linear && this.selectedIndex !== null) { const index = this.steps.indexOf(step); return index < this.selectedIndex || index === this.selectedIndex + 1; } @@ -139,15 +139,15 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { return true; } - private _select(step: SbbStepElement | undefined): void { + private _select(step: SbbStepElement | null): void { if (!this._isValidStep(step)) { return; } const validatePayload: SbbStepValidateEventDetails = { currentIndex: this.selectedIndex, currentStep: this.selected, - nextIndex: this.selectedIndex !== undefined ? this.selectedIndex + 1 : undefined, - nextStep: this.selectedIndex !== undefined ? this.steps[this.selectedIndex + 1] : undefined, + nextIndex: this.selectedIndex !== null ? this.selectedIndex + 1 : null, + nextStep: this.selectedIndex !== null ? this.steps[this.selectedIndex + 1] : null, }; if (this.selected && !this.selected.validate(validatePayload)) { return; @@ -181,7 +181,7 @@ class SbbStepperElement extends SbbHydrationMixin(LitElement) { } private _calculateLabelOffsetTop(): number | undefined { - if (this.selectedIndex === undefined) { + if (this.selectedIndex === null) { return; } let offset = 0;