diff --git a/common/changes/@uifabric/utilities/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json b/common/changes/@uifabric/utilities/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json new file mode 100644 index 0000000000000..6f9cd3e61623e --- /dev/null +++ b/common/changes/@uifabric/utilities/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "@uifabric/utilities", + "comment": "Handling the case where a FocusTrapZone would not properly trap when a FocusZone was the last element in the trap zone.", + "type": "patch" + } + ], + "packageName": "@uifabric/utilities", + "email": "sllynn8907@gmail.com" +} \ No newline at end of file diff --git a/common/changes/office-ui-fabric-react/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json b/common/changes/office-ui-fabric-react/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json new file mode 100644 index 0000000000000..50991d32efbb1 --- /dev/null +++ b/common/changes/office-ui-fabric-react/focustrapzone-last-child-focuszone-2_2018-10-13-07-34.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "Updating a FocusTrapZone test to ensure focus is properly trapped with a FocusZone as the last child of the trap zone.", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "sllynn8907@gmail.com" +} \ No newline at end of file diff --git a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx index 5f05ea3bdec67..3ac15617acccb 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.test.tsx @@ -493,4 +493,73 @@ describe('FocusTrapZone', () => { expect(focusTrapZoneFocusStack[0]).toBe(baseFocusTrapZone); }); }); + + it('can tab across FocusZones with different button structures', () => { + const topLevelDiv = ReactTestUtils.renderIntoDocument( +
+ + + + + + + + +
+ ) as HTMLElement; + + const buttonA = topLevelDiv.querySelector('.a') as HTMLElement; + const buttonD = topLevelDiv.querySelector('.d') as HTMLElement; + const buttonE = topLevelDiv.querySelector('.e') as HTMLElement; + const buttonF = topLevelDiv.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); + }); }); diff --git a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx index a01cf304f7db1..78ae52c8bb443 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -106,9 +106,7 @@ export class FocusTrapZone extends BaseComponent implem } const focusSelector = - typeof firstFocusableSelector === 'string' - ? firstFocusableSelector - : firstFocusableSelector && firstFocusableSelector(); + typeof firstFocusableSelector === 'string' ? firstFocusableSelector : firstFocusableSelector && firstFocusableSelector(); let _firstFocusableChild; @@ -116,14 +114,7 @@ export class FocusTrapZone extends BaseComponent implem if (focusSelector) { _firstFocusableChild = this._root.current.querySelector('.' + focusSelector); } else { - _firstFocusableChild = getNextElement( - this._root.current, - this._root.current.firstChild as HTMLElement, - true, - false, - false, - true - ); + _firstFocusableChild = getNextElement(this._root.current, this._root.current.firstChild as HTMLElement, true, false, false, true); } } if (_firstFocusableChild) { @@ -160,11 +151,7 @@ export class FocusTrapZone extends BaseComponent implem return; } - const _firstTabbableChild = getFirstTabbable( - this._root.current, - this._root.current.firstChild as HTMLElement, - true - ); + const _firstTabbableChild = getFirstTabbable(this._root.current, this._root.current.firstChild as HTMLElement, true); const _lastTabbableChild = getLastTabbable(this._root.current, this._root.current.lastChild as HTMLElement, true); if (ev.shiftKey && _firstTabbableChild === ev.target) { diff --git a/packages/utilities/src/focus.ts b/packages/utilities/src/focus.ts index 5d67270328a99..1c16a6ebc02f4 100644 --- a/packages/utilities/src/focus.ts +++ b/packages/utilities/src/focus.ts @@ -144,6 +144,23 @@ export function getPreviousElement( isCurrentElementVisible && (includeElementsInFocusZones || !(isElementFocusZone(currentElement) || isElementFocusSubZone(currentElement))) ) { + // If current is a focus zone, check for first element, since only one is tabbable at a time + if ((tabbable && isElementFocusZone(currentElement)) || isElementFocusSubZone(currentElement)) { + const firstMatch = getNextElement( + rootElement, + currentElement.firstElementChild as HTMLElement, + true, + true, + true, + includeElementsInFocusZones, + allowFocusRoot, + tabbable + ); + if (firstMatch) { + return firstMatch; + } + } + const childMatch = getPreviousElement( rootElement, currentElement.lastElementChild as HTMLElement, @@ -429,10 +446,7 @@ export function doesElementContainFocus(element: HTMLElement): boolean { * @param noWrapDataAttribute - the no wrap data attribute to match (either) * @returns true if focus should wrap, false otherwise */ -export function shouldWrapFocus( - element: HTMLElement, - noWrapDataAttribute: 'data-no-vertical-wrap' | 'data-no-horizontal-wrap' -): boolean { +export function shouldWrapFocus(element: HTMLElement, noWrapDataAttribute: 'data-no-vertical-wrap' | 'data-no-horizontal-wrap'): boolean { return elementContainsAttribute(element, noWrapDataAttribute) === 'true' ? false : true; }