Skip to content

Commit

Permalink
Merge pull request #2310 from ing-bank/fix/modal-dialog-esc
Browse files Browse the repository at this point in the history
fix(overlays): prevent closing of a modal dialog on pressing Esc and …
  • Loading branch information
tlouisse authored Jul 24, 2024
2 parents b15d2aa + f9ba215 commit 436d5ee
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 1 deletion.
5 changes: 5 additions & 0 deletions .changeset/poor-bears-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@lion/ui': patch
---

[overlays] prevent closing of a modal dialog on pressing Esc and hidesOnEsc is set to false
28 changes: 27 additions & 1 deletion docs/fundamentals/systems/overlays/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
16 changes: 16 additions & 0 deletions packages/ui/components/overlays/src/OverlayController.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/**
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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 */
Expand Down
11 changes: 11 additions & 0 deletions packages/ui/components/overlays/test/OverlayController.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down

0 comments on commit 436d5ee

Please sign in to comment.