Revert "FocusTrapZone does not correctly trap focus when last child i…

common/changes/@uifabric/utilities/revert-focus_2018-03-15-01-48.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "@uifabric/utilities",
+      "comment": "Revert focus changes",
+      "type": "patch"
+    }
+  ],
+  "packageName": "@uifabric/utilities",
+  "email": "joschect@microsoft.com"
+}

common/changes/office-ui-fabric-react/revert-focus_2018-03-15-01-48.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "comment": "",
+      "packageName": "office-ui-fabric-react",
+      "type": "none"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "joschect@microsoft.com"
+}

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

@@ -239,75 +239,4 @@ describe('FocusTrapZone', () => {
     ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab });
     expect(lastFocusedElement).toBe(buttonX);
   });
-
-  it('can tab across FocusZones with different button structures', () => {
-    const component = ReactTestUtils.renderIntoDocument(
-      <div { ...{ onFocusCapture: _onFocus } }>
-        <FocusTrapZone forceFocusInsideTrap={ false }>
-          <button className='a'>a</button>
-          <FocusZone direction={ FocusZoneDirection.horizontal } data-is-visible={ true }>
-            <button className='d'>d</button>
-            <button className='e'>e</button>
-            <button className='f'>f</button>
-          </FocusZone>
-        </FocusTrapZone>
-      </div>
-    );
-
-    const focusTrapZone = ReactDOM.findDOMNode(component as React.ReactInstance) as Element;
-
-    const buttonA = focusTrapZone.querySelector('.a') as HTMLElement;
-    const buttonD = focusTrapZone.querySelector('.d') as HTMLElement;
-    const buttonE = focusTrapZone.querySelector('.e') as HTMLElement;
-    const buttonF = focusTrapZone.querySelector('.f') as HTMLElement;
-
-    // Assign bounding locations to buttons.
-    setupElement(buttonA, {
-      clientRect: {
-        top: 0,
-        bottom: 30,
-        left: 0,
-        right: 30
-      }
-    });
-
-    setupElement(buttonD, {
-      clientRect: {
-        top: 30,
-        bottom: 60,
-        left: 0,
-        right: 30
-      }
-    });
-
-    setupElement(buttonE, {
-      clientRect: {
-        top: 30,
-        bottom: 60,
-        left: 30,
-        right: 60
-      }
-    });
-
-    setupElement(buttonF, {
-      clientRect: {
-        top: 30,
-        bottom: 60,
-        left: 60,
-        right: 90
-      }
-    });
-
-    // Focus the first button.
-    ReactTestUtils.Simulate.focus(buttonD);
-    expect(lastFocusedElement).toBe(buttonD);
-
-    // Pressing tab should go to a.
-    ReactTestUtils.Simulate.keyDown(buttonD, { which: KeyCodes.tab });
-    expect(lastFocusedElement).toBe(buttonA);
-
-    // Pressing shift + tab should go to a.
-    ReactTestUtils.Simulate.keyDown(buttonA, { which: KeyCodes.tab, shiftKey: true });
-    expect(lastFocusedElement).toBe(buttonD);
-  });
 });

packages/utilities/src/focus.ts

@@ -77,7 +77,7 @@ export function getPreviousElement(
   traverseChildren?: boolean,
   includeElementsInFocusZones?: boolean,
   allowFocusRoot?: boolean,
-  elementShouldBeTabbable?: boolean): HTMLElement | null {
+  tabbable?: boolean): HTMLElement | null {
 
   if (!currentElement ||
     (!allowFocusRoot && currentElement === rootElement)) {
@@ -97,10 +97,10 @@ export function getPreviousElement(
       true,
       includeElementsInFocusZones,
       allowFocusRoot,
-      elementShouldBeTabbable);
+      tabbable);
 
     if (childMatch) {
-      if ((elementShouldBeTabbable && isElementTabbable(childMatch, true)) || !elementShouldBeTabbable) {
+      if ((tabbable && (isElementTabbable(childMatch, true))) || !tabbable) {
         return childMatch;
       }
 
@@ -112,7 +112,7 @@ export function getPreviousElement(
         true,
         includeElementsInFocusZones,
         allowFocusRoot,
-        elementShouldBeTabbable
+        tabbable
       );
       if (childMatchSiblingMatch) {
         return childMatchSiblingMatch;
@@ -133,7 +133,7 @@ export function getPreviousElement(
           true,
           includeElementsInFocusZones,
           allowFocusRoot,
-          elementShouldBeTabbable
+          tabbable
         );
 
         if (childMatchParentMatch) {
@@ -146,8 +146,7 @@ export function getPreviousElement(
   }
 
   // Check the current node, if it's not the first traversal.
-  if (checkNode && isCurrentElementVisible &&
-    ((elementShouldBeTabbable && isElementTabbable(currentElement, true)) || !elementShouldBeTabbable)) {
+  if (checkNode && isCurrentElementVisible && isElementTabbable(currentElement)) {
     return currentElement;
   }
 
@@ -160,7 +159,7 @@ export function getPreviousElement(
     true,
     includeElementsInFocusZones,
     allowFocusRoot,
-    elementShouldBeTabbable);
+    tabbable);
 
   if (siblingMatch) {
     return siblingMatch;
@@ -169,7 +168,7 @@ export function getPreviousElement(
   // Check its parent.
   if (!suppressParentTraversal) {
     return getPreviousElement(rootElement, currentElement.parentElement, true, false, false, includeElementsInFocusZones,
-      allowFocusRoot, elementShouldBeTabbable);
+      allowFocusRoot, tabbable);
   }
 
   return null;