From 7c6784dcee5ce6affa1f89b6e8c2b95006189ea9 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Thu, 8 Aug 2019 16:20:00 +0200 Subject: [PATCH 1/6] WIP --- .../lib/accessibility/FocusZone/FocusZone.tsx | 173 ++++++++++++---- .../accessibility/FocusZone/focusUtilities.ts | 68 +++++++ .../react/test/specs/lib/FocusZone-test.tsx | 184 ++++++++++++++++++ 3 files changed, 392 insertions(+), 33 deletions(-) diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx index a4997e5a86..303881ec8c 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx @@ -18,6 +18,10 @@ import { isElementFocusSubZone, isElementTabbable, getWindow, + getDocument, + getElementIndexPath, + getFocusableByIndexPath, + getParent, IS_FOCUSABLE_ATTRIBUTE, FOCUSZONE_ID_ATTRIBUTE, } from './focusUtilities' @@ -37,10 +41,6 @@ interface Point { } const ALLOWED_INPUT_TYPES = ['text', 'number', 'password', 'email', 'tel', 'url', 'search'] -function getParent(child: HTMLElement): HTMLElement | null { - return child && child.parentElement -} - export default class FocusZone extends React.Component implements IFocusZone { static propTypes = { className: PropTypes.string, @@ -78,6 +78,19 @@ export default class FocusZone extends React.Component implement _id: string /** The most recently focused child element. */ _activeElement: HTMLElement | null + + /** + * The index path to the last focused child element. + */ + _lastIndexPath: number[] | undefined + + /** + * Flag to define when we've intentionally parked focus on the root element to temporarily + * hold focus until items appear within the zone. + */ + _isParked: boolean + _parkedTabIndex: string | null | undefined + /** The child element with tabindex=0. */ _defaultFocusElement: HTMLElement | null _focusAlignment: Point @@ -104,30 +117,61 @@ export default class FocusZone extends React.Component implement componentDidMount(): void { _allInstances[this._id] = this - this.setRef(this) // called here to support functional components, we only need HTMLElement ref anyway - if (this._root.current) { - this.windowElement = getWindow(this._root.current) - let parentElement = getParent(this._root.current) + if (!this._root.current) { + return + } - while (parentElement && parentElement !== document.body && parentElement.nodeType === 1) { - if (isElementFocusZone(parentElement)) { - this._isInnerZone = true - break - } - parentElement = getParent(parentElement) - } + this.windowElement = getWindow(this._root.current) + let parentElement = getParent(this._root.current) - if (!this._isInnerZone) { - this.windowElement.addEventListener('keydown', this.onKeyDownCapture, true) + while (parentElement && parentElement !== document.body && parentElement.nodeType === 1) { + if (isElementFocusZone(parentElement)) { + this._isInnerZone = true + break } + parentElement = getParent(parentElement) + } - // Assign initial tab indexes so that we can set initial focus as appropriate. - this.updateTabIndexes() + if (!this._isInnerZone) { + this.windowElement.addEventListener('keydown', this.onKeyDownCapture, true) + this._root.current.addEventListener('blur', this._onBlur, true) + } + + // Assign initial tab indexes so that we can set initial focus as appropriate. + this.updateTabIndexes() + + if (this.props.shouldFocusOnMount) { + this.focus() + } + } + + componentDidUpdate(): void { + if (!this._root.current) { + return + } + const doc = getDocument(this._root.current) - if (this.props.shouldFocusOnMount) { - this.focus() + if ( + doc && + this._lastIndexPath && + (doc.activeElement === doc.body || doc.activeElement === this._root.current) + ) { + // The element has been removed after the render, attempt to restore focus. + const elementToFocus = getFocusableByIndexPath( + this._root.current as HTMLElement, + this._lastIndexPath, + ) + + if (elementToFocus) { + this.setActiveElement(elementToFocus, true) + elementToFocus.focus() + this.setParkedFocus(false) + } else { + // We had a focus path to restore, but now that path is unresolvable. Park focus + // on the container until we can try again. + this.setParkedFocus(true) } } } @@ -148,6 +192,13 @@ export default class FocusZone extends React.Component implement this.props, ) + // Note, right before rendering/reconciling proceeds, we need to record if focus + // was in the zone before the update. This helper will track this and, if focus + // was actually in the zone, what the index path to the element is at this time. + // Then, later in componentDidUpdate, we can evaluate if we need to restore it in + // the case the element was removed. + this.evaluateFocusBeforeRender() + return ( implement this._root.current = ReactDOM.findDOMNode(elem) as HTMLElement } + // Record if focus was in the zone, what the index path to the element is at this time. + evaluateFocusBeforeRender(): void { + if (!this._root.current) { + return + } + const doc = getDocument(this._root.current) + + if (doc) { + const focusedElement = doc.activeElement as HTMLElement + + // Only update the index path if we are not parked on the root. + if (focusedElement !== this._root.current) { + const shouldRestoreFocus = this._root.current.contains(focusedElement) + + this._lastIndexPath = shouldRestoreFocus + ? getElementIndexPath(this._root.current as HTMLElement, doc.activeElement as HTMLElement) + : undefined + } + } + } + + /** + * When focus is in the zone at render time but then all focusable elements are removed, + * we "park" focus temporarily on the root. Once we update with focusable children, we restore + * focus to the closest path from previous. If the user tabs away from the parked container, + * we restore focusability to the pre-parked state. + */ + setParkedFocus(isParked: boolean): void { + if (this._root.current && this._isParked !== isParked) { + this._isParked = isParked + + if (isParked) { + if (!this.props.allowFocusRoot) { + this._parkedTabIndex = this._root.current.getAttribute('tabindex') + this._root.current.setAttribute('tabindex', '-1') + } + this._root.current.focus() + } else if (!this.props.allowFocusRoot) { + if (this._parkedTabIndex) { + this._root.current.setAttribute('tabindex', this._parkedTabIndex) + this._parkedTabIndex = undefined + } else { + this._root.current.removeAttribute('tabindex') + } + } + } + } + + _onBlur = () => { + this.setParkedFocus(false) + } + _onFocus = (ev: React.FocusEvent): void => { const { onActiveElementChanged, @@ -262,8 +365,9 @@ export default class FocusZone extends React.Component implement } = this.props let newActiveElement: HTMLElement | undefined + const isImmediateDescendant = this.isImmediateDescendantOfZone(ev.target as HTMLElement) - if (this.isImmediateDescendantOfZone(ev.target as HTMLElement)) { + if (isImmediateDescendant) { newActiveElement = ev.target as HTMLElement } else { let parentElement = ev.target as HTMLElement @@ -298,12 +402,15 @@ export default class FocusZone extends React.Component implement if (newActiveElement && newActiveElement !== this._activeElement) { this._activeElement = newActiveElement - this.setFocusAlignment(newActiveElement, true) - this.updateTabIndexes() - } - if (onActiveElementChanged) { - onActiveElementChanged(this._activeElement as HTMLElement, ev) + if (isImmediateDescendant) { + this.setFocusAlignment(this._activeElement) + this.updateTabIndexes() + } + + if (onActiveElementChanged) { + onActiveElementChanged(this._activeElement as HTMLElement, ev) + } } if (stopFocusPropagation) { @@ -387,12 +494,6 @@ export default class FocusZone extends React.Component implement return undefined } - if (document.activeElement === this._root.current && this._isInnerZone) { - // If this element has focus, it is being controlled by a parent. - // Ignore the keystroke. - return undefined - } - if (this.props.onKeyDown) { this.props.onKeyDown(ev) } @@ -402,6 +503,12 @@ export default class FocusZone extends React.Component implement return undefined } + if (document.activeElement === this._root.current && this._isInnerZone) { + // If this element has focus, it is being controlled by a parent. + // Ignore the keystroke. + return undefined + } + if ( shouldEnterInnerZone && shouldEnterInnerZone(ev) && diff --git a/packages/react/src/lib/accessibility/FocusZone/focusUtilities.ts b/packages/react/src/lib/accessibility/FocusZone/focusUtilities.ts index f75ec99fe5..0cfb810f15 100644 --- a/packages/react/src/lib/accessibility/FocusZone/focusUtilities.ts +++ b/packages/react/src/lib/accessibility/FocusZone/focusUtilities.ts @@ -443,3 +443,71 @@ export function getWindow(rootElement?: Element | null): Window | undefined { (rootElement && rootElement.ownerDocument && rootElement.ownerDocument.defaultView) || window ) } + +/** + * Helper to get the document object. + * + * @public + */ +export function getDocument(rootElement?: Element | null): Document | undefined { + return (rootElement && rootElement.ownerDocument) || document +} + +/** + * Returns parent element of passed child element if exists + * @param child element to find parent for + */ +export function getParent(child: HTMLElement): HTMLElement | null { + return child && child.parentElement +} + +/** + * Finds the closest focusable element via an index path from a parent. See + * `getElementIndexPath` for getting an index path from an element to a child. + */ +export function getFocusableByIndexPath( + parent: HTMLElement, + path: number[], +): HTMLElement | undefined { + let element = parent + + for (const index of path) { + const nextChild = element.children[Math.min(index, element.children.length - 1)] as HTMLElement + + if (!nextChild) { + break + } + element = nextChild + } + + element = + isElementTabbable(element) && isElementVisible(element) + ? element + : getNextElement(parent, element, true) || getPreviousElement(parent, element)! + + return element as HTMLElement +} + +/** + * Finds the element index path from a parent element to a child element. + * + * If you had this node structure: "A has children [B, C] and C has child D", + * the index path from A to D would be [1, 0], or `parent.chidren[1].children[0]`. + */ +export function getElementIndexPath(fromElement: HTMLElement, toElement: HTMLElement): number[] { + const path: number[] = [] + let currentElement: HTMLElement = toElement + + while (currentElement && fromElement && currentElement !== fromElement) { + const parent = getParent(currentElement) + + if (parent === null) { + return [] + } + + path.unshift(Array.prototype.indexOf.call(parent.children, currentElement)) + currentElement = parent + } + + return path +} diff --git a/packages/react/test/specs/lib/FocusZone-test.tsx b/packages/react/test/specs/lib/FocusZone-test.tsx index a485ea7ae8..ec8a47da04 100644 --- a/packages/react/test/specs/lib/FocusZone-test.tsx +++ b/packages/react/test/specs/lib/FocusZone-test.tsx @@ -13,6 +13,8 @@ import { IS_FOCUSABLE_ATTRIBUTE } from 'src/lib/accessibility/FocusZone/focusUti describe('FocusZone', () => { let lastFocusedElement: HTMLElement | undefined + let host: HTMLElement + function onFocus(ev: any): void { lastFocusedElement = ev.target } @@ -50,6 +52,13 @@ describe('FocusZone', () => { lastFocusedElement = undefined }) + afterEach(() => { + if (host) { + ReactDOM.unmountComponentAtNode(host) + ;(host as any) = undefined + } + }) + it('can use arrows vertically', () => { const component = ReactTestUtils.renderIntoDocument<{}, React.Component>(
@@ -1467,4 +1476,179 @@ describe('FocusZone', () => { buttonB.focus() expect(lastFocusedElement).toBe(buttonB) }) + + it('should call onKeyDown handler even within another FocusZone', () => { + const keyDownHandler = jest.fn() + + const component = ReactTestUtils.renderIntoDocument<{}, React.Component>( +
+ + + Inner Focus Zone + + +
, + ) + + const focusZone = ReactDOM.findDOMNode(component)!.firstChild as Element + const innerFocusZone = focusZone.querySelector('.innerFocusZone') as HTMLElement + ReactTestUtils.Simulate.keyDown(innerFocusZone, { which: keyboardKey.Enter }) + + expect(keyDownHandler).toBeCalled() + }) + + describe('restores focus', () => { + it('to the following item when item removed', () => { + host = document.createElement('div') + + // Render component. + ReactDOM.render( + + + + + , + host, + ) + + const buttonB = host.querySelector('#b') as HTMLElement + + buttonB.focus() + + // Render component without button A. + ReactDOM.render( + + + + , + host, + ) + + expect(document.activeElement).toBe(host.querySelector('#c')) + }) + + it('can restore focus to the previous item when end item removed', () => { + host = document.createElement('div') + + // Render component. + ReactDOM.render( + + + + + , + host, + ) + + const buttonC = host.querySelector('#c') as HTMLElement + + buttonC.focus() + + // Render component without button A. + ReactDOM.render( + + + + , + host, + ) + + expect(document.activeElement).toBe(host.querySelector('#b')) + }) + }) + + describe('parking and unparking', () => { + let buttonA: HTMLElement + + beforeEach(() => { + host = document.createElement('div') + + // Render component. + ReactDOM.render( +
+ + +
, + host, + ) + buttonA = host.querySelector('#a') as HTMLElement + buttonA.focus() + + // Render component without button A. + ReactDOM.render( +
+
, + host, + ) + }) + + it('can move focus to container when last item removed', () => { + expect(document.activeElement).toBe(host.querySelector('#fz')) + }) + + it('can move focus from container to first item when added', () => { + ReactDOM.render( +
+ + +
, + host, + ) + expect(document.activeElement).toBe(host.querySelector('#a')) + }) + + it('removes focusability when moving from focused container', () => { + expect(host.querySelector('#fz')!.getAttribute('tabindex')).toEqual('-1') + ;(host.querySelector('#z') as HTMLElement).focus() + expect(host.querySelector('#fz')!.getAttribute('tabindex')).toBeNull() + }) + + it('does not move focus when items added without container focus', () => { + expect(host.querySelector('#fz')!.getAttribute('tabindex')).toEqual('-1') + ;(host.querySelector('#z') as HTMLElement).focus() + + ReactDOM.render( +
+ + +
, + host, + ) + expect(document.activeElement).toBe(host.querySelector('#z')) + }) + }) }) From ea294fa160955b99f364f8694447943ea2d1ce0f Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Thu, 8 Aug 2019 18:16:49 +0200 Subject: [PATCH 2/6] Upgrade FocusZone --- .../lib/accessibility/FocusZone/FocusZone.tsx | 46 +++++++++++++------ .../react/test/specs/lib/FocusZone-test.tsx | 29 ++++++++++++ 2 files changed, 62 insertions(+), 13 deletions(-) diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx index 303881ec8c..2e02925225 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx @@ -35,6 +35,8 @@ const _allInstances: { [key: string]: FocusZone } = {} +const _outerZones: Set = new Set() + interface Point { left: number top: number @@ -112,11 +114,11 @@ export default class FocusZone extends React.Component implement } this._processingTabKey = false - this.onKeyDownCapture = this.onKeyDownCapture.bind(this) } componentDidMount(): void { _allInstances[this._id] = this + this.setRef(this) // called here to support functional components, we only need HTMLElement ref anyway if (!this._root.current) { @@ -135,10 +137,15 @@ export default class FocusZone extends React.Component implement } if (!this._isInnerZone) { - this.windowElement.addEventListener('keydown', this.onKeyDownCapture, true) - this._root.current.addEventListener('blur', this._onBlur, true) + _outerZones.add(this) + } + + if (this.windowElement && _outerZones.size === 1) { + this.windowElement.addEventListener('keydown', this._onKeyDownCapture, true) } + this._root.current.addEventListener('blur', this._onBlur, true) + // Assign initial tab indexes so that we can set initial focus as appropriate. this.updateTabIndexes() @@ -178,8 +185,17 @@ export default class FocusZone extends React.Component implement componentWillUnmount() { delete _allInstances[this._id] + + if (!this._isInnerZone) { + _outerZones.delete(this) + } + if (this.windowElement) { - this.windowElement.removeEventListener('keydown', this.onKeyDownCapture, true) + this.windowElement.removeEventListener('keydown', this._onKeyDownCapture, true) + } + + if (this._root.current) { + this._root.current.removeEventListener('blur', this._onBlur, true) } } @@ -407,10 +423,10 @@ export default class FocusZone extends React.Component implement this.setFocusAlignment(this._activeElement) this.updateTabIndexes() } + } - if (onActiveElementChanged) { - onActiveElementChanged(this._activeElement as HTMLElement, ev) - } + if (onActiveElementChanged) { + onActiveElementChanged(this._activeElement as HTMLElement, ev) } if (stopFocusPropagation) { @@ -423,9 +439,9 @@ export default class FocusZone extends React.Component implement /** * Handle global tab presses so that we can patch tabindexes on the fly. */ - onKeyDownCapture(ev: KeyboardEvent) { + _onKeyDownCapture = (ev: KeyboardEvent) => { if (keyboardKey.getCode(ev) === keyboardKey.Tab) { - this.updateTabIndexes() + _outerZones.forEach(zone => zone.updateTabIndexes()) } } @@ -888,9 +904,11 @@ export default class FocusZone extends React.Component implement // Going left at a leftmost rectangle will go down a line instead of up a line like in LTR. // This is important, because we want to be comparing the top of the target rect // with the bottom of the active rect. - topBottomComparison = targetRect.top.toFixed(3) < activeRect.bottom.toFixed(3) + topBottomComparison = + parseFloat(targetRect.top.toFixed(3)) < parseFloat(activeRect.bottom.toFixed(3)) } else { - topBottomComparison = targetRect.bottom.toFixed(3) > activeRect.top.toFixed(3) + topBottomComparison = + parseFloat(targetRect.bottom.toFixed(3)) > parseFloat(activeRect.top.toFixed(3)) } if ( @@ -927,9 +945,11 @@ export default class FocusZone extends React.Component implement // Going right at a rightmost rectangle will go up a line instead of down a line like in LTR. // This is important, because we want to be comparing the bottom of the target rect // with the top of the active rect. - topBottomComparison = targetRect.bottom.toFixed(3) > activeRect.top.toFixed(3) + topBottomComparison = + parseFloat(targetRect.bottom.toFixed(3)) > parseFloat(activeRect.top.toFixed(3)) } else { - topBottomComparison = targetRect.top.toFixed(3) < activeRect.bottom.toFixed(3) + topBottomComparison = + parseFloat(targetRect.top.toFixed(3)) < parseFloat(activeRect.bottom.toFixed(3)) } if ( diff --git a/packages/react/test/specs/lib/FocusZone-test.tsx b/packages/react/test/specs/lib/FocusZone-test.tsx index ec8a47da04..4d641c89d6 100644 --- a/packages/react/test/specs/lib/FocusZone-test.tsx +++ b/packages/react/test/specs/lib/FocusZone-test.tsx @@ -1497,6 +1497,35 @@ describe('FocusZone', () => { expect(keyDownHandler).toBeCalled() }) + it('can call onActiveItemChanged when the active item is changed', () => { + let called = false + const component = ReactTestUtils.renderIntoDocument<{}, React.Component>( + (called = true)}> + + + , + ) + const focusZone = ReactDOM.findDOMNode(component)!.firstChild as Element + const buttonA = focusZone.querySelector('#a') as HTMLElement + const buttonB = focusZone.querySelector('#b') as HTMLElement + + ReactTestUtils.Simulate.mouseDown(focusZone, { target: buttonA }) + ReactTestUtils.Simulate.focus(focusZone, { target: buttonA }) + + expect(called).toEqual(true) + called = false + + ReactTestUtils.Simulate.mouseDown(focusZone, { target: buttonB }) + ReactTestUtils.Simulate.focus(focusZone, { target: buttonB }) + + expect(called).toEqual(true) + called = false + }) + describe('restores focus', () => { it('to the following item when item removed', () => { host = document.createElement('div') From 6878ac095919f919e3db6d49ff97219ebcacc842 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Thu, 8 Aug 2019 23:11:14 +0200 Subject: [PATCH 3/6] Improvements and fixes: remove "allowFocusRoot" prop. Add prop "restoreFocusFromRoot" --- .../lib/accessibility/FocusZone/FocusZone.tsx | 27 +++++++++---------- .../FocusZone/FocusZone.types.ts | 10 +++---- .../accessibility/FocusZone/focusUtilities.ts | 19 ++----------- .../react/test/specs/lib/FocusZone-test.tsx | 2 +- 4 files changed, 20 insertions(+), 38 deletions(-) diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx index 2e02925225..f3d9d50207 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx @@ -57,13 +57,13 @@ export default class FocusZone extends React.Component implement shouldEnterInnerZone: PropTypes.func, onActiveElementChanged: PropTypes.func, shouldReceiveFocus: PropTypes.func, - allowFocusRoot: PropTypes.bool, handleTabKey: PropTypes.number, shouldInputLoseFocusOnArrowKey: PropTypes.func, stopFocusPropagation: PropTypes.bool, onFocus: PropTypes.func, preventDefaultWhenHandled: PropTypes.bool, isRtl: PropTypes.bool, + restoreFocusFromRoot: PropTypes.bool, } static defaultProps: FocusZoneProps = { @@ -163,7 +163,8 @@ export default class FocusZone extends React.Component implement if ( doc && this._lastIndexPath && - (doc.activeElement === doc.body || doc.activeElement === this._root.current) + (doc.activeElement === doc.body || + (this.props.restoreFocusFromRoot && doc.activeElement === this._root.current)) ) { // The element has been removed after the render, attempt to restore focus. const elementToFocus = getFocusableByIndexPath( @@ -352,19 +353,15 @@ export default class FocusZone extends React.Component implement this._isParked = isParked if (isParked) { - if (!this.props.allowFocusRoot) { - this._parkedTabIndex = this._root.current.getAttribute('tabindex') - this._root.current.setAttribute('tabindex', '-1') - } + this._parkedTabIndex = this._root.current.getAttribute('tabindex') + this._root.current.setAttribute('tabindex', '-1') this._root.current.focus() - } else if (!this.props.allowFocusRoot) { - if (this._parkedTabIndex) { + } else if (this._parkedTabIndex) { this._root.current.setAttribute('tabindex', this._parkedTabIndex) this._parkedTabIndex = undefined } else { this._root.current.removeAttribute('tabindex') } - } } } @@ -510,6 +507,12 @@ export default class FocusZone extends React.Component implement return undefined } + if (document.activeElement === this._root.current && this._isInnerZone) { + // If this element has focus, it is being controlled by a parent. + // Ignore the keystroke. + return undefined + } + if (this.props.onKeyDown) { this.props.onKeyDown(ev) } @@ -519,12 +522,6 @@ export default class FocusZone extends React.Component implement return undefined } - if (document.activeElement === this._root.current && this._isInnerZone) { - // If this element has focus, it is being controlled by a parent. - // Ignore the keystroke. - return undefined - } - if ( shouldEnterInnerZone && shouldEnterInnerZone(ev) && diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.types.ts b/packages/react/src/lib/accessibility/FocusZone/FocusZone.types.ts index c4383ef887..78bb6ba277 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.types.ts +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.types.ts @@ -109,11 +109,6 @@ export interface FocusZoneProps extends React.HTMLAttributes boolean - /** - * Allow focus to move to root container - */ - allowFocusRoot?: boolean - /** * Allows TAB key to be handled, thus alows tabbing through a focusable list of items in the * focus zone. A side effect is that users will not be able to TAB out of the focus zone and @@ -148,6 +143,11 @@ export interface FocusZoneProps extends React.HTMLAttributes { ReactDOM.render(
From e7b75283f2e4b31f218b542a752f96c2e4c8a4ed Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Thu, 8 Aug 2019 23:21:58 +0200 Subject: [PATCH 4/6] Fix prettier --- .../src/lib/accessibility/FocusZone/FocusZone.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx index f3d9d50207..7645c23345 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx @@ -357,11 +357,11 @@ export default class FocusZone extends React.Component implement this._root.current.setAttribute('tabindex', '-1') this._root.current.focus() } else if (this._parkedTabIndex) { - this._root.current.setAttribute('tabindex', this._parkedTabIndex) - this._parkedTabIndex = undefined - } else { - this._root.current.removeAttribute('tabindex') - } + this._root.current.setAttribute('tabindex', this._parkedTabIndex) + this._parkedTabIndex = undefined + } else { + this._root.current.removeAttribute('tabindex') + } } } From f8128a4b8541eda1f8de39ab5c5125123d2ccfb7 Mon Sep 17 00:00:00 2001 From: Sofiya Huts Date: Fri, 9 Aug 2019 17:17:06 +0200 Subject: [PATCH 5/6] Improvements after CR comments --- CHANGELOG.md | 3 +++ .../lib/accessibility/FocusZone/CHANGELOG.md | 6 ++++++ .../lib/accessibility/FocusZone/FocusZone.tsx | 20 ++++++++++--------- .../react/test/specs/lib/FocusZone-test.tsx | 7 ++----- 4 files changed, 22 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b65a14d23..b1f4172f0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,9 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## [Unreleased] +### Features +- Upgrade `FocusZone` to the latest version from `fabric-ui` @sophieH29 ([#1772](https://github.com/stardust-ui/react/pull/1772)) + ### Documentation - Restore docs for `Ref` component @layershifter ([#1777](https://github.com/stardust-ui/react/pull/1777)) diff --git a/packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md b/packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md index 2ef38ac023..232d973a13 100644 --- a/packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md +++ b/packages/react/src/lib/accessibility/FocusZone/CHANGELOG.md @@ -26,6 +26,12 @@ This is a list of changes made to this Stardust copy of FocusZone in comparison - Handle keyDownCapture based on `shouldHandleKeyDownCapture` prop @sophieH29 ([#563](https://github.com/stardust-ui/react/pull/563)) - Add `bidirectionalDomOrder` direction allowing arrow keys navigation following DOM order @sophieH29 ([#1637](https://github.com/stardust-ui/react/pull/1647)) +### Upgrade `FocusZone` to the latest version from `fabric-ui` @sophieH29 ([#1772](https://github.com/stardust-ui/react/pull/1772)) +- Restore focus on removing item ([OfficeDev/office-ui-fabric-react#7818](https://github.com/OfficeDev/office-ui-fabric-react/pull/7818)) +- Reduce global event listeners ([OfficeDev/office-ui-fabric-react#7958](https://github.com/OfficeDev/office-ui-fabric-react/pull/7958)) +- Track innerzones correctly ([OfficeDev/office-ui-fabric-react#8560](https://github.com/OfficeDev/office-ui-fabric-react/pull/8560)) +- Check for no wrap fix ([OfficeDev/office-ui-fabric-react#9542](https://github.com/OfficeDev/office-ui-fabric-react/pull/9542)) + #### feat(FocusZone): Implement FocusZone into renderComponent [#116](https://github.com/stardust-ui/react/pull/116) - Prettier and linting fixes, e.g., removing semicolons, removing underscores from private methods. diff --git a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx index 7645c23345..a76d89f8f3 100644 --- a/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx +++ b/packages/react/src/lib/accessibility/FocusZone/FocusZone.tsx @@ -328,17 +328,19 @@ export default class FocusZone extends React.Component implement } const doc = getDocument(this._root.current) - if (doc) { - const focusedElement = doc.activeElement as HTMLElement + if (!doc) { + return + } - // Only update the index path if we are not parked on the root. - if (focusedElement !== this._root.current) { - const shouldRestoreFocus = this._root.current.contains(focusedElement) + const focusedElement = doc.activeElement as HTMLElement - this._lastIndexPath = shouldRestoreFocus - ? getElementIndexPath(this._root.current as HTMLElement, doc.activeElement as HTMLElement) - : undefined - } + // Only update the index path if we are not parked on the root. + if (focusedElement !== this._root.current) { + const shouldRestoreFocus = this._root.current.contains(focusedElement) + + this._lastIndexPath = shouldRestoreFocus + ? getElementIndexPath(this._root.current as HTMLElement, doc.activeElement as HTMLElement) + : undefined } } diff --git a/packages/react/test/specs/lib/FocusZone-test.tsx b/packages/react/test/specs/lib/FocusZone-test.tsx index a9c2186ad3..1f8270bc8b 100644 --- a/packages/react/test/specs/lib/FocusZone-test.tsx +++ b/packages/react/test/specs/lib/FocusZone-test.tsx @@ -1530,7 +1530,6 @@ describe('FocusZone', () => { it('to the following item when item removed', () => { host = document.createElement('div') - // Render component. ReactDOM.render( + + , + host, + ) + + expect(FocusZone.getOuterZones()).toEqual(activeZones + 1) + + ReactDOM.unmountComponentAtNode(host) + + expect(FocusZone.getOuterZones()).toEqual(activeZones) + }) + describe('restores focus', () => { it('to the following item when item removed', () => { host = document.createElement('div')