feat(ui): [SlotMixin] allow to (re)render scoped elements as direct light dom child

.changeset/smooth-suits-compare.md

@@ -0,0 +1,5 @@
+---
+'@lion/ui': patch
+---
+
+[core/SlotMixin] allow to (re)render scoped elements as direct light dom child

packages/ui/components/core/src/SlotMixin.js

@@ -1,18 +1,37 @@
 /* eslint-disable class-methods-use-this */
+import { isTemplateResult } from 'lit/directive-helpers.js';
 import { dedupeMixin } from '@open-wc/dedupe-mixin';
 import { render } from 'lit';
-import { isTemplateResult } from 'lit/directive-helpers.js';
 
 /**
- * @typedef {import('../types/SlotMixinTypes.js').SlotMixin} SlotMixin
+ * @typedef {{renderBefore:Comment; renderTargetThatRespectsShadowRootScoping: HTMLDivElement}} RenderMetaObj
- * @typedef {import('../types/SlotMixinTypes.js').SlotsMap} SlotsMap
  * @typedef {import('../types/SlotMixinTypes.js').SlotFunctionResult} SlotFunctionResult
  * @typedef {import('../types/SlotMixinTypes.js').SlotRerenderObject} SlotRerenderObject
+ * @typedef {import('../types/SlotMixinTypes.js').SlotMixin} SlotMixin
+ * @typedef {import('../types/SlotMixinTypes.js').SlotsMap} SlotsMap
  * @typedef {import('lit').LitElement} LitElement
  */
 
-const isRerenderConfig = (/** @type {SlotFunctionResult} */ o) =>
+/**
-  !Array.isArray(o) && typeof o === 'object' && 'template' in o;
+ * @param {SlotFunctionResult} slotFunctionResult
+ * @returns {'template-result'|'node'|'slot-rerender-object'|null}
+ */
+function determineSlotFunctionResultType(slotFunctionResult) {
+  if (slotFunctionResult instanceof Node) {
+    return 'node';
+  }
+  if (isTemplateResult(slotFunctionResult)) {
+    return 'template-result';
+  }
+  if (
+    !Array.isArray(slotFunctionResult) &&
+    typeof slotFunctionResult === 'object' &&
+    'template' in slotFunctionResult
+  ) {
+    return 'slot-rerender-object';
+  }
+  return null;
+}
 
 /**
  * All intricacies involved in managing light dom can be delegated to SlotMixin. Amongst others, it automatically:
@@ -45,8 +64,8 @@ const SlotMixinImplementation = superclass =>
        * so that all interactions work as intended and no focus issues can arise (which would be the case
        * when (cloned) nodes of a render outcome would be moved around)
        * @private
-       * @type { Map<string, HTMLDivElement> } */
+       * @type { Map<string, RenderMetaObj> } */
-      this.__scopedRenderRoots = new Map();
+      this.__renderMetaPerSlot = new Map();
       /**
        * @private
        * @type {Set<string>}
@@ -79,9 +98,9 @@ const SlotMixinImplementation = superclass =>
     __rerenderSlot(slotName) {
       const slotFunctionResult = /** @type {SlotRerenderObject} */ (this.slots[slotName]());
       this.__renderTemplateInScopedContext({
+        renderAsDirectHostChild: slotFunctionResult.renderAsDirectHostChild,
         template: slotFunctionResult.template,
         slotName,
-        shouldRerender: true,
       });
       // TODO: this is deprecated, remove later
       slotFunctionResult.afterRender?.();
