Skip to content

Commit

Permalink
fix(overlays): no hiding of nested overlays having hideOnEsc config…
Browse files Browse the repository at this point in the history
…ured
  • Loading branch information
tlouisse committed Oct 31, 2024
1 parent a5b2c2d commit 576d1d8
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 15 deletions.
5 changes: 5 additions & 0 deletions .changeset/rude-flies-reflect.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[overlays] no hiding of nested overlays having `hideOnEsc` configured
21 changes: 17 additions & 4 deletions packages/ui/components/overlays/src/OverlayController.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,13 @@ import { overlayShadowDomStyle } from './overlayShadowDomStyle.js';
import { _adoptStyleUtils } from './utils/adopt-styles.js';

/**
* @typedef {import('@lion/ui/types/overlays.js').OverlayConfig} OverlayConfig
* @typedef {'setup'|'init'|'teardown'|'before-show'|'show'|'hide'|'add'|'remove'} OverlayPhase
* @typedef {import('@lion/ui/types/overlays.js').ViewportConfig} ViewportConfig
* @typedef {import('@popperjs/core').createPopper} Popper
* @typedef {import('@lion/ui/types/overlays.js').OverlayConfig} OverlayConfig
* @typedef {import('@popperjs/core').Options} PopperOptions
* @typedef {import('@popperjs/core').Placement} Placement
* @typedef {import('@popperjs/core').createPopper} Popper
* @typedef {{ createPopper: Popper }} PopperModule
* @typedef {'setup'|'init'|'teardown'|'before-show'|'show'|'hide'|'add'|'remove'} OverlayPhase
*/

/**
Expand Down Expand Up @@ -1148,7 +1148,20 @@ export class OverlayController extends EventTarget {

/** @private */
__escKeyHandler(/** @type {KeyboardEvent} */ ev) {
return ev.key === 'Escape' && this.hide();
// @ts-expect-error
if (ev.key !== 'Escape' || ev._hasAlreadyHandledEscape) return;

this.hide();

const shouldNotCloseParents = this.hidesOnEsc && !this.hidesOnOutsideEsc;
if (shouldNotCloseParents) {
// We could do ev.stopPropagation() here, but we don't want to hide info for
// the outside world about user interactions. Instead, we piggyback
// on the event so our parent overlay can read it.
// @ts-expect-error
// eslint-disable-next-line no-param-reassign
ev._hasAlreadyHandledEscape = true;
}
}

/**
Expand Down
53 changes: 42 additions & 11 deletions packages/ui/components/overlays/test/OverlayController.test.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,26 @@
/* eslint-disable no-new */
import { OverlayController, overlays, OverlayMixin } from '@lion/ui/overlays.js';

Check failure on line 2 in packages/ui/components/overlays/test/OverlayController.test.js

View workflow job for this annotation

GitHub Actions / Verify changes

'OverlayMixin' is defined but never used
import { mimicClick } from '@lion/ui/overlays-test-helpers.js';
import { LitElement } from 'lit';

Check failure on line 4 in packages/ui/components/overlays/test/OverlayController.test.js

View workflow job for this annotation

GitHub Actions / Verify changes

'LitElement' is defined but never used
import sinon from 'sinon';
import {
aTimeout,
unsafeStatic,
fixtureSync,
defineCE,
expect,
aTimeout,
fixture,
expect,
html,
unsafeStatic,
fixtureSync,
} from '@open-wc/testing';
import sinon from 'sinon';
import { OverlayController, overlays } from '@lion/ui/overlays.js';
import { mimicClick } from '@lion/ui/overlays-test-helpers.js';
import { keyCodes } from '../src/utils/key-codes.js';
import { simulateTab } from '../src/utils/simulate-tab.js';
import { _adoptStyleUtils } from '../src/utils/adopt-styles.js';

import { createShadowHost } from '../test-helpers/createShadowHost.js';
import { _adoptStyleUtils } from '../src/utils/adopt-styles.js';
import { simulateTab } from '../src/utils/simulate-tab.js';
import { keyCodes } from '../src/utils/key-codes.js';

/**
* @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
* @typedef {import('../types/OverlayConfig.js').ViewportPlacement} ViewportPlacement
* @typedef {import('../types/OverlayConfig.js').OverlayConfig} OverlayConfig
*/

const wrappingDialogNodeStyle = 'display: none; z-index: 9999; padding: 0px;';
Expand Down Expand Up @@ -617,6 +619,35 @@ describe('OverlayController', () => {
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
});

it('does not hide when [escape] while the overlay has a child open', async () => {
const childContent = /** @type {HTMLElement} */ (
await fixture(html`<div id="child">child</div>`)
);
const childOverlay = new OverlayController({
...withGlobalTestConfig(),
contentNode: childContent,
hidesOnEsc: true,
});

const parentContent = /** @type {HTMLElement} */ (
await fixture(html`<div id="parent">parent${childContent}</div>`)
);
const parentOverlay = new OverlayController({
...withGlobalTestConfig(),
contentNode: parentContent,
hidesOnEsc: true,
});

await parentOverlay.show();
await childOverlay.show();

childContent.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape', bubbles: true }));
await aTimeout(0);

expect(childOverlay.isShown).to.be.false;
expect(parentOverlay.isShown).to.be.true;
});
});

describe('hidesOnOutsideEsc', () => {
Expand Down

0 comments on commit 576d1d8

Please sign in to comment.