refactor: review

sbb-design-systems · Jan 23, 2024 · c60dd1c · c60dd1c
1 parent 3233f94
commit c60dd1c
Show file tree

Hide file tree

Showing 27 changed files with 142 additions and 123 deletions.
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="child-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="child-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="child-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', '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
@@ -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-1" href="/" icon-name="pie-small" slot="child-0" dir="ltr" role="link" tabindex="0"></sbb-breadcrumb>
-        <sbb-breadcrumb id="sbb-breadcrumb-2" href="/one" slot="child-1" dir="ltr" role="link" tabindex="0">
+        <sbb-breadcrumb id="sbb-breadcrumb-1" href="/" icon-name="pie-small" slot="li-0" dir="ltr" role="link" tabindex="0"></sbb-breadcrumb>
+        <sbb-breadcrumb id="sbb-breadcrumb-2" href="/one" slot="li-1" dir="ltr" role="link" tabindex="0">
           One
         </sbb-breadcrumb>
-        <sbb-breadcrumb id="sbb-breadcrumb-3" href="/one" slot="child-2" aria-current="page" dir="ltr" role="link" tabindex="0">
+        <sbb-breadcrumb id="sbb-breadcrumb-3" 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
@@ -67,6 +67,7 @@ export class SbbBreadcrumbGroupElement extends NamedSlotListElement<SbbBreadcrum
   }
 
   protected override willUpdate(changedProperties: PropertyValueMap<WithListChildren<this>>): void {
+    super.willUpdate(changedProperties);
     if (changedProperties.has('listChildren')) {
       this._syncBreadcrumbs();
     }
@@ -143,7 +144,7 @@ export class SbbBreadcrumbGroupElement extends NamedSlotListElement<SbbBreadcrum
   private _renderCollapsed(): TemplateResult {
     return html`
       <li class="sbb-breadcrumb-group__item">
-        <slot name="child-0"></slot>
+        <slot name="li-0"></slot>
       </li>
       <li class="sbb-breadcrumb-group__item" id="sbb-breadcrumb-group-ellipsis">
         <sbb-icon
@@ -165,7 +166,7 @@ export class SbbBreadcrumbGroupElement extends NamedSlotListElement<SbbBreadcrum
           name="chevron-small-right-small"
           class="sbb-breadcrumb-group__divider-icon"
         ></sbb-icon>
-        <slot name=${`child-${this.listChildren.length - 1}`}></slot>
+        <slot name=${`li-${this.listChildren.length - 1}`}></slot>
       </li>
     `;
   }

diff --git 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
@@ -41,15 +41,15 @@ export const SlotChildObserver = <T extends Constructor<LitElement>>(
     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();
     }
 
     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) {
+      if (!this.hasUpdated && !this._needsHydration) {
         this.checkChildren();
       }
     }