@@ -94,14 +113,8 @@ const SlotMixinImplementation = superclass =>
     update(changedProperties) {
       super.update(changedProperties);
 
-      if (this.__slotsThatNeedRerender.size) {
+      for (const slotName of this.__slotsThatNeedRerender) {
-        for (const slotName of Array.from(this.__slotsThatNeedRerender)) {
+        this.__rerenderSlot(slotName);
-          this.__rerenderSlot(slotName);
-        }
-      }
-
-      if (this.__isFirstSlotUpdate) {
-        this.__isFirstSlotUpdate = false;
       }
     }
 
@@ -110,32 +123,59 @@ const SlotMixinImplementation = superclass =>
      * @param {object} opts
      * @param {import('lit').TemplateResult} opts.template
      * @param {string} opts.slotName
-     * @param {boolean} [opts.shouldRerender] false when TemplateResult, true when SlotRerenderObject
+     * @param {boolean} [opts.renderAsDirectHostChild] when false, the render parent (wrapper div) will be kept in the light dom
+     * @returns {void}
      */
-    __renderTemplateInScopedContext({ template, slotName, shouldRerender }) {
+    __renderTemplateInScopedContext({ template, slotName, renderAsDirectHostChild }) {
-      // @ts-expect-error wait for browser support
+      const isFirstRender = !this.__renderMetaPerSlot.has(slotName);
-      const supportsScopedRegistry = !!ShadowRoot.prototype.createElement;
+      if (isFirstRender) {
-      const registryRoot = supportsScopedRegistry ? this.shadowRoot : document;
-
-      let renderTarget;
-      // Reuse the existing offline renderTargets for results consistent with that of rendering to one target (shadow dom)
-      if (this.__scopedRenderRoots.has(slotName)) {
-        renderTarget = this.__scopedRenderRoots.get(slotName);
-      } else {
         // @ts-expect-error wait for browser support
-        renderTarget = registryRoot.createElement('div');
+        const supportsScopedRegistry = !!ShadowRoot.prototype.createElement;
-        if (shouldRerender) {
+        const registryRoot = supportsScopedRegistry ? this.shadowRoot || document : document;
-          renderTarget.slot = slotName;
+
-          this.appendChild(renderTarget);
+        // @ts-expect-error wait for browser support
+        const renderTargetThatRespectsShadowRootScoping = registryRoot.createElement('div');
+        const startComment = document.createComment(`_start_slot_${slotName}_`);
+        const endComment = document.createComment(`_end_slot_${slotName}_`);
+
+        renderTargetThatRespectsShadowRootScoping.appendChild(startComment);
+        renderTargetThatRespectsShadowRootScoping.appendChild(endComment);
+
+        // Providing all options breaks Safari; keep host and creationScope
+        const { creationScope, host } = this.renderOptions;
+        render(template, renderTargetThatRespectsShadowRootScoping, {
+          renderBefore: endComment,
+          creationScope,
+          host,
+        });
+
+        if (renderAsDirectHostChild) {
+          const nodes = Array.from(renderTargetThatRespectsShadowRootScoping.childNodes);
+          this.__appendNodes({ nodes, renderParent: this, slotName });
+        } else {
+          renderTargetThatRespectsShadowRootScoping.slot = slotName;
+          this.appendChild(renderTargetThatRespectsShadowRootScoping);
         }
-        this.__scopedRenderRoots.set(slotName, renderTarget);
+
+        this.__renderMetaPerSlot.set(slotName, {
+          renderTargetThatRespectsShadowRootScoping,
+          renderBefore: endComment,
+        });
+
+        return;
       }
 
-      // Providing all options breaks Safari; keep host and creationScope
+      // Rerender
-      const { creationScope, host } = this.renderOptions;
+      const { renderBefore, renderTargetThatRespectsShadowRootScoping } =
-      render(template, renderTarget, { creationScope, host });
+        /** @type {RenderMetaObj} */ (this.__renderMetaPerSlot.get(slotName));
 
-      return renderTarget;
+      const rerenderTarget = renderAsDirectHostChild
+        ? this
+        : renderTargetThatRespectsShadowRootScoping;
+
+      // Providing all options breaks Safari: we keep host and creationScope
+      const { creationScope, host } = this.renderOptions;
+      render(template, rerenderTarget, { creationScope, host, renderBefore });
     }
 
     /**
@@ -145,13 +185,8 @@ const SlotMixinImplementation = superclass =>
      * which can be used as a render target for most
      * @param {string} options.slotName For the first render, it's best to use slotName
      */
-    __appendNodesForOneTimeRender({ nodes, renderParent = this, slotName }) {
+    __appendNodes({ nodes, renderParent = this, slotName }) {
       for (const node of nodes) {
-        if (!(node instanceof Node)) {
-          return;
-        }
-        // Here, we add the slot name to the node that is an element
-        // (ignoring helper comment nodes that might be in our nodes array)
         if (node instanceof Element && slotName && slotName !== '') {
           node.setAttribute('slot', slotName);
         }
@@ -182,29 +217,36 @@ const SlotMixinImplementation = superclass =>
           this.__privateSlots.add(slotName);
         }
 
-        if (isTemplateResult(slotFunctionResult)) {
+        const slotFunctionResultType = determineSlotFunctionResultType(slotFunctionResult);
-          const renderTarget = this.__renderTemplateInScopedContext({
-            template: slotFunctionResult,
-            slotName,
-          });
-          const nodes = Array.from(renderTarget.childNodes);
-          this.__appendNodesForOneTimeRender({ nodes, renderParent: this, slotName });
-        } else if (slotFunctionResult instanceof Node) {
-          const nodes = [/** @type {Node} */ (slotFunctionResult)];
-          this.__appendNodesForOneTimeRender({ nodes, renderParent: this, slotName });
-        } else if (isRerenderConfig(slotFunctionResult)) {
-          // Rerenderable slots are scheduled in the "updated loop"
-          this.__slotsThatNeedRerender.add(slotName);
 
-          // For backw. compat, we allow a first render on connectedCallback
+        switch (slotFunctionResultType) {
-          if (slotFunctionResult.firstRenderOnConnected) {
+          case 'template-result':
-            this.__rerenderSlot(slotName);
+            this.__renderTemplateInScopedContext({
-          }
+              template: /** @type {import('lit').TemplateResult} */ (slotFunctionResult),
-        } else {
+              renderAsDirectHostChild: true,
-          throw new Error(
+              slotName,
-            `Slot "${slotName}" configured inside "get slots()" (in prototype) of ${this.constructor.name} may return these types: TemplateResult | Node | {template:TemplateResult, afterRender?:function} | undefined.
+            });
-            You provided: ${slotFunctionResult}`,
+            break;
-          );
+          case 'node':
+            this.__appendNodes({
+              nodes: [/** @type {Node} */ (slotFunctionResult)],
+              renderParent: this,
+              slotName,
+            });
+            break;
+          case 'slot-rerender-object':
+            // Rerenderable slots are scheduled in the "update loop"
+            this.__slotsThatNeedRerender.add(slotName);
+            // For backw. compat, we allow a first render on connectedCallback
+            if (/** @type {SlotRerenderObject} */ (slotFunctionResult).firstRenderOnConnected) {
+              this.__rerenderSlot(slotName);
+            }
+            break;
+          default:
+            throw new Error(
+              `Slot "${slotName}" configured inside "get slots()" (in prototype) of ${this.constructor.name} may return these types: TemplateResult | Node | {template:TemplateResult, afterRender?:function} | undefined.
+              You provided: ${slotFunctionResult}`,
+            );
         }
       }
     }

packages/ui/components/core/test/SlotMixin.test.js

@@ -208,6 +208,7 @@ describe('SlotMixin', () => {
               ...super.slots,
               'focusable-node': () => ({
                 template: html`<input /> `,
+                renderAsDirectHostChild: false,
               }),
             };
           }
@@ -225,7 +226,6 @@ describe('SlotMixin', () => {
         },
       );
       const el = /** @type {* & SlotHost} */ (await fixture(`<${tag}></${tag}>`));
-
       el._focusableNode.focus();
       expect(document.activeElement).to.equal(el._focusableNode);
 
@@ -298,6 +298,69 @@ describe('SlotMixin', () => {
       expect(el._focusableNode.shadowRoot.activeElement).to.equal(el._focusableNode._buttonNode);
     });
 
+    it('allows for rerendering complex shadow root into slot as a direct child', async () => {
+      const complexSlotTagName = defineCE(
+        class extends LitElement {
+          render() {
+            return html`
+              <input />
+              <button>I will be focused</button>
+            `;
+          }
+
+          get _buttonNode() {
+            // @ts-expect-error
+            return this.shadowRoot.querySelector('button');
+          }
+        },
+      );
+
+      const complexSlotTag = unsafeStatic(complexSlotTagName);
+
+      const tagName = defineCE(
+        // @ts-expect-error
+        class extends SlotMixin(LitElement) {
+          static properties = { currentValue: Number };
+
+          constructor() {
+            super();
+            this.currentValue = 0;
+          }
+
+          get slots() {
+            return {
+              ...super.slots,
+              'focusable-node': () => ({
+                template: html`<${complexSlotTag}> </${complexSlotTag}> `,
+                renderAsDirectHostChild: true,
+              }),
+            };
+          }
+
+          render() {
+            return html`<slot name="focusable-node"></slot>`;
+          }
+
+          get _focusableNode() {
+            return /** @type HTMLSpanElement */ (
+              Array.from(this.children).find(elm => elm.slot === 'focusable-node')
+            );
+          }
+        },
+      );
+      const el = /** @type {* & SlotHost} */ (await fixture(`<${tagName}></${tagName}>`));
+
+      el._focusableNode._buttonNode.focus();
+
+      expect(el._focusableNode.shadowRoot.activeElement).to.equal(el._focusableNode._buttonNode);
+
+      el.currentValue = 1;
+      await el.updateComplete;
+
+      expect(document.activeElement).to.equal(el._focusableNode);
+      expect(el._focusableNode.shadowRoot.activeElement).to.equal(el._focusableNode._buttonNode);
+    });
+
     describe('firstRenderOnConnected (for backwards compatibility)', () => {
       it('does render on connected when firstRenderOnConnected:true', async () => {
         // Start with elem that does not render on connectedCallback

packages/ui/components/core/types/SlotMixinTypes.ts

@@ -18,6 +18,14 @@ export type SlotRerenderObject = {
    * For new components, please align with ReactiveElement/LitElement reactive cycle callbacks.
    */
   firstRenderOnConnected?: boolean;
+  /**
+   * This is recommended to set to true always, as its behavior is usually desired and more in line with slot nodes that
+   * are not configured as rerenderable.
+   * For backward compatibility, it is set to false by default.
+   * When not configured, content is wrapped in a div (this can be problematic for ::slotted css selectors and for
+   * querySelectors that expect [slot=x] to have some semantic or (presentational) value).
+   */
+  renderAsDirectHostChild?: boolean;
 };
 
 export type SlotFunctionResult = TemplateResult | Element | SlotRerenderObject | undefined;

packages/ui/components/input-file/src/LionInputFile.js

@@ -84,6 +84,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))
             .multiple=${this.multiple}
           ></lion-selected-file-list>
         `,
+        renderAsDirectHostChild: true,
       }),
     };
   }
@@ -183,7 +184,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField))
    */
   get _fileListNode() {
     return /** @type {LionSelectedFileList} */ (
-      Array.from(this.children).find(child => child.slot === 'selected-file-list')?.children[0]
+      Array.from(this.children).find(child => child.slot === 'selected-file-list')
     );
   }
 

packages/ui/components/input-tel-dropdown/src/LionInputTelDropdown.js

@@ -180,6 +180,7 @@ export class LionInputTelDropdown extends LionInputTel {
 
         return {
           template: templates.dropdown(this._templateDataDropdown),
+          renderAsDirectHostChild: Boolean,
         };
       },
     };

packages/ui/components/input-tel-dropdown/test-suites/LionInputTelDropdown.suite.js

@@ -215,7 +215,7 @@ export function runInputTelDropdownSuite({ klass } = { klass: LionInputTelDropdo
       it('renders to prefix slot in light dom', async () => {
         const el = await fixture(html` <${tag} .allowedRegions="${['DE']}"></${tag}> `);
         const prefixSlot = /** @type {HTMLElement} */ (
-          /** @type {HTMLElement} */ (el.refs.dropdown.value).parentElement
+          /** @type {HTMLElement} */ (el.refs.dropdown.value)
         );
         expect(prefixSlot.getAttribute('slot')).to.equal('prefix');
         expect(prefixSlot.slot).to.equal('prefix');