Skip to content

Commit

Permalink
fix: do not close dialog when closing popover on Escape (#7420)
Browse files Browse the repository at this point in the history
  • Loading branch information
web-padawan authored May 17, 2024
1 parent dc9b8c7 commit 4af7633
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 1 deletion.
1 change: 1 addition & 0 deletions integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
"@vaadin/overlay": "24.5.0-alpha0",
"@vaadin/password-field": "24.5.0-alpha0",
"@vaadin/polymer-legacy-adapter": "24.5.0-alpha0",
"@vaadin/popover": "24.5.0-alpha0",
"@vaadin/radio-group": "24.5.0-alpha0",
"@vaadin/select": "24.5.0-alpha0",
"@vaadin/tabs": "24.5.0-alpha0",
Expand Down
73 changes: 73 additions & 0 deletions integration/tests/dialog-popover.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
import { expect } from '@esm-bundle/chai';
import { fixtureSync, nextFrame, nextRender, nextUpdate } from '@vaadin/testing-helpers';
import { sendKeys } from '@web/test-runner-commands';
import './not-animated-styles.js';
import '@vaadin/dialog';
import '@vaadin/popover';

describe('popover in dialog', () => {
let dialog, popover, button, overlay;

beforeEach(async () => {
dialog = fixtureSync('<vaadin-dialog></vaadin-dialog>');
dialog.renderer = (root) => {
if (!root.firstChild) {
const button = document.createElement('button');
button.textContent = 'Open popover';
root.append(button);

const popover = document.createElement('vaadin-popover');
popover.renderer = (root2) => {
if (!root2.firstChild) {
const button2 = document.createElement('button');
button2.textContent = 'Inside popover';
root2.append(button2);
}
};

popover.target = button;
root.append(popover);
}
};
dialog.opened = true;
await nextRender();
button = dialog.$.overlay.querySelector('button');
popover = dialog.$.overlay.querySelector('vaadin-popover');
overlay = popover._overlayElement;
});

afterEach(async () => {
dialog.opened = false;
await nextFrame();
});

['modal', 'modeless'].forEach((type) => {
describe(`${type} popover`, () => {
beforeEach(async () => {
if (type === 'modal') {
popover.modal = true;
await nextUpdate(popover);
}

button.focus();
button.click();
await nextRender();
});

it(`should not close the dialog when closing ${type} popover on Escape`, async () => {
await sendKeys({ press: 'Escape' });

expect(overlay.opened).to.be.false;
expect(dialog.opened).to.be.true;
});

it(`should close the dialog on subsequent Escape after the ${type} popover is closed`, async () => {
await sendKeys({ press: 'Escape' });

await sendKeys({ press: 'Escape' });

expect(dialog.opened).to.be.false;
});
});
});
});
4 changes: 3 additions & 1 deletion packages/popover/src/vaadin-popover.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,9 @@ class Popover extends PopoverPositionMixin(

/** @private */
__onTargetKeydown(event) {
if (event.key === 'Escape' && !this.noCloseOnEsc) {
if (event.key === 'Escape' && !this.noCloseOnEsc && this._opened) {
// Prevent closing parent overlay (e.g. dialog)
event.stopPropagation();
this._opened = false;
}
}
Expand Down

0 comments on commit 4af7633

Please sign in to comment.