From 8f5ebc087baafcce54b8c72d6a236384f221cfe9 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Sat, 20 Jan 2024 21:06:48 +0100 Subject: [PATCH 01/10] refactor: extract base functionality of list components into `NamedSlotListElement` --- .../breadcrumb-group.spec.snap.js | 6 +- .../breadcrumb-group/breadcrumb-group.e2e.ts | 4 +- .../breadcrumb-group/breadcrumb-group.spec.ts | 6 +- .../breadcrumb-group/breadcrumb-group.ts | 109 ++++------ .../breadcrumb/breadcrumb/breadcrumb.spec.ts | 35 +++- .../breadcrumb/breadcrumb/breadcrumb.ts | 16 +- src/components/core/common-behaviors/index.ts | 1 + .../named-slot-list-element.ts | 113 ++++++++++ .../common-behaviors/slot-child-observer.ts | 67 +++--- src/components/icon/icon-base.ts | 10 +- src/components/icon/icon.spec.ts | 2 +- .../__snapshots__/link-list.spec.snap.js | 194 +++++++++++++++--- src/components/link-list/link-list.spec.ts | 132 ++++-------- src/components/link-list/link-list.ts | 85 +++----- .../menu/menu/__snapshots__/menu.spec.snap.js | 10 +- src/components/menu/menu/menu.spec.ts | 10 +- src/components/menu/menu/menu.ts | 84 +++----- .../navigation-list.spec.snap.js | 28 +-- .../navigation-list/navigation-list.e2e.ts | 4 +- .../navigation-list/navigation-list.spec.ts | 10 +- .../navigation-list/navigation-list.ts | 50 ++--- .../navigation-marker.e2e.ts | 4 +- .../navigation-marker.spec.ts | 2 +- .../navigation-marker/navigation-marker.ts | 34 +-- .../__snapshots__/skiplink-list.spec.snap.js | 18 +- .../skiplink-list/skiplink-list.e2e.ts | 5 +- .../skiplink-list/skiplink-list.spec.ts | 16 +- src/components/skiplink-list/skiplink-list.ts | 66 ++---- src/components/tag/tag-group/tag-group.scss | 4 +- .../tag/tag-group/tag-group.spec.ts | 24 +-- src/components/tag/tag-group/tag-group.ts | 49 ++--- .../train-formation.spec.snap.js | 25 ++- .../train-formation/train-formation.scss | 9 +- .../train-formation/train-formation.spec.ts | 6 +- .../train/train-formation/train-formation.ts | 57 ++--- .../train/train-wagon/train-wagon.e2e.ts | 16 +- .../train/train-wagon/train-wagon.scss | 4 +- .../train/train-wagon/train-wagon.spec.ts | 36 +++- .../train/train-wagon/train-wagon.ts | 64 ++---- src/components/train/train/train.spec.ts | 6 +- src/components/train/train/train.ts | 53 ++--- 41 files changed, 750 insertions(+), 724 deletions(-) create mode 100644 src/components/core/common-behaviors/named-slot-list-element.ts diff --git a/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js b/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js index 06c6a30362..233319709f 100644 --- a/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js +++ b/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js @@ -4,7 +4,7 @@ export const snapshots = {}; snapshots["sbb-breadcrumb-group renders"] = `
  1. - +
  2. - +
  3. - +
diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts index 089dd3d7af..4926f67a00 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts @@ -91,8 +91,8 @@ describe('sbb-breadcrumb-group', () => { // only two slots are displayed, and the second is the last one const slots = breadcrumbGroup.shadowRoot!.querySelectorAll('li > slot'); expect(slots.length).to.be.equal(2); - expect(slots[0]).to.have.attribute('name', 'breadcrumb-0'); - expect(slots[1]).to.have.attribute('name', 'breadcrumb-6'); + expect(slots[0]).to.have.attribute('name', 'child-0'); + expect(slots[1]).to.have.attribute('name', 'child-6'); }); it('keyboard navigation with ellipsis', async () => { diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts index 859548f7b0..0414d47cd9 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts @@ -19,11 +19,11 @@ describe('sbb-breadcrumb-group', () => { expect(root).dom.to.be.equal(` - - + + One - + Two diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts index 771a6f5e8d..9ef71b86ec 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts @@ -1,9 +1,10 @@ -import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit'; -import { html, LitElement, nothing } from 'lit'; +import type { CSSResultGroup, PropertyValueMap, PropertyValues, TemplateResult } from 'lit'; +import { html, nothing } from 'lit'; import { customElement, state } from 'lit/decorators.js'; import { getNextElementIndex, isArrowKeyPressed, sbbInputModalityDetector } from '../../core/a11y'; -import { LanguageController, SlotChildObserver } from '../../core/common-behaviors'; +import type { WithListChildren } from '../../core/common-behaviors'; +import { LanguageController, NamedSlotListElement } from '../../core/common-behaviors'; import { setAttribute } from '../../core/dom'; import { ConnectedAbortController } from '../../core/eventing'; import { i18nBreadcrumbEllipsisButtonLabel } from '../../core/i18n'; @@ -20,11 +21,9 @@ import '../../icon'; * @slot - Use the unnamed slot to add `sbb-breadcrumb` elements. */ @customElement('sbb-breadcrumb-group') -export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { +export class SbbBreadcrumbGroupElement extends NamedSlotListElement { public static override styles: CSSResultGroup = style; - - /** Local instance of slotted sbb-breadcrumb elements */ - @state() private _breadcrumbs: SbbBreadcrumbElement[] = []; + protected override readonly listChildTagNames = ['SBB-BREADCRUMB']; @state() private _state?: 'collapsed' | 'manually-expanded'; @@ -35,7 +34,7 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { private _handleKeyDown(evt: KeyboardEvent): void { if ( - !this._breadcrumbs || + !this.listChildren.length || // don't trap nested handling ((evt.target as HTMLElement) !== this && (evt.target as HTMLElement).parentElement !== this) ) { @@ -62,52 +61,40 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { this.toggleAttribute('data-loaded', true); } - protected override updated(): void { - if (this._markForFocus && sbbInputModalityDetector.mostRecentModality === 'keyboard') { - this._breadcrumbs[1]?.focus(); - - // Reset mark for focus - this._markForFocus = false; - } - } - public override disconnectedCallback(): void { super.disconnectedCallback(); this._resizeObserver.disconnect(); } - /** Creates and sets an array with only the sbb-breadcrumb children. */ - protected override checkChildren(): void { - this._evaluateCollapsedState(); + protected override willUpdate(changedProperties: PropertyValueMap>): void { + if (changedProperties.has('listChildren')) { + this._syncBreadcrumbs(); + } + } - const breadcrumbs = Array.from(this.children ?? []).filter( - (e): e is SbbBreadcrumbElement => e.tagName === 'SBB-BREADCRUMB', - ); - // If the slotted sbb-breadcrumb instances have not changed, - // we can skip syncing and updating the breadcrumb reference list. - if ( - this._breadcrumbs && - breadcrumbs.length === this._breadcrumbs.length && - this._breadcrumbs.every((e, i) => breadcrumbs[i] === e) - ) { - return; + protected override updated(changedProperties: PropertyValueMap>): void { + super.updated(changedProperties); + if (changedProperties.has('listChildren')) { + Promise.resolve().then(() => this._evaluateCollapsedState()); + } + if (this._markForFocus && sbbInputModalityDetector.mostRecentModality === 'keyboard') { + this.listChildren[1]?.focus(); + + // Reset mark for focus + this._markForFocus = false; } - this._breadcrumbs = breadcrumbs; - this._syncBreadcrumbs(); } /** Apply the aria-current attribute to the last sbb-breadcrumb element. */ private _syncBreadcrumbs(): void { - this._breadcrumbs.forEach((breadcrumb, index) => { - breadcrumb.removeAttribute('aria-current'); - if (!breadcrumb.id) { - breadcrumb.id = `sbb-breadcrumb-${index}`; - } - }); - this._breadcrumbs[this._breadcrumbs.length - 1]?.setAttribute('aria-current', 'page'); + this.listChildren + .slice(0, -1) + .filter((c) => c.hasAttribute('aria-current')) + .forEach((c) => c.removeAttribute('aria-current')); + this.listChildren[this.listChildren.length - 1]?.setAttribute('aria-current', 'page'); // If it is not expandable, reset state - if (this._breadcrumbs.length < 3) { + if (this.listChildren.length < 3) { this._state = undefined; } } @@ -117,16 +104,16 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { */ private _focusNextCollapsed(evt: KeyboardEvent): void { const arrayCollapsed: SbbBreadcrumbElement[] = [ - this._breadcrumbs[0], + this.listChildren[0], this.shadowRoot!.querySelector('#sbb-breadcrumb-ellipsis') as SbbBreadcrumbElement, - this._breadcrumbs[this._breadcrumbs.length - 1], + this.listChildren[this.listChildren.length - 1], ]; this._focusNext(evt, arrayCollapsed); } private _focusNext( evt: KeyboardEvent, - breadcrumbs: SbbBreadcrumbElement[] = this._breadcrumbs, + breadcrumbs: SbbBreadcrumbElement[] = this.listChildren, ): void { const current: number = breadcrumbs.findIndex( (e) => e === document.activeElement || e === this.shadowRoot!.activeElement, @@ -154,17 +141,9 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { } private _renderCollapsed(): TemplateResult { - for (let i = 0; i < this._breadcrumbs.length; i++) { - if (i === 0 || i === this._breadcrumbs.length - 1) { - this._breadcrumbs[i].setAttribute('slot', `breadcrumb-${i}`); - } else { - this._breadcrumbs[i].removeAttribute('slot'); - } - } - return html`
  • - +
  • - +
    • `; } private _renderExpanded(): TemplateResult[] { - const slotName = (index: number): string => `breadcrumb-${index}`; - - return this._breadcrumbs.map((element: SbbBreadcrumbElement, index: number) => { - element.setAttribute('slot', slotName(index)); - - return html` + return this.listSlotNames().map( + (name, index, array) => html`
  • - - ${index !== this._breadcrumbs.length - 1 + + ${index !== array.length - 1 ? html`` : nothing}
    • - `; - }); + `, + ); } protected override render(): TemplateResult { @@ -216,12 +191,10 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) { setAttribute(this, 'data-state', this._state); return html` -
      +
        ${this._state === 'collapsed' ? this._renderCollapsed() : this._renderExpanded()}
      - + ${this.renderHiddenSlot()} `; } } diff --git a/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts b/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts index 1b2c9bbf16..902a3dd849 100644 --- a/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts +++ b/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts @@ -1,17 +1,28 @@ import { expect, fixture } from '@open-wc/testing'; import { html } from 'lit/static-html.js'; + +import { waitForLitRender } from '../../core/testing'; + import './breadcrumb'; describe('sbb-breadcrumb', () => { it('renders with text', async () => { const root = await fixture(html` - Breadcrumb `); expect(root).dom.to.be.equal(` - + Breadcrumb `); @@ -24,8 +35,15 @@ describe('sbb-breadcrumb', () => { `); + await waitForLitRender(root); expect(root).dom.to.be.equal(` - + `); await expect(root).shadowDom.to.equalSnapshot(); @@ -36,8 +54,15 @@ describe('sbb-breadcrumb', () => { Home `); + await waitForLitRender(root); expect(root).dom.to.be.equal(` - + Home `); @@ -49,7 +74,7 @@ describe('sbb-breadcrumb', () => { const root = await fixture(html`Breadcrumb`); expect(root).dom.to.be.equal(` - + Breadcrumb `); diff --git a/src/components/breadcrumb/breadcrumb/breadcrumb.ts b/src/components/breadcrumb/breadcrumb/breadcrumb.ts index 9e465a0abb..9d1506f58a 100644 --- a/src/components/breadcrumb/breadcrumb/breadcrumb.ts +++ b/src/components/breadcrumb/breadcrumb/breadcrumb.ts @@ -15,6 +15,8 @@ import style from './breadcrumb.scss?lit&inline'; import '../../icon'; +let nextId = 1; + /** * It displays a link to a page; used within a `sbb-breadcrumb-group` it can display the path to the current page. * @@ -46,14 +48,16 @@ export class SbbBreadcrumbElement extends SlotChildObserver(LitElement) { @state() private _hasText = false; + public constructor() { + super(); + this.id = this.id || `sbb-breadcrumb-${nextId++}`; + } + private _language = new LanguageController(this); private _handlerRepository = new HandlerRepository(this, actionElementHandlerAspect); public override connectedCallback(): void { super.connectedCallback(); - this._hasText = Array.from(this.childNodes ?? []).some( - (n) => !(n as Element).slot && n.textContent?.trim(), - ); this._handlerRepository.connect(); } @@ -63,9 +67,9 @@ export class SbbBreadcrumbElement extends SlotChildObserver(LitElement) { } protected override checkChildren(): void { - this._hasText = !!this.shadowRoot!.querySelector('slot:not([name])') - ?.assignedNodes() - .some((n) => !!n.textContent?.trim()); + this._hasText = Array.from(this.childNodes ?? []).some( + (n) => !(n as Element).slot && n.textContent?.trim(), + ); } protected override render(): TemplateResult { diff --git a/src/components/core/common-behaviors/index.ts b/src/components/core/common-behaviors/index.ts index f6f89ce314..c643161d9e 100644 --- a/src/components/core/common-behaviors/index.ts +++ b/src/components/core/common-behaviors/index.ts @@ -1,5 +1,6 @@ export * from './constructor'; export * from './language-controller'; export * from './named-slot-state-controller'; +export * from './named-slot-list-element'; export * from './slot-child-observer'; export * from './update-scheduler'; diff --git a/src/components/core/common-behaviors/named-slot-list-element.ts b/src/components/core/common-behaviors/named-slot-list-element.ts new file mode 100644 index 0000000000..014c01392f --- /dev/null +++ b/src/components/core/common-behaviors/named-slot-list-element.ts @@ -0,0 +1,113 @@ +import type { TemplateResult } from 'lit'; +import { LitElement, html, nothing } from 'lit'; +import { state } from 'lit/decorators.js'; + +import { SlotChildObserver } from './slot-child-observer'; + +/** + * Helper type for willUpdate or similar checks. + * Allows the usage of the string literal 'listChildren'. + * + * @example + * protected override willUpdate(changedProperties: PropertyValueMap>): void { + * if (changedProperties.has('listChildren')) { + * ... + * } + * } + */ +export type WithListChildren< + T extends NamedSlotListElement, + C extends HTMLElement = HTMLElement, +> = T & { listChildren: C[] }; + +/** + * This base class provides named slot list observer functionality. + * This allows using the pattern of rendering a named slot for each child, which allows + * wrapping children in a ul/li list. + */ +export abstract class NamedSlotListElement< + C extends HTMLElement = HTMLElement, +> extends SlotChildObserver(LitElement) { + /** A list of upper-cased tag names to match against. (e.g. SBB-LINK) */ + protected abstract readonly listChildTagNames: string[]; + + /** + * A list of children with the defined tag names. + * This array is only updated, if there is an actual change + * to the child elements. + */ + @state() protected listChildren: C[] = []; + + protected override checkChildren(): void { + const listChildren = Array.from(this.children).filter((e): e is C => + this.listChildTagNames.includes(e.tagName), + ); + // If the slotted child instances have not changed, we can skip syncing and updating + // the link reference list. + if ( + this.listChildren && + listChildren.length === this.listChildren.length && + this.listChildren.every((e, i) => listChildren[i] === e) + ) { + return; + } + + this.listChildren + .filter((c) => !listChildren.includes(c)) + .forEach((c) => c.removeAttribute('slot')); + this.listChildren = listChildren; + this.listChildren.forEach((child, index) => { + child.setAttribute('slot', `child-${index}`); + this.formatChild?.(child); + }); + } + + protected formatChild?(child: C): void; + + /** + * Renders list slots for slotted children or an amount of list slots + * corresponding to the `data-ssr-child-count` attribute value. + * + * This is a possible optimization for SSR, as in an SSR Lit environment + * other elements are not available, but might be available in the meta + * framework wrapper (like e.g. React). This allows to provide the amount of + * children to be passed via the `data-ssr-child-count` attribute value. + */ + protected renderListSlots(): TemplateResult { + return html`${this.listSlotNames().map((name) => html`
      2. `)}`; + } + + /** + * Returns an array of list slot names with the length corresponding to the amount of matched + * children or the `data-ssr-child-count` attribute value. + * + * This is a possible optimization for SSR, as in an SSR Lit environment + * other elements are not available, but might be available in the meta + * framework wrapper (like e.g. React). This allows to provide the amount of + * children to be passed via the `data-ssr-child-count` attribute value. + */ + protected listSlotNames(): string[] { + const listChildren = this.listChildren.length + ? this.listChildren + : Array.from({ length: +this.getAttribute('data-ssr-child-count') }); + return listChildren.map((_, i) => `child-${i}`); + } + + /** + * Returns a hidden slot, which is intended as the children change detection. + * When an element without a slot attribute is slotted to the element, it triggers + * the slotchange event, which can be used to assign it to the appropriate named slot. + */ + protected renderHiddenSlot(): TemplateResult { + return html``; + } + + /** + * Returns 'presentation' when less than two children are available. + * This is an accessibility improvement, as only lists with more than one + * children should be marked as lists. + */ + protected roleOverride(): 'presentation' | typeof nothing { + return this.listSlotNames().length > 1 ? nothing : 'presentation'; + } +} diff --git a/src/components/core/common-behaviors/slot-child-observer.ts b/src/components/core/common-behaviors/slot-child-observer.ts index 530926c469..0333da71c0 100644 --- a/src/components/core/common-behaviors/slot-child-observer.ts +++ b/src/components/core/common-behaviors/slot-child-observer.ts @@ -1,12 +1,10 @@ import type { LitElement, PropertyValues } from 'lit'; -import { ConnectedAbortController } from '../eventing'; - import type { Constructor } from './constructor'; // Define the interface for the mixin -export declare class SlotChildObserverType { - protected checkChildren(): void; +export declare abstract class SlotChildObserverType { + protected abstract checkChildren(): void; } /** @@ -22,51 +20,52 @@ export const SlotChildObserver = >( ): Constructor & T => { class SlotChildObserverClass extends base implements Partial { /** + * Whether the component needs hydration. + * @see https://github.com/lit/lit/blob/main/packages/labs/ssr-client/src/lit-element-hydrate-support.ts + * * We have a problem with SSR, in that we don't have a reference to children. * Due to this, the SSR/hydration can fail, when internal elements depend on children * and will therefore not be rendered server side, but will be immediately rendered client side. * Due to this, we add this initialized property, which will prevent child detection * until it has been initialized/hydrated. - * - * https://github.com/lit/lit/discussions/4407 + * @see https://github.com/lit/lit/discussions/4407 */ - private _hydrated = new Promise((r) => (this._hydratedResolve = r)); - private _hydratedResolve?: () => void; - private _hydratedResolved = false; - private _slotchangeAbortController = new ConnectedAbortController(this); + private _needsHydration = false; + private _slotchangeHandler = (): void => { + if (this._needsHydration) { + return; + } + this.checkChildren(); + }; + + protected override createRenderRoot(): HTMLElement | DocumentFragment { + this._needsHydration = !!this.shadowRoot; + return super.createRenderRoot(); + } + + protected override update(changedProperties: PropertyValues): void { + super.update(changedProperties); + if (this._needsHydration) { + this._needsHydration = false; + Promise.resolve().then(() => this.checkChildren()); + } + } public override connectedCallback(): void { super.connectedCallback(); - if (this._hydratedResolved) { + if (!this._needsHydration) { this.checkChildren(); } - this.shadowRoot?.addEventListener( - 'slotchange', - () => { - if (!this._hydratedResolved) { - return; - } - this.checkChildren(); - }, - { signal: this._slotchangeAbortController.signal }, - ); + this.shadowRoot!.addEventListener('slotchange', this._slotchangeHandler); } - protected checkChildren(): void { - // Needs to be implemented by inherited classes + public override disconnectedCallback(): void { + super.disconnectedCallback(); + this.shadowRoot!.removeEventListener('slotchange', this._slotchangeHandler); } - protected override firstUpdated(changedProperties: PropertyValues): void { - super.firstUpdated(changedProperties); - this._hydratedResolved = true; - this._hydratedResolve?.(); - Promise.resolve().then(() => this.checkChildren()); - } - - protected override async getUpdateComplete(): Promise { - const result = await super.getUpdateComplete(); - await this._hydrated; - return result; + protected checkChildren(): void { + // Needs to be implemented by inherited classes } } return SlotChildObserverClass as unknown as Constructor & T; diff --git a/src/components/icon/icon-base.ts b/src/components/icon/icon-base.ts index 8db12e58e1..e92e776dd2 100644 --- a/src/components/icon/icon-base.ts +++ b/src/components/icon/icon-base.ts @@ -2,12 +2,13 @@ import type { CSSResultGroup, TemplateResult } from 'lit'; import { html, LitElement } from 'lit'; import { property, state } from 'lit/decorators.js'; +import { UpdateScheduler } from '../core/common-behaviors'; import { setAttribute } from '../core/dom'; import { getSvgContent } from './icon-request'; import style from './icon.scss?lit&inline'; -export abstract class SbbIconBase extends LitElement { +export abstract class SbbIconBase extends UpdateScheduler(LitElement) { public static override styles: CSSResultGroup = style; @state() private _svgNamespace = 'default'; @@ -30,13 +31,18 @@ export abstract class SbbIconBase extends LitElement { return; } + this.startUpdate(); const [namespace, name] = this._splitIconName(iconName); if (namespace) { this._svgNamespace = namespace; } - this._svgIcon = await this.fetchSvgIcon(this._svgNamespace, name); + try { + this._svgIcon = await this.fetchSvgIcon(this._svgNamespace, name); + } finally { + this.completeUpdate(); + } } protected async fetchSvgIcon(namespace: string, name: string): Promise { diff --git a/src/components/icon/icon.spec.ts b/src/components/icon/icon.spec.ts index b39a936f92..4dbade117e 100644 --- a/src/components/icon/icon.spec.ts +++ b/src/components/icon/icon.spec.ts @@ -110,7 +110,7 @@ describe('sbb-icon', () => { await waitForLitRender(root); expect(root).dom.to.be.equal(` - `, @@ -140,8 +146,17 @@ describe('sbb-train-wagon', () => { - - Additional wagon information + + @@ -157,6 +172,7 @@ describe('sbb-train-wagon', () => { >`, ); + await waitForLitRender(root); expect(root).dom.to.be.equal( ` @@ -165,13 +181,13 @@ describe('sbb-train-wagon', () => { data-namespace="default" name="sa-rs" role="img" - slot="sbb-train-wagon-icon-0"> + slot="child-0"> + slot="child-1"> `, ); @@ -193,14 +209,14 @@ describe('sbb-train-wagon', () => {
        -
      • - +
      • +
        • -
      • - +
      • +
      - diff --git a/src/components/train/train-wagon/train-wagon.ts b/src/components/train/train-wagon/train-wagon.ts index 11636795a1..cbc9c98d46 100644 --- a/src/components/train/train-wagon/train-wagon.ts +++ b/src/components/train/train-wagon/train-wagon.ts @@ -1,9 +1,9 @@ import type { CSSResultGroup, TemplateResult } from 'lit'; -import { LitElement, nothing } from 'lit'; -import { customElement, property, state } from 'lit/decorators.js'; +import { nothing } from 'lit'; +import { customElement, property } from 'lit/decorators.js'; import { html, unsafeStatic } from 'lit/static-html.js'; -import { LanguageController, SlotChildObserver } from '../../core/common-behaviors'; +import { LanguageController, NamedSlotListElement } from '../../core/common-behaviors'; import { setAttribute } from '../../core/dom'; import { EventEmitter } from '../../core/eventing'; import { @@ -28,11 +28,12 @@ import style from './train-wagon.scss?lit&inline'; * @slot - Use the unnamed slot to add one or more `sbb-icon` for meta-information of the `sbb-train-wagon`. */ @customElement('sbb-train-wagon') -export class SbbTrainWagonElement extends SlotChildObserver(LitElement) { +export class SbbTrainWagonElement extends NamedSlotListElement { public static override styles: CSSResultGroup = style; public static readonly events = { sectorChange: 'sectorChange', } as const; + protected override readonly listChildTagNames = ['SBB-ICON']; /** Wagon type. */ @property({ reflect: true }) public type: 'locomotive' | 'closed' | 'wagon' = 'wagon'; @@ -65,9 +66,6 @@ export class SbbTrainWagonElement extends SlotChildObserver(LitElement) { @property({ attribute: 'additional-accessibility-text' }) public additionalAccessibilityText?: string; - /** Slotted Sbb-icons. */ - @state() private _icons: SbbIconElement[] = []; - private _language = new LanguageController(this); /** @@ -87,25 +85,7 @@ export class SbbTrainWagonElement extends SlotChildObserver(LitElement) { this._sectorChange.emit(); } - /** - * Create an array with only the sbb-icon children. - */ - protected override checkChildren(): void { - this._icons = Array.from(this.children ?? []).filter( - (e): e is SbbIconElement => e.tagName === 'SBB-ICON', - ); - } - protected override render(): TemplateResult { - // We should avoid lists with only one entry - if (this._icons?.length > 1) { - this._icons.forEach((icon, index) => - icon.setAttribute('slot', `sbb-train-wagon-icon-${index}`), - ); - } else { - this._icons?.forEach((icon) => icon.removeAttribute('slot')); - } - const label = (tagName: string): TemplateResult => { const TAG_NAME = tagName; /* eslint-disable lit/binding-positions */ @@ -204,31 +184,15 @@ export class SbbTrainWagonElement extends SlotChildObserver(LitElement) { ? html`, ${this.additionalAccessibilityText}` : nothing} ${this.type === 'wagon' - ? html` - ${this._icons?.length > 1 - ? html`
        - ${this._icons.map( - (_, index) => - html`
      • - -
        • `, - )} -
      ` - : nothing} - - ${this._icons?.length === 1 - ? html` - ${i18nAdditionalWagonInformationHeading[this._language.current]} - ` - : nothing} - - + ? html` +
        + ${this.renderListSlots()} +
      + ${this.renderHiddenSlot()}       ` : nothing} diff --git a/src/components/train/train/train.spec.ts b/src/components/train/train/train.spec.ts index 26ef0479df..8d2ea5ed50 100644 --- a/src/components/train/train/train.spec.ts +++ b/src/components/train/train/train.spec.ts @@ -1,5 +1,8 @@ import { expect, fixture } from '@open-wc/testing'; import { html } from 'lit/static-html.js'; + +import { waitForLitRender } from '../../core/testing'; + import './train'; import '../../icon'; @@ -9,6 +12,7 @@ describe('sbb-train', () => { html``, ); + await waitForLitRender(root); expect(root).dom.to.be.equal( ``, ); @@ -16,7 +20,7 @@ describe('sbb-train', () => { `
      Train, Driving direction Bern.
      -
        + diff --git a/src/components/train/train/train.ts b/src/components/train/train/train.ts index f5c98d0e75..1d89730508 100644 --- a/src/components/train/train/train.ts +++ b/src/components/train/train/train.ts @@ -1,9 +1,10 @@ -import type { CSSResultGroup, TemplateResult } from 'lit'; -import { LitElement, nothing } from 'lit'; -import { customElement, property, state } from 'lit/decorators.js'; +import type { CSSResultGroup, PropertyValueMap, TemplateResult } from 'lit'; +import { nothing } from 'lit'; +import { customElement, property } from 'lit/decorators.js'; import { html, unsafeStatic } from 'lit/static-html.js'; -import { LanguageController, SlotChildObserver } from '../../core/common-behaviors'; +import type { WithListChildren } from '../../core/common-behaviors'; +import { LanguageController, NamedSlotListElement } from '../../core/common-behaviors'; import { EventEmitter } from '../../core/eventing'; import { i18nTrain, i18nWagonsLabel } from '../../core/i18n'; import type { TitleLevel } from '../../title'; @@ -20,11 +21,14 @@ import '../../icon'; * @slot - Use the unnamed slot to add 'sbb-train-wagon' elements to the `sbb-train`. */ @customElement('sbb-train') -export class SbbTrainElement extends SlotChildObserver(LitElement) { +export class SbbTrainElement extends NamedSlotListElement< + SbbTrainWagonElement | SbbTrainBlockedPassageElement +> { public static override styles: CSSResultGroup = style; public static readonly events = { trainSlotChange: 'trainSlotChange', } as const; + protected override readonly listChildTagNames = ['SBB-TRAIN-WAGON', 'SBB-TRAIN-BLOCKED-PASSAGE']; /** General label for "driving direction". */ @property({ attribute: 'direction-label' }) public directionLabel!: string; @@ -41,8 +45,6 @@ export class SbbTrainElement extends SlotChildObserver(LitElement) { /** Controls the direction indicator to show the arrow left or right. Default is left. */ @property({ reflect: true }) public direction: 'left' | 'right' = 'left'; - @state() private _wagons: (SbbTrainBlockedPassageElement | SbbTrainWagonElement)[] = []; - private _language = new LanguageController(this); /** @@ -75,28 +77,14 @@ export class SbbTrainElement extends SlotChildObserver(LitElement) { return `${textParts.join(', ')}.`; } - protected override checkChildren(): void { - const wagons = Array.from(this.children ?? []).filter( - (e): e is SbbTrainBlockedPassageElement | SbbTrainWagonElement => - e.tagName === 'SBB-TRAIN-WAGON' || e.tagName === 'SBB-TRAIN-BLOCKED-PASSAGE', - ); - // If the slotted sbb-train-wagon and sbb-train-blocked-passage instances have not changed, we can skip syncing and updating - // the link reference list. - if ( - this._wagons && - wagons.length === this._wagons.length && - this._wagons.every((e, i) => wagons[i] === e) - ) { - return; + protected override willUpdate(changedProperties: PropertyValueMap>): void { + if (changedProperties.has('listChildren')) { + this._trainSlotChange.emit(); } - - this._trainSlotChange.emit(); - this._wagons = wagons; } protected override render(): TemplateResult { const TITLE_TAG_NAME = `h${this.directionLabelLevel}`; - this._wagons.forEach((wagon, index) => wagon.setAttribute('slot', `wagon-${index}`)); /* eslint-disable lit/binding-positions */ return html` @@ -104,17 +92,14 @@ export class SbbTrainElement extends SlotChildObserver(LitElement) { <${unsafeStatic(TITLE_TAG_NAME)} class="sbb-train__direction-label-sr"> ${this._getDirectionAriaLabel()} -
          - ${this._wagons.map( - (_, index) => - html`
        • - -
          • `, - )} +
            + ${this.renderListSlots()}
          - + ${this.renderHiddenSlot()} ${ this.directionLabel From ac1fd307203079d23e65a3b4c46bacebc4d12d21 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Sat, 20 Jan 2024 23:39:13 +0100 Subject: [PATCH 02/10] refactor: optimize logic --- .../named-slot-list-element.ts | 53 +++++++++++++------ .../common-behaviors/slot-child-observer.ts | 15 ++++++ src/components/link-list/link-list.ts | 16 +++--- src/components/menu/menu/menu.ts | 11 +--- .../navigation-list/navigation-list.ts | 12 ++--- .../navigation-marker/navigation-marker.ts | 7 +-- src/components/skiplink-list/skiplink-list.ts | 9 +--- src/components/tag/tag-group/tag-group.ts | 12 ++--- .../train/train-formation/train-formation.ts | 12 ++--- .../train/train-wagon/train-wagon.ts | 12 ++--- src/components/train/train/train.ts | 12 ++--- 11 files changed, 81 insertions(+), 90 deletions(-) diff --git a/src/components/core/common-behaviors/named-slot-list-element.ts b/src/components/core/common-behaviors/named-slot-list-element.ts index 014c01392f..aaa323b275 100644 --- a/src/components/core/common-behaviors/named-slot-list-element.ts +++ b/src/components/core/common-behaviors/named-slot-list-element.ts @@ -4,6 +4,9 @@ import { state } from 'lit/decorators.js'; import { SlotChildObserver } from './slot-child-observer'; +const SSR_CHILD_COUNT_ATTRIBUTE = 'data-ssr-child-count'; +const SLOTNAME_PREFIX = 'child'; + /** * Helper type for willUpdate or similar checks. * Allows the usage of the string literal 'listChildren'. @@ -39,13 +42,12 @@ export abstract class NamedSlotListElement< @state() protected listChildren: C[] = []; protected override checkChildren(): void { - const listChildren = Array.from(this.children).filter((e): e is C => + const listChildren = Array.from(this.children ?? []).filter((e): e is C => this.listChildTagNames.includes(e.tagName), ); // If the slotted child instances have not changed, we can skip syncing and updating // the link reference list. if ( - this.listChildren && listChildren.length === this.listChildren.length && this.listChildren.every((e, i) => listChildren[i] === e) ) { @@ -57,15 +59,18 @@ export abstract class NamedSlotListElement< .forEach((c) => c.removeAttribute('slot')); this.listChildren = listChildren; this.listChildren.forEach((child, index) => { - child.setAttribute('slot', `child-${index}`); + child.setAttribute('slot', `${SLOTNAME_PREFIX}-${index}`); this.formatChild?.(child); }); + + // Remove the ssr attribute, once we have actually initialized the children elements. + this.removeAttribute(SSR_CHILD_COUNT_ATTRIBUTE); } protected formatChild?(child: C): void; /** - * Renders list slots for slotted children or an amount of list slots + * Renders list and list slots for slotted children or an amount of list slots * corresponding to the `data-ssr-child-count` attribute value. * * This is a possible optimization for SSR, as in an SSR Lit environment @@ -73,8 +78,20 @@ export abstract class NamedSlotListElement< * framework wrapper (like e.g. React). This allows to provide the amount of * children to be passed via the `data-ssr-child-count` attribute value. */ - protected renderListSlots(): TemplateResult { - return html`${this.listSlotNames().map((name) => html`
          • `)}`; + protected renderList( + attributes: { class?: string; ariaLabel?: string; ariaLabelledby?: string } = {}, + ): TemplateResult { + return html` +
            + ${this.listSlotNames().map((name) => html`
            • `)} +
          + ${this.renderHiddenSlot()} + `; } /** @@ -89,8 +106,19 @@ export abstract class NamedSlotListElement< protected listSlotNames(): string[] { const listChildren = this.listChildren.length ? this.listChildren - : Array.from({ length: +this.getAttribute('data-ssr-child-count') }); - return listChildren.map((_, i) => `child-${i}`); + : Array.from({ length: +this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE) }); + return listChildren.map((_, i) => `${SLOTNAME_PREFIX}-${i}`); + } + + /** + * Returns 'presentation' when less than two children are available. + * This is an accessibility improvement, as only lists with more than one + * children should be marked as lists. + */ + protected roleOverride(): 'presentation' | typeof nothing { + return (this.listChildren.length || +this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE)) >= 2 + ? nothing + : 'presentation'; } /** @@ -101,13 +129,4 @@ export abstract class NamedSlotListElement< protected renderHiddenSlot(): TemplateResult { return html``; } - - /** - * Returns 'presentation' when less than two children are available. - * This is an accessibility improvement, as only lists with more than one - * children should be marked as lists. - */ - protected roleOverride(): 'presentation' | typeof nothing { - return this.listSlotNames().length > 1 ? nothing : 'presentation'; - } } diff --git a/src/components/core/common-behaviors/slot-child-observer.ts b/src/components/core/common-behaviors/slot-child-observer.ts index 0333da71c0..98a21a8fd5 100644 --- a/src/components/core/common-behaviors/slot-child-observer.ts +++ b/src/components/core/common-behaviors/slot-child-observer.ts @@ -39,14 +39,29 @@ export const SlotChildObserver = >( }; protected override createRenderRoot(): HTMLElement | DocumentFragment { + // Check whether hydration is needed by checking whether the shadow root + // is available before createRenderRoot is called. this._needsHydration = !!this.shadowRoot; return super.createRenderRoot(); } + protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); + // If hydration is not needed we can immediately check for children + // in the first update. + if (!this._needsHydration && !this.hasUpdated) { + this.checkChildren(); + } + } + protected override update(changedProperties: PropertyValues): void { + // When hydration is needed, we wait the hydration process to finish, which is patched + // into the update method of the LitElement base class. super.update(changedProperties); if (this._needsHydration) { this._needsHydration = false; + // Scheduling checkChildren needs to be delayed by a micro-task, else lit + // will detect a change immediately after an update and warn in the console. Promise.resolve().then(() => this.checkChildren()); } } diff --git a/src/components/link-list/link-list.ts b/src/components/link-list/link-list.ts index 3d1b96cfbc..a3e02a8197 100644 --- a/src/components/link-list/link-list.ts +++ b/src/components/link-list/link-list.ts @@ -76,16 +76,12 @@ export class SbbLinkListElement extends NamedSlotListElement { > this.requestUpdate()}>${this.titleContent} -
            - ${this.renderListSlots()} -
          - ${this.renderHiddenSlot()} + ${this.renderList({ + ariaLabelledby: + this._namedSlots.slots.has('title') || this.titleContent + ? 'sbb-link-list-title-id' + : undefined, + })}
        `; } diff --git a/src/components/menu/menu/menu.ts b/src/components/menu/menu/menu.ts index 25ddbcf3b5..915c862b0c 100644 --- a/src/components/menu/menu/menu.ts +++ b/src/components/menu/menu/menu.ts @@ -229,7 +229,7 @@ export class SbbMenuElement extends NamedSlotListElement { // If all children are sbb-menu-action instances, we render them as a list. if ( this.children?.length && - Array.from(this.children).every((c) => c.tagName === 'SBB-MENU-ACTION') + Array.from(this.children ?? []).every((c) => c.tagName === 'SBB-MENU-ACTION') ) { super.checkChildren(); } else if (this.listChildren.length) { @@ -383,14 +383,7 @@ export class SbbMenuElement extends NamedSlotListElement { class="sbb-menu__content" > ${this.listChildren.length - ? html`
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()}` + ? this.renderList({ class: 'sbb-menu-list', ariaLabel: this.listAccessibilityLabel }) : html``} diff --git a/src/components/navigation/navigation-list/navigation-list.ts b/src/components/navigation/navigation-list/navigation-list.ts index b885f1da16..d592a530b8 100644 --- a/src/components/navigation/navigation-list/navigation-list.ts +++ b/src/components/navigation/navigation-list/navigation-list.ts @@ -37,14 +37,10 @@ export class SbbNavigationListElement extends NamedSlotListElement ${this.label}         -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ + class: 'sbb-navigation-list__content', + ariaLabelledby: 'sbb-navigation-link-label-id', + })} `; } } diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index 39d471f84f..df5ad35105 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -98,12 +98,7 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement - ${this.renderListSlots()} - - ${this.renderHiddenSlot()} - `; + return this.renderList(); } } diff --git a/src/components/skiplink-list/skiplink-list.ts b/src/components/skiplink-list/skiplink-list.ts index 918e3871b7..52820837f0 100644 --- a/src/components/skiplink-list/skiplink-list.ts +++ b/src/components/skiplink-list/skiplink-list.ts @@ -49,14 +49,7 @@ export class SbbSkiplinkListElement extends NamedSlotListElement > ${this.titleContent} -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ ariaLabelledby: 'sbb-skiplink-list-title-id' })} `; } diff --git a/src/components/tag/tag-group/tag-group.ts b/src/components/tag/tag-group/tag-group.ts index f87a456d94..25acebdfa7 100644 --- a/src/components/tag/tag-group/tag-group.ts +++ b/src/components/tag/tag-group/tag-group.ts @@ -152,14 +152,10 @@ export class SbbTagGroupElement extends NamedSlotListElement { return html`
        -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ + class: 'sbb-tag-group__list', + ariaLabel: this.listAccessibilityLabel, + })}
        `; } diff --git a/src/components/train/train-formation/train-formation.ts b/src/components/train/train-formation/train-formation.ts index cf4c51db0e..52954e6227 100644 --- a/src/components/train/train-formation/train-formation.ts +++ b/src/components/train/train-formation/train-formation.ts @@ -148,14 +148,10 @@ export class SbbTrainFormationElement extends NamedSlotListElement
        -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ + class: 'sbb-train-formation__train-list', + ariaLabel: i18nTrains[this._language.current], + })}
        `; diff --git a/src/components/train/train-wagon/train-wagon.ts b/src/components/train/train-wagon/train-wagon.ts index cbc9c98d46..a96fc99a30 100644 --- a/src/components/train/train-wagon/train-wagon.ts +++ b/src/components/train/train-wagon/train-wagon.ts @@ -185,14 +185,10 @@ export class SbbTrainWagonElement extends NamedSlotListElement { : nothing} ${this.type === 'wagon' ? html` -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ + class: 'sbb-train-wagon__icons-list', + ariaLabel: i18nAdditionalWagonInformationHeading[this._language.current], + })}         ` : nothing} diff --git a/src/components/train/train/train.ts b/src/components/train/train/train.ts index 1d89730508..53ddc88abd 100644 --- a/src/components/train/train/train.ts +++ b/src/components/train/train/train.ts @@ -92,14 +92,10 @@ export class SbbTrainElement extends NamedSlotListElement< <${unsafeStatic(TITLE_TAG_NAME)} class="sbb-train__direction-label-sr"> ${this._getDirectionAriaLabel()} -
          - ${this.renderListSlots()} -
        - ${this.renderHiddenSlot()} + ${this.renderList({ + class: 'sbb-train__wagons', + ariaLabel: i18nWagonsLabel[this._language.current], + })} ${ this.directionLabel From 9c2ba3b1bb4e7c18bf983911c131896b6aba2920 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Mon, 22 Jan 2024 10:25:13 +0100 Subject: [PATCH 03/10] docs: fix --- src/components/breadcrumb/breadcrumb/breadcrumb.ts | 1 + src/components/menu/menu/menu.ts | 1 + 2 files changed, 2 insertions(+) diff --git a/src/components/breadcrumb/breadcrumb/breadcrumb.ts b/src/components/breadcrumb/breadcrumb/breadcrumb.ts index 9d1506f58a..6e2313782c 100644 --- a/src/components/breadcrumb/breadcrumb/breadcrumb.ts +++ b/src/components/breadcrumb/breadcrumb/breadcrumb.ts @@ -50,6 +50,7 @@ export class SbbBreadcrumbElement extends SlotChildObserver(LitElement) { public constructor() { super(); + /** @ignore id is already a well known property. */ this.id = this.id || `sbb-breadcrumb-${nextId++}`; } diff --git a/src/components/menu/menu/menu.ts b/src/components/menu/menu/menu.ts index 915c862b0c..70bbf45f2a 100644 --- a/src/components/menu/menu/menu.ts +++ b/src/components/menu/menu/menu.ts @@ -113,6 +113,7 @@ export class SbbMenuElement extends NamedSlotListElement { public constructor() { super(); + /** @ignore id is already a well known property. */ this.id = this.id || `sbb-menu-${nextId++}`; } From e5fada7e56283fb5c2cec4eee6ff490c64a86efe Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Mon, 22 Jan 2024 10:30:52 +0100 Subject: [PATCH 04/10] fix: linting --- src/components/menu/menu/menu.ts | 2 +- .../navigation/navigation-marker/navigation-marker.ts | 1 - src/components/tag/tag-group/tag-group.ts | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/components/menu/menu/menu.ts b/src/components/menu/menu/menu.ts index 70bbf45f2a..ff7f26b5cd 100644 --- a/src/components/menu/menu/menu.ts +++ b/src/components/menu/menu/menu.ts @@ -1,5 +1,5 @@ import type { CSSResultGroup, TemplateResult } from 'lit'; -import { html, nothing } from 'lit'; +import { html } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import { ref } from 'lit/directives/ref.js'; diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index df5ad35105..3781ed2364 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -1,5 +1,4 @@ import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit'; -import { html } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; import { NamedSlotListElement } from '../../core/common-behaviors'; diff --git a/src/components/tag/tag-group/tag-group.ts b/src/components/tag/tag-group/tag-group.ts index 25acebdfa7..e63abb726c 100644 --- a/src/components/tag/tag-group/tag-group.ts +++ b/src/components/tag/tag-group/tag-group.ts @@ -1,5 +1,5 @@ import type { CSSResultGroup, TemplateResult, PropertyValueMap } from 'lit'; -import { html, nothing } from 'lit'; +import { html } from 'lit'; import { customElement, property } from 'lit/decorators.js'; import type { WithListChildren } from '../../core/common-behaviors'; From a97e02ad84250e5143a470781510b537a97cef5a Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Mon, 22 Jan 2024 13:28:52 +0100 Subject: [PATCH 05/10] fix: navigation marker position --- .../navigation-marker/navigation-marker.ts | 23 +++++++------------ 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index 3781ed2364..22df63e91f 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -1,7 +1,7 @@ import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit'; import { customElement, property, state } from 'lit/decorators.js'; -import { NamedSlotListElement } from '../../core/common-behaviors'; +import { NamedSlotListElement, type WithListChildren } from '../../core/common-behaviors'; import { setAttribute } from '../../core/dom'; import { AgnosticResizeObserver } from '../../core/observers'; import type { SbbNavigationActionElement } from '../navigation-action'; @@ -33,19 +33,19 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement): void { - if (changedProperties.has('size')) { + protected override willUpdate(changedProperties: PropertyValues>): void { + if (changedProperties.has('size') || changedProperties.has('listChildren')) { this._updateMarkerActions(); } } private _updateMarkerActions(): void { - for (const action of this._navigationActions) { + for (const action of this.listChildren) { action.size = this.size; } - this._hasActiveAction = !!this._activeNavigationAction; - this._currentActiveAction = this._activeNavigationAction; + this._currentActiveAction = this.listChildren.find((action) => action.active); + this._hasActiveAction = !!this._currentActiveAction; this._setMarkerPosition(); } @@ -77,19 +77,12 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement action.active); - } - private _setMarkerPosition(): void { if (this._hasActiveAction) { + const index = this.listChildren.indexOf(this._currentActiveAction)!; this.style?.setProperty( '--sbb-navigation-marker-position-y', - `${(this.shadowRoot!.querySelector('[data-active]') as HTMLElement)?.offsetTop}px`, + `${this.shadowRoot.querySelector(`li:nth-child(${index + 1})`).offsetTop}px`, ); } } From 3233f9429783f0d8bc37e94fd0bbc96083d1a1f7 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Tue, 23 Jan 2024 12:10:57 +0100 Subject: [PATCH 06/10] refactor: adapt strict types --- .../named-slot-list-element.ts | 4 ++-- .../navigation-marker/navigation-marker.ts | 22 ++++++------------- src/components/skiplink-list/skiplink-list.ts | 2 +- .../train/train-formation/train-formation.ts | 2 +- 4 files changed, 11 insertions(+), 19 deletions(-) diff --git a/src/components/core/common-behaviors/named-slot-list-element.ts b/src/components/core/common-behaviors/named-slot-list-element.ts index aaa323b275..e729071f8a 100644 --- a/src/components/core/common-behaviors/named-slot-list-element.ts +++ b/src/components/core/common-behaviors/named-slot-list-element.ts @@ -106,7 +106,7 @@ export abstract class NamedSlotListElement< protected listSlotNames(): string[] { const listChildren = this.listChildren.length ? this.listChildren - : Array.from({ length: +this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE) }); + : Array.from({ length: +(this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE) ?? 0) }); return listChildren.map((_, i) => `${SLOTNAME_PREFIX}-${i}`); } @@ -116,7 +116,7 @@ export abstract class NamedSlotListElement< * children should be marked as lists. */ protected roleOverride(): 'presentation' | typeof nothing { - return (this.listChildren.length || +this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE)) >= 2 + return (this.listChildren.length || +(this.getAttribute(SSR_CHILD_COUNT_ATTRIBUTE) ?? 0)) >= 2 ? nothing : 'presentation'; } diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index 22df63e91f..2aaf20f83d 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -23,17 +23,14 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement this._setMarkerPosition(), ); protected override willUpdate(changedProperties: PropertyValues>): void { + setAttribute(this, 'data-has-active-action', !!this._currentActiveAction); if (changedProperties.has('size') || changedProperties.has('listChildren')) { this._updateMarkerActions(); } @@ -45,7 +42,6 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement action.active); - this._hasActiveAction = !!this._currentActiveAction; this._setMarkerPosition(); } @@ -63,33 +59,29 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement this._setMarkerPosition()); } public reset(): void { - if (!this._hasActiveAction) { - return; - } if (this._currentActiveAction) { this._currentActiveAction.active = false; + this._currentActiveAction = undefined; } - this._hasActiveAction = false; } private _setMarkerPosition(): void { - if (this._hasActiveAction) { + if (this._currentActiveAction) { const index = this.listChildren.indexOf(this._currentActiveAction)!; this.style?.setProperty( '--sbb-navigation-marker-position-y', - `${this.shadowRoot.querySelector(`li:nth-child(${index + 1})`).offsetTop}px`, + `${ + this.shadowRoot!.querySelector(`li:nth-child(${index + 1})`)!.offsetTop + }px`, ); } } protected override render(): TemplateResult { - setAttribute(this, 'data-has-active-action', this._hasActiveAction); - return this.renderList(); } } diff --git a/src/components/skiplink-list/skiplink-list.ts b/src/components/skiplink-list/skiplink-list.ts index 52820837f0..73df58f992 100644 --- a/src/components/skiplink-list/skiplink-list.ts +++ b/src/components/skiplink-list/skiplink-list.ts @@ -1,5 +1,5 @@ import type { CSSResultGroup, TemplateResult } from 'lit'; -import { html } from 'lit'; +import { html, nothing } from 'lit'; import { customElement, property } from 'lit/decorators.js'; import { NamedSlotListElement, NamedSlotStateController } from '../core/common-behaviors'; diff --git a/src/components/train/train-formation/train-formation.ts b/src/components/train/train-formation/train-formation.ts index 52954e6227..8a1cb108c2 100644 --- a/src/components/train/train-formation/train-formation.ts +++ b/src/components/train/train-formation/train-formation.ts @@ -104,7 +104,7 @@ export class SbbTrainFormationElement extends NamedSlotListElement { + private async _updateFormationDiv(el: Element | undefined): Promise { if (!el) { return; } From c60dd1c56b6f419e525fd3ecdebc488a5d4aee6d Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Tue, 23 Jan 2024 14:20:37 +0100 Subject: [PATCH 07/10] refactor: review --- src/components/autocomplete/autocomplete.ts | 1 + .../breadcrumb-group.spec.snap.js | 6 +- .../breadcrumb-group/breadcrumb-group.e2e.ts | 4 +- .../breadcrumb-group/breadcrumb-group.spec.ts | 6 +- .../breadcrumb-group/breadcrumb-group.ts | 5 +- .../named-slot-list-element.ts | 9 +-- .../common-behaviors/slot-child-observer.ts | 4 +- .../__snapshots__/link-list.spec.snap.js | 72 +++++++++---------- src/components/link-list/link-list.ts | 1 + .../menu/menu/__snapshots__/menu.spec.snap.js | 8 +-- src/components/menu/menu/menu.spec.ts | 8 +-- .../navigation-list.spec.snap.js | 14 ++-- .../navigation-list/navigation-list.spec.ts | 10 +-- .../navigation-list/navigation-list.ts | 15 ++-- .../navigation-marker/navigation-marker.ts | 1 + src/components/option/optgroup/optgroup.ts | 1 + .../__snapshots__/skiplink-list.spec.snap.js | 18 ++--- .../skiplink-list/skiplink-list.spec.ts | 16 ++--- src/components/skiplink-list/skiplink-list.ts | 19 +++-- .../tag/tag-group/tag-group.spec.ts | 16 ++--- src/components/tag/tag-group/tag-group.ts | 1 + .../train-formation.spec.snap.js | 6 +- .../train-formation/train-formation.spec.ts | 6 +- .../train/train-formation/train-formation.ts | 1 + .../train/train-wagon/train-wagon.e2e.ts | 4 +- .../train/train-wagon/train-wagon.spec.ts | 12 ++-- src/components/train/train/train.ts | 1 + 27 files changed, 142 insertions(+), 123 deletions(-) diff --git a/src/components/autocomplete/autocomplete.ts b/src/components/autocomplete/autocomplete.ts index d8a18b3b24..29a48152f2 100644 --- a/src/components/autocomplete/autocomplete.ts +++ b/src/components/autocomplete/autocomplete.ts @@ -239,6 +239,7 @@ export class SbbAutocompleteElement extends SlotChildObserver(LitElement) { } protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); if (changedProperties.has('origin')) { this._resetOriginClickListener(this.origin, changedProperties.get('origin')); } diff --git a/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js b/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js index 233319709f..9892581766 100644 --- a/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js +++ b/src/components/breadcrumb/breadcrumb-group/__snapshots__/breadcrumb-group.spec.snap.js @@ -4,7 +4,7 @@ export const snapshots = {}; snapshots["sbb-breadcrumb-group renders"] = `
        1. - +
        2. - +
        3. - +
        diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts index 4926f67a00..2dc512f10c 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.e2e.ts @@ -91,8 +91,8 @@ describe('sbb-breadcrumb-group', () => { // only two slots are displayed, and the second is the last one const slots = breadcrumbGroup.shadowRoot!.querySelectorAll('li > slot'); expect(slots.length).to.be.equal(2); - expect(slots[0]).to.have.attribute('name', 'child-0'); - expect(slots[1]).to.have.attribute('name', 'child-6'); + expect(slots[0]).to.have.attribute('name', 'li-0'); + expect(slots[1]).to.have.attribute('name', 'li-6'); }); it('keyboard navigation with ellipsis', async () => { diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts index 0414d47cd9..d8d2b5c4f8 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.spec.ts @@ -19,11 +19,11 @@ describe('sbb-breadcrumb-group', () => { expect(root).dom.to.be.equal(` - - + + One - + Two diff --git a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts index 9ef71b86ec..392459b6ba 100644 --- a/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts +++ b/src/components/breadcrumb/breadcrumb-group/breadcrumb-group.ts @@ -67,6 +67,7 @@ export class SbbBreadcrumbGroupElement extends NamedSlotListElement>): void { + super.willUpdate(changedProperties); if (changedProperties.has('listChildren')) { this._syncBreadcrumbs(); } @@ -143,7 +144,7 @@ export class SbbBreadcrumbGroupElement extends NamedSlotListElement - +
      • - +
        • `; } diff --git a/src/components/core/common-behaviors/named-slot-list-element.ts b/src/components/core/common-behaviors/named-slot-list-element.ts index e729071f8a..f3fc3541fe 100644 --- a/src/components/core/common-behaviors/named-slot-list-element.ts +++ b/src/components/core/common-behaviors/named-slot-list-element.ts @@ -5,7 +5,7 @@ import { state } from 'lit/decorators.js'; import { SlotChildObserver } from './slot-child-observer'; const SSR_CHILD_COUNT_ATTRIBUTE = 'data-ssr-child-count'; -const SLOTNAME_PREFIX = 'child'; +const SLOTNAME_PREFIX = 'li'; /** * Helper type for willUpdate or similar checks. @@ -58,17 +58,12 @@ export abstract class NamedSlotListElement< .filter((c) => !listChildren.includes(c)) .forEach((c) => c.removeAttribute('slot')); this.listChildren = listChildren; - this.listChildren.forEach((child, index) => { - child.setAttribute('slot', `${SLOTNAME_PREFIX}-${index}`); - this.formatChild?.(child); - }); + this.listChildren.forEach((c, index) => c.setAttribute('slot', `${SLOTNAME_PREFIX}-${index}`)); // Remove the ssr attribute, once we have actually initialized the children elements. this.removeAttribute(SSR_CHILD_COUNT_ATTRIBUTE); } - protected formatChild?(child: C): void; - /** * Renders list and list slots for slotted children or an amount of list slots * corresponding to the `data-ssr-child-count` attribute value. diff --git a/src/components/core/common-behaviors/slot-child-observer.ts b/src/components/core/common-behaviors/slot-child-observer.ts index 98a21a8fd5..2cff8635c7 100644 --- a/src/components/core/common-behaviors/slot-child-observer.ts +++ b/src/components/core/common-behaviors/slot-child-observer.ts @@ -41,7 +41,7 @@ export const SlotChildObserver = >( protected override createRenderRoot(): HTMLElement | DocumentFragment { // Check whether hydration is needed by checking whether the shadow root // is available before createRenderRoot is called. - this._needsHydration = !!this.shadowRoot; + this._needsHydration = !!this.shadowRoot && 'litElementHydrateSupport' in globalThis; return super.createRenderRoot(); } @@ -49,7 +49,7 @@ export const SlotChildObserver = >( super.willUpdate(changedProperties); // If hydration is not needed we can immediately check for children // in the first update. - if (!this._needsHydration && !this.hasUpdated) { + if (!this.hasUpdated && !this._needsHydration) { this.checkChildren(); } } diff --git a/src/components/link-list/__snapshots__/link-list.spec.snap.js b/src/components/link-list/__snapshots__/link-list.spec.snap.js index 87e8bb1c12..4614aa59f2 100644 --- a/src/components/link-list/__snapshots__/link-list.spec.snap.js +++ b/src/components/link-list/__snapshots__/link-list.spec.snap.js @@ -16,15 +16,15 @@ snapshots["sbb-link-list should render named slots if data-ssr-child-count attri
        • - +
        • - +
        • - +
        @@ -38,7 +38,7 @@ snapshots["sbb-link-list should render named slots if data-ssr-child-count attri snapshots["sbb-link-list rendered with a slotted title in light DOM"] = ` @@ -64,7 +64,7 @@ snapshots["sbb-link-list rendered with a slotted title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-1" + slot="li-1" tabindex="0" variant="block" > @@ -76,7 +76,7 @@ snapshots["sbb-link-list rendered with a slotted title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-2" + slot="li-2" tabindex="0" variant="block" > @@ -88,7 +88,7 @@ snapshots["sbb-link-list rendered with a slotted title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-3" + slot="li-3" tabindex="0" variant="block" > @@ -100,7 +100,7 @@ snapshots["sbb-link-list rendered with a slotted title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-4" + slot="li-4" tabindex="0" variant="block" > @@ -128,23 +128,23 @@ snapshots["sbb-link-list rendered with a slotted title in shadow DOM"] = class="sbb-link-list" >
      • - +
      • - +
      • - +
      • - +
      • - +
        • @@ -158,7 +158,7 @@ snapshots["sbb-link-list rendered with a slotted title in shadow DOM"] = snapshots["sbb-link-list rendered with a title from properties in light DOM"] = ` @@ -182,7 +182,7 @@ snapshots["sbb-link-list rendered with a title from properties in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-1" + slot="li-1" tabindex="0" variant="block" > @@ -194,7 +194,7 @@ snapshots["sbb-link-list rendered with a title from properties in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-2" + slot="li-2" tabindex="0" variant="block" > @@ -206,7 +206,7 @@ snapshots["sbb-link-list rendered with a title from properties in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-3" + slot="li-3" tabindex="0" variant="block" > @@ -218,7 +218,7 @@ snapshots["sbb-link-list rendered with a title from properties in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-4" + slot="li-4" tabindex="0" variant="block" > @@ -247,23 +247,23 @@ snapshots["sbb-link-list rendered with a title from properties in shadow DOM"] = class="sbb-link-list" >
      • - +
      • - +
      • - +
      • - +
      • - +
        • @@ -277,7 +277,7 @@ snapshots["sbb-link-list rendered with a title from properties in shadow DOM"] = snapshots["sbb-link-list rendered without a title in light DOM"] = ` @@ -287,7 +287,7 @@ snapshots["sbb-link-list rendered without a title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-0" + slot="li-0" tabindex="0" variant="block" > @@ -299,7 +299,7 @@ snapshots["sbb-link-list rendered without a title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-1" + slot="li-1" tabindex="0" variant="block" > @@ -311,7 +311,7 @@ snapshots["sbb-link-list rendered without a title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-2" + slot="li-2" tabindex="0" variant="block" > @@ -323,7 +323,7 @@ snapshots["sbb-link-list rendered without a title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-3" + slot="li-3" tabindex="0" variant="block" > @@ -335,7 +335,7 @@ snapshots["sbb-link-list rendered without a title in light DOM"] = href="https://www.sbb.ch/de/hilfe-und-kontakt/erstattung-entschaedigung/rueckerstattung-von-billetten.html" role="link" size="s" - slot="child-4" + slot="li-4" tabindex="0" variant="block" > @@ -360,23 +360,23 @@ snapshots["sbb-link-list rendered without a title in shadow DOM"] =
        • - +
        • - +
        • - +
        • - +
        • - +
        diff --git a/src/components/link-list/link-list.ts b/src/components/link-list/link-list.ts index a3e02a8197..063ece694b 100644 --- a/src/components/link-list/link-list.ts +++ b/src/components/link-list/link-list.ts @@ -51,6 +51,7 @@ export class SbbLinkListElement extends NamedSlotListElement { private _namedSlots = new NamedSlotStateController(this); protected override willUpdate(changedProperties: PropertyValues>): void { + super.willUpdate(changedProperties); if ( changedProperties.has('size') || changedProperties.has('negative') || diff --git a/src/components/menu/menu/__snapshots__/menu.spec.snap.js b/src/components/menu/menu/__snapshots__/menu.spec.snap.js index 1fc19a9677..a469fdc83f 100644 --- a/src/components/menu/menu/__snapshots__/menu.spec.snap.js +++ b/src/components/menu/menu/__snapshots__/menu.spec.snap.js @@ -60,19 +60,19 @@ snapshots["sbb-menu renders with list"] =
        • - +
        • - +
        • - +
        • - +
        diff --git a/src/components/menu/menu/menu.spec.ts b/src/components/menu/menu/menu.spec.ts index 5b43c525c6..0bbb7786c3 100644 --- a/src/components/menu/menu/menu.spec.ts +++ b/src/components/menu/menu/menu.spec.ts @@ -51,16 +51,16 @@ describe('sbb-menu', () => { ` - + View - + Edit - + Details - + Cancel diff --git a/src/components/navigation/navigation-list/__snapshots__/navigation-list.spec.snap.js b/src/components/navigation/navigation-list/__snapshots__/navigation-list.spec.snap.js index 7f1ce82aaf..297d3f69f2 100644 --- a/src/components/navigation/navigation-list/__snapshots__/navigation-list.spec.snap.js +++ b/src/components/navigation/navigation-list/__snapshots__/navigation-list.spec.snap.js @@ -14,15 +14,15 @@ snapshots["sbb-navigation-list should render named slots if data-ssr-child-count class="sbb-navigation-list__content" >
      • - +
      • - +
      • - +
        • @@ -46,19 +46,19 @@ snapshots["sbb-navigation-list renders"] = class="sbb-navigation-list__content" >
      • - +
      • - +
      • - +
      • - +
        • diff --git a/src/components/navigation/navigation-list/navigation-list.spec.ts b/src/components/navigation/navigation-list/navigation-list.spec.ts index 418a1055c3..dc7e24e26a 100644 --- a/src/components/navigation/navigation-list/navigation-list.spec.ts +++ b/src/components/navigation/navigation-list/navigation-list.spec.ts @@ -15,17 +15,17 @@ describe('sbb-navigation-list', () => { expect(root).dom.to.be.equal( ` - - + + Tickets & Offers - + Vacations & Recreation - + Travel information - + Help & Contact diff --git a/src/components/navigation/navigation-list/navigation-list.ts b/src/components/navigation/navigation-list/navigation-list.ts index d592a530b8..b4614b4ceb 100644 --- a/src/components/navigation/navigation-list/navigation-list.ts +++ b/src/components/navigation/navigation-list/navigation-list.ts @@ -1,8 +1,12 @@ -import type { CSSResultGroup, TemplateResult } from 'lit'; +import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit'; import { html } from 'lit'; import { customElement, property } from 'lit/decorators.js'; -import { NamedSlotListElement, NamedSlotStateController } from '../../core/common-behaviors'; +import { + NamedSlotListElement, + NamedSlotStateController, + type WithListChildren, +} from '../../core/common-behaviors'; import type { SbbNavigationActionElement } from '../navigation-action'; import style from './navigation-list.scss?lit&inline'; @@ -28,8 +32,11 @@ export class SbbNavigationListElement extends NamedSlotListElement>): void { + super.willUpdate(changedProperties); + if (changedProperties.has('listChildren')) { + this.listChildren.forEach((c) => (c.size = 'm')); + } } protected override render(): TemplateResult { diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index 2aaf20f83d..559815279d 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -30,6 +30,7 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement>): void { + super.willUpdate(changedProperties); setAttribute(this, 'data-has-active-action', !!this._currentActiveAction); if (changedProperties.has('size') || changedProperties.has('listChildren')) { this._updateMarkerActions(); diff --git a/src/components/option/optgroup/optgroup.ts b/src/components/option/optgroup/optgroup.ts index ffd48d55e3..7cfa2f7ee3 100644 --- a/src/components/option/optgroup/optgroup.ts +++ b/src/components/option/optgroup/optgroup.ts @@ -64,6 +64,7 @@ export class SbbOptGroupElement extends SlotChildObserver(LitElement) { } protected override willUpdate(changedProperties: PropertyValues): void { + super.willUpdate(changedProperties); if (changedProperties.has('disabled')) { this._proxyDisabledToOptions(); } diff --git a/src/components/skiplink-list/__snapshots__/skiplink-list.spec.snap.js b/src/components/skiplink-list/__snapshots__/skiplink-list.spec.snap.js index d259bed3a6..fff23acc73 100644 --- a/src/components/skiplink-list/__snapshots__/skiplink-list.spec.snap.js +++ b/src/components/skiplink-list/__snapshots__/skiplink-list.spec.snap.js @@ -21,15 +21,15 @@ snapshots["sbb-skiplink-list should render named slots if data-ssr-child-count a class="sbb-skiplink-list" >
      • - +
      • - +
      • - +
        • @@ -61,15 +61,15 @@ snapshots["sbb-skiplink-list renders"] = class="sbb-skiplink-list" >
      • - +
      • - +
      • - +
        • @@ -102,15 +102,15 @@ snapshots["sbb-skiplink-list renders with title"] = class="sbb-skiplink-list" >
      • - +
      • - +
      • - +
        • diff --git a/src/components/skiplink-list/skiplink-list.spec.ts b/src/components/skiplink-list/skiplink-list.spec.ts index d92e45f264..8a813bcb06 100644 --- a/src/components/skiplink-list/skiplink-list.spec.ts +++ b/src/components/skiplink-list/skiplink-list.spec.ts @@ -16,10 +16,10 @@ describe('sbb-skiplink-list', () => { expect(root).dom.to.be.equal( ` - - Link 1 - Link 2 - Link 3 + + Link 1 + Link 2 + Link 3 `, ); @@ -37,10 +37,10 @@ describe('sbb-skiplink-list', () => { expect(root).dom.to.be.equal( ` - - Link 1 - Link 2 - Link 3 + + Link 1 + Link 2 + Link 3 `, ); diff --git a/src/components/skiplink-list/skiplink-list.ts b/src/components/skiplink-list/skiplink-list.ts index 73df58f992..9cd8f30928 100644 --- a/src/components/skiplink-list/skiplink-list.ts +++ b/src/components/skiplink-list/skiplink-list.ts @@ -1,8 +1,12 @@ -import type { CSSResultGroup, TemplateResult } from 'lit'; +import type { CSSResultGroup, PropertyValues, TemplateResult } from 'lit'; import { html, nothing } from 'lit'; import { customElement, property } from 'lit/decorators.js'; -import { NamedSlotListElement, NamedSlotStateController } from '../core/common-behaviors'; +import { + NamedSlotListElement, + NamedSlotStateController, + type WithListChildren, +} from '../core/common-behaviors'; import type { SbbLinkElement } from '../link'; import type { TitleLevel } from '../title'; @@ -31,9 +35,14 @@ export class SbbSkiplinkListElement extends NamedSlotListElement new NamedSlotStateController(this); } - protected override formatChild(child: SbbLinkElement): void { - child.size = 'm'; - child.negative = true; + protected override willUpdate(changedProperties: PropertyValues>): void { + super.willUpdate(changedProperties); + if (changedProperties.has('listChildren')) { + for (const child of this.listChildren) { + child.size = 'm'; + child.negative = true; + } + } } protected override render(): TemplateResult { diff --git a/src/components/tag/tag-group/tag-group.spec.ts b/src/components/tag/tag-group/tag-group.spec.ts index 5caf387e54..42171dba5b 100644 --- a/src/components/tag/tag-group/tag-group.spec.ts +++ b/src/components/tag/tag-group/tag-group.spec.ts @@ -16,14 +16,14 @@ describe('sbb-tag-group', () => { expect(root).dom.to.be.equal( ` - + First tag - + Second tag -
        - +
        + Third tag         @@ -34,16 +34,16 @@ describe('sbb-tag-group', () => {
        • - +
        • - +
        • - +
        • - +
        `; } diff --git a/src/components/menu/menu/menu.ts b/src/components/menu/menu/menu.ts index ff7f26b5cd..b7de228ff2 100644 --- a/src/components/menu/menu/menu.ts +++ b/src/components/menu/menu/menu.ts @@ -111,12 +111,6 @@ export class SbbMenuElement extends NamedSlotListElement { private _focusHandler = new FocusHandler(); private _scrollHandler = new ScrollHandler(); - public constructor() { - super(); - /** @ignore id is already a well known property. */ - this.id = this.id || `sbb-menu-${nextId++}`; - } - /** * Opens the menu on trigger click. */ @@ -253,6 +247,7 @@ export class SbbMenuElement extends NamedSlotListElement { return; } + this.id = this.id || `sbb-menu-${nextId++}`; setAriaOverlayTriggerAttributes(this._triggerElement, 'menu', this.id, this._state); this._menuController?.abort(); this._menuController = new AbortController(); diff --git a/src/components/navigation/navigation-marker/navigation-marker.ts b/src/components/navigation/navigation-marker/navigation-marker.ts index bc376fa4e5..e31711b61c 100644 --- a/src/components/navigation/navigation-marker/navigation-marker.ts +++ b/src/components/navigation/navigation-marker/navigation-marker.ts @@ -41,7 +41,9 @@ export class SbbNavigationMarkerElement extends NamedSlotListElement action.active); + this._currentActiveAction = this.listChildren.find( + (action) => action.active ?? action.getAttribute('active'), + ); this._setMarkerPosition(); } From 75d15212b56120246f89844c16c7cb9d50f7a121 Mon Sep 17 00:00:00 2001 From: Lukas Spirig Date: Thu, 25 Jan 2024 16:21:54 +0100 Subject: [PATCH 10/10] fix: react --- config/custom-elements-manifest.config.js | 23 +-------------- src/react/vite.config.ts | 36 +++++++++++++++-------- 2 files changed, 25 insertions(+), 34 deletions(-) diff --git a/config/custom-elements-manifest.config.js b/config/custom-elements-manifest.config.js index 665249747c..fb19700740 100644 --- a/config/custom-elements-manifest.config.js +++ b/config/custom-elements-manifest.config.js @@ -1,5 +1,3 @@ -import { parse } from 'comment-parser'; - /** * Docs: https://custom-elements-manifest.open-wc.org/analyzer/getting-started/ */ @@ -13,26 +11,7 @@ export default { plugins: [ { analyzePhase({ ts, node, moduleDoc }) { - if (ts.isPropertyDeclaration(node)) { - const className = node.parent.name.getText(); - const classDoc = moduleDoc.declarations.find( - (declaration) => declaration.name === className, - ); - - for (const jsDoc of node.jsDoc ?? []) { - for (const parsedJsDoc of parse(jsDoc.getFullText())) { - for (const tag of parsedJsDoc.tags) { - if (tag.tag === 'ssrchildcounter') { - const member = classDoc.members.find((m) => m.name === node.name.getText()); - member['_ssrchildcounter'] = true; - } - } - } - } - } else if ( - ts.isNewExpression(node) && - node.expression.getText() === 'NamedSlotStateController' - ) { + if (ts.isNewExpression(node) && node.expression.getText() === 'NamedSlotStateController') { let classNode = node; while (classNode) { if (ts.isClassDeclaration(classNode)) { diff --git a/src/react/vite.config.ts b/src/react/vite.config.ts index 282b31d201..e4c91a2208 100644 --- a/src/react/vite.config.ts +++ b/src/react/vite.config.ts @@ -89,7 +89,7 @@ function generateReactWrappers(): PluginOption { .filter((m) => m.kind === 'javascript-module') .reduce((current, next) => current.concat(next.declarations ?? []), [] as Declaration[]); for (const module of manifest.modules) { - for (const declaration of module.declarations.filter( + for (const declaration of module.declarations?.filter( (d): d is CustomElementDeclaration => 'customElement' in d && d.customElement, ) ?? []) { const targetPath = new URL(`./${module.path}/index.ts`, packageRoot); @@ -115,8 +115,8 @@ function generateReactWrappers(): PluginOption { } } - config.build.lib = { - ...(config.build.lib ? config.build.lib : {}), + config.build!.lib = { + ...(config.build!.lib ? config.build!.lib : {}), entry: globIndexMap(packageRoot), }; }, @@ -189,7 +189,7 @@ function findExtensionUsage( if (usesSsrSlotState(declaration, declarations)) { extensions.set('withSsrDataSlotNames', (v) => `withSsrDataSlotNames(${v})`); } - const childTypes = usesSsrSlotChildCounter(declaration); + const childTypes = namedSlotListElements(declaration); if (childTypes.length) { extensions.set( 'withSsrDataChildCount', @@ -199,32 +199,44 @@ function findExtensionUsage( return extensions; } +// eslint-disable-next-line @typescript-eslint/naming-convention +type ClassDeclarationSSR = ClassDeclaration & { _ssrslotstate?: boolean }; const ssrSlotStateKey = '_ssrslotstate'; -function usesSsrSlotState(declaration: ClassDeclaration, declarations: Declaration[]): boolean { +function usesSsrSlotState( + declaration: ClassDeclarationSSR | undefined, + declarations: Declaration[], +): boolean { while (declaration) { if ( declaration[ssrSlotStateKey] || declaration.mixins?.some((m) => - declarations.find((d) => d.name === m.name && d[ssrSlotStateKey]), + declarations.find((d) => d.name === m.name && (d as ClassDeclarationSSR)[ssrSlotStateKey]), ) ) { return true; } declaration = declarations.find( - (d): d is ClassDeclaration => d.name === declaration.superclass?.name, + (d): d is ClassDeclarationSSR => d.name === declaration!.superclass?.name, ); } return false; } -const ssrSlotChildCountKey = '_ssrchildcounter'; -function usesSsrSlotChildCounter(declaration: ClassDeclaration): string[] { +function namedSlotListElements(declaration: ClassDeclaration): string[] { return ( declaration.members - ?.find((m): m is ClassField => m[ssrSlotChildCountKey]) - ?.type?.text.replace(/[()[\] ]/g, '') - .split('|') ?? [] + ?.find( + (m): m is ClassField => + m.inheritedFrom?.name === 'NamedSlotListElement' && m.name === 'listChildTagNames', + ) + ?.default?.match(/([\w-]+)/g) + ?.map((m) => + m + .split('-') + .map((s) => s[0] + s.substring(1).toLowerCase()) + .join(''), + ) ?? [] ); }