refactor: set aria-modal and tabindex on the login overlay (#10003)

Author: web-padawan

dev/login-overlay.html

@@ -11,14 +11,18 @@
       import '@vaadin/login/vaadin-login-overlay.js';
 
       const login = document.querySelector('vaadin-login-overlay');
-      login.addEventListener('login', () => {
-        login.opened = false;
-      });
 
       const button = document.querySelector('button');
       button.addEventListener('click', () => {
+        login.disabled = false;
         login.opened = true;
       });
+
+      login.addEventListener('login', () => {
+        login.opened = false;
+        // Login does not restore focus
+        button.focus();
+      });
     </script>
   </head>
 

packages/login/src/vaadin-login-overlay-mixin.js

@@ -49,6 +49,8 @@ export const LoginOverlayMixin = (superClass) =>
       super.firstUpdated();
 
       this.setAttribute('role', 'dialog');
+      this.setAttribute('aria-modal', 'true');
+      this.setAttribute('tabindex', '0');
 
       this.__titleController = new TitleController(this);
       this.addController(this.__titleController);

packages/login/src/vaadin-login-overlay-wrapper.js

@@ -49,11 +49,29 @@ class LoginOverlayWrapper extends OverlayMixin(DirMixin(ThemableMixin(PolylitMix
     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;
+  }
+
   /** @protected */
   render() {
     return html`
       <div id="backdrop" part="backdrop" ?hidden="${!this.withBackdrop}"></div>
-      <div part="overlay" id="overlay" tabindex="0">
+      <div part="overlay" id="overlay">
         <div part="content" id="content">
           <section part="card">
             <div part="brand">

packages/login/src/vaadin-login-overlay.js

@@ -68,13 +68,21 @@ class LoginOverlay extends LoginFormMixin(LoginOverlayMixin(ElementMixin(Themabl
 
   static get styles() {
     return css`
-      :host {
-        display: block;
+      :host([opened]),
+      :host([opening]),
+      :host([closing]) {
+        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);
+      }
     `;
   }
 

packages/login/test/dom/__snapshots__/login-overlay.test.snap.js

@@ -4,8 +4,10 @@ export const snapshots = {};
 snapshots["vaadin-login-overlay host default"] = 
 `<vaadin-login-overlay
   aria-labelledby="title-vaadin-login-overlay-8"
+  aria-modal="true"
   opened=""
   role="dialog"
+  tabindex="0"
   with-backdrop=""
 >
   <form
@@ -126,8 +128,10 @@ snapshots["vaadin-login-overlay host default"] =
 snapshots["vaadin-login-overlay host i18n"] = 
 `<vaadin-login-overlay
   aria-labelledby="title-vaadin-login-overlay-8"
+  aria-modal="true"
   opened=""
   role="dialog"
+  tabindex="0"
   with-backdrop=""
 >
   <form
@@ -248,8 +252,10 @@ snapshots["vaadin-login-overlay host i18n"] =
 snapshots["vaadin-login-overlay host i18n-partial"] = 
 `<vaadin-login-overlay
   aria-labelledby="title-vaadin-login-overlay-8"
+  aria-modal="true"
   opened=""
   role="dialog"
+  tabindex="0"
   with-backdrop=""
 >
   <form
@@ -436,7 +442,6 @@ snapshots["vaadin-login-overlay shadow wrapper"] =
 <div
   id="overlay"
   part="overlay"
-  tabindex="0"
 >
   <div
     id="content"
@@ -469,7 +474,6 @@ snapshots["vaadin-login-overlay shadow i18n"] =
 <div
   id="overlay"
   part="overlay"
-  tabindex="0"
 >
   <div
     id="content"

packages/login/test/login-overlay.test.js

@@ -95,6 +95,26 @@ describe('login overlay', () => {
   });
 });
 
+describe('display', () => {
+  let login;
+
+  beforeEach(async () => {
+    login = fixtureSync('<vaadin-login-overlay></vaadin-login-overlay>');
+    await nextRender();
+  });
+
+  it('should use display: none when not opened', () => {
+    expect(getComputedStyle(login).display).to.equal('none');
+  });
+
+  ['opened', 'opening', 'closing'].forEach((state) => {
+    it(`should use display: block when ${state} attribute is set`, () => {
+      login.setAttribute(state, '');
+      expect(getComputedStyle(login).display).to.equal('block');
+    });
+  });
+});
+
 describe('no autofocus', () => {
   let login, overlay;
 
@@ -107,8 +127,8 @@ describe('no autofocus', () => {
   it('should not focus the username field', async () => {
     login.opened = true;
     await oneEvent(overlay, 'vaadin-overlay-open');
-    // Overlay traps focus and focuses the wrapper by default
-    expect(getDeepActiveElement()).to.equal(overlay.$.overlay);
+    // Overlay traps focus and focuses the host by default
+    expect(getDeepActiveElement()).to.equal(login);
   });
 });