fix: close modeless dialog on Esc when dialog itself has focus (#10037)

web-padawan · web-flow · commit f293e813449b · 2025-08-20T12:30:55.000+03:00
diff --git a/packages/dialog/test/dialog.test.js b/packages/dialog/test/dialog.test.js
@@ -1,14 +1,6 @@
 import { expect } from '@vaadin/chai-plugins';
-import {
-  aTimeout,
-  click,
-  esc,
-  fixtureSync,
-  listenOnce,
-  nextRender,
-  nextUpdate,
-  oneEvent,
-} from '@vaadin/testing-helpers';
+import { sendKeys } from '@vaadin/test-runner-commands';
+import { aTimeout, click, fixtureSync, listenOnce, nextRender, nextUpdate, oneEvent } from '@vaadin/testing-helpers';
 import sinon from 'sinon';
 import '../src/vaadin-dialog.js';
 import { getDeepActiveElement } from '@vaadin/a11y-base/src/focus-utils.js';
@@ -61,14 +53,22 @@ describe('vaadin-dialog', () => {
   });
 
   describe('opened', () => {
-    let dialog, backdrop, overlay;
+    let dialog, focusable, backdrop, overlay;
 
     beforeEach(async () => {
-      dialog = fixtureSync('<vaadin-dialog opened></vaadin-dialog>');
+      [dialog, focusable] = fixtureSync(`
+        <div>
+          <vaadin-dialog opened></vaadin-dialog>
+          <input />
+        </div>
+      `).children;
       await nextRender();
 
       dialog.renderer = (root) => {
-        root.innerHTML = '<div>Simple dialog</div>';
+        root.innerHTML = `
+          <div>Simple dialog</div>
+          <input />
+        `;
       };
       await nextUpdate(dialog);
 
@@ -83,15 +83,35 @@ describe('vaadin-dialog', () => {
 
     describe('no-close-on-esc', () => {
       it('should close itself on ESC press by default', async () => {
-        esc(document.body);
+        await sendKeys({ press: 'Escape' });
         await nextUpdate(dialog);
         expect(dialog.opened).to.be.false;
       });
 
       it('should not close itself on ESC press when no-close-on-esc is true', async () => {
         dialog.noCloseOnEsc = true;
-        await nextUpdate(dialog);
-        esc(document.body);
+        await sendKeys({ press: 'Escape' });
+        expect(dialog.opened).to.be.true;
+      });
+
+      it('should close on Escape press when modeless and dialog itself is focused', async () => {
+        dialog.modeless = true;
+        dialog.focus();
+        await sendKeys({ press: 'Escape' });
+        expect(dialog.opened).to.be.false;
+      });
+
+      it('should close on Escape press when modeless and content element is focused', async () => {
+        dialog.modeless = true;
+        dialog.querySelector('input').focus();
+        await sendKeys({ press: 'Escape' });
+        expect(dialog.opened).to.be.false;
+      });
+
+      it('should not close on Escape press when modeless and not focused', async () => {
+        dialog.modeless = true;
+        focusable.focus();
+        await sendKeys({ press: 'Escape' });
         expect(dialog.opened).to.be.true;
       });
     });
diff --git a/packages/overlay/src/vaadin-overlay-mixin.js b/packages/overlay/src/vaadin-overlay-mixin.js
@@ -557,7 +557,7 @@ export const OverlayMixin = (superClass) =>
       }
 
       // Only close modeless overlay on Esc press when it contains focus
-      if (!this._shouldAddGlobalListeners() && !event.composedPath().includes(this.$.overlay)) {
+      if (!this._shouldAddGlobalListeners() && !event.composedPath().includes(this._focusTrapRoot)) {
         return;
       }
 

Original file line number	Diff line number	Diff line change
`@@ -557,7 +557,7 @@ export const OverlayMixin = (superClass) =>`
`557`	`557`	`}`
`558`	`558`
`559`	`559`	`// Only close modeless overlay on Esc press when it contains focus`
`560`		`- if (!this._shouldAddGlobalListeners() && !event.composedPath().includes(this.$.overlay)) {`
	`560`	`+ if (!this._shouldAddGlobalListeners() && !event.composedPath().includes(this._focusTrapRoot)) {`
`561`	`561`	`return;`
`562`	`562`	`}`
`563`	`563`