refactor: set aria-modal and tabindex on the confirm-dialog (#9980)

web-padawan · web-flow · commit 8c529b8fedc8 · 2025-08-15T11:40:35.000+03:00
diff --git a/dev/confirm-dialog.html b/dev/confirm-dialog.html
@@ -12,8 +12,11 @@
     </script>
   </head>
   <body>
-    <vaadin-button class="short">Short Message</vaadin-button>
-    <vaadin-button class="long">Long Message</vaadin-button>
+    <section>
+      <h3>Examples</h3>
+      <vaadin-button class="short">Short Message</vaadin-button>
+      <vaadin-button class="long">Long Message</vaadin-button>
+    </section>
 
     <vaadin-confirm-dialog class="short" header="Save Changes" confirm-text="Save" cancel-button-visible>
       Save or not?
diff --git a/packages/confirm-dialog/src/vaadin-confirm-dialog-mixin.js b/packages/confirm-dialog/src/vaadin-confirm-dialog-mixin.js
@@ -246,6 +246,8 @@ export const ConfirmDialogMixin = (superClass) =>
       super.ready();
 
       this.role = 'alertdialog';
+      this.setAttribute('aria-modal', 'true');
+      this.setAttribute('tabindex', '0');
 
       this._headerController = new SlotController(this, 'header', 'h3', {
         initializer: (node) => {
diff --git a/packages/confirm-dialog/src/vaadin-confirm-dialog-overlay.js b/packages/confirm-dialog/src/vaadin-confirm-dialog-overlay.js
@@ -49,7 +49,7 @@ class ConfirmDialogOverlay extends OverlayMixin(DirMixin(ThemableMixin(PolylitMi
   render() {
     return html`
       <div part="backdrop" id="backdrop" ?hidden="${!this.withBackdrop}"></div>
-      <div part="overlay" id="overlay" tabindex="0">
+      <div part="overlay" id="overlay">
         <header part="header"><slot name="header"></slot></header>
         <div part="content" id="content">
           <div part="message"><slot></slot></div>
@@ -98,6 +98,24 @@ class ConfirmDialogOverlay extends OverlayMixin(DirMixin(ThemableMixin(PolylitMi
   get _modalRoot() {
     return this.owner;
   }
+
+  /**
+   * Override method from OverlayFocusMixin to use owner as focus trap root
+   * @protected
+   * @override
+   */
+  get _focusTrapRoot() {
+    return this.owner;
+  }
+
+  /**
+   * Override method from OverlayFocusMixin to not set `aria-hidden`
+   * @protected
+   * @override
+   */
+  get _useAriaHidden() {
+    return false;
+  }
 }
 
 defineCustomElement(ConfirmDialogOverlay);
diff --git a/packages/confirm-dialog/src/vaadin-confirm-dialog.js b/packages/confirm-dialog/src/vaadin-confirm-dialog.js
@@ -75,13 +75,18 @@ class ConfirmDialog extends ConfirmDialogMixin(ElementMixin(ThemePropertyMixin(P
       :host([opened]),
       :host([opening]),
       :host([closing]) {
-        display: contents !important;
+        display: block !important;
+        position: absolute;
       }
 
       :host,
       :host([hidden]) {
         display: none !important;
       }
+
+      :host(:focus) ::part(overlay) {
+        outline: var(--vaadin-focus-ring-width) solid var(--vaadin-focus-ring-color);
+      }
     `;
   }
 
diff --git a/packages/confirm-dialog/test/confirm-dialog.test.js b/packages/confirm-dialog/test/confirm-dialog.test.js
@@ -36,14 +36,15 @@ describe('vaadin-confirm-dialog', () => {
     });
 
     ['opened', 'opening', 'closing'].forEach((state) => {
-      it(`should use display: contents when ${state} attribute is set`, () => {
+      it(`should use display: block when ${state} attribute is set`, () => {
         confirm.setAttribute(state, '');
-        expect(getComputedStyle(confirm).display).to.equal('contents');
+        expect(getComputedStyle(confirm).display).to.equal('block');
       });
     });
 
     it('should use display: none when hidden while opened', async () => {
       confirm.opened = true;
+      await oneEvent(confirm.$.overlay, 'vaadin-overlay-open');
       confirm.hidden = true;
       await nextRender();
       expect(getComputedStyle(confirm).display).to.equal('none');
diff --git a/packages/confirm-dialog/test/dom/__snapshots__/confirm-dialog.test.snap.js b/packages/confirm-dialog/test/dom/__snapshots__/confirm-dialog.test.snap.js
@@ -5,9 +5,11 @@ snapshots["vaadin-confirm-dialog host"] =
 `<vaadin-confirm-dialog
   aria-description="Do you want to save or discard the changes?"
   aria-label="Unsaved changes"
+  aria-modal="true"
   header="Unsaved changes"
   opened=""
   role="alertdialog"
+  tabindex="0"
   with-backdrop=""
 >
   Do you want to save or discard the changes?
@@ -131,7 +133,6 @@ snapshots["vaadin-confirm-dialog overlay"] =
 <div
   id="overlay"
   part="overlay"
-  tabindex="0"
 >
   <header part="header">
     <slot name="header">
diff --git a/packages/crud/test/dom/__snapshots__/crud.test.snap.js b/packages/crud/test/dom/__snapshots__/crud.test.snap.js
@@ -9,10 +9,12 @@ snapshots["vaadin-crud host default"] =
   <vaadin-confirm-dialog
     aria-description="There are unsaved changes to this item."
     aria-label="Discard changes"
+    aria-modal="true"
     cancel-button-visible=""
     confirm-theme="primary"
     role="alertdialog"
     slot="confirm-cancel"
+    tabindex="0"
     with-backdrop=""
   >
     <h3 slot="header">
@@ -50,10 +52,12 @@ snapshots["vaadin-crud host default"] =
   <vaadin-confirm-dialog
     aria-description="Are you sure you want to delete this item? This action cannot be undone."
     aria-label="Delete item"
+    aria-modal="true"
     cancel-button-visible=""
     confirm-theme="primary error"
     role="alertdialog"
     slot="confirm-delete"
+    tabindex="0"
     with-backdrop=""
   >
     <h3 slot="header">
diff --git a/packages/overlay/src/vaadin-overlay-focus-mixin.js b/packages/overlay/src/vaadin-overlay-focus-mixin.js
@@ -80,14 +80,34 @@ export const OverlayFocusMixin = (superClass) =>
       return this;
     }
 
+    /**
+     * Override to specify another element used as a focus trap root,
+     * e.g. the overlay's owner element, rather than overlay part.
+     * @protected
+     */
+    get _focusTrapRoot() {
+      return this.$.overlay;
+    }
+
+    /**
+     * Override not use a controller for setting `aria-hidden` on
+     * elements outside the overlay, e.g. when using `aria-modal`.
+     * @protected
+     */
+    _useAriaHidden() {
+      return true;
+    }
+
     /**
      * Release focus and restore focus after the overlay is closed.
      *
      * @protected
      */
     _resetFocus() {
       if (this.focusTrap) {
-        this.__ariaModalController.close();
+        if (this._useAriaHidden) {
+          this.__ariaModalController.close();
+        }
         this.__focusTrapController.releaseFocus();
       }
 
@@ -115,8 +135,10 @@ export const OverlayFocusMixin = (superClass) =>
      */
     _trapFocus() {
       if (this.focusTrap) {
-        this.__ariaModalController.showModal();
-        this.__focusTrapController.trapFocus(this.$.overlay);
+        if (this._useAriaHidden) {
+          this.__ariaModalController.showModal();
+        }
+        this.__focusTrapController.trapFocus(this._focusTrapRoot);
       }
     }
 
diff --git a/packages/rich-text-editor/test/dom/__snapshots__/rich-text-editor.test.snap.js b/packages/rich-text-editor/test/dom/__snapshots__/rich-text-editor.test.snap.js
@@ -6,10 +6,12 @@ snapshots["vaadin-rich-text-editor host"] =
   <vaadin-confirm-dialog
     aria-description=""
     aria-label="Link address"
+    aria-modal="true"
     cancel-button-visible=""
     reject-theme="error"
     role="alertdialog"
     slot="link-dialog"
+    tabindex="0"
     with-backdrop=""
   >
     <vaadin-text-field style="width: 100%;">

Original file line number	Diff line number	Diff line change
`@@ -75,13 +75,18 @@ class ConfirmDialog extends ConfirmDialogMixin(ElementMixin(ThemePropertyMixin(P`
`75`	`75`	`:host([opened]),`
`76`	`76`	`:host([opening]),`
`77`	`77`	`:host([closing]) {`
`78`		`- display: contents !important;`
	`78`	`+ display: block !important;`
	`79`	`+ position: absolute;`
`79`	`80`	`}`
`80`	`81`
`81`	`82`	`:host,`
`82`	`83`	`:host([hidden]) {`
`83`	`84`	`display: none !important;`
`84`	`85`	`}`
	`86`	`+`
	`87`	`+ :host(:focus) ::part(overlay) {`
	`88`	`+ outline: var(--vaadin-focus-ring-width) solid var(--vaadin-focus-ring-color);`
	`89`	`+ }`
`85`	`90`	`;
`86`	`91`	`}`
`87`	`92`