From f0333bbc1cc049a07f14964bf723c5c9927e2c18 Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Fri, 29 Mar 2024 00:52:50 +0100 Subject: [PATCH] feat(ui): [SlotMixin] allow to (re)render scoped elements as direct light dom child --- .changeset/smooth-suits-compare.md | 5 + packages/ui/components/core/src/SlotMixin.js | 170 +++++++++++------- .../ui/components/core/test/SlotMixin.test.js | 65 ++++++- .../components/core/types/SlotMixinTypes.ts | 8 + .../input-file/src/LionInputFile.js | 3 +- .../src/LionInputTelDropdown.js | 1 + .../test-suites/LionInputTelDropdown.suite.js | 2 +- 7 files changed, 187 insertions(+), 67 deletions(-) create mode 100644 .changeset/smooth-suits-compare.md diff --git a/.changeset/smooth-suits-compare.md b/.changeset/smooth-suits-compare.md new file mode 100644 index 000000000..2f6961401 --- /dev/null +++ b/.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 diff --git a/packages/ui/components/core/src/SlotMixin.js b/packages/ui/components/core/src/SlotMixin.js index 73a5636cb..1442f20b2 100644 --- a/packages/ui/components/core/src/SlotMixin.js +++ b/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 {import('../types/SlotMixinTypes.js').SlotsMap} SlotsMap + * @typedef {{renderBefore:Comment; renderTargetThatRespectsShadowRootScoping: HTMLDivElement}} RenderMetaObj * @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 } */ - this.__scopedRenderRoots = new Map(); + * @type { Map } */ + this.__renderMetaPerSlot = new Map(); /** * @private * @type {Set} @@ -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 Array.from(this.__slotsThatNeedRerender)) { - this.__rerenderSlot(slotName); - } - } - - if (this.__isFirstSlotUpdate) { - this.__isFirstSlotUpdate = false; + for (const slotName of this.__slotsThatNeedRerender) { + this.__rerenderSlot(slotName); } } @@ -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 }) { - // @ts-expect-error wait for browser support - const supportsScopedRegistry = !!ShadowRoot.prototype.createElement; - 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 { + __renderTemplateInScopedContext({ template, slotName, renderAsDirectHostChild }) { + const isFirstRender = !this.__renderMetaPerSlot.has(slotName); + if (isFirstRender) { // @ts-expect-error wait for browser support - renderTarget = registryRoot.createElement('div'); - if (shouldRerender) { - renderTarget.slot = slotName; - this.appendChild(renderTarget); + const supportsScopedRegistry = !!ShadowRoot.prototype.createElement; + const registryRoot = supportsScopedRegistry ? this.shadowRoot || document : document; + + // @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 - const { creationScope, host } = this.renderOptions; - render(template, renderTarget, { creationScope, host }); + // Rerender + const { renderBefore, renderTargetThatRespectsShadowRootScoping } = + /** @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 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); + const slotFunctionResultType = determineSlotFunctionResultType(slotFunctionResult); - // For backw. compat, we allow a first render on connectedCallback - if (slotFunctionResult.firstRenderOnConnected) { - this.__rerenderSlot(slotName); - } - } else { - 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}`, - ); + switch (slotFunctionResultType) { + case 'template-result': + this.__renderTemplateInScopedContext({ + template: /** @type {import('lit').TemplateResult} */ (slotFunctionResult), + renderAsDirectHostChild: true, + slotName, + }); + 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}`, + ); } } } diff --git a/packages/ui/components/core/test/SlotMixin.test.js b/packages/ui/components/core/test/SlotMixin.test.js index 6e968d7f8..413dbf77c 100644 --- a/packages/ui/components/core/test/SlotMixin.test.js +++ b/packages/ui/components/core/test/SlotMixin.test.js @@ -208,6 +208,7 @@ describe('SlotMixin', () => { ...super.slots, 'focusable-node': () => ({ template: html` `, + renderAsDirectHostChild: false, }), }; } @@ -225,7 +226,6 @@ describe('SlotMixin', () => { }, ); const el = /** @type {* & SlotHost} */ (await fixture(`<${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` + + + `; + } + + 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}> `, + renderAsDirectHostChild: true, + }), + }; + } + + render() { + return html``; + } + + get _focusableNode() { + return /** @type HTMLSpanElement */ ( + Array.from(this.children).find(elm => elm.slot === 'focusable-node') + ); + } + }, + ); + const el = /** @type {* & SlotHost} */ (await fixture(`<${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 diff --git a/packages/ui/components/core/types/SlotMixinTypes.ts b/packages/ui/components/core/types/SlotMixinTypes.ts index fce8bf80c..f01494831 100644 --- a/packages/ui/components/core/types/SlotMixinTypes.ts +++ b/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; diff --git a/packages/ui/components/input-file/src/LionInputFile.js b/packages/ui/components/input-file/src/LionInputFile.js index 5c5337963..894ab36bf 100644 --- a/packages/ui/components/input-file/src/LionInputFile.js +++ b/packages/ui/components/input-file/src/LionInputFile.js @@ -84,6 +84,7 @@ export class LionInputFile extends ScopedElementsMixin(LocalizeMixin(LionField)) .multiple=${this.multiple} > `, + 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') ); } diff --git a/packages/ui/components/input-tel-dropdown/src/LionInputTelDropdown.js b/packages/ui/components/input-tel-dropdown/src/LionInputTelDropdown.js index 328264334..357f137f9 100644 --- a/packages/ui/components/input-tel-dropdown/src/LionInputTelDropdown.js +++ b/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, }; }, }; diff --git a/packages/ui/components/input-tel-dropdown/test-suites/LionInputTelDropdown.suite.js b/packages/ui/components/input-tel-dropdown/test-suites/LionInputTelDropdown.suite.js index 5523b5dbd..4e79ae657 100644 --- a/packages/ui/components/input-tel-dropdown/test-suites/LionInputTelDropdown.suite.js +++ b/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']}"> `); 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');