Introduced focusAsync for cheaper element focusing, and made FocusTrapZone utilize it

common/changes/@uifabric/utilities/mariust-focusAsync_2018-03-19-15-01.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/utilities",
+      "comment": "Introduced focusAsync for cheaper element focusing",
+      "type": "minor"
+    }
+  ],
+  "packageName": "@uifabric/utilities",
+  "email": "mariust@microsoft.com"
+}

common/changes/office-ui-fabric-react/mariust-focusAsync_2018-03-19-15-01.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Introduced focusAsync for cheaper element focusing, and made FocusTrapZone utilize it",
+      "type": "minor"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "mariust@microsoft.com"
+}

packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx

@@ -9,6 +9,13 @@ import { KeyCodes } from '../../Utilities';
 import { FocusZone, FocusZoneDirection } from '../FocusZone';
 import { FocusTrapZone } from './FocusTrapZone';
 
+// rAF does not exist in node - let's mock it
+window.requestAnimationFrame = (callback: FrameRequestCallback) => {
+  return window.setTimeout(callback, 16);
+};
+
+jest.useFakeTimers();
+
 describe('FocusTrapZone', () => {
   let lastFocusedElement: HTMLElement | undefined;
   function _onFocus(ev: any) {
@@ -139,15 +146,23 @@ describe('FocusTrapZone', () => {
 
     // Focus the first button.
     ReactTestUtils.Simulate.focus(buttonA);
-    expect(lastFocusedElement).toBe(buttonA);
+    window.requestAnimationFrame(() => {
+      expect(lastFocusedElement).toBe(buttonA);
+
+      // Pressing shift + tab should go to d.
+      ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab, shiftKey: true });
+      window.requestAnimationFrame(() => {
+        expect(lastFocusedElement).toBe(buttonD);
 
-    // Pressing shift + tab should go to d.
-    ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab, shiftKey: true });
-    expect(lastFocusedElement).toBe(buttonD);
+        // Pressing tab should go to a.
+        ReactTestUtils.Simulate.keyDown(buttonD, { which: KeyCodes.tab });
+        window.requestAnimationFrame(() => {
+          expect(lastFocusedElement).toBe(buttonA);
+        });
+      });
+    });
 
-    // Pressing tab should go to a.
-    ReactTestUtils.Simulate.keyDown(buttonD, { which: KeyCodes.tab });
-    expect(lastFocusedElement).toBe(buttonA);
+    jest.runAllTimers();
   });
 
   it('can tab across a FocusZone with different button structures', () => {
@@ -229,14 +244,22 @@ describe('FocusTrapZone', () => {
 
     // Focus the first button.
     ReactTestUtils.Simulate.focus(buttonX);
-    expect(lastFocusedElement).toBe(buttonX);
+    window.requestAnimationFrame(() => {
+      expect(lastFocusedElement).toBe(buttonX);
+
+      // Pressing shift + tab should go to a.
+      ReactTestUtils.Simulate.keyDown(buttonX, { which: KeyCodes.tab, shiftKey: true });
+      window.requestAnimationFrame(() => {
+        expect(lastFocusedElement).toBe(buttonA);
 
-    // Pressing shift + tab should go to a.
-    ReactTestUtils.Simulate.keyDown(buttonX, { which: KeyCodes.tab, shiftKey: true });
-    expect(lastFocusedElement).toBe(buttonA);
+        // Pressing tab should go to x.
+        ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab });
+        window.requestAnimationFrame(() => {
+          expect(lastFocusedElement).toBe(buttonX);
+        });
+      });
+    });
 
-    // Pressing tab should go to x.
-    ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab });
-    expect(lastFocusedElement).toBe(buttonX);
+    jest.runAllTimers();
   });
 });

packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx

@@ -8,6 +8,7 @@ import {
   getFirstFocusable,
   getLastTabbable,
   getNextElement,
+  focusAsync,
   createRef
 } from '../../Utilities';
 import { IFocusTrapZone, IFocusTrapZoneProps } from './FocusTrapZone.types';
@@ -75,7 +76,7 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
     }
 
     if (!ignoreExternalFocusing && this._previouslyFocusedElement && typeof this._previouslyFocusedElement.focus === 'function') {
-      this._previouslyFocusedElement.focus();
+      focusAsync(this._previouslyFocusedElement);
     }
   }
 
@@ -115,7 +116,7 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
       }
     }
     if (_firstFocusableChild) {
-      (_firstFocusableChild as any).focus();
+      focusAsync(_firstFocusableChild);
     }
   }
 
