[EuiPopover][EuiContextMenu] Attempt to manually restore focus to the…

… toggling button on popover close if focus becomes stranded (#5760)

* Upgrade `tabbable` to latest to get `focusable` API

+ update import/fix types

* Update popovers to manually attempt to always focus back onto the toggle button on trap deactivation

* Update downstream snapshots

* Add changelog entry

* Move `refocusButtonOnClose` logic to only occur on escape key

+ improve unit tests to use .invoke()

* Narrow down scope of bugfix further

- Instead of always running this logic, wait the popover to finish closing and check for stranded document.activeElement first before manually restoring focus

* hello snapshots my old friend
i've come to update you again
because a code change softly creeping
left its diffs while I was sleeping

* Tweak changelog entry

* Add clearTimeout for unmount/open of stranded focus logic

+ improve listener cleanup unit tests

* [PR feedback] fix accidental deletion

package.json

@@ -91,7 +91,7 @@
     "remark-emoji": "^2.1.0",
     "remark-parse": "^8.0.3",
     "remark-rehype": "^8.0.0",
-    "tabbable": "^3.0.0",
+    "tabbable": "^5.2.1",
     "text-diff": "^1.0.1",
     "unified": "^9.2.0",
     "unist-util-visit": "^2.0.3",
@@ -133,7 +133,7 @@
     "@types/react-dom": "^17.0.11",
     "@types/react-is": "^17.0.3",
     "@types/react-router-dom": "^5.1.5",
-    "@types/tabbable": "^3.1.0",
+    "@types/tabbable": "^3.1.2",
     "@types/url-parse": "^1.4.8",
     "@types/uuid": "^8.3.0",
     "@typescript-eslint/eslint-plugin": "^5.10.2",

src/components/context_menu/context_menu_panel.tsx

@@ -14,7 +14,7 @@ import React, {
   ReactNode,
 } from 'react';
 import classNames from 'classnames';
-import tabbable from 'tabbable';
+import { tabbable } from 'tabbable';
 
 import { CommonProps, NoArgCallback, keysOf } from '../common';
 import { EuiIcon } from '../icon';

src/components/datagrid/body/data_grid_cell.tsx

@@ -18,7 +18,7 @@ import React, {
   MutableRefObject,
 } from 'react';
 import { createPortal } from 'react-dom';
-import tabbable from 'tabbable';
+import { tabbable } from 'tabbable';
 import { keys } from '../../../services';
 import { EuiScreenReaderOnly } from '../../accessibility';
 import { EuiFocusTrap } from '../../focus_trap';

src/components/datagrid/body/header/data_grid_header_cell_wrapper.tsx

@@ -14,7 +14,7 @@ import React, {
   useRef,
   useState,
 } from 'react';
-import tabbable from 'tabbable';
+import { tabbable } from 'tabbable';
 import { keys } from '../../../../services';
 import { DataGridFocusContext } from '../../utils/focus';
 import { EuiDataGridHeaderCellWrapperProps } from '../../data_grid_types';

src/components/datagrid/body/header/header_is_interactive.ts

@@ -7,7 +7,7 @@
  */
 
 import { useCallback, useEffect, useState } from 'react';
-import tabbable from 'tabbable';
+import { tabbable } from 'tabbable';
 
 export const useHeaderIsInteractive = (gridElement: HTMLElement | null) => {
   const [headerIsInteractive, setHeaderIsInteractive] = useState(false);

src/components/datagrid/utils/focus.ts

@@ -19,7 +19,7 @@ import {
   MutableRefObject,
 } from 'react';
 import { GridOnItemsRenderedProps } from 'react-window';
-import tabbable from 'tabbable';
+import { tabbable } from 'tabbable';
 import { keys } from '../../../services';
 import {
   DataGridFocusContextShape,

src/components/popover/input_popover.tsx

@@ -14,7 +14,7 @@ import React, {
   useCallback,
 } from 'react';
 import classnames from 'classnames';
-import tabbable from 'tabbable';
+import { tabbable, FocusableElement } from 'tabbable';
 
 import { CommonProps } from '../common';
 import { EuiFocusTrap } from '../focus_trap';
@@ -81,7 +81,7 @@ export const EuiInputPopover: FunctionComponent<EuiInputPopoverProps> = ({
 
   const onKeyDown = (event: React.KeyboardEvent) => {
     if (panelEl && event.key === cascadingMenuKeys.TAB) {
-      const tabbableItems = tabbable(panelEl).filter((el: HTMLElement) => {
+      const tabbableItems = tabbable(panelEl).filter((el: FocusableElement) => {
         return (
           Array.from(el.attributes)
             .map((el) => el.name)

src/components/popover/popover.test.tsx

@@ -9,6 +9,7 @@
 import React, { ReactNode } from 'react';
 import { render, mount } from 'enzyme';
 import { requiredProps } from '../../test/required_props';
+import { EuiFocusTrap } from '../';
 
 import {
   EuiPopover,
@@ -383,33 +384,36 @@ describe('EuiPopover', () => {
   });
 
   describe('listener cleanup', () => {
-    let _raf: typeof window['requestAnimationFrame'];
-    let _caf: typeof window['cancelAnimationFrame'];
+    let rafSpy: jest.SpyInstance;
+    let cafSpy: jest.SpyInstance;
+    const activeAnimationFrames = new Map<number, number>();
+    let nextAnimationFrameId = 0;
+
     beforeAll(() => {
       jest.useFakeTimers();
-      _raf = window.requestAnimationFrame;
-      _caf = window.cancelAnimationFrame;
-
-      const activeAnimationFrames = new Map<number, number>();
-      let nextAnimationFrameId = 0;
-      window.requestAnimationFrame = (fn) => {
-        const animationFrameId = nextAnimationFrameId++;
-        activeAnimationFrames.set(animationFrameId, setTimeout(fn));
-        return animationFrameId;
-      };
-      window.cancelAnimationFrame = (id: number) => {
-        const timeoutId = activeAnimationFrames.get(id);
-        if (timeoutId) {
-          clearTimeout(timeoutId);
-          activeAnimationFrames.delete(id);
-        }
-      };
+      jest.spyOn(window, 'clearTimeout');
+      rafSpy = jest
+        .spyOn(window, 'requestAnimationFrame')
+        .mockImplementation((fn) => {
+          const animationFrameId = nextAnimationFrameId++;
+          activeAnimationFrames.set(animationFrameId, setTimeout(fn));
+          return animationFrameId;
+        });
+      cafSpy = jest
+        .spyOn(window, 'cancelAnimationFrame')
+        .mockImplementation((id: number) => {
+          const timeoutId = activeAnimationFrames.get(id);
+          if (timeoutId) {
+            clearTimeout(timeoutId);
+            activeAnimationFrames.delete(id);
+          }
+        });
     });
 
     afterAll(() => {
       jest.useRealTimers();
-      window.requestAnimationFrame = _raf;
-      window.cancelAnimationFrame = _caf;
+      rafSpy.mockRestore();
+      cafSpy.mockRestore();
     });
 
     it('cleans up timeouts and rAFs on unmount', () => {
@@ -422,10 +426,21 @@ describe('EuiPopover', () => {
           isOpen={false}
         />
       );
+      expect(window.clearTimeout).toHaveBeenCalledTimes(0);
 
       component.setProps({ isOpen: true });
+      expect(window.clearTimeout).toHaveBeenCalledTimes(3);
+      expect(rafSpy).toHaveBeenCalledTimes(1);
+      expect(activeAnimationFrames.size).toEqual(1);
+
+      jest.advanceTimersByTime(10);
+      expect(rafSpy).toHaveBeenCalledTimes(2);
+      expect(activeAnimationFrames.size).toEqual(2);
 
       component.unmount();
+      expect(window.clearTimeout).toHaveBeenCalledTimes(10);
+      expect(cafSpy).toHaveBeenCalledTimes(2);
+      expect(activeAnimationFrames.size).toEqual(0);
 
       // EUI's jest configuration throws an error if there are any console.error calls, like
       // React's setState on an unmounted component warning
@@ -436,7 +451,89 @@ describe('EuiPopover', () => {
 
       // execute any pending timeouts or animation frame callbacks
       // and validate the timeout/rAF clearing done by EuiPopover
-      jest.advanceTimersByTime(10);
+      jest.advanceTimersByTime(300);
+    });
+  });
+
+  describe('onEscapeKey', () => {
+    const closePopover = jest.fn();
+    const closingTransitionTime = 250; // TODO: DRY out var when converting to CSS-in-JS
+
+    const mockEvent = {
+      preventDefault: () => {},
+      stopPropagation: () => {},
+    } as Event;
+
+    beforeAll(() => jest.useFakeTimers());
+    beforeEach(() => {
+      jest.clearAllMocks();
+      (document.activeElement as HTMLElement)?.blur(); // Reset focus between tests
+    });
+    afterAll(() => jest.useRealTimers());
+
+    it('closes the popover and refocuses the toggle button', () => {
+      const toggleButtonEl = React.createRef<HTMLButtonElement>();
+      const toggleButton = <button ref={toggleButtonEl} />;
+
+      const component = mount(
+        <EuiPopover
+          isOpen={true}
+          button={toggleButton}
+          closePopover={closePopover}
+          {...requiredProps}
+        />
+      );
+      component.find(EuiFocusTrap).invoke('onEscapeKey')!(mockEvent);
+      component.setProps({ isOpen: false });
+      jest.advanceTimersByTime(closingTransitionTime);
+
+      expect(closePopover).toHaveBeenCalled();
+      expect(document.activeElement).toEqual(toggleButtonEl.current);
+    });
+
+    it('refocuses the first nested toggle button on focus trap deactivation', () => {
+      const toggleButtonEl = React.createRef<HTMLButtonElement>();
+      const toggleDiv = (
+        <div>
+          <button ref={toggleButtonEl} tabIndex={-1} />
+          <button tabIndex={-1} />
+        </div>
+      );
+
+      const component = mount(
+        <EuiPopover
+          isOpen={true}
+          button={toggleDiv}
+          closePopover={closePopover}
+          {...requiredProps}
+        />
+      );
+      component.find(EuiFocusTrap).invoke('onEscapeKey')!(mockEvent);
+      component.setProps({ isOpen: false });
+      jest.advanceTimersByTime(closingTransitionTime);
+
+      expect(closePopover).toHaveBeenCalled();
+      expect(document.activeElement).toEqual(toggleButtonEl.current);
+    });
+
+    it('does not refocus if the toggle button is not focusable', () => {
+      const toggleDivEl = React.createRef<HTMLDivElement>();
+      const toggleDiv = <div ref={toggleDivEl} />;
+
+      const component = mount(
+        <EuiPopover
+          button={toggleDiv}
+          isOpen={true}
+          closePopover={closePopover}
+          {...requiredProps}
+        />
+      );
+      component.find(EuiFocusTrap).invoke('onEscapeKey')!(mockEvent);
+      component.setProps({ isOpen: false });
+      jest.advanceTimersByTime(closingTransitionTime);
+
+      expect(closePopover).toHaveBeenCalled();
+      expect(document.activeElement).not.toEqual(toggleDivEl.current);
     });
   });
 });

src/components/popover/popover.tsx

@@ -16,7 +16,7 @@ import React, {
   RefCallback,
 } from 'react';
 import classNames from 'classnames';
-import tabbable from 'tabbable';
+import { tabbable, focusable } from 'tabbable';
 
 import { CommonProps, NoArgCallback } from '../common';
 import { FocusTarget, EuiFocusTrap, EuiFocusTrapProps } from '../focus_trap';
@@ -281,6 +281,7 @@ function getElementFromInitialFocus(
 }
 
 const returnFocusConfig = { preventScroll: true };
+const closingTransitionTime = 250; // TODO: DRY out var when converting to CSS-in-JS
 
 export type Props = CommonProps &
   HTMLAttributes<HTMLDivElement> &
@@ -346,6 +347,7 @@ export class EuiPopover extends Component<Props, State> {
   }
 
   private respositionTimeout: number | undefined;
+  private strandedFocusTimeout: number | undefined;
   private closingTransitionTimeout: number | undefined;
   private closingTransitionAnimationFrame: number | undefined;
   private updateFocusAnimationFrame: number | undefined;
@@ -383,9 +385,26 @@ export class EuiPopover extends Component<Props, State> {
       event.preventDefault();
       event.stopPropagation();
       this.closePopover();
+      this.handleStrandedFocus();
     }
   };
 
+  handleStrandedFocus = () => {
+    this.strandedFocusTimeout = window.setTimeout(() => {
+      // If `returnFocus` failed and focus was stranded on the body,
+      // attempt to manually restore focus to the toggle button
+      if (document.activeElement === document.body) {
+        if (!this.button) return;
+
+        const focusableItems = focusable(this.button);
+        if (!focusableItems.length) return;
+
+        const toggleButton = focusableItems[0];
+        toggleButton.focus(returnFocusConfig);
+      }
+    }, closingTransitionTime);
+  };
+
   onKeyDown = (event: KeyboardEvent) => {
     if (event.key === cascadingMenuKeys.ESCAPE) {
       this.onEscapeKey((event as unknown) as Event);
@@ -463,6 +482,7 @@ export class EuiPopover extends Component<Props, State> {
   }
 
   onOpenPopover = () => {
+    clearTimeout(this.strandedFocusTimeout);
     clearTimeout(this.closingTransitionTimeout);
     if (this.closingTransitionAnimationFrame) {
       cancelAnimationFrame(this.closingTransitionAnimationFrame);
@@ -541,13 +561,14 @@ export class EuiPopover extends Component<Props, State> {
         this.setState({
           isClosing: false,
         });
-      }, 250);
+      }, closingTransitionTime);
     }
   }
 
   componentWillUnmount() {
     window.removeEventListener('scroll', this.positionPopoverFixed, true);
     clearTimeout(this.respositionTimeout);
+    clearTimeout(this.strandedFocusTimeout);
     clearTimeout(this.closingTransitionTimeout);
     cancelAnimationFrame(this.closingTransitionAnimationFrame!);
     cancelAnimationFrame(this.updateFocusAnimationFrame!);

upcoming_changelogs/5760.md

@@ -0,0 +1,3 @@
+**Bug fixes**
+
+- Fixed keyboard focus being stranded on `EuiContextMenu` popover close

yarn.lock

@@ -3465,10 +3465,10 @@
   resolved "https://registry.yarnpkg.com/@types/stack-utils/-/stack-utils-1.0.1.tgz#0a851d3bd96498fa25c33ab7278ed3bd65f06c3e"
   integrity sha512-l42BggppR6zLmpfU6fq9HEa2oGPEI8yrSPL3GITjfRInppYFahObbIQOQK3UGxEnyQpltZLaPe75046NOZQikw==
 
-"@types/tabbable@^3.1.0":
-  version "3.1.0"
-  resolved "https://registry.yarnpkg.com/@types/tabbable/-/tabbable-3.1.0.tgz#540d4c2729872560badcc220e73c9412c1d2bffe"
-  integrity sha512-LL0q/bTlzseaXQ8j91eZ+Z8FQUzo0nwkng00B8365qULvFyiSOWylxV8m31Gmee3QuidkDqR72a9NRfR8s4qTw==
+"@types/tabbable@^3.1.2":
+  version "3.1.2"
+  resolved "https://registry.yarnpkg.com/@types/tabbable/-/tabbable-3.1.2.tgz#5046f043fef50961d7727920b0076b37737e31ad"
+  integrity sha512-Yp+M5IjNZxYjsflBbSalyjUAIqiJyEISg++gLAstGrZlp9lzVi5KAsZvJqThT2qeoqGYnFqdZXorPEYtaVBAkg==
 
 "@types/tapable@*", "@types/tapable@^1.0.5":
   version "1.0.6"
@@ -18843,10 +18843,10 @@ syntax-error@^1.1.1:
   dependencies:
     acorn-node "^1.2.0"
 
-tabbable@^3.0.0:
-  version "3.1.2"
-  resolved "https://registry.yarnpkg.com/tabbable/-/tabbable-3.1.2.tgz#f2d16cccd01f400e38635c7181adfe0ad965a4a2"
-  integrity sha512-wjB6puVXTYO0BSFtCmWQubA/KIn7Xvajw0x0l6eJUudMG/EAiJvIUnyNX6xO4NpGrJ16lbD0eUseB9WxW0vlpQ==
+tabbable@^5.2.1:
+  version "5.2.1"
+  resolved "https://registry.yarnpkg.com/tabbable/-/tabbable-5.2.1.tgz#e3fda7367ddbb172dcda9f871c0fdb36d1c4cd9c"
+  integrity sha512-40pEZ2mhjaZzK0BnI+QGNjJO8UYx9pP5v7BGe17SORTO0OEuuaAwQTkAp8whcZvqon44wKFOikD+Al11K3JICQ==
 
 table@^3.7.8:
   version "3.8.3"