Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: improve null/undefined type handling for Angular wrapper #3302

Merged
merged 4 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/elements/calendar/calendar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 3 additions & 3 deletions src/elements/calendar/calendar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ class SbbCalendarElement<T = Date> 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<T> | undefined) {
public set max(value: SbbDateLike<T> | null) {
this._max = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
}
public get max(): T | null {
Expand All @@ -133,7 +133,7 @@ class SbbCalendarElement<T = Date> 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<T> | undefined) {
public set now(value: SbbDateLike<T> | null) {
this._now = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
}
public get now(): T {
Expand All @@ -143,7 +143,7 @@ class SbbCalendarElement<T = Date> 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<T> | undefined) {
public set selected(value: SbbDateLike<T> | null) {
this._selectedDate = this._dateAdapter.getValidDateOrNull(this._dateAdapter.deserialize(value));
if (
!!this._selectedDate &&
Expand Down
12 changes: 6 additions & 6 deletions src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,14 @@ class SbbDatepickerToggleElement<T = Date> 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<T>;
if (this._calendarElement) {
this._calendarElement.selected = this._datePickerElement.valueAsDate || undefined;
this._calendarElement.selected = this._datePickerElement.valueAsDate ?? null;
}
}

Expand All @@ -179,7 +179,7 @@ class SbbDatepickerToggleElement<T = Date> 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();
}
Expand All @@ -190,8 +190,8 @@ class SbbDatepickerToggleElement<T = Date> 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 {
Expand Down Expand Up @@ -220,7 +220,7 @@ class SbbDatepickerToggleElement<T = Date> 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<T>) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
DavideMininni-Fincons marked this conversation as resolved.
Show resolved Hide resolved
}

// eslint-disable-next-line @typescript-eslint/naming-convention
Expand Down
4 changes: 2 additions & 2 deletions src/elements/slider/slider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions src/elements/stepper/step/step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

/**
Expand Down
18 changes: 9 additions & 9 deletions src/elements/stepper/stepper/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
40 changes: 20 additions & 20 deletions src/elements/stepper/stepper/stepper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@

/** 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 | null) {
this._horizontalFrom = value && breakpoints.includes(value) ? value : null;

Check warning on line 41 in src/elements/stepper/stepper/stepper.ts

View check run for this annotation

Codecov / codecov/patch

src/elements/stepper/stepper/stepper.ts#L41

Added line #L41 was not covered by tests
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 })
Expand All @@ -60,24 +60,24 @@

/** The currently selected step. */
@property({ attribute: false })
public set selected(step: SbbStepElement) {
public set selected(step: SbbStepElement | null) {
if (this._loaded) {
this._select(step);
}
}
public get selected(): SbbStepElement | undefined {
return this.querySelector?.<SbbStepElement>('sbb-step[data-selected]') ?? undefined;
public get selected(): SbbStepElement | null {
return this.querySelector?.<SbbStepElement>('sbb-step[data-selected]') ?? null;
}

/** The currently selected step index. */
@property({ attribute: 'selected-index', type: Number })
public set selectedIndex(index: number) {
if (this._loaded) {
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. */
Expand All @@ -95,14 +95,14 @@

/** 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]);
}
}
Expand All @@ -122,7 +122,7 @@
}
}

private _isValidStep(step: SbbStepElement): boolean {
private _isValidStep(step: SbbStepElement | null): boolean {
if (!step || (!this.linear && step.label?.hasAttribute('disabled'))) {
return false;
}
Expand All @@ -131,30 +131,30 @@
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;
}

return true;
}

private _select(step: SbbStepElement): 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;
}
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.
Expand All @@ -181,7 +181,7 @@
}

private _calculateLabelOffsetTop(): number | undefined {
if (this.selectedIndex === undefined) {
if (this.selectedIndex === null) {

Check warning on line 184 in src/elements/stepper/stepper/stepper.ts

View check run for this annotation

Codecov / codecov/patch

src/elements/stepper/stepper/stepper.ts#L184

Added line #L184 was not covered by tests
return;
}
let offset = 0;
Expand Down
4 changes: 2 additions & 2 deletions src/elements/tag/tag-group/tag-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class SbbTagGroupElement extends SbbNamedSlotListMixin<SbbTagElement, typeof Lit
* If set multiple to true, the value is an array.
*/
@property()
public set value(value: string | string[] | null) {
public set value(value: string | (string | null)[] | null) {
jeripeierSBB marked this conversation as resolved.
Show resolved Hide resolved
const tags = this.tags;
if (isServer) {
this._value = value;
Expand Down Expand Up @@ -91,7 +91,7 @@ class SbbTagGroupElement extends SbbNamedSlotListMixin<SbbTagElement, typeof Lit
? this.tags.filter((t) => 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;
jeripeierSBB marked this conversation as resolved.
Show resolved Hide resolved

/** The child instances of sbb-tag as an array. */
public get tags(): SbbTagElement[] {
Expand Down
2 changes: 1 addition & 1 deletion src/elements/time-input/time-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
Loading