From f6e299aedbf3193c21569546b158101d5341f0fb Mon Sep 17 00:00:00 2001
From: Serhii Kulykov <iamkulykov@gmail.com>
Date: Mon, 2 Sep 2024 21:19:56 +0300
Subject: [PATCH] fix: set login overlay role to dialog, use title as
 aria-label (#7723)

---
 .../src/vaadin-lit-login-overlay-wrapper.js   | 23 +-----
 .../login/src/vaadin-lit-login-overlay.js     |  1 +
 .../src/vaadin-login-overlay-wrapper-mixin.js | 71 +++++++++++++++++++
 .../login/src/vaadin-login-overlay-wrapper.js | 26 +------
 packages/login/src/vaadin-login-overlay.js    |  1 +
 .../__snapshots__/login-overlay.test.snap.js  |  6 ++
 packages/login/test/login-overlay.common.js   | 44 ++++++++++++
 7 files changed, 128 insertions(+), 44 deletions(-)
 create mode 100644 packages/login/src/vaadin-login-overlay-wrapper-mixin.js

diff --git a/packages/login/src/vaadin-lit-login-overlay-wrapper.js b/packages/login/src/vaadin-lit-login-overlay-wrapper.js
index 517d1f29b9..1d7467e58d 100644
--- a/packages/login/src/vaadin-lit-login-overlay-wrapper.js
+++ b/packages/login/src/vaadin-lit-login-overlay-wrapper.js
@@ -5,11 +5,10 @@
  */
 import { html, LitElement } from 'lit';
 import { defineCustomElement } from '@vaadin/component-base/src/define.js';
-import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
 import { PolylitMixin } from '@vaadin/component-base/src/polylit-mixin.js';
-import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';
 import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js';
 import { ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
+import { LoginOverlayWrapperMixin } from './vaadin-login-overlay-wrapper-mixin.js';
 import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles.js';
 
 /**
@@ -18,7 +17,7 @@ import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles
  * @extends HTMLElement
  * @private
  */
-class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolylitMixin(LitElement)))) {
+class LoginOverlayWrapper extends LoginOverlayWrapperMixin(ThemableMixin(PolylitMixin(LitElement))) {
   static get is() {
     return 'vaadin-login-overlay-wrapper';
   }
@@ -27,24 +26,6 @@ class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolylitMix
     return [overlayStyles, loginOverlayWrapperStyles];
   }
 
-  static get properties() {
-    return {
-      /**
-       * Title of the application.
-       */
-      title: {
-        type: String,
-      },
-
-      /**
-       * Application description. Displayed under the title.
-       */
-      description: {
-        type: String,
-      },
-    };
-  }
-
   /** @protected */
   render() {
     return html`
diff --git a/packages/login/src/vaadin-lit-login-overlay.js b/packages/login/src/vaadin-lit-login-overlay.js
index 251fdb5bd2..2d51e3ea59 100644
--- a/packages/login/src/vaadin-lit-login-overlay.js
+++ b/packages/login/src/vaadin-lit-login-overlay.js
@@ -35,6 +35,7 @@ class LoginOverlay extends LoginOverlayMixin(ElementMixin(ThemableMixin(PolylitM
         .opened="${this.opened}"
         .title="${this.title}"
         .description="${this.description}"
+        role="dialog"
         focus-trap
         with-backdrop
         theme="${ifDefined(this._theme)}"
diff --git a/packages/login/src/vaadin-login-overlay-wrapper-mixin.js b/packages/login/src/vaadin-login-overlay-wrapper-mixin.js
new file mode 100644
index 0000000000..62f921ed03
--- /dev/null
+++ b/packages/login/src/vaadin-login-overlay-wrapper-mixin.js
@@ -0,0 +1,71 @@
+/**
+ * @license
+ * Copyright (c) 2018 - 2024 Vaadin Ltd.
+ * This program is available under Apache License Version 2.0, available at https://vaadin.com/license/
+ */
+import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
+import { SlotObserver } from '@vaadin/component-base/src/slot-observer';
+import { generateUniqueId } from '@vaadin/component-base/src/unique-id-utils';
+import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';
+
+/**
+ * @polymerMixin
+ * @mixes DirMixin
+ * @mixes OverlayMixin
+ */
+export const LoginOverlayWrapperMixin = (superClass) =>
+  class LoginOverlayWrapperMixin extends OverlayMixin(DirMixin(superClass)) {
+    static get properties() {
+      return {
+        /**
+         * Title of the application.
+         */
+        title: {
+          type: String,
+          observer: '_titleChanged',
+        },
+
+        /**
+         * Application description. Displayed under the title.
+         */
+        description: {
+          type: String,
+        },
+      };
+    }
+
+    /** @protected */
+    ready() {
+      super.ready();
+
+      // Use slot observer instead of slot controller since the latter
+      // does not work well with teleporting (it removes custom title).
+      const slot = this.shadowRoot.querySelector('slot[name="title"]');
+      this._titleSlotObserver = new SlotObserver(slot, () => {
+        const title = slot.assignedElements({ flatten: true })[0];
+        if (!title) {
+          return;
+        }
+
+        // Only set ID on the custom slotted title and link it using
+        // aria-labelledby as the default title is in the shadow DOM.
+        if (title.getAttribute('part') === 'title') {
+          this.setAttribute('aria-label', this.title);
+          this.removeAttribute('aria-labelledby');
+        } else {
+          if (!title.id) {
+            title.id = `login-overlay-title-${generateUniqueId()}`;
+          }
+          this.removeAttribute('aria-label');
+          this.setAttribute('aria-labelledby', title.id);
+        }
+      });
+    }
+
+    /** @private */
+    _titleChanged(title) {
+      if (title && this.hasAttribute('aria-label')) {
+        this.setAttribute('aria-label', title);
+      }
+    }
+  };
diff --git a/packages/login/src/vaadin-login-overlay-wrapper.js b/packages/login/src/vaadin-login-overlay-wrapper.js
index ff5e9b8ff1..74a21935f3 100644
--- a/packages/login/src/vaadin-login-overlay-wrapper.js
+++ b/packages/login/src/vaadin-login-overlay-wrapper.js
@@ -5,10 +5,9 @@
  */
 import { html, PolymerElement } from '@polymer/polymer/polymer-element.js';
 import { defineCustomElement } from '@vaadin/component-base/src/define.js';
-import { DirMixin } from '@vaadin/component-base/src/dir-mixin.js';
-import { OverlayMixin } from '@vaadin/overlay/src/vaadin-overlay-mixin.js';
 import { overlayStyles } from '@vaadin/overlay/src/vaadin-overlay-styles.js';
 import { registerStyles, ThemableMixin } from '@vaadin/vaadin-themable-mixin/vaadin-themable-mixin.js';
+import { LoginOverlayWrapperMixin } from './vaadin-login-overlay-wrapper-mixin.js';
 import { loginOverlayWrapperStyles } from './vaadin-login-overlay-wrapper-styles.js';
 
 registerStyles('vaadin-login-overlay-wrapper', [overlayStyles, loginOverlayWrapperStyles], {
@@ -19,34 +18,15 @@ registerStyles('vaadin-login-overlay-wrapper', [overlayStyles, loginOverlayWrapp
  * An element used internally by `<vaadin-login-overlay>`. Not intended to be used separately.
  *
  * @extends HTMLElement
- * @mixes DirMixin
- * @mixes OverlayMixin
+ * @mixes LoginOverlayWrapperMixin
  * @mixes ThemableMixin
  * @private
  */
-class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolymerElement))) {
+class LoginOverlayWrapper extends LoginOverlayWrapperMixin(ThemableMixin(PolymerElement)) {
   static get is() {
     return 'vaadin-login-overlay-wrapper';
   }
 
-  static get properties() {
-    return {
-      /**
-       * Title of the application.
-       */
-      title: {
-        type: String,
-      },
-
-      /**
-       * Application description. Displayed under the title.
-       */
-      description: {
-        type: String,
-      },
-    };
-  }
-
   static get template() {
     return html`
       <div id="backdrop" part="backdrop" hidden$="[[!withBackdrop]]"></div>
diff --git a/packages/login/src/vaadin-login-overlay.js b/packages/login/src/vaadin-login-overlay.js
index 7a04fd14fb..ad9dabd1f4 100644
--- a/packages/login/src/vaadin-login-overlay.js
+++ b/packages/login/src/vaadin-login-overlay.js
@@ -56,6 +56,7 @@ class LoginOverlay extends LoginOverlayMixin(ElementMixin(ThemableMixin(PolymerE
       <vaadin-login-overlay-wrapper
         id="vaadinLoginOverlayWrapper"
         opened="{{opened}}"
+        role="dialog"
         focus-trap
         with-backdrop
         title="[[title]]"
diff --git a/packages/login/test/dom/__snapshots__/login-overlay.test.snap.js b/packages/login/test/dom/__snapshots__/login-overlay.test.snap.js
index c5750f724f..fd557935e0 100644
--- a/packages/login/test/dom/__snapshots__/login-overlay.test.snap.js
+++ b/packages/login/test/dom/__snapshots__/login-overlay.test.snap.js
@@ -3,9 +3,11 @@ export const snapshots = {};
 
 snapshots["vaadin-login-overlay host default"] = 
 `<vaadin-login-overlay-wrapper
+  aria-label="App name"
   focus-trap=""
   id="vaadinLoginOverlayWrapper"
   opened=""
+  role="dialog"
   with-backdrop=""
 >
   <vaadin-login-form
@@ -125,9 +127,11 @@ snapshots["vaadin-login-overlay host default"] =
 
 snapshots["vaadin-login-overlay host i18n"] = 
 `<vaadin-login-overlay-wrapper
+  aria-label="Sovelluksen nimi"
   focus-trap=""
   id="vaadinLoginOverlayWrapper"
   opened=""
+  role="dialog"
   with-backdrop=""
 >
   <vaadin-login-form
@@ -247,10 +251,12 @@ snapshots["vaadin-login-overlay host i18n"] =
 
 snapshots["vaadin-login-overlay host overlay class"] = 
 `<vaadin-login-overlay-wrapper
+  aria-label="App name"
   class="custom login-overlay"
   focus-trap=""
   id="vaadinLoginOverlayWrapper"
   opened=""
+  role="dialog"
   with-backdrop=""
 >
   <vaadin-login-form
diff --git a/packages/login/test/login-overlay.common.js b/packages/login/test/login-overlay.common.js
index 5ad98e49bb..aba817209f 100644
--- a/packages/login/test/login-overlay.common.js
+++ b/packages/login/test/login-overlay.common.js
@@ -184,6 +184,12 @@ describe('title and description', () => {
     expect(overlay.title).to.be.equal(overlay.i18n.header.title);
     expect(overlay.description).to.be.equal(overlay.i18n.header.description);
   });
+
+  it('should update aria-label when title property changes', async () => {
+    overlay.title = 'New title';
+    await nextUpdate(overlay);
+    expect(overlay.$.vaadinLoginOverlayWrapper.getAttribute('aria-label')).to.equal('New title');
+  });
 });
 
 describe('title slot', () => {
@@ -219,6 +225,44 @@ describe('title slot', () => {
     expect(titleElements.length).to.be.equal(1);
     expect(titleElements[0].textContent).to.be.equal('Teleported title');
   });
+
+  it('should link slotted title using aria-labelledby', () => {
+    const title = overlayWrapper.querySelector('[slot=title]');
+    expect(title.id).to.be.ok;
+    expect(overlayWrapper.hasAttribute('aria-label')).to.be.false;
+    expect(overlayWrapper.getAttribute('aria-labelledby')).to.equal(title.id);
+  });
+
+  it('should reset aria-labelledby when slotted title is removed', async () => {
+    overlay.opened = false;
+    await nextRender();
+
+    overlay.querySelector('[slot=title]').remove();
+
+    overlay.opened = true;
+    await nextRender();
+    expect(overlayWrapper.hasAttribute('aria-labelledby')).to.be.false;
+    expect(overlayWrapper.getAttribute('aria-label')).to.equal(overlay.title);
+  });
+
+  it('should not override custom id set on the slotted title', async () => {
+    overlay.opened = false;
+    await nextRender();
+
+    overlay.querySelector('[slot=title]').remove();
+
+    // Attach new slotted title with a custom ID
+    const title = document.createElement('h1');
+    title.id = 'custom-title';
+    title.setAttribute('slot', 'title');
+    overlay.appendChild(title);
+
+    overlay.opened = true;
+    await nextRender();
+
+    expect(overlayWrapper.getAttribute('aria-labelledby')).to.equal('custom-title');
+    expect(title.id).to.equal('custom-title');
+  });
 });
 
 describe('custom-form-area slot', () => {