refactor: restore focus for popover click and focus triggers

packages/popover/src/vaadin-popover.js

@@ -127,6 +127,13 @@ class Popover extends PopoverPositionMixin(
         type: Boolean,
         value: false,
       },
+
+      /** @private */
+      __shouldRestoreFocus: {
+        type: Boolean,
+        value: false,
+        sync: true,
+      },
     };
   }
 
@@ -165,9 +172,11 @@ class Popover extends PopoverPositionMixin(
         @focusin="${this.__onOverlayFocusIn}"
         @focusout="${this.__onOverlayFocusOut}"
         @opened-changed="${this.__onOpenedChanged}"
+        .restoreFocusOnClose="${this.__shouldRestoreFocus}"
         .restoreFocusNode="${this.target}"
         @vaadin-overlay-escape-press="${this.__onEscapePress}"
         @vaadin-overlay-outside-click="${this.__onOutsideClick}"
+        @vaadin-overlay-closed="${this.__onOverlayClosed}"
       ></vaadin-popover-overlay>
     `;
   }
@@ -257,6 +266,9 @@ class Popover extends PopoverPositionMixin(
   /** @private */
   __onTargetClick() {
     if (this.__hasTrigger('click')) {
+      if (!this.opened) {
+        this.__shouldRestoreFocus = true;
+      }
       this.opened = !this.opened;
     }
   }
@@ -268,6 +280,11 @@ class Popover extends PopoverPositionMixin(
       event.stopPropagation();
       this.opened = false;
     }
+
+    // Prevent restoring focus after target blur on Tab key
+    if (event.key === 'Tab' && this.__shouldRestoreFocus) {
+      this.__shouldRestoreFocus = false;
+    }
   }
 
   /** @private */
@@ -282,7 +299,11 @@ class Popover extends PopoverPositionMixin(
         return;
       }
 
-      this.opened = true;
+      // Prevent overlay re-opening when restoring focus on close.
+      if (!this.__shouldRestoreFocus) {
+        this.__shouldRestoreFocus = true;
+        this.opened = true;
+      }
     }
   }
 
@@ -299,7 +320,11 @@ class Popover extends PopoverPositionMixin(
   __onTargetMouseEnter() {
     this.__hoverInside = true;
 
-    if (this.__hasTrigger('hover')) {
+    if (this.__hasTrigger('hover') && !this.opened) {
+      // Prevent closing due to `pointer-events: none` set on body.
+      if (this.modal) {
+        this.target.style.pointerEvents = 'auto';
+      }
       this.opened = true;
     }
   }
@@ -316,6 +341,12 @@ class Popover extends PopoverPositionMixin(
   /** @private */
   __onOverlayFocusIn() {
     this.__focusInside = true;
+
+    // When using Tab to move focus, restoring focus is reset. However, if pressing Tab
+    // causes focus to be moved inside the overlay, we should restore focus on close.
+    if ((this.__hasTrigger('focus') || this.__hasTrigger('click')) && !this.__shouldRestoreFocus) {
+      this.__shouldRestoreFocus = true;
+    }
   }
 
   /** @private */
@@ -372,6 +403,22 @@ class Popover extends PopoverPositionMixin(
     this.opened = event.detail.value;
   }
 
+  /** @private */
+  __onOverlayClosed() {
+    // Reset restoring focus state after a timeout to make sure focus was restored
+    // and then allow re-opening overlay on re-focusing target with focus trigger.
+    if (this.__shouldRestoreFocus) {
+      setTimeout(() => {
+        this.__shouldRestoreFocus = false;
+      });
+    }
+
+    // Restore pointer-events set when opening on hover.
+    if (this.modal && this.target.style.pointerEvents) {
+      this.target.style.pointerEvents = '';
+    }
+  }
+
   /**
    * Close the popover if `noCloseOnEsc` isn't set to true.
    * @private

packages/popover/test/a11y.test.js

@@ -0,0 +1,210 @@
+import { expect } from '@esm-bundle/chai';
+import { esc, fixtureSync, focusout, nextRender, nextUpdate, outsideClick, tab } from '@vaadin/testing-helpers';
+import sinon from 'sinon';
+import './not-animated-styles.js';
+import '../vaadin-popover.js';
+import { mouseenter, mouseleave } from './helpers.js';
+
+describe('a11y', () => {
+  let popover, target, overlay;
+
+  beforeEach(async () => {
+    popover = fixtureSync('<vaadin-popover></vaadin-popover>');
+    target = fixtureSync('<button>Target</button>');
+    popover.target = target;
+    popover.renderer = (root) => {
+      if (!root.firstChild) {
+        root.appendChild(document.createElement('input'));
+      }
+    };
+    await nextRender();
+    overlay = popover.shadowRoot.querySelector('vaadin-popover-overlay');
+  });
+
+  describe('focus restoration', () => {
+    describe('focus trigger', () => {
+      beforeEach(async () => {
+        popover.trigger = ['focus'];
+        await nextUpdate(popover);
+
+        target.focus();
+        await nextRender();
+      });
+
+      it('should restore focus on Esc with trigger set to focus', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+
+      it('should not restore focus on Tab with trigger set to focus', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        tab(target);
+        focusout(target);
+        await nextRender();
+
+        expect(focusSpy).to.not.be.called;
+      });
+
+      it('should restore focus on close after Tab to overlay with trigger set to focus', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        tab(target);
+        focusout(target, overlay);
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+
+      it('should not re-open when restoring focus on Esc with trigger set to focus', async () => {
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(popover.opened).to.be.false;
+      });
+
+      it('should not re-open when restoring focus on outside click with trigger set to focus', async () => {
+        overlay.$.overlay.focus();
+        outsideClick();
+        await nextRender();
+
+        expect(popover.opened).to.be.false;
+      });
+
+      it('should re-open when re-focusing after closing on outside click with trigger set to focus', async () => {
+        overlay.$.overlay.focus();
+        outsideClick();
+        await nextRender();
+
+        target.blur();
+        target.focus();
+        await nextRender();
+
+        expect(popover.opened).to.be.true;
+      });
+    });
+
+    describe('click trigger', () => {
+      beforeEach(async () => {
+        popover.trigger = ['click'];
+        await nextUpdate(popover);
+
+        target.click();
+        await nextRender();
+      });
+
+      it('should restore focus on Esc with trigger set to click', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+
+      it('should restore focus on outside click with trigger set to click', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        outsideClick();
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+
+      it('should restore focus on close after Tab to overlay with trigger set to click', async () => {
+        const focusSpy = sinon.spy(target, 'focus');
+        tab(target);
+        focusout(target, overlay);
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+    });
+
+    describe('hover trigger', () => {
+      beforeEach(async () => {
+        popover.trigger = ['hover'];
+        await nextUpdate(popover);
+      });
+
+      it('should not restore focus on Esc with trigger set to hover', async () => {
+        mouseenter(target);
+        await nextRender();
+
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.not.be.called;
+      });
+
+      it('should set pointer-events: auto on the target when opened if modal', async () => {
+        popover.modal = true;
+        await nextUpdate(popover);
+
+        mouseenter(target);
+        await nextRender();
+
+        expect(target.style.pointerEvents).to.equal('auto');
+      });
+
+      it('should remove pointer-events on the target when closed if modal', async () => {
+        popover.modal = true;
+        await nextUpdate(popover);
+
+        mouseenter(target);
+        await nextRender();
+
+        mouseleave(target);
+        await nextRender();
+
+        expect(target.style.pointerEvents).to.equal('');
+      });
+    });
+
+    describe('hover and focus trigger', () => {
+      beforeEach(async () => {
+        popover.trigger = ['hover', 'focus'];
+        await nextUpdate(popover);
+
+        target.focus();
+        await nextRender();
+      });
+
+      it('should not restore focus when re-opening on hover after being restored', async () => {
+        outsideClick();
+        await nextRender();
+
+        // Re-open overlay
+        mouseenter(target);
+        await nextRender();
+
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.not.be.called;
+      });
+
+      it('should restore focus when hover occurs after opening on focus', async () => {
+        mouseenter(target);
+
+        const focusSpy = sinon.spy(target, 'focus');
+        overlay.$.overlay.focus();
+        esc(overlay.$.overlay);
+        await nextRender();
+
+        expect(focusSpy).to.be.calledOnce;
+      });
+    });
+  });
+});

packages/popover/test/helpers.js

@@ -0,0 +1,10 @@
+import { fire } from '@vaadin/testing-helpers';
+
+export function mouseenter(target) {
+  fire(target, 'mouseenter');
+}
+
+export function mouseleave(target, relatedTarget) {
+  const eventProps = relatedTarget ? { relatedTarget } : {};
+  fire(target, 'mouseleave', undefined, eventProps);
+}

packages/popover/test/trigger.test.js

@@ -11,19 +11,11 @@ import {
 } from '@vaadin/testing-helpers';
 import './not-animated-styles.js';
 import '../vaadin-popover.js';
+import { mouseenter, mouseleave } from './helpers.js';
 
 describe('trigger', () => {
   let popover, target, overlay;
 
-  function mouseenter(target) {
-    fire(target, 'mouseenter');
-  }
-
-  function mouseleave(target, relatedTarget) {
-    const eventProps = relatedTarget ? { relatedTarget } : {};
-    fire(target, 'mouseleave', undefined, eventProps);
-  }
-
   beforeEach(async () => {
     popover = fixtureSync('<vaadin-popover></vaadin-popover>');
     target = fixtureSync('<button>Target</button>');