ayo/lion
1
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix: hidesOnEsc for nested overlays

Browse source
This commit is contained in:
Oleksii Kadurin 2025-10-30 12:47:53 +01:00 committed by GitHub
parent e6a8fa70e5
commit abcc6fdd69
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 268 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

5
.changeset/yellow-hotels-sell.md Normal file
View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
fix hidesOnEsc for nested overlays

170
packages/ui/components/dialog/test/lion-dialog.integration.test.js Normal file
View file

@ -0,0 +1,170 @@
/* eslint-disable lit-a11y/no-autofocus */
import { expect, fixture as _fixture, html, waitUntil } from '@open-wc/testing';
import { sendKeys } from '@web/test-runner-commands';
import '@lion/ui/define/lion-button.js';
import '@lion/ui/define/lion-combobox.js';
import '@lion/ui/define/lion-select-rich.js';
import '@lion/ui/define/lion-option.js';
import '@lion/ui/define/lion-dialog.js';
import '@lion/ui/define/lion-tooltip.js';
import { mimicUserTyping } from '@lion/ui/combobox-test-helpers.js';
import sinon from 'sinon';
/**
* @typedef {import('../src/LionDialog.js').LionDialog} LionDialog
* @typedef {import('lit').TemplateResult} TemplateResult
*/
const fixture = /** @type {(arg: TemplateResult) => Promise<LionDialog>} */ (_fixture);
const dropDownEntries = ['Apple', 'Banana'];
describe('lion-dialog', () => {
describe('lion-combobox integration', () => {
it('should close lion-combobox dropdown on Escape and should not close the parent lion-dialog', async () => {
const el = await fixture(html`
<lion-dialog has-close-button>
<lion-button slot="invoker">Open Dialog</lion-button>
<div slot="header">Combobox example</div>
<div slot="content">
<lion-combobox name="combo" label="Select a fruit">
${dropDownEntries.map(
(/** @type { string } */ entry) =>
html` <lion-option .choiceValue="${entry}">${entry}</lion-option>`,
)}
</lion-combobox>
</div>
</lion-dialog>
`);
const dialogInvoker = /** @type {HTMLElement} */ (el.querySelector('[slot="invoker"]'));
dialogInvoker.click();
const combobox = /** @type {HTMLElement} */ el.querySelector('lion-combobox');
const isComboboxRendered = () => !!combobox?.shadowRoot?.childNodes.length;
await waitUntil(isComboboxRendered);
// @ts-ignore
await mimicUserTyping(combobox, 'a');
const firstOption = /** @type { HTMLElement | undefined } */ (
el.querySelectorAll('lion-option')?.[0]
);
// @ts-ignore
const isComboboxFirstOptionInDropdownVisible = () => firstOption?.checkVisibility();
await waitUntil(isComboboxFirstOptionInDropdownVisible);
const comboboxDialog = combobox?.shadowRoot?.querySelector('dialog');
// Note, do not remove `console.log` down below. There is a bug in test engine.
// Referring dialog.close from the test environement fixes the bug
console.log(sinon.spy(comboboxDialog?.close));
const comboboxInput = combobox?.querySelector('input');
comboboxInput?.focus();
const dropdownDialog = combobox?.shadowRoot?.querySelector('dialog');
// @ts-ignore
const dropdownDialogCloseSpy = sinon.spy(dropdownDialog, 'close');
await sendKeys({
press: 'Escape',
});
// @ts-ignore
const isDialogVisible = () => el?.shadowRoot?.querySelector('dialog')?.checkVisibility();
await waitUntil(() => dropdownDialogCloseSpy.called);
expect(isDialogVisible()).to.equal(true);
});
});
describe('lion-select-rich integration', () => {
it('should close lion-select-rich dropdown on Escape and should not close the parent lion-dialog', async () => {
const el = await fixture(html`
<lion-dialog has-close-button>
<lion-button slot="invoker">Open Dialog</lion-button>
<div slot="header">Select rich example</div>
<div slot="content">
<lion-select-rich label="Select a fruit">
${dropDownEntries.map(
(/** @type { string } */ entry) =>
html` <lion-option .choiceValue="${entry}">${entry}</lion-option>`,
)}
</lion-select-rich>
</div>
</lion-dialog>
`);
const dialogInvoker = /** @type {HTMLElement} */ (el.querySelector('[slot="invoker"]'));
dialogInvoker.click();
const selectRichInvoker = /** @type {HTMLElement} */ (
el.querySelector('lion-select-invoker')
);
const isSelectRichInvokerRendered = () => !!selectRichInvoker?.shadowRoot?.childNodes.length;
await waitUntil(isSelectRichInvokerRendered);
const selectRich = el?.querySelector('lion-select-rich');
const selectRichDialog = selectRich?.shadowRoot?.querySelector('dialog');
// Note, do not remove `console.log` down below. There is a bug in test engine.
// Referring dialog.close from the test environement fixes the bug
console.log(selectRichDialog?.close);
selectRichInvoker?.click();
const dropdownDialog = el
?.querySelector('lion-select-rich')
?.shadowRoot?.querySelector('dialog');
// @ts-ignore
const dropdownDialogCloseSpy = sinon.spy(dropdownDialog, 'close');
const isDropdownVisible = () =>
el
?.querySelector('lion-select-rich')
?.shadowRoot?.querySelector('dialog')
// @ts-ignore
?.checkVisibility();
await waitUntil(isDropdownVisible);
const lionOptions = /** @type {HTMLElement} */ (el.querySelector('lion-options'));
lionOptions?.focus();
await sendKeys({
press: 'Escape',
});
// @ts-ignore
const isDialogVisible = () => el?.shadowRoot?.querySelector('dialog')?.checkVisibility();
await waitUntil(() => dropdownDialogCloseSpy.called);
expect(isDialogVisible()).to.equal(true);
});
});
describe('lion-tooltip integration', () => {
it('should close lion-tooltip on first Escape and should close the parent lion-dialog on the second Escape', async () => {
const el = await fixture(html`
<lion-dialog has-close-button>
<lion-button slot="invoker" class="dialog-invoker">Open Dialog</lion-button>
<div slot="header">Tooltip example</div>
<div slot="content">
<lion-tooltip has-arrow>
<button slot="invoker" class="demo-tooltip-invoker">Focus on me</button>
<div slot="content">This is a tooltip</div>
</lion-tooltip>
</div>
</lion-dialog>
`);
await el.updateComplete;
const dialogInvoker = /** @type {HTMLElement} */ (el.querySelector('.dialog-invoker'));
dialogInvoker.click();
const tooltip = /** @type {HTMLElement} */ el.querySelector('lion-tooltip');
const isTooltipRendered = () => !!tooltip?.shadowRoot?.childNodes.length;
await waitUntil(isTooltipRendered);
const tooltipButton = /** @type {HTMLElement} */ (el.querySelector('.demo-tooltip-invoker'));
tooltipButton?.focus();
const getTooltipContent = () =>
el
.querySelector('lion-tooltip')
?.shadowRoot?.querySelector('#overlay-content-node-wrapper');
// @ts-ignore
const isTooltipContentVisible = () => getTooltipContent()?.checkVisibility();
await waitUntil(isTooltipContentVisible);
await sendKeys({
press: 'Escape',
});
// @ts-ignore
const isDialogVisible = () => el?.shadowRoot?.querySelector('dialog')?.checkVisibility();
expect(isTooltipContentVisible()).to.equal(false);
expect(isDialogVisible()).to.equal(true);
await sendKeys({
press: 'Escape',
});
expect(isTooltipContentVisible()).to.equal(false);
expect(isDialogVisible()).to.equal(false);
});
});
});

41
packages/ui/components/overlays/src/OverlayController.js
View file

@ -197,15 +197,24 @@ export class OverlayController extends EventTarget {
this._contentId = `overlay-content--${Math.random().toString(36).slice(2, 10)}`;
/** @private */
this.__originalAttrs = new Map();
/** @private */
this.__escKeyHandler = this.__escKeyHandler.bind(this);
this.updateConfig(config);
/** @private */
this.__hasActiveTrapsKeyboardFocus = false;
/** @private */
this.__hasActiveBackdrop = true;
/** @private */
this.__escKeyHandler = this.__escKeyHandler.bind(this);
/** @private */
this.__cancelHandler = this.__cancelHandler.bind(this);
/**
* The property is used to skip `__escKeyHandler` handler for overlays that have been closed
* by `__escKeyHandlerCalled` previously.
* `__escKeyHandlerCalled` is set to `false` right before overlay show up
* `__escKeyHandlerCalled` is set to `true` when `__escKeyHandler` is called at least one time
* after each 'show' phase
* @private
*/
this.__escKeyHandlerCalled = false;
}
/**
@ -1211,13 +1220,21 @@ export class OverlayController extends EventTarget {
* @returns {void}
*/
__escKeyHandler(event) {
if (event.key !== 'Escape' || childDialogsClosedInEventLoopWeakmap.has(event)) return;
if (
event.key !== 'Escape' ||
childDialogsClosedInEventLoopWeakmap.has(event) ||
(!this.isShown && this.__escKeyHandlerCalled)
) {
return;
}
const hasPressedInside =
event.composedPath().includes(this.contentNode) ||
(this.invokerNode && event.composedPath().includes(this.invokerNode)) ||
deepContains(this.contentNode, /** @type {HTMLElement|ShadowRoot} */ (event.target));
if (hasPressedInside) {
this.__escKeyHandlerCalled = true;
this.hide();
// We could do event.stopPropagation() here, but we don't want to hide info for
// the outside world about user interactions. Instead, we store the event in a WeakMap
@ -1245,12 +1262,21 @@ export class OverlayController extends EventTarget {
* @protected
*/
_handleHidesOnEsc({ phase }) {
if (phase === 'show') {
if (phase === 'init') {
// we remove previously added (if any) event listener to guarantee
// there is only one Escape handler added here.
// Note `init` phase triggered on every `updateConfig` call and that
// could happen multiple times during the component life cycle
this.contentNode.removeEventListener('keyup', this.__escKeyHandler);
this.contentNode.addEventListener('keyup', this.__escKeyHandler);
if (this.invokerNode) {
this.invokerNode.addEventListener('keyup', this.__escKeyHandler);
}
} else if (phase === 'hide' || phase === 'teardown') {
}
if (phase === 'show') {
this.__escKeyHandlerCalled = false;
}
if (phase === 'teardown') {
this.contentNode.removeEventListener('keyup', this.__escKeyHandler);
if (this.invokerNode) {
this.invokerNode.removeEventListener('keyup', this.__escKeyHandler);
@ -1263,9 +1289,10 @@ export class OverlayController extends EventTarget {
* @protected
*/
_handleHidesOnOutsideEsc({ phase }) {
if (phase === 'show') {
if (phase === 'init') {
document.removeEventListener('keyup', this.#outsideEscKeyHandler);
document.addEventListener('keyup', this.#outsideEscKeyHandler);
} else if (phase === 'hide' || phase === 'teardown') {
} else if (phase === 'teardown') {
document.removeEventListener('keyup', this.#outsideEscKeyHandler);
}
}

57
packages/ui/components/overlays/test-suites/OverlayMixin.suite.js
View file

@ -111,6 +111,63 @@ export function runOverlayMixinSuite({ tagString, tag, suffix = '' }) {
expect(el.opened).to.be.false;
});
/**
* In case OverlayMixin has a handler for Escape which sets `opened` to `false`,
* make sure it does not conflict with the Escape handler in OverlayController.
* */
it('syncs opened between overlayController and OverlayMixin on Escape', async () => {
const config = { hidesOnEsc: true };
const el = /** @type {OverlayEl} */ (
await fixture(html`
<${tag} id="main-dialog" .config="${config}">
<div slot="content" id="mainContent">
open nested overlay:
<${tag} id="sub-dialog" .config="${config}">
<div slot="content" id="nestedContent">
<input id="nestedContentInput">
</div>
<button slot="invoker" id="nestedInvoker">nested invoker button</button>
</${tag}>
</div>
<button slot="invoker" id="mainInvoker">invoker button</button>
</${tag}>
`)
);
await el.updateComplete;
const subDialog = /** @type {OverlayEl} */ (el.querySelector('#sub-dialog'));
const nestedContentInput = /** @type {OverlayEl} */ (el.querySelector('#nestedContentInput'));
// Note, OverlayController has Escape key listener added to "#nestedContent".
// And the sub-dialog adds the Escape key listener to "#nestedContentInput" which is a child evelement of "#nestedContent"
// In this case when "#nestedContentInput" is focused and Escape is pressed, the listener of "#nestedContentInput"
// is always called before the listener set for "#nestedContent" inside OverlayController.
// This way we check the scenario when OverlayController's `hide()` is called first from
// The OverlayMixin and it does not prevent OverlayController `hidesOnEsc` handler from being called
nestedContentInput?.addEventListener('keyup', (/** @type {* & KeyboardEvent} */ event) => {
if (event.key === 'Escape') {
subDialog.opened = false;
}
});
el.opened = true;
await el.updateComplete;
await el._overlayCtrl._showComplete;
expect(el.opened).to.be.true;
expect(el._overlayCtrl.isShown).to.be.true;
subDialog.opened = true;
await subDialog.updateComplete;
await subDialog._overlayCtrl._showComplete;
expect(subDialog.opened).to.be.true;
expect(subDialog._overlayCtrl.isShown).to.be.true;
await mimicEscapePress(nestedContentInput);
expect(subDialog.opened).to.be.false;
expect(subDialog._overlayCtrl.isShown).to.be.false;
expect(el.opened).to.be.true;
expect(el._overlayCtrl.isShown).to.be.true;
});
// TODO: put this tests in OverlayController.test.js instead?
it('does not change the body size when opened', async () => {
const parentNode = document.createElement('div');

4
packages/ui/components/overlays/test/OverlayMixin.test.js
View file

@ -7,9 +7,9 @@ const tagString = defineCE(
class extends OverlayMixin(LitElement) {
render() {
return html`
<button slot="invoker">invoker button</button>
<slot name="invoker"></slot>
<div id="overlay-content-node-wrapper">
<div slot="content">content of the overlay</div>
<slot name="content"></slot>
</div>
`;
}