From e5ff2b01d2386f6d3616869f8b2cf48ffa25a63c Mon Sep 17 00:00:00 2001 From: Thijs Louisse Date: Wed, 18 Dec 2024 19:34:12 +0100 Subject: [PATCH] fix(overlays): overlayController teardown cleanup --- .changeset/smooth-boats-argue.md | 5 ++++ .../overlays/src/OverlayController.js | 30 +++++++++---------- .../overlays/test/OverlayController.test.js | 19 +++++++----- .../overlays/types/ArrowMixinTypes.ts | 13 ++++++-- .../overlays/types/OverlayConfig.ts | 28 +++++++++-------- 5 files changed, 57 insertions(+), 38 deletions(-) create mode 100644 .changeset/smooth-boats-argue.md diff --git a/.changeset/smooth-boats-argue.md b/.changeset/smooth-boats-argue.md new file mode 100644 index 000000000..6a93afc4c --- /dev/null +++ b/.changeset/smooth-boats-argue.md @@ -0,0 +1,5 @@ +--- +'@lion/ui': patch +--- + +overlayController teardown cleanup (removes logs in test files) diff --git a/packages/ui/components/overlays/src/OverlayController.js b/packages/ui/components/overlays/src/OverlayController.js index c64e1baff..9ff3e8295 100644 --- a/packages/ui/components/overlays/src/OverlayController.js +++ b/packages/ui/components/overlays/src/OverlayController.js @@ -1003,16 +1003,16 @@ export class OverlayController extends EventTarget { * @param {{ phase: OverlayPhase }} config */ _handleVisibilityTriggers({ phase }) { - if (typeof this.visibilityTriggerFunction === 'function') { - if (phase === 'init') { - this.__visibilityTriggerHandler = this.visibilityTriggerFunction({ - phase, - controller: this, - }); - } - if (this.__visibilityTriggerHandler[phase]) { - this.__visibilityTriggerHandler[phase](); - } + if (typeof this.visibilityTriggerFunction !== 'function') return; + + if (phase === 'init') { + this.__visibilityTriggerHandler = this.visibilityTriggerFunction({ + phase, + controller: this, + }); + } + if (this.__visibilityTriggerHandler[phase]) { + this.__visibilityTriggerHandler[phase](); } } @@ -1191,9 +1191,9 @@ export class OverlayController extends EventTarget { const hasPressedInside = event.composedPath().includes(this.contentNode) || deepContains(this.contentNode, /** @type {HTMLElement|ShadowRoot} */ (event.target)); - if (!hasPressedInside) { - this.hide(); - } + if (hasPressedInside) return; + + this.hide(); }; /** @@ -1206,7 +1206,7 @@ export class OverlayController extends EventTarget { if (this.invokerNode) { this.invokerNode.addEventListener('keyup', this.__escKeyHandler); } - } else if (phase === 'hide') { + } else if (phase === 'hide' || phase === 'teardown') { this.contentNode.removeEventListener('keyup', this.__escKeyHandler); if (this.invokerNode) { this.invokerNode.removeEventListener('keyup', this.__escKeyHandler); @@ -1221,7 +1221,7 @@ export class OverlayController extends EventTarget { _handleHidesOnOutsideEsc({ phase }) { if (phase === 'show') { document.addEventListener('keyup', this.#outsideEscKeyHandler); - } else if (phase === 'hide') { + } else if (phase === 'hide' || phase === 'teardown') { document.removeEventListener('keyup', this.#outsideEscKeyHandler); } } diff --git a/packages/ui/components/overlays/test/OverlayController.test.js b/packages/ui/components/overlays/test/OverlayController.test.js index 025ce71f5..5eca2c44c 100644 --- a/packages/ui/components/overlays/test/OverlayController.test.js +++ b/packages/ui/components/overlays/test/OverlayController.test.js @@ -103,12 +103,12 @@ const withLocalTestConfig = () => async function createNestedEscControllers(parentContent) { const childContent = /** @type {HTMLDivElement} */ (parentContent.querySelector('div[id]')); // Assert valid fixure - const isValid = - (parentContent.id === 'parent-overlay--hidesOnEsc' || - parentContent.id === 'parent-overlay--hidesOnOutsideEsc') && - (childContent.id === 'child-overlay--hidesOnEsc' || - childContent.id === 'child-overlay--hidesOnOutsideEsc'); - if (!isValid) { + const isValidFixture = + (parentContent.id.startsWith('parent-overlay--hidesOnEsc') || + parentContent.id.startsWith('parent-overlay--hidesOnOutsideEsc')) && + (childContent.id.startsWith('child-overlay--hidesOnEsc') || + childContent.id.startsWith('child-overlay--hidesOnOutsideEsc')); + if (!isValidFixture) { throw new Error('Provide a valid fixture'); } @@ -117,8 +117,8 @@ async function createNestedEscControllers(parentContent) { shadowRootParent.appendChild(childContent); } - const parentHasOutsideOnEsc = parentContent.id === 'parent-overlay--hidesOnOutsideEsc'; - const childHasOutsideOnEsc = childContent.id === 'child-overlay--hidesOnOutsideEsc'; + const parentHasOutsideOnEsc = parentContent.id.startsWith('parent-overlay--hidesOnOutsideEsc'); + const childHasOutsideOnEsc = childContent.id.startsWith('child-overlay--hidesOnOutsideEsc'); const parentConfig = parentHasOutsideOnEsc ? { hidesOnOutsideEsc: true } : { hidesOnEsc: true }; const childConfig = childHasOutsideOnEsc ? { hidesOnOutsideEsc: true } : { hidesOnEsc: true }; @@ -787,6 +787,9 @@ describe('OverlayController', () => { expect(parentOverlay.isShown).to.be.false; expect(childOverlay.isShown).to.be.true; + + await childOverlay.teardown(); + await parentOverlay.teardown(); }); it('on [Escape] press outside overlays: parent stays shown, child hides', async () => { diff --git a/packages/ui/components/overlays/types/ArrowMixinTypes.ts b/packages/ui/components/overlays/types/ArrowMixinTypes.ts index a18b48fc9..047f288eb 100644 --- a/packages/ui/components/overlays/types/ArrowMixinTypes.ts +++ b/packages/ui/components/overlays/types/ArrowMixinTypes.ts @@ -4,25 +4,34 @@ import { Options as PopperOptions, State } from '@popperjs/core/lib/popper.js'; import { OverlayConfig } from './OverlayConfig.js'; export declare class ArrowHost { - static get properties(): { + static properties: { hasArrow: { type: BooleanConstructor; reflect: boolean; attribute: string; }; }; + hasArrow: boolean; + repositionComplete: Promise; - static get styles(): CSSResultArray; + static styles: CSSResultArray; render(): TemplateResult; + protected _arrowTemplate(): TemplateResult; + protected _arrowNodeTemplate(): TemplateResult; + protected _defineOverlayConfig(): OverlayConfig; + protected _getPopperArrowConfig(popperConfigToExtendFrom: Partial): Partial; + private __setupRepositionCompletePromise(): void; + get _arrowNode(): Element | null; + private __syncFromPopperState(data: Partial): void; } diff --git a/packages/ui/components/overlays/types/OverlayConfig.ts b/packages/ui/components/overlays/types/OverlayConfig.ts index 6db671cd5..4d1bf6361 100644 --- a/packages/ui/components/overlays/types/OverlayConfig.ts +++ b/packages/ui/components/overlays/types/OverlayConfig.ts @@ -1,5 +1,20 @@ import { Options } from '@popperjs/core'; +export type ViewportPlacement = + | 'center' + | 'top-left' + | 'top' + | 'top-right' + | 'right' + | 'bottom-right' + | 'bottom' + | 'bottom-left' + | 'left'; + + +export interface ViewportConfig { + placement: ViewportPlacement; +} export interface OverlayConfig { // Positioning @@ -80,17 +95,4 @@ export interface OverlayConfig { visibilityTriggerFunction?: Function; } -export type ViewportPlacement = - | 'center' - | 'top-left' - | 'top' - | 'top-right' - | 'right' - | 'bottom-right' - | 'bottom' - | 'bottom-left' - | 'left'; -export interface ViewportConfig { - placement: ViewportPlacement; -}