Skip to content

Commit

Permalink
fix(overlays): allow Subclassers to teardown at the right moment
Browse files Browse the repository at this point in the history
  • Loading branch information
tlouisse committed Jan 6, 2025
1 parent 9825cbf commit 3f4f48d
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/tricky-suns-change.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[overlays]: allow Subclassers to teardown at the right moment
9 changes: 5 additions & 4 deletions packages/ui/components/overlays/src/OverlayMixin.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,10 @@ export const OverlayMixinImplementation = superclass =>
super.disconnectedCallback();

if (!this._overlayCtrl) return;
if (await this.#isMovingInDom()) return;

this._teardownOverlayCtrl();
if (await this._isPermanentlyDisconnected()) {
this._teardownOverlayCtrl();
}
}

/**
Expand Down Expand Up @@ -396,9 +397,9 @@ export const OverlayMixinImplementation = superclass =>
* Before we decide to teardown, let's wait to see if we were not just moving nodes around.
* @return {Promise<boolean>}
*/
async #isMovingInDom() {
async _isPermanentlyDisconnected() {
await this.updateComplete;
return this.isConnected;
return !this.isConnected;
}
};
export const OverlayMixin = dedupeMixin(OverlayMixinImplementation);
30 changes: 30 additions & 0 deletions packages/ui/components/overlays/test-suites/OverlayMixin.suite.js
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,36 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) {
}).to.not.throw;
stub.restore();
});

it('does not teardown when moving dom nodes', async () => {
const el = /** @type {OverlayEl} */ (
await fixture(html`
<${tag}>
<div slot="content">content</div>
<button slot="invoker">invoker button</button>
</${tag}>
`)
);
// @ts-expect-error [allow-protected] in tests
const teardownSpy = sinon.spy(el, '_teardownOverlayCtrl');
// @ts-expect-error [allow-protected] in tests
const spyDisconnected = sinon.spy(el, '_isPermanentlyDisconnected');

// move around in dom
const newParent = document.createElement('div');
document.body.appendChild(newParent);
newParent.appendChild(el);
await aTimeout(0);

expect(spyDisconnected.callCount).to.equal(1);
expect(teardownSpy.callCount).to.equal(0);

// simulate a permanent disconnect
newParent.removeChild(el);
await aTimeout(0);

expect(teardownSpy.callCount).to.equal(1);
});
});

describe(`OverlayMixin${suffix} nested`, () => {
Expand Down

0 comments on commit 3f4f48d

Please sign in to comment.