From da22fdb7d0dfbc8e6bd330d81e2daf7076824799 Mon Sep 17 00:00:00 2001 From: Oleksii Kadurin Date: Mon, 25 Aug 2025 09:47:14 +0200 Subject: [PATCH] fix: allow the popup dialog to close when it is setup by `lit` `cache` (#2563) --- .changeset/six-ghosts-crash.md | 5 + .../dialog/test/lion-dialog.test.js | 115 +++++++++++++++++- .../components/overlays/src/OverlayMixin.js | 17 ++- .../test-suites/OverlayMixin.suite.js | 5 +- .../overlays/test/OverlayController.test.js | 2 + 5 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 .changeset/six-ghosts-crash.md diff --git a/.changeset/six-ghosts-crash.md b/.changeset/six-ghosts-crash.md new file mode 100644 index 000000000..a5f77b9d4 --- /dev/null +++ b/.changeset/six-ghosts-crash.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': patch +--- + +[overlays] allow the popup dialog to close when it is setup by `lit` `cache` diff --git a/packages/ui/components/dialog/test/lion-dialog.test.js b/packages/ui/components/dialog/test/lion-dialog.test.js index 2902ea7a4..b7a7258c4 100644 --- a/packages/ui/components/dialog/test/lion-dialog.test.js +++ b/packages/ui/components/dialog/test/lion-dialog.test.js @@ -1,9 +1,20 @@ /* eslint-disable lit-a11y/no-autofocus */ -import { expect, fixture as _fixture, html, unsafeStatic, aTimeout } from '@open-wc/testing'; +import { + expect, + fixture as _fixture, + html, + unsafeStatic, + aTimeout, + defineCE, + waitUntil, +} from '@open-wc/testing'; +import { cache } from 'lit/directives/cache.js'; +import { LitElement, nothing } from 'lit'; import { runOverlayMixinSuite } from '../../overlays/test-suites/OverlayMixin.suite.js'; import { isActiveElement } from '../../core/test-helpers/isActiveElement.js'; import '../test-helpers/test-router.js'; import '@lion/ui/define/lion-dialog.js'; +import '@lion/ui/define/lion-tabs.js'; /** * @typedef {import('../src/LionDialog.js').LionDialog} LionDialog @@ -296,5 +307,107 @@ describe('lion-dialog', () => { await dialogAfterRouteChange._overlayCtrl._showComplete; expect(dialogAfterRouteChange.opened).to.be.true; }); + + it('should close the popup dialog after rendered from cache', async () => { + /** + * + * @param {Event} e + * @returns + */ + const closeButtonHandler = e => + e.target?.dispatchEvent(new Event('close-overlay', { bubbles: true })); + const dialog = html` + +
+ Hello! You can close this dialog here: + +
+ `; + + /** + * Note, inactive tab content is **destroyed** on every tab switch. + */ + class Wrapper extends LitElement { + static properties = { + ...super.properties, + activeTabIndex: { type: Number }, + }; + + constructor() { + super(); + this.activeTabIndex = 0; + } + + /** + * @param {number} index + */ + changeActiveTabIndex(index) { + this.activeTabIndex = index; + } + + render() { + const changeActiveTabIndexRef = this.changeActiveTabIndex.bind(this); + return html` + + +

${cache(this.activeTabIndex === 0 ? dialog : nothing)}

+ +

Info page with lots of information about us.

+ + `; + } + } + + const wrapperFixture = /** @type {(arg: TemplateResult) => Promise} */ (_fixture); + const tagString = defineCE(Wrapper); + const wrapperTag = unsafeStatic(tagString); + const wrapperElement = /** @type {Wrapper} */ ( + await wrapperFixture(html`<${wrapperTag}>`) + ); + await wrapperElement.updateComplete; + const wrapperElementShadowRoot = wrapperElement.shadowRoot; + /** + * @returns { HTMLElement | null | undefined } + */ + const getFirstButton = () => wrapperElementShadowRoot?.querySelector('.first-button'); + /** + * @returns { HTMLElement | null | undefined } + */ + const getSecondButton = () => wrapperElementShadowRoot?.querySelector('.second-button'); + /** + * @returns { HTMLElement | null | undefined } + */ + const getInvokerButton = () => wrapperElementShadowRoot?.querySelector('.invoker-button'); + /** + * @returns { HTMLElement | null | undefined } + */ + const getCloseButton = () => wrapperElementShadowRoot?.querySelector('.close-button'); + /** + * @returns { Element | null | undefined } + */ + const getDialog = () => + wrapperElementShadowRoot?.querySelector('lion-dialog')?.shadowRoot?.querySelector('dialog'); + // @ts-ignore + const isDialogVisible = () => getDialog()?.checkVisibility() === true; + const isDialogRendered = () => + !!wrapperElement.shadowRoot?.querySelector('lion-dialog')?.shadowRoot?.childNodes.length; + getInvokerButton()?.click(); + await waitUntil(isDialogVisible); + getCloseButton()?.click(); + await waitUntil(() => !isDialogVisible()); + getSecondButton()?.click(); + await waitUntil(() => !isDialogRendered()); + getFirstButton()?.click(); + await waitUntil(isDialogRendered); + getInvokerButton()?.click(); + await waitUntil(isDialogVisible); + getCloseButton()?.click(); + await waitUntil(() => !isDialogVisible()); + expect(isDialogVisible()).to.equal(false); + }); }); }); diff --git a/packages/ui/components/overlays/src/OverlayMixin.js b/packages/ui/components/overlays/src/OverlayMixin.js index 116b09e84..4b29f0cfe 100644 --- a/packages/ui/components/overlays/src/OverlayMixin.js +++ b/packages/ui/components/overlays/src/OverlayMixin.js @@ -246,16 +246,23 @@ export const OverlayMixinImplementation = superclass => /** @protected */ _setupOverlayCtrl() { - if (this._overlayCtrl) return; - - /** @type {OverlayController} */ - this._overlayCtrl = this._defineOverlay({ + if (this.#hasSetup) return; + const config = { contentNode: this._overlayContentNode, contentWrapperNode: this._overlayContentWrapperNode, invokerNode: this._overlayInvokerNode, referenceNode: this._overlayReferenceNode, backdropNode: this._overlayBackdropNode, - }); + }; + + if (this._overlayCtrl) { + // when `lit` `cache` attaches node to the DOM, register the controller back in the OverlaysManager + this._overlayCtrl.updateConfig(config); + } else { + /** @type {OverlayController} */ + this._overlayCtrl = this._defineOverlay(config); + } + this.__syncToOverlayController(); this.__setupSyncFromOverlayController(); this._setupOpenCloseListeners(); diff --git a/packages/ui/components/overlays/test-suites/OverlayMixin.suite.js b/packages/ui/components/overlays/test-suites/OverlayMixin.suite.js index c0a4e96de..983f55cb7 100644 --- a/packages/ui/components/overlays/test-suites/OverlayMixin.suite.js +++ b/packages/ui/components/overlays/test-suites/OverlayMixin.suite.js @@ -533,8 +533,11 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) { el.switched = false; await el.updateComplete; - expect(setupSpy.callCount).to.equal(1); + + const elCtrl = /** @type {OverlayEl} */ (el.overlayEl)?._overlayCtrl; + const isCtrlRegisteredAtOverlaysManager = elCtrl.manager.list.some(ctrl => elCtrl === ctrl); + expect(isCtrlRegisteredAtOverlaysManager).to.equal(true); }); it('correctly removes event listeners when disconnected from dom', async () => { diff --git a/packages/ui/components/overlays/test/OverlayController.test.js b/packages/ui/components/overlays/test/OverlayController.test.js index ff515f380..16d4c1215 100644 --- a/packages/ui/components/overlays/test/OverlayController.test.js +++ b/packages/ui/components/overlays/test/OverlayController.test.js @@ -153,6 +153,8 @@ async function createNestedEscControllers(parentContent) { afterEach(() => { overlays.teardown(); + // clean document.body from the DOM nodes left by previous tests + document.body.innerHTML = ''; }); describe('OverlayController', () => {