Merge pull request #2310 from ing-bank/fix/modal-dialog-esc

fix(overlays): prevent closing of a modal dialog on pressing Esc and …
This commit is contained in:
Thijs Louisse 2024-07-24 16:09:31 +02:00 committed by GitHub
commit 436d5eebbe
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 59 additions and 1 deletions

View file

@ -0,0 +1,5 @@
---
'@lion/ui': patch
---
[overlays] prevent closing of a modal dialog on pressing Esc and hidesOnEsc is set to false

View file

@ -4,7 +4,7 @@
import { html } from '@mdjs/mdjs-preview';
import './assets/demo-el-using-overlaymixin.mjs';
import './assets/applyDemoOverlayStyles.mjs';
import { withDropdownConfig, withTooltipConfig } from '@lion/ui/overlays.js';
import { withDropdownConfig, withModalDialogConfig, withTooltipConfig } from '@lion/ui/overlays.js';
```
The `OverlayController` has many configuration options.
@ -145,6 +145,32 @@ export const hidesOnEsc = () => {
};
```
And how it works if `hidesOnEsc` is disabled. In most cases `hidesOnOutsideEsc` needs also to be set to `false`.
```js preview-story
export const hidesOnEscFalse = () => {
const hidesOnEscConfig = {
...withModalDialogConfig(),
hidesOnEsc: false,
hidesOnOutsideEsc: false,
};
return html`
<demo-el-using-overlaymixin .config=${hidesOnEscConfig}>
<button slot="invoker">Click me to open the overlay!</button>
<div slot="content" class="demo-overlay">
Hello! You can close this notification here:
<button
class="close-button"
@click=${e => e.target.dispatchEvent(new Event('close-overlay', { bubbles: true }))}
>
</button>
</div>
</demo-el-using-overlaymixin>
`;
};
```
## hidesOnOutsideEsc
Boolean property. When enabled allows closing the overlay on ESC key, even when contentNode has no focus.

View file

@ -201,6 +201,8 @@ export class OverlayController extends EventTarget {
this.__hasActiveBackdrop = true;
/** @private */
this.__escKeyHandler = this.__escKeyHandler.bind(this);
/** @private */
this.__cancelHandler = this.__cancelHandler.bind(this);
}
/**
@ -510,6 +512,8 @@ export class OverlayController extends EventTarget {
this.__contentHasBeenInitialized = true;
}
this.__wrappingDialogNode?.addEventListener('cancel', this.__cancelHandler);
// Reset all positioning styles (local, c.q. Popper) and classes (global)
this.contentWrapperNode.removeAttribute('style');
this.contentWrapperNode.removeAttribute('class');
@ -1126,6 +1130,17 @@ export class OverlayController extends EventTarget {
}
}
/**
* When the overlay is a modal dialog hidesOnEsc works out of the box, so we prevent that.
*
* There is currently a bug in chrome that makes the dialog close when pressing Esc the second time
* @private
*/
// eslint-disable-next-line class-methods-use-this
__cancelHandler(/** @type {Event} */ ev) {
ev.preventDefault();
}
/** @private */
__escKeyHandler(/** @type {KeyboardEvent} */ ev) {
return ev.key === 'Escape' && this.hide();
@ -1292,6 +1307,7 @@ export class OverlayController extends EventTarget {
teardown() {
this.__handleOverlayStyles({ phase: 'teardown' });
this._handleFeatures({ phase: 'teardown' });
this.__wrappingDialogNode?.removeEventListener('cancel', this.__cancelHandler);
}
/** @private */

View file

@ -585,6 +585,17 @@ describe('OverlayController', () => {
expect(ctrl.isShown).to.be.false;
});
it("doesn't hide when [escape] is pressed and hidesOnEsc is set to false", async () => {
const ctrl = new OverlayController({
...withGlobalTestConfig(),
hidesOnEsc: false,
});
await ctrl.show();
ctrl.contentNode.dispatchEvent(new KeyboardEvent('keyup', { key: 'Escape' }));
await aTimeout(0);
expect(ctrl.isShown).to.be.true;
});
it('stays shown when [escape] is pressed on outside element', async () => {
const ctrl = new OverlayController({
...withGlobalTestConfig(),