refactor: extract base functionality of list components into `NamedSl…

…otListElement` (#2359) This PR refactors the list components, by extracting the common functionality into the `NamedSlotListElement` base class. It also refactors the `SlotChildObserver` to be mostly synchronous, instead of asynchronous.
sbb-design-systems · Jan 25, 2024 · 35a0173 · 35a0173
1 parent d427c50
commit 35a0173
Show file tree

Hide file tree

Showing 46 changed files with 813 additions and 805 deletions.
diff --git 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/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<this>): 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
@@ -4,7 +4,7 @@ export const snapshots = {};
 snapshots["sbb-breadcrumb-group renders"] = 
 `<ol class="sbb-breadcrumb-group">
   <li class="sbb-breadcrumb-group__item">
-    <slot name="breadcrumb-0">
+    <slot name="li-0">
     </slot>
     <sbb-icon
       aria-hidden="true"
@@ -16,7 +16,7 @@ snapshots["sbb-breadcrumb-group renders"] =
     </sbb-icon>
   </li>
   <li class="sbb-breadcrumb-group__item">
-    <slot name="breadcrumb-1">
+    <slot name="li-1">
     </slot>
     <sbb-icon
       aria-hidden="true"
@@ -28,7 +28,7 @@ snapshots["sbb-breadcrumb-group renders"] =
     </sbb-icon>
   </li>
   <li class="sbb-breadcrumb-group__item">
-    <slot name="breadcrumb-2">
+    <slot name="li-2">
     </slot>
   </li>
 </ol>

diff --git 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', '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
@@ -19,11 +19,11 @@ describe('sbb-breadcrumb-group', () => {
 
     expect(root).dom.to.be.equal(`
       <sbb-breadcrumb-group role='navigation' data-loaded>
-        <sbb-breadcrumb id="sbb-breadcrumb-0" href="/" icon-name="pie-small" slot="breadcrumb-0" dir="ltr" role="link" tabindex="0"></sbb-breadcrumb>
-        <sbb-breadcrumb id="sbb-breadcrumb-1" href="/one" slot="breadcrumb-1" dir="ltr" role="link" tabindex="0">
+        <sbb-breadcrumb href="/" icon-name="pie-small" slot="li-0" dir="ltr" role="link" tabindex="0"></sbb-breadcrumb>
+        <sbb-breadcrumb href="/one" slot="li-1" dir="ltr" role="link" tabindex="0">
           One
         </sbb-breadcrumb>
-        <sbb-breadcrumb id="sbb-breadcrumb-2" href="/one" slot="breadcrumb-2" aria-current="page" dir="ltr" role="link" tabindex="0">
+        <sbb-breadcrumb href="/one" slot="li-2" aria-current="page" dir="ltr" role="link" tabindex="0">
           Two
         </sbb-breadcrumb>
       </sbb-breadcrumb-group>

diff --git 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<SbbBreadcrumbElement> {
   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,41 @@ 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<WithListChildren<this>>): void {
+    super.willUpdate(changedProperties);
+    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<WithListChildren<this>>): 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 +105,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 +142,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`
       <li class="sbb-breadcrumb-group__item">
-        <slot name="breadcrumb-0"></slot>
+        <slot name="li-0"></slot>
       </li>
       <li class="sbb-breadcrumb-group__item" id="sbb-breadcrumb-group-ellipsis">
         <sbb-icon
@@ -186,42 +166,36 @@ export class SbbBreadcrumbGroupElement extends SlotChildObserver(LitElement) {
           name="chevron-small-right-small"
           class="sbb-breadcrumb-group__divider-icon"
         ></sbb-icon>
-        <slot name=${`breadcrumb-${this._breadcrumbs.length - 1}`}></slot>
+        <slot name=${`li-${this.listChildren.length - 1}`}></slot>
       </li>
     `;
   }
 
   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`
         <li class="sbb-breadcrumb-group__item">
-          <slot name="${slotName(index)}"></slot>
-          ${index !== this._breadcrumbs.length - 1
+          <slot name=${name}></slot>
+          ${index !== array.length - 1
             ? html`<sbb-icon
                 name="chevron-small-right-small"
                 class="sbb-breadcrumb-group__divider-icon"
               ></sbb-icon>`
             : nothing}
         </li>
-      `;
-    });
+      `,
+    );
   }
 
   protected override render(): TemplateResult {
     setAttribute(this, 'role', 'navigation');
     setAttribute(this, 'data-state', this._state);
 
     return html`
-      <ol class="sbb-breadcrumb-group">
+      <ol class="sbb-breadcrumb-group" role=${this.roleOverride()}>
         ${this._state === 'collapsed' ? this._renderCollapsed() : this._renderExpanded()}
       </ol>
-      <span hidden>
-        <slot></slot>
-      </span>
+      ${this.renderHiddenSlot()}
     `;
   }
 }

diff --git a/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts b/src/components/breadcrumb/breadcrumb/breadcrumb.spec.ts
@@ -1,17 +1,27 @@
 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`
-      <sbb-breadcrumb href="/test" target="_blank" download="true" rel="subsection"
+      <sbb-breadcrumb href="/test" target="_blank" download rel="subsection"
         >Breadcrumb</sbb-breadcrumb
       >
     `);
 
     expect(root).dom.to.be.equal(`
-      <sbb-breadcrumb dir="ltr" role="link" tabindex="0" href="/test" target="_blank" download="true" rel="subsection">
+      <sbb-breadcrumb
+        dir="ltr"
+        role="link"
+        tabindex="0"
+        href="/test"
+        target="_blank"
+        download
+        rel="subsection">
         Breadcrumb
       </sbb-breadcrumb>
     `);
@@ -24,8 +34,14 @@ describe('sbb-breadcrumb', () => {
       <sbb-breadcrumb href="/" icon-name="house-small"></sbb-breadcrumb>
     `);
 
+    await waitForLitRender(root);
     expect(root).dom.to.be.equal(`
-      <sbb-breadcrumb dir="ltr" role="link" tabindex="0" href="/" icon-name="house-small"></sbb-breadcrumb>
+      <sbb-breadcrumb
+        dir="ltr"
+        role="link"
+        tabindex="0"
+        href="/"
+        icon-name="house-small"></sbb-breadcrumb>
     `);
 
     await expect(root).shadowDom.to.equalSnapshot();
@@ -36,8 +52,14 @@ describe('sbb-breadcrumb', () => {
       <sbb-breadcrumb href="/" icon-name="house-small">Home</sbb-breadcrumb>
     `);
 
+    await waitForLitRender(root);
     expect(root).dom.to.be.equal(`
-      <sbb-breadcrumb dir="ltr" role="link" tabindex="0" href="/" icon-name="house-small">
+      <sbb-breadcrumb
+        dir="ltr"
+        role="link"
+        tabindex="0"
+        href="/"
+        icon-name="house-small">
         Home
       </sbb-breadcrumb>
     `);

diff --git a/src/components/breadcrumb/breadcrumb/breadcrumb.ts b/src/components/breadcrumb/breadcrumb/breadcrumb.ts
@@ -51,9 +51,6 @@ export class SbbBreadcrumbElement extends SlotChildObserver(LitElement) {
 
   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 +60,9 @@ export class SbbBreadcrumbElement extends SlotChildObserver(LitElement) {
   }
 
   protected override checkChildren(): void {
-    this._hasText = !!this.shadowRoot!.querySelector<HTMLSlotElement>('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
@@ -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';