fix(overlays): overlayController teardown cleanup

This commit is contained in:
Thijs Louisse 2024-12-18 19:34:12 +01:00 committed by Thijs Louisse
parent 9825cbf656
commit e5ff2b01d2
5 changed files with 57 additions and 38 deletions

View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
overlayController teardown cleanup (removes logs in test files)

View file

@ -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);
}
}

View file

@ -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 () => {

View file

@ -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<void>;
static get styles(): CSSResultArray;
static styles: CSSResultArray;
render(): TemplateResult;
protected _arrowTemplate(): TemplateResult;
protected _arrowNodeTemplate(): TemplateResult;
protected _defineOverlayConfig(): OverlayConfig;
protected _getPopperArrowConfig(popperConfigToExtendFrom: Partial<PopperOptions>): Partial<PopperOptions>;
private __setupRepositionCompletePromise(): void;
get _arrowNode(): Element | null;
private __syncFromPopperState(data: Partial<State>): void;
}

View file

@ -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;
}