Fix/2059 date field clicking a date in a dialog can close the dialog

.changeset/chatty-places-throw.md

@@ -0,0 +1,5 @@
+---
+'@sl-design-system/combobox': patch
+---
+
+Fixes the issue where pressing the `Escape` key inside the combobox closes parent containers (such as dialogs).

.changeset/chubby-ravens-decide.md

@@ -0,0 +1,5 @@
+---
+'@sl-design-system/menu': patch
+---
+
+Fixes the issue where pressing the `Escape` key inside the menu closes parent containers (such as dialogs).

.changeset/itchy-onions-lead.md

@@ -0,0 +1,6 @@
+---
+'@sl-design-system/dialog': patch
+---
+
+Fixes closing the dialog when clicking the backdrop.
+The dialog should close only when the dialog element itself is clicked, not when a child of the dialog is clicked.

.changeset/mighty-schools-itch.md

@@ -0,0 +1,6 @@
+---
+'@sl-design-system/date-field': patch
+---
+
+Fixes the issue where pressing the `Escape` key inside the date picker (calendar) closes parent containers (such as dialogs).
+Prevents the Escape key event from bubbling up, so pressing Escape inside the date field does not close the dialog (or other parent container).

.changeset/poor-moose-hang.md

@@ -0,0 +1,5 @@
+---
+'@sl-design-system/select': patch
+---
+
+Fixes the issue where pressing the `Escape` key inside the select closes parent containers (such as dialogs).

.changeset/thirty-dancers-arrive.md

@@ -0,0 +1,6 @@
+---
+'@sl-design-system/popover': patch
+'@sl-design-system/shared': patch
+---
+
+Fixes the issue where pressing the `Escape` key inside the popover closes parent containers (such as dialogs).

packages/components/combobox/src/combobox.ts

@@ -443,6 +443,7 @@ export class Combobox<T = any, U = T> extends FormControlMixin(ScopedElementsMix
         })}
         @beforetoggle=${this.#onBeforeToggle}
         @click=${this.#onOptionClick}
+        @keydown=${this.#onKeydown}
         @slotchange=${() => this.#onSlotChange()}
         @toggle=${this.#onToggle}
         part="wrapper"
@@ -625,6 +626,10 @@ export class Combobox<T = any, U = T> extends FormControlMixin(ScopedElementsMix
       index = (index + delta + items.length) % items.length;
 
       this.#updateCurrent(items[index]);
+    } else if (event.key === 'Escape') {
+      // Prevents the Escape key event from bubbling up, so that pressing 'Escape' inside the combobox
+      // does not close parent containers (such as dialogs).
+      event.stopPropagation();
     }
   }
 

packages/components/date-field/src/date-field.ts

@@ -118,7 +118,7 @@ export class DateField extends LocaleMixin(FormControlMixin(ScopedElementsMixin(
 
   /**
    * Whether the component is select only. This means you cannot type in the text field,
-   * but you can still pick a date via de popover.
+   * but you can still pick a date via the popover.
    * @default false
    */
   @property({ type: Boolean, reflect: true, attribute: 'select-only' }) selectOnly?: boolean;
@@ -231,6 +231,7 @@ export class DateField extends LocaleMixin(FormControlMixin(ScopedElementsMixin(
         })}
         @beforetoggle=${this.#onBeforeToggle}
         @toggle=${this.#onToggle}
+        @keydown=${this.#onKeydown}
         name="calendar"
         part="wrapper"
         popover
@@ -336,4 +337,12 @@ export class DateField extends LocaleMixin(FormControlMixin(ScopedElementsMixin(
     // Trigger a rerender so the calendar will be rendered
     this.requestUpdate();
   }
+
+  #onKeydown(event: KeyboardEvent): void {
+    if (event.key === 'Escape') {
+      // Prevents the Escape key event from bubbling up, so that pressing 'Escape' inside the date field
+      // does not close parent containers (such as dialogs).
+      event.stopPropagation();
+    }
+  }
 }

packages/components/dialog/src/dialog.spec.ts

@@ -1,4 +1,4 @@
-import { expect, fixture } from '@open-wc/testing';
+import { expect, fixture, oneEvent } from '@open-wc/testing';
 import { type Button } from '@sl-design-system/button';
 import '@sl-design-system/button/register.js';
 import { sendKeys } from '@web/test-runner-commands';
@@ -134,7 +134,6 @@ describe('sl-dialog', () => {
       const onCancel = spy();
       el.addEventListener('sl-cancel', onCancel);
 
-      // Mock the click event
       const clickEvent = new PointerEvent('click');
       stub(clickEvent, 'clientX').value(100);
       stub(clickEvent, 'clientY').value(100);
@@ -143,14 +142,56 @@ describe('sl-dialog', () => {
       expect(onCancel).to.have.been.calledOnce;
     });
 
+    it('should only handle backdrop clicks when event.target is a dialog', async () => {
+      stub(dialog, 'getBoundingClientRect').returns({
+        top: 400,
+        right: 1400,
+        bottom: 900,
+        left: 700
+      } as DOMRect);
+
+      const onCancel = spy();
+      el.addEventListener('sl-cancel', onCancel);
+
+      const clickEvent = new PointerEvent('click');
+      stub(clickEvent, 'clientX').value(100);
+      stub(clickEvent, 'clientY').value(100);
+      dialog.dispatchEvent(clickEvent);
+
+      // Wait for the event to be emitted
+      await new Promise(resolve => setTimeout(resolve));
+
+      expect(onCancel).to.have.been.calledOnce;
+
+      onCancel.resetHistory();
+
+      // Simulate a click inside the dialog (not a backdrop click)
+      const button = document.createElement('sl-button');
+      button.innerHTML = 'Click me';
+      dialog.appendChild(button);
+
+      await el.updateComplete;
+
+      button.click();
+      await el.updateComplete;
+
+      // Wait for the event to be emitted
+      await new Promise(resolve => setTimeout(resolve));
+
+      expect(onCancel).not.to.have.been.called;
+    });
+
     it('should emit an sl-close event when calling close()', async () => {
       const onClose = spy();
 
       el.addEventListener('sl-close', onClose);
       el.close();
 
-      // Wait for the event to be emitted
-      await new Promise(resolve => setTimeout(resolve, 50));
+      // Using `oneEvent` https://open-wc.org/docs/testing/helpers/#testing-events
+      // instead of `await new Promise(resolve => setTimeout(resolve))`
+      // ensures the test waits for the actual 'sl-close' event to be emitted by the component, rather than relying on a timeout.
+      await oneEvent(el, 'sl-close', false);
+      await el.updateComplete;
 
       expect(onClose).to.have.been.calledOnce;
     });
@@ -172,8 +213,9 @@ describe('sl-dialog', () => {
       stub(clickEvent, 'clientY').value(100);
       dialog.dispatchEvent(clickEvent);
 
-      // Wait for the event to be emitted
-      await new Promise(resolve => setTimeout(resolve, 50));
+      // ensures the test waits for the actual 'sl-close' event to be emitted by the component, rather than relying on a timeout.
+      await oneEvent(el, 'sl-close', false);
+      await el.updateComplete;
 
       expect(onClose).to.have.been.calledOnce;
     });
@@ -184,8 +226,10 @@ describe('sl-dialog', () => {
       el.addEventListener('sl-close', onClose);
       el.renderRoot.querySelector<Button>('sl-button[aria-label="Close"]')?.click();
 
-      // Wait for the event to be emitted
-      await new Promise(resolve => setTimeout(resolve, 50));
+      // ensures the test waits for the actual 'sl-close' event to be emitted by the component, rather than relying on a timeout.
+      await oneEvent(el, 'sl-close', false);
+
+      await el.updateComplete;
 
       expect(onClose).to.have.been.calledOnce;
     });
@@ -196,8 +240,9 @@ describe('sl-dialog', () => {
       el.addEventListener('sl-close', onClose);
       el.querySelector('sl-button')?.click();
 
-      // Wait for the event to be emitted
-      await new Promise(resolve => setTimeout(resolve, 50));
+      // ensures the test waits for the actual 'sl-close' event to be emitted by the component, rather than relying on a timeout.
+      await oneEvent(el, 'sl-close', false);
+      await el.updateComplete;
 
       expect(onClose).to.have.been.calledOnce;
     });

packages/components/dialog/src/dialog.stories.ts

@@ -1,10 +1,15 @@
 import { faBurst } from '@fortawesome/pro-regular-svg-icons';
 import '@sl-design-system/button/register.js';
 import '@sl-design-system/button-bar/register.js';
+import '@sl-design-system/combobox/register.js';
+import '@sl-design-system/date-field/register.js';
 import '@sl-design-system/form/register.js';
 import { Icon } from '@sl-design-system/icon';
 import '@sl-design-system/icon/register.js';
+import '@sl-design-system/listbox/register.js';
 import { FormInDialog } from '@sl-design-system/lit-examples';
+import '@sl-design-system/popover/register.js';
+import '@sl-design-system/select/register.js';
 import '@sl-design-system/text-area/register.js';
 import '@sl-design-system/text-field/register.js';
 import { type Meta, type StoryObj } from '@storybook/web-components-vite';
@@ -230,3 +235,107 @@ export const All: Story = {
     `;
   }
 };
+
+export const DialogWithOverlayComponents: Story = {
+  render: () => {
+    const onClick = async (event: Event & { target: HTMLElement }) => {
+      const dialog = document.createElement('sl-dialog');
+
+      const onClickPopover = (): void => {
+        const popover = document.getElementById('popover-example') as HTMLElement & { togglePopover(): void };
+        popover?.togglePopover();
+      };
+
+      const hidePopover = (): void => {
+        const popover = document.getElementById('popover-example') as HTMLElement & { togglePopover(): void };
+        popover?.hidePopover();
+      };
+
+      dialog.innerHTML = `
+        <span slot="title">Dialog with overlay components</span>
+        <div class="container">
+          This dialog should not close when any overlay component is closed using the Escape key.
+
+          <sl-date-field autofocus select-only placeholder="Choose a date" style="width: fit-content"> </sl-date-field>
+
+          <sl-select placeholder="Select an option">
+            <sl-option value="1">Option 1</sl-option>
+            <sl-option value="2">Option 2</sl-option>
+            <sl-option value="3">Option 3</sl-option>
+            <sl-option value="3">Option 4</sl-option>
+            <sl-option value="3">Option 5</sl-option>
+          </sl-select>
+
+          <sl-combobox multiple value='["0","2"]'>
+            <sl-listbox>
+              <sl-option value="0">Mathematics</sl-option>
+              <sl-option value="1">Geography</sl-option>
+              <sl-option value="2">Physics</sl-option>
+              <sl-option value="3">History</sl-option>
+              <sl-option value="4">Biology</sl-option>
+              <sl-option value="4">Art</sl-option>
+            </sl-listbox>
+          </sl-combobox>
+
+          <sl-button id="anchor" variant="primary">Show definition</sl-button>
+          <sl-popover id="popover-example" anchor="anchor">
+            <header style="font-size: 1.1em; padding-block-end: 1rem;">Word Definition</header>
+            <section style="padding-block-end: 1rem;">
+              <strong>Photosynthesis</strong> is the process by which green plants and some other organisms <br/>
+               use sunlight to synthesize foods from carbon dioxide and water.
+            </section>
+            <footer>
+              <sl-button id="closeButton" size="sm" variant="primary">Got it</sl-button>
+            </footer>
+          </sl-popover>
+
+          <sl-menu-button position="bottom">
+            <span slot="button">Actions</span>
+            <sl-menu-item><sl-icon name="smile"></sl-icon>Profile</sl-menu-item>
+            <sl-menu-item><sl-icon name="calendar"></sl-icon>Settings</sl-menu-item>
+            <sl-menu-item><sl-icon name="far-trash"></sl-icon>Remove</sl-menu-item>
+          </sl-menu-button>
+        </div>
+        <sl-button slot="primary-actions" sl-dialog-close variant="primary">Close</sl-button>
+      `;
+
+      dialog.closeButton = true;
+
+      dialog.addEventListener('sl-close', () => {
+        dialog.remove();
+      });
+
+      event.target.insertAdjacentElement('afterend', dialog);
+
+      dialog.querySelector('#anchor')?.addEventListener('click', onClickPopover);
+
+      dialog.querySelector('#closeButton')?.addEventListener('click', hidePopover);
+
+      await dialog.updateComplete;
+
+      dialog.showModal();
+    };
+
+    return html`
+      <style>
+        .container {
+          display: flex;
+          flex-direction: column;
+          gap: 0.8rem;
+          margin-block-end: 0.5rem;
+        }
+
+        section {
+          margin-block-end: 1rem;
+        }
+      </style>
+      <section>
+        This example shows a dialog with overlay components (such as date fields, selects, comboboxes, popovers, and
+        menu buttons). <br />
+        The main purpose is to verify that closing any of these overlay components (for example, by pressing the
+        <code>Escape</code> key) does not accidentally close the parent dialog.
+      </section>
+      <sl-button @click=${onClick}>Open dialog</sl-button>
+    `;
+  }
+};

packages/components/dialog/src/dialog.ts

@@ -82,8 +82,9 @@ export class Dialog extends ScopedElementsMixin(LitElement) {
   @property({ attribute: 'dialog-role' }) dialogRole: 'dialog' | 'alertdialog' = 'dialog';
 
   /**
-   * Disables the ability to cancel the dialog by pressing the Escape key
-   * or clicking on the backdrop.
+   * Disables the ability to cancel the dialog by pressing the Escape key or clicking on the backdrop.
+   * We recommend setting this to true when the dialog contains a form that must be submitted or cancelled,
+   * to prevent accidental closing when clicking on the backdrop.
    * @default false
    */
   @property({ type: Boolean, attribute: 'disable-cancel' }) disableCancel?: boolean;
@@ -257,7 +258,11 @@ export class Dialog extends ScopedElementsMixin(LitElement) {
   }
 
   #onBackdropClick(event: MouseEvent): void {
-    const rect = this.dialog!.getBoundingClientRect();
+    if (this.dialog !== event.composedPath()[0]) {
+      return;
+    }
+
+    const rect = this.dialog.getBoundingClientRect();
 
     // Check if the user clicked on the backdrop
     if (

packages/components/menu/src/menu-button.ts

@@ -132,6 +132,7 @@ export class MenuButton extends ObserveAttributesMixin(ScopedElementsMixin(LitEl
       </sl-button>
       <sl-menu
         @click=${this.#onMenuClick}
+        @keydown=${this.#onKeydownMenu}
         @toggle=${this.#onToggle}
         @sl-select=${this.#onSelect}
         .position=${this.position ?? 'bottom-start'}
@@ -147,7 +148,11 @@ export class MenuButton extends ObserveAttributesMixin(ScopedElementsMixin(LitEl
   }
 
   #onKeydown(event: KeyboardEvent): void {
-    if (this.#popoverState !== 'open' && event.key === 'ArrowDown') {
+    if (event.key === 'Escape') {
+      // Prevents the Escape key event from bubbling up, so that pressing 'Escape' inside the menu
+      // does not close parent containers (such as dialogs).
+      event.stopPropagation();
+    } else if (this.#popoverState !== 'open' && event.key === 'ArrowDown') {
       this.menu.showPopover();
       this.menu.focus();
     } else {
@@ -161,6 +166,14 @@ export class MenuButton extends ObserveAttributesMixin(ScopedElementsMixin(LitEl
     }
   }
 
+  #onKeydownMenu(event: KeyboardEvent): void {
+    if (event.key === 'Escape') {
+      // Prevents the Escape key event from bubbling up, so that pressing 'Escape' inside the menu
+      // does not close parent containers (such as dialogs).
+      event.stopPropagation();
+    }
+  }
+
   #onMenuClick(event: Event & { target: HTMLElement }): void {
     // Only hide the menu if the user clicked on a menu item
     if (event.composedPath().find(el => el instanceof MenuItem)) {

packages/components/menu/src/menu.ts

@@ -139,7 +139,11 @@ export class Menu extends LitElement {
 
     const placement = this.getAttribute('actual-placement');
 
-    if (
+    if (event.key === 'Escape') {
+      // Prevents the Escape key event from bubbling up, so that pressing 'Escape' inside the menu
+      // does not close parent containers (such as dialogs).
+      event.stopPropagation();
+    } else if (
       (placement?.startsWith('right') && event.key === 'ArrowLeft') ||
       (placement?.startsWith('left') && event.key === 'ArrowRight')
     ) {