diff --git a/packages/dialog/test/lion-dialog.test.js b/packages/dialog/test/lion-dialog.test.js index 296ddb16b..dc9395124 100644 --- a/packages/dialog/test/lion-dialog.test.js +++ b/packages/dialog/test/lion-dialog.test.js @@ -30,7 +30,7 @@ describe('lion-dialog', () => { expect(el.opened).to.be.true; }); - it.skip('supports nested overlays', async () => { + it('supports nested overlays', async () => { const el = await fixture(html`
diff --git a/packages/overlays/src/OverlayMixin.js b/packages/overlays/src/OverlayMixin.js index fdce45c36..4bbfa5149 100644 --- a/packages/overlays/src/OverlayMixin.js +++ b/packages/overlays/src/OverlayMixin.js @@ -3,7 +3,7 @@ import { OverlayController } from './OverlayController.js'; /** * @type {Function()} - * @polymerMixin + * @polymerMixinOverlayMixin * @mixinFunction */ export const OverlayMixin = dedupeMixin( @@ -133,15 +133,24 @@ export const OverlayMixin = dedupeMixin( } } - connectedCallback() { + async connectedCallback() { if (super.connectedCallback) { super.connectedCallback(); } - this._overlaySetupComplete = new Promise(resolve => { - this.__overlaySetupCompleteResolve = resolve; + + // Wait for DOM to be ready before setting up the overlay, else extensions like rich select breaks + this.updateComplete.then(() => { + if (!this.__isOverlaySetup) { + this._setupOverlayCtrl(); + } }); - // Wait for DOM to be ready before setting up the overlay - this.updateComplete.then(() => this._setupOverlayCtrl()); + + // When dom nodes are being moved around (meaning connected/disconnected are being fired + // repeatedly), we need to delay the teardown until we find a 'permanent disconnect' + if (this.__rejectOverlayDisconnectComplete) { + // makes sure _overlayDisconnectComplete never resolves: we don't want a teardown + this.__rejectOverlayDisconnectComplete(); + } } disconnectedCallback() { @@ -149,11 +158,24 @@ export const OverlayMixin = dedupeMixin( super.disconnectedCallback(); } + this._overlayDisconnectComplete = new Promise((resolve, reject) => { + this.__resolveOverlayDisconnectComplete = resolve; + this.__rejectOverlayDisconnectComplete = reject; + }); + + setTimeout(() => { + // we start the teardown below + this.__resolveOverlayDisconnectComplete(); + }); + if (this._overlayCtrl) { - this.__tornDown = true; - this.__overlayContentNodeWrapperBeforeTeardown = this._overlayContentNodeWrapper; + // We need to prevent that we create a setup/teardown cycle during startup, where it + // is common that the overlay system moves around nodes. Therefore, we make the + // teardown async, so that it only happens when we are permanently disconnecting from dom + this._overlayDisconnectComplete.then(() => { + this._teardownOverlayCtrl(); + }); } - this._teardownOverlayCtrl(); } get _overlayInvokerNode() { @@ -178,34 +200,32 @@ export const OverlayMixin = dedupeMixin( } _setupOverlayCtrl() { - // When we reconnect, this is for recovering from disconnectedCallback --> teardown which removes the - // the content node wrapper contents (which is necessary for global overlays to remove them from bottom of body) - if (this.__tornDown) { - this.__reappendContentNodeWrapperNodes(); - this.__tornDown = false; - } - this._overlayCtrl = this._defineOverlay({ contentNode: this._overlayContentNode, + contentWrapperNode: this._overlayContentWrapperNode, invokerNode: this._overlayInvokerNode, backdropNode: this._overlayBackdropNode, - contentWrapperNode: this._overlayContentWrapperNode, }); this.__syncToOverlayController(); this.__setupSyncFromOverlayController(); this._setupOpenCloseListeners(); this.__overlaySetupCompleteResolve(); + this.__isOverlaySetup = true; } - async _teardownOverlayCtrl() { - if (this._overlayCtrl) { - this.__teardownSyncFromOverlayController(); - this._overlayCtrl.teardown(); - } - await this.updateComplete; + _teardownOverlayCtrl() { this._teardownOpenCloseListeners(); + this.__teardownSyncFromOverlayController(); + this._overlayCtrl.teardown(); + this.__isOverlaySetup = false; } + /** + * When the opened state is changed by an Application Developer,cthe OverlayController is + * requested to show/hide. It might happen that this request is not honoured + * (intercepted in before-hide for instance), so that we need to sync the controller state + * to this webcomponent again, preventing eternal loops. + */ async _setOpenedWithoutPropertyEffects(newOpened) { this.__blockSyncToOverlayCtrl = true; this.opened = newOpened; @@ -262,13 +282,5 @@ export const OverlayMixin = dedupeMixin( this._overlayCtrl.hide(); } } - - // TODO: Simplify this logic of tearing down / reappending overlay content node wrapper - // after we have moved this wrapper to ShadowDOM. - __reappendContentNodeWrapperNodes() { - Array.from(this.__overlayContentNodeWrapperBeforeTeardown.children).forEach(child => { - this.appendChild(child); - }); - } }, ); diff --git a/packages/overlays/test-suites/OverlayMixin.suite.js b/packages/overlays/test-suites/OverlayMixin.suite.js index d911a607f..984e5e50d 100644 --- a/packages/overlays/test-suites/OverlayMixin.suite.js +++ b/packages/overlays/test-suites/OverlayMixin.suite.js @@ -157,7 +157,7 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) { }); describe(`OverlayMixin${suffix} nested`, () => { - it.skip('supports nested overlays', async () => { + it('supports nested overlays', async () => { const el = await fixture(html` <${tag}>
@@ -223,5 +223,43 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) { expect(contentNode.innerText).to.equal('content of the nested overlay'); } }); + + it("doesn't tear down controller when dom nodes are being moved around", async () => { + const nestedEl = await fixture(html` + <${tag} id="nest"> +
content of the nested overlay
+ + + `); + + const setupOverlayCtrlSpy = sinon.spy(nestedEl, '_setupOverlayCtrl'); + const teardownOverlayCtrlSpy = sinon.spy(nestedEl, '_teardownOverlayCtrl'); + + const mainEl = await fixture(html` + <${tag} id="main"> +
+ open nested overlay: + ${nestedEl} +
+ + + `); + + // Even though many connected/disconnected calls take place, + // we detect we are in the middle of a 'move' + expect(teardownOverlayCtrlSpy).to.not.have.been.called; + expect(setupOverlayCtrlSpy).to.not.have.been.called; + + // Now move nestedEl to an offline node + const offlineNode = document.createElement('div'); + offlineNode.appendChild(nestedEl); + await aTimeout(); + // And we detect this time the disconnect was 'permanent' + expect(teardownOverlayCtrlSpy.callCount).to.equal(1); + + mainEl._overlayContentNode.appendChild(nestedEl); + await aTimeout(); + expect(setupOverlayCtrlSpy.callCount).to.equal(1); + }); }); }