Merge pull request #2255 from Shopify/popover-focus-next

[Popover ] Fixed incorrect element being focused when closed

UNRELEASED.md

@@ -4,6 +4,7 @@
 
 ### Enhancements
 
+- Updated `Popover` to focus the correct element when closed ([#2255](https://github.com/Shopify/polaris-react/pull/2255))
 - Updated the type of the `title` prop in `ChoiceList` from `string` to `ReactNode` ([#2355](https://github.com/Shopify/polaris-react/pull/2355))
 - Added `disabled` prop to `Filters` component ([2389](https://github.com/Shopify/polaris-react/pull/2389))
 

src/components/Popover/Popover.tsx

@@ -1,12 +1,11 @@
 import React from 'react';
 import {createUniqueIDFactory} from '@shopify/javascript-utilities/other';
-import {
-  focusFirstFocusableNode,
-  findFirstFocusableNode,
-} from '@shopify/javascript-utilities/focus';
+import {findFirstFocusableNode} from '@shopify/javascript-utilities/focus';
+import {focusNextFocusableNode} from '../../utilities/focus';
 
 import {PreferredPosition, PreferredAlignment} from '../PositionedOverlay';
 import {Portal} from '../Portal';
+import {portal} from '../shared';
 import {CloseSource, Pane, PopoverOverlay, Section} from './components';
 
 export {CloseSource};
@@ -126,16 +125,25 @@ export class Popover extends React.PureComponent<PopoverProps, State> {
   }
 
   private handleClose = (source: CloseSource) => {
+    const {activatorNode} = this.state;
     this.props.onClose(source);
 
     if (this.activatorContainer == null) {
       return;
     }
+
     if (
-      source === CloseSource.FocusOut ||
-      source === CloseSource.EscapeKeypress
+      (source === CloseSource.FocusOut ||
+        source === CloseSource.EscapeKeypress) &&
+      activatorNode
     ) {
-      focusFirstFocusableNode(this.activatorContainer, false);
+      const focusableActivator =
+        findFirstFocusableNode(activatorNode) ||
+        findFirstFocusableNode(this.activatorContainer) ||
+        this.activatorContainer;
+      if (!focusNextFocusableNode(focusableActivator, isInPortal)) {
+        focusableActivator.focus();
+      }
     }
   };
 
@@ -150,3 +158,14 @@ export class Popover extends React.PureComponent<PopoverProps, State> {
     this.activatorContainer = node;
   };
 }
+
+function isInPortal(element: Element) {
+  let parentElement = element.parentElement;
+
+  while (parentElement) {
+    if (parentElement.matches(portal.selector)) return false;
+    parentElement = parentElement.parentElement;
+  }
+
+  return true;
+}

src/components/Popover/tests/Popover.test.tsx

@@ -1,6 +1,8 @@
 import React, {useState, useCallback} from 'react';
 import {mountWithAppProvider, findByTestID} from 'test-utilities/legacy';
+import {mountWithApp} from 'test-utilities';
 import {Popover} from '../Popover';
+import {PopoverOverlay} from '../components';
 
 describe('<Popover />', () => {
   const spy = jest.fn();
@@ -173,4 +175,45 @@ describe('<Popover />', () => {
 
     expect(onCloseSpy).not.toHaveBeenCalled();
   });
+
+  it('focuses the next available element when the popover is closed', () => {
+    const id = 'focus-target';
+    function PopoverTest() {
+      return (
+        <React.Fragment>
+          <div>
+            <Popover active activator={<div />} onClose={noop} />
+          </div>
+          <button id={id} />
+        </React.Fragment>
+      );
+    }
+
+    const popover = mountWithApp(<PopoverTest />);
+
+    popover.find(PopoverOverlay)!.trigger('onClose', 1);
+    const focusTarget = popover.find('button', {id})!.domNode;
+
+    expect(document.activeElement).toBe(focusTarget);
+  });
+
+  it('focuses the activator when another focusable element is not available when the popover is closed', () => {
+    const id = 'activator';
+    function PopoverTest() {
+      return (
+        <React.Fragment>
+          <Popover active activator={<button id={id} />} onClose={noop} />
+        </React.Fragment>
+      );
+    }
+
+    const popover = mountWithApp(<PopoverTest />);
+
+    popover.find(PopoverOverlay)!.trigger('onClose', 1);
+    const focusTarget = popover.find('button', {id})!.domNode;
+
+    expect(document.activeElement).toBe(focusTarget);
+  });
 });
+
+function noop() {}

src/components/Portal/Portal.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 import {createPortal} from 'react-dom';
 import {createUniqueIDFactory} from '@shopify/javascript-utilities/other';
 import {ThemeContext} from '../../utilities/theme';
+import {portal} from '../shared';
 
 export interface PortalProps {
   children?: React.ReactNode;
@@ -31,7 +32,7 @@ export class Portal extends React.PureComponent<PortalProps, State> {
 
   componentDidMount() {
     this.portalNode = document.createElement('div');
-    this.portalNode.setAttribute('data-portal-id', this.portalId);
+    this.portalNode.setAttribute(portal.props[0], this.portalId);
 
     if (this.context != null) {
       /* eslint-disable babel/camelcase */

src/components/Portal/tests/Portal.test.tsx

@@ -2,6 +2,7 @@ import React from 'react';
 import {mount} from 'enzyme';
 import {mountWithAppProvider} from 'test-utilities/legacy';
 import {Portal} from '../Portal';
+import {portal} from '../../shared';
 
 jest.mock('react-dom', () => ({
   ...require.requireActual('react-dom'),
@@ -34,15 +35,15 @@ describe('<Portal />', () => {
       const idPrefix = 'test';
       mountWithAppProvider(<Portal idPrefix={idPrefix} />);
       const [, portalNode] = lastSpyCall(createPortalSpy);
-      expect(portalNode.getAttribute('data-portal-id')).toMatch(
+      expect(portalNode.getAttribute(portal.props[0])).toMatch(
         new RegExp(`^${idPrefix}-portal`),
       );
     });
 
     it('is ignored when not defined', () => {
       mountWithAppProvider(<Portal />);
       const [, portalNode] = lastSpyCall(createPortalSpy);
-      expect(portalNode.getAttribute('data-portal-id')).toMatch(/^portal/);
+      expect(portalNode.getAttribute(portal.props[0])).toMatch(/^portal/);
     });
   });
 

src/components/shared.ts

@@ -28,6 +28,11 @@ export const headerCell = {
   selector: '[data-polaris-header-cell]',
 };
 
+export const portal = {
+  props: ['data-portal-id'],
+  selector: '[data-portal-id]',
+};
+
 export const DATA_ATTRIBUTE = {
   overlay,
   layer,

src/utilities/focus.ts

@@ -1,5 +1,42 @@
+import {FOCUSABLE_SELECTOR} from '@shopify/javascript-utilities/focus';
+import {isElementInViewport} from './is-element-in-viewport';
+
+type Filter = (element: Element) => void;
+
 export function handleMouseUpByBlurring({
   currentTarget,
 }: React.MouseEvent<HTMLAnchorElement | HTMLButtonElement>) {
   currentTarget.blur();
 }
+
+export function nextFocusableNode(
+  node: HTMLElement,
+  filter?: Filter,
+): HTMLElement | Element | null {
+  const allFocusableElements = [
+    ...document.querySelectorAll(FOCUSABLE_SELECTOR),
+  ];
+  const sliceLocation = allFocusableElements.indexOf(node) + 1;
+  const focusableElementsAfterNode = allFocusableElements.slice(sliceLocation);
+
+  for (const focusableElement of focusableElementsAfterNode) {
+    if (
+      isElementInViewport(focusableElement) &&
+      (!filter || (filter && filter(focusableElement)))
+    ) {
+      return focusableElement;
+    }
+  }
+
+  return null;
+}
+
+export function focusNextFocusableNode(node: HTMLElement, filter?: Filter) {
+  const nextFocusable = nextFocusableNode(node, filter);
+  if (nextFocusable && nextFocusable instanceof HTMLElement) {
+    nextFocusable.focus();
+    return true;
+  }
+
+  return false;
+}

src/utilities/is-element-in-viewport.ts

@@ -0,0 +1,10 @@
+export function isElementInViewport(element: Element) {
+  const {top, left, bottom, right} = element.getBoundingClientRect();
+
+  return (
+    top >= 0 &&
+    right <= window.innerWidth &&
+    bottom <= window.innerHeight &&
+    left >= 0
+  );
+}

src/utilities/tests/focus.test.ts

@@ -1,14 +1,128 @@
 import {MouseEvent} from 'react';
-import {handleMouseUpByBlurring} from '../focus';
-
-describe('focus', () => {
-  describe('handleMouseUpByBlurring()', () => {
-    it('calls blur on the currentTarget', () => {
-      const currentTarget = document.createElement('button');
-      jest.spyOn(currentTarget, 'blur');
-      const mouseEvent = {currentTarget};
-      handleMouseUpByBlurring(mouseEvent as MouseEvent<HTMLButtonElement>);
-      expect(currentTarget.blur).toHaveBeenCalled();
+import {
+  handleMouseUpByBlurring,
+  focusNextFocusableNode,
+  nextFocusableNode,
+} from '../focus';
+
+describe('handleMouseUpByBlurring()', () => {
+  it('calls blur on the currentTarget', () => {
+    const currentTarget = document.createElement('button');
+    jest.spyOn(currentTarget, 'blur');
+    const mouseEvent = {currentTarget};
+    handleMouseUpByBlurring(mouseEvent as MouseEvent<HTMLButtonElement>);
+    expect(currentTarget.blur).toHaveBeenCalled();
+  });
+});
+
+describe('nextFocusableNode', () => {
+  it('does not return the initial element as the focusable node', () => {
+    const {activator, otherNode} = domSetup();
+
+    expect(nextFocusableNode(activator)).toBe(otherNode);
+  });
+
+  it('returns null when a focusable element is not found', () => {
+    const {activator} = domSetup({
+      otherNodeTag: 'div',
+    });
+
+    expect(nextFocusableNode(activator)).toBeNull();
+  });
+
+  it('filters out elements', () => {
+    const {activator} = domSetup();
+
+    expect(nextFocusableNode(activator, () => false)).toBeNull();
+  });
+
+  it("returns the parent of an adjacent element when it's focusable", () => {
+    const {activator, otherNode} = domSetup();
+
+    expect(nextFocusableNode(activator)).toBe(otherNode);
+  });
+
+  it('searches adjacent elements for focusable children', () => {
+    const {activator, otherNodeNested} = domSetup({
+      otherNodeTag: 'div',
+      nested: true,
+    });
+
+    expect(nextFocusableNode(activator)).toBe(otherNodeNested);
+  });
+
+  it('searches parent elements for focusable children', () => {
+    const {activator, parentsFocusableNode} = domSetup({
+      otherNodeTag: 'div',
+      parents: true,
     });
+
+    expect(nextFocusableNode(activator)).toBe(parentsFocusableNode);
   });
 });
+
+describe('focusNextFocusableNode', () => {
+  it('returns true when the node was focused', () => {
+    const {activator} = domSetup();
+
+    expect(focusNextFocusableNode(activator)).toBe(true);
+  });
+
+  it('returns false when the node was not focused', () => {
+    const {activator} = domSetup({otherNodeTag: 'div'});
+
+    expect(focusNextFocusableNode(activator)).toBe(false);
+  });
+
+  it('focused the node', () => {
+    const {activator, otherNode} = domSetup();
+
+    focusNextFocusableNode(activator);
+
+    expect(document.activeElement).toBe(otherNode);
+  });
+});
+
+function domSetup(
+  options: {
+    wrapperTag?: string;
+    activatorTag?: string;
+    otherNodeTag?: string;
+    otherNodeNestedTag?: string;
+    nested?: boolean;
+    parents?: true;
+  } = {},
+) {
+  const div = 'div';
+  const button = 'button';
+  const {
+    wrapperTag = div,
+    activatorTag = button,
+    otherNodeTag = button,
+    otherNodeNestedTag = button,
+    nested,
+    parents,
+  } = options;
+  const wrapper = document.createElement(wrapperTag);
+  const activator = document.createElement(activatorTag);
+  const otherNode = document.createElement(otherNodeTag);
+  let otherNodeNested = null;
+  let parentNode = null;
+  let parentsFocusableNode = null;
+
+  if (nested) {
+    otherNodeNested = document.createElement(otherNodeNestedTag);
+    otherNode.appendChild(otherNodeNested);
+  }
+
+  wrapper.append(activator, otherNode);
+
+  if (parents) {
+    parentNode = document.createElement(div);
+    parentsFocusableNode = document.createElement(button);
+    parentNode.append(wrapper, parentsFocusableNode);
+  }
+
+  document.body.appendChild(parentNode || wrapper);
+  return {wrapper, activator, otherNode, otherNodeNested, parentsFocusableNode};
+}