@@ -132,11 +133,11 @@ export class FocusTrapZone extends BaseComponent<IFocusTrapZoneProps, {}> implem
     const _lastFocusableChild = getLastTabbable(this._root.value, this._root.value.lastChild as HTMLElement, true);
 
     if (ev.shiftKey && _firstFocusableChild === ev.target) {
-      _lastFocusableChild!.focus();
+      focusAsync(_lastFocusableChild);
       ev.preventDefault();
       ev.stopPropagation();
     } else if (!ev.shiftKey && _lastFocusableChild === ev.target) {
-      _firstFocusableChild!.focus();
+      focusAsync(_firstFocusableChild);
       ev.preventDefault();
       ev.stopPropagation();
     }

packages/utilities/src/focus.test.tsx

@@ -3,7 +3,7 @@ import * as React from 'react';
 /* tslint:enable:no-unused-variable */
 import * as ReactDOM from 'react-dom';
 import * as ReactTestUtils from 'react-dom/test-utils';
-import { isElementVisible, isElementTabbable } from './focus';
+import { isElementVisible, isElementTabbable, focusAsync } from './focus';
 
 let _hiddenElement: HTMLElement | undefined;
 let _visibleElement: HTMLElement | undefined;
@@ -121,4 +121,44 @@ describe('isElementTabbable', () => {
     expect(isElementTabbable(button)).toEqual(false);
   });
 
+});
+
+describe('focusAsync', () => {
+  // rAF does not exist in node - let's mock it
+  window.requestAnimationFrame = (callback: FrameRequestCallback) => {
+    return window.setTimeout(callback, 16);
+  };
+
+  jest.useFakeTimers();
+
+  it('focuses on an item on the next frame', () => {
+    const component = renderIntoDocument(
+      <div>
+        <button className='a'>a</button>
+        <button className='b'>b</button>
+        <button className='c'>c</button>
+      </div>
+    );
+
+    const container = ReactDOM.findDOMNode(component as React.ReactInstance) as Element;
+
+    const buttonA = container.querySelector('.a') as HTMLElement;
+    const buttonB = container.querySelector('.b') as HTMLElement;
+    const buttonC = container.querySelector('.c') as HTMLElement;
+
+    // Focus the first button.
+    focusAsync(buttonA);
+    window.requestAnimationFrame(() => {
+      expect(container.ownerDocument.activeElement).toBe(buttonA);
+
+      // Focus the second button, then the third before the next frame
+      focusAsync(buttonB);
+      focusAsync(buttonC);
+      window.requestAnimationFrame(() => {
+        expect(container.ownerDocument.activeElement).toBe(buttonC);
+      });
+    });
+
+    jest.runAllTimers();
+  });
 });

packages/utilities/src/focus.ts

@@ -58,7 +58,7 @@ export function focusFirstChild(
   let element: HTMLElement | null = getNextElement(rootElement, rootElement, true, false, false, true);
 
   if (element) {
-    element.focus();
+    focusAsync(element);
     return true;
   }
   return false;
@@ -351,3 +351,31 @@ export function shouldWrapFocus(element: HTMLElement, noWrapDataAttribute: 'data
 
   return elementContainsAttribute(element, noWrapDataAttribute) === 'true' ? false : true;
 }
+
+let targetToFocusOnNextRepaint: HTMLElement | null | undefined = undefined;
+
+/**
+ * Sets focus to an element asynchronously. The focus will be set at the next browser repaint,
+ * meaning it won't cause any extra recalculations. If more than one focusAsync is called during one frame,
+ * only the latest called focusAsync element will actually be focused
+ * @param element The element to focus
+ */
+export function focusAsync(element: HTMLElement | undefined | null): void {
+  if (element) {
+    // An element was already queued to be focused, so replace that one with the new element
+    if (targetToFocusOnNextRepaint) {
+      targetToFocusOnNextRepaint = element;
+      return;
+    }
+
+    targetToFocusOnNextRepaint = element;
+
+    // element.focus() is a no-op if the element is no longer in the DOM, meaning this is always safe
+    element.ownerDocument.defaultView.requestAnimationFrame(() => {
+      targetToFocusOnNextRepaint && targetToFocusOnNextRepaint.focus();
+
+      // We are done focusing for this frame, so reset the queued focus element
+      targetToFocusOnNextRepaint = undefined;
+    });
+  }
+}