Skip to content

Commit

Permalink
refactor: move event handlers to constructors (#3314)
Browse files Browse the repository at this point in the history
This is the recommended approach for event handlers for web components.
  • Loading branch information
kyubisation authored and github-actions committed Dec 19, 2024
1 parent 7369a3e commit 18660a3
Show file tree
Hide file tree
Showing 39 changed files with 226 additions and 328 deletions.
12 changes: 4 additions & 8 deletions src/elements/accordion/accordion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit';
import { html, LitElement } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import { SbbConnectedAbortController } from '../core/controllers.js';
import { forceType, handleDistinctChange } from '../core/decorators.js';
import { isLean } from '../core/dom.js';
import { isEventPrevented } from '../core/eventing.js';
Expand Down Expand Up @@ -43,20 +42,17 @@ class SbbAccordionElement extends SbbHydrationMixin(LitElement) {
@property({ type: Boolean })
public accessor multi: boolean = false;

private _abort = new SbbConnectedAbortController(this);

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener(
public constructor() {
super();
this.addEventListener?.(
'willOpen',
async (e: CustomEvent<void>) => {
if (!(await isEventPrevented(e))) {
this._closePanels(e);
}
},
{
signal,
// We use capture here, because willOpen does not bubble.
capture: true,
},
);
Expand Down
12 changes: 4 additions & 8 deletions src/elements/alert/alert-group/alert-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { LitElement, nothing } from 'lit';
import { customElement, property, state } from 'lit/decorators.js';
import { html, unsafeStatic } from 'lit/static-html.js';

import { SbbConnectedAbortController } from '../../core/controllers.js';
import { forceType } from '../../core/decorators.js';
import { EventEmitter, isEventPrevented } from '../../core/eventing.js';
import { SbbHydrationMixin } from '../../core/mixins.js';
Expand Down Expand Up @@ -51,20 +50,17 @@ class SbbAlertGroupElement extends SbbHydrationMixin(LitElement) {
/** Emits when `sbb-alert-group` becomes empty. */
private _empty: EventEmitter<void> = new EventEmitter(this, SbbAlertGroupElement.events.empty);

private _abort = new SbbConnectedAbortController(this);

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener(
public constructor() {
super();
this.addEventListener?.(
'didClose',
async (e: CustomEvent<void>) => {
if (!(await isEventPrevented(e))) {
this._alertClosed(e);
}
},
{
signal,
// We use capture here, because didClose does not bubble.
capture: true,
},
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,10 @@ class SbbAutocompleteGridElement extends SbbAutocompleteBaseElement {
);
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this.abort.signal;
this.addEventListener(
'autocompleteOptionSelectionChange',
(e: CustomEvent<void>) => this.onOptionSelected(e),
{ signal },
public constructor() {
super();
this.addEventListener?.('autocompleteOptionSelectionChange', (e: CustomEvent<void>) =>
this.onOptionSelected(e),
);
}

Expand Down
1 change: 1 addition & 0 deletions src/elements/autocomplete/autocomplete-base-element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ export abstract class SbbAutocompleteBaseElement extends SbbNegativeMixin(

protected abstract overlayId: string;
protected abstract panelRole: string;
/** @deprecated No longer used internally. */
protected abort = new SbbConnectedAbortController(this);
private _overlay!: HTMLElement;
private _optionContainer!: HTMLElement;
Expand Down
11 changes: 4 additions & 7 deletions src/elements/autocomplete/autocomplete.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,10 @@ class SbbAutocompleteElement extends SbbAutocompleteBaseElement {
return Array.from(this.querySelectorAll?.('sbb-option') ?? []);
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this.abort.signal;
this.addEventListener(
'optionSelectionChange',
(e: CustomEvent<void>) => this.onOptionSelected(e),
{ signal },
public constructor() {
super();
this.addEventListener?.('optionSelectionChange', (e: CustomEvent<void>) =>
this.onOptionSelected(e),
);
}

Expand Down
14 changes: 6 additions & 8 deletions src/elements/breadcrumb/breadcrumb-group/breadcrumb-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
isArrowKeyPressed,
sbbInputModalityDetector,
} from '../../core/a11y.js';
import { SbbConnectedAbortController, SbbLanguageController } from '../../core/controllers.js';
import { SbbLanguageController } from '../../core/controllers.js';
import { hostAttributes } from '../../core/decorators.js';
import { setOrRemoveAttribute } from '../../core/dom.js';
import { i18nBreadcrumbEllipsisButtonLabel } from '../../core/i18n.js';
Expand Down Expand Up @@ -58,10 +58,14 @@ class SbbBreadcrumbGroupElement extends SbbNamedSlotListMixin<
skipInitial: true,
callback: () => this._evaluateCollapsedState(),
});
private _abort = new SbbConnectedAbortController(this);
private _language = new SbbLanguageController(this);
private _markForFocus = false;

public constructor() {
super();
this.addEventListener?.('keydown', (e) => this._handleKeyDown(e));
}

private _handleKeyDown(evt: KeyboardEvent): void {
if (
!this.listChildren.length ||
Expand All @@ -79,12 +83,6 @@ class SbbBreadcrumbGroupElement extends SbbNamedSlotListMixin<
}
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener('keydown', (e) => this._handleKeyDown(e), { signal });
}

protected override firstUpdated(changedProperties: PropertyValues<this>): void {
super.firstUpdated(changedProperties);

Expand Down
8 changes: 4 additions & 4 deletions src/elements/checkbox/checkbox-group/checkbox-group.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { LitElement, html } from 'lit';
import { customElement, property } from 'lit/decorators.js';

import { getNextElementIndex, interactivityChecker, isArrowKeyPressed } from '../../core/a11y.js';
import { SbbConnectedAbortController } from '../../core/controllers.js';
import { forceType, slotState } from '../../core/decorators.js';
import { isLean } from '../../core/dom.js';
import type { SbbHorizontalFrom, SbbOrientation } from '../../core/interfaces.js';
Expand Down Expand Up @@ -54,12 +53,13 @@ class SbbCheckboxGroupElement extends SbbDisabledMixin(LitElement) {
);
}

private _abort: SbbConnectedAbortController = new SbbConnectedAbortController(this);
public constructor() {
super();
this.addEventListener?.('keydown', (e) => this._handleKeyDown(e));
}

public override connectedCallback(): void {
super.connectedCallback();
const signal = this._abort.signal;
this.addEventListener('keydown', (e) => this._handleKeyDown(e), { signal });
this.toggleAttribute(
'data-has-panel',
!!this.querySelector?.('sbb-selection-expansion-panel, sbb-checkbox-panel'),
Expand Down
4 changes: 4 additions & 0 deletions src/elements/core/controllers/connected-abort-controller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import type { ReactiveController, ReactiveControllerHost } from 'lit';

/**
* @deprecated No replacement should be necessary, as this was intended
* for event listeners, which should be applied in the constructor.
*/
export class SbbConnectedAbortController implements ReactiveController {
private _abortController?: AbortController = new AbortController();

Expand Down
18 changes: 3 additions & 15 deletions src/elements/core/mixins/form-associated-checkbox-mixin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,22 +100,10 @@ export const SbbFormAssociatedCheckboxMixin = <T extends Constructor<LitElement>
super();
/** @internal */
this.internals.role = 'checkbox';
}

public override connectedCallback(): void {
super.connectedCallback();

this.addEventListener('click', this._handleUserInteraction);
this.addEventListener('keydown', preventScrollOnSpacebarPress);
this.addEventListener('keyup', this._handleKeyboardInteraction);
}

public override disconnectedCallback(): void {
super.disconnectedCallback();

this.removeEventListener('click', this._handleUserInteraction);
this.removeEventListener('keydown', preventScrollOnSpacebarPress);
this.removeEventListener('keyup', this._handleKeyboardInteraction);
this.addEventListener?.('click', this._handleUserInteraction);
this.addEventListener?.('keydown', preventScrollOnSpacebarPress);
this.addEventListener?.('keyup', this._handleKeyboardInteraction);
}

public override attributeChangedCallback(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export declare class SbbFormAssociatedRadioButtonMixinType
public required: boolean;

protected associatedRadioButtons?: Set<SbbFormAssociatedRadioButtonMixinType>;
/** @deprecated No longer used internally. */
protected abort: SbbConnectedAbortController;

public formResetCallback(): void;
Expand Down Expand Up @@ -76,6 +77,7 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme
return 'radio';
}

/** @deprecated No longer used internally. */
protected abort = new SbbConnectedAbortController(this);

/**
Expand All @@ -90,14 +92,12 @@ export const SbbFormAssociatedRadioButtonMixin = <T extends Constructor<LitEleme
super();
/** @internal */
this.internals.role = 'radio';
this.addEventListener?.('keydown', (e) => this._handleArrowKeyDown(e));
}

public override connectedCallback(): void {
super.connectedCallback();
this._connectToRegistry();
this.addEventListener('keydown', (e) => this._handleArrowKeyDown(e), {
signal: this.abort.signal,
});
}

public override disconnectedCallback(): void {
Expand Down
10 changes: 7 additions & 3 deletions src/elements/datepicker/common/datepicker-button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { property, state } from 'lit/decorators.js';

import { SbbButtonBaseElement } from '../../core/base-elements.js';
import { readConfig } from '../../core/config.js';
import { SbbConnectedAbortController, SbbLanguageController } from '../../core/controllers.js';
import { SbbLanguageController } from '../../core/controllers.js';
import { type DateAdapter, defaultDateAdapter } from '../../core/datetime.js';
import { i18nToday } from '../../core/i18n.js';
import { SbbNegativeMixin } from '../../core/mixins.js';
Expand Down Expand Up @@ -35,17 +35,21 @@ export abstract class SbbDatepickerButton<T = Date> extends SbbNegativeMixin(Sbb
protected datePickerElement?: SbbDatepickerElement<T> | null = null;
private _dateAdapter: DateAdapter<T> = readConfig().datetime?.dateAdapter ?? defaultDateAdapter;
private _datePickerController!: AbortController;
private _abort = new SbbConnectedAbortController(this);
private _language = new SbbLanguageController(this).withHandler(() => this._setAriaLabel());

protected abstract iconName: string;
protected abstract i18nOffBoundaryDay: Record<string, string>;
protected abstract i18nSelectOffBoundaryDay: (_currentDate: string) => Record<string, string>;

public constructor() {
super();
this.addEventListener?.('click', () => this._handleClick());
}

protected abstract findAvailableDate(_date: T): T;

public override connectedCallback(): void {
super.connectedCallback();
this.addEventListener('click', () => this._handleClick(), { signal: this._abort.signal });
this._syncUpstreamProperties();
if (!this.datePicker) {
this._init();
Expand Down
18 changes: 6 additions & 12 deletions src/elements/datepicker/datepicker-toggle/datepicker-toggle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { ref } from 'lit/directives/ref.js';
import type { SbbMiniButtonElement } from '../../button/mini-button.js';
import type { CalendarView, SbbCalendarElement } from '../../calendar.js';
import { sbbInputModalityDetector } from '../../core/a11y.js';
import { SbbConnectedAbortController, SbbLanguageController } from '../../core/controllers.js';
import { SbbLanguageController } from '../../core/controllers.js';
import { hostAttributes } from '../../core/decorators.js';
import { i18nShowCalendar } from '../../core/i18n.js';
import { SbbHydrationMixin, SbbNegativeMixin } from '../../core/mixins.js';
Expand Down Expand Up @@ -54,10 +54,14 @@ class SbbDatepickerToggleElement<T = Date> extends SbbNegativeMixin(SbbHydration
private _popoverElement!: SbbPopoverElement;
private _datePickerController!: AbortController;
private _language = new SbbLanguageController(this);
private _abort = new SbbConnectedAbortController(this);

public constructor() {
super();
this.addEventListener?.('click', (event) => {
if (event.composedPath()[0] === this) {
this.open();
}
});
if (!isServer) {
this.hydrationComplete.then(() => (this._renderCalendar = true));
}
Expand All @@ -83,16 +87,6 @@ class SbbDatepickerToggleElement<T = Date> extends SbbNegativeMixin(SbbHydration
if (formField) {
this.negative = formField.hasAttribute('negative');
}

this.addEventListener(
'click',
(event) => {
if (event.composedPath()[0] === this) {
this.open();
}
},
{ signal: this._abort.signal },
);
}

public override willUpdate(changedProperties: PropertyValues<this>): void {
Expand Down
11 changes: 6 additions & 5 deletions src/elements/datepicker/datepicker/datepicker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
import { customElement, property, state } from 'lit/decorators.js';

import { readConfig } from '../../core/config.js';
import { SbbConnectedAbortController, SbbLanguageController } from '../../core/controllers.js';
import { SbbLanguageController } from '../../core/controllers.js';
import { type DateAdapter, defaultDateAdapter } from '../../core/datetime.js';
import { forceType } from '../../core/decorators.js';
import { findInput, findReferencedElement } from '../../core/dom.js';
Expand Down Expand Up @@ -157,7 +157,6 @@ class SbbDatepickerElement<T = Date> extends LitElement {

private _dateAdapter: DateAdapter<T> = readConfig().datetime?.dateAdapter ?? defaultDateAdapter;

private _abort = new SbbConnectedAbortController(this);
private _language = new SbbLanguageController(this).withHandler(() => {
if (this._inputElement) {
if (this._inputElementPlaceholderMutable) {
Expand All @@ -169,11 +168,13 @@ class SbbDatepickerElement<T = Date> extends LitElement {
}
});

public constructor() {
super();
this.addEventListener?.('datepickerControlRegistered', () => this._emitInputUpdated());
}

public override connectedCallback(): void {
super.connectedCallback();
this.addEventListener('datepickerControlRegistered', () => this._emitInputUpdated(), {
signal: this._abort.signal,
});
this._attachInput();
if (this._inputElement) {
this._emitInputUpdated();
Expand Down
19 changes: 7 additions & 12 deletions src/elements/dialog/dialog/dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,13 @@ class SbbDialogElement extends SbbOverlayBaseElement {
private _dialogId = `sbb-dialog-${nextId++}`;
protected closeAttribute: string = 'sbb-dialog-close';

public constructor() {
super();
// Close dialog on backdrop click
this.addEventListener?.('pointerdown', this._pointerDownListener);
this.addEventListener?.('pointerup', this._closeOnBackdropClick);
}

/** Opens the component. */
public open(): void {
if (this.state !== 'closed') {
Expand Down Expand Up @@ -133,18 +140,6 @@ class SbbDialogElement extends SbbOverlayBaseElement {
this.focusHandler.trap(this);
}

public override connectedCallback(): void {
super.connectedCallback();

// Close dialog on backdrop click
this.addEventListener('pointerdown', this._pointerDownListener, {
signal: this.overlayController.signal,
});
this.addEventListener('pointerup', this._closeOnBackdropClick, {
signal: this.overlayController.signal,
});
}

protected override firstUpdated(changedProperties: PropertyValues<this>): void {
// Synchronize the negative state before the first opening to avoid a possible color flash if it is negative.
this._dialogTitleElement = this.querySelector('sbb-dialog-title')!;
Expand Down
Loading

0 comments on commit 18660a3

Please sign in to comment.