From e322d81dd72b37bd0364da032aa0724d5ae20bcc Mon Sep 17 00:00:00 2001 From: David Goff Date: Thu, 23 Feb 2017 14:45:27 -0800 Subject: [PATCH 1/7] Prevent multiple FocusTrapZones from causing an infinite loop --- .../FocusTrapZone/FocusTrapZone.tsx | 31 +++++++++++++++---- 1 file changed, 25 insertions(+), 6 deletions(-) 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 5828b0f668eb51..a9e4096176bf55 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -20,6 +20,16 @@ export class FocusTrapZone extends BaseComponent implem }; private _previouslyFocusedElement: HTMLElement; + private static _priorityStack: FocusTrapZone[] = []; + private _isInStack: boolean = false; + + public componentWillMount() { + if (this.props.forceFocusInsideTrap) { + this._isInStack = true; + FocusTrapZone._priorityStack.push(this); + } + // todo: add to the stack + } public componentDidMount() { let { elementToFocusOnDismiss, isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true } = this.props; @@ -39,6 +49,13 @@ export class FocusTrapZone extends BaseComponent implem public componentWillUnmount() { let { ignoreExternalFocusing } = this.props; + this._events.dispose(); + if (this._isInStack) { + FocusTrapZone._priorityStack = FocusTrapZone._priorityStack.filter((value: FocusTrapZone) => { + return this !== value; + }); + } + if (!ignoreExternalFocusing && this._previouslyFocusedElement) { this._previouslyFocusedElement.focus(); } @@ -101,12 +118,14 @@ export class FocusTrapZone extends BaseComponent implem } private _forceFocusInTrap(ev: FocusEvent) { - const focusedElement = document.activeElement as HTMLElement; - - if (!elementContains(this.refs.root, focusedElement)) { - this.focus(); - ev.preventDefault(); - ev.stopPropagation(); + if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[0]) { + const focusedElement = document.activeElement as HTMLElement; + + if (!elementContains(this.refs.root, focusedElement)) { + this.focus(); + ev.preventDefault(); + ev.stopPropagation(); + } } } From 628742f480b003e20189ba7823f7fc96260d3bd0 Mon Sep 17 00:00:00 2001 From: David Goff Date: Thu, 23 Feb 2017 14:47:31 -0800 Subject: [PATCH 2/7] Rush change --- ...dagoff-focusTrapZone_2017-02-23-22-47.json | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json diff --git a/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json b/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json new file mode 100644 index 00000000000000..0b42eda517b3b9 --- /dev/null +++ b/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json @@ -0,0 +1,20 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "Prevent multiple FocusTrapZones from fighting over focus", + "type": "patch" + }, + { + "comment": "", + "packageName": "@uifabric/utilities", + "type": "none" + }, + { + "comment": "", + "packageName": "@uifabric/example-app-base", + "type": "none" + } + ], + "email": "dagoff@microsoft.com" +} \ No newline at end of file From 701f7c0a4e88292710698dbf9689a29970839e3f Mon Sep 17 00:00:00 2001 From: David Goff Date: Thu, 23 Feb 2017 15:01:21 -0800 Subject: [PATCH 3/7] FocusTrapZone checks the wrong side of the priorityStack --- .../src/components/FocusTrapZone/FocusTrapZone.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a9e4096176bf55..161f23807fd1c8 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -118,7 +118,7 @@ export class FocusTrapZone extends BaseComponent implem } private _forceFocusInTrap(ev: FocusEvent) { - if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[0]) { + if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[FocusTrapZone._priorityStack.length - 1]) { const focusedElement = document.activeElement as HTMLElement; if (!elementContains(this.refs.root, focusedElement)) { From 7a6a50d523fee34662de3a8e1c22f309156b4f57 Mon Sep 17 00:00:00 2001 From: David Goff Date: Thu, 23 Feb 2017 17:34:18 -0800 Subject: [PATCH 4/7] Fix bug related to default parameters --- .../src/components/FocusTrapZone/FocusTrapZone.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 161f23807fd1c8..11d9e4e5ec6bf9 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -24,11 +24,11 @@ export class FocusTrapZone extends BaseComponent implem private _isInStack: boolean = false; public componentWillMount() { - if (this.props.forceFocusInsideTrap) { + let { forceFocusInsideTrap = true } = this.props; + if (forceFocusInsideTrap) { this._isInStack = true; FocusTrapZone._priorityStack.push(this); } - // todo: add to the stack } public componentDidMount() { @@ -52,7 +52,7 @@ export class FocusTrapZone extends BaseComponent implem this._events.dispose(); if (this._isInStack) { FocusTrapZone._priorityStack = FocusTrapZone._priorityStack.filter((value: FocusTrapZone) => { - return this !== value; + return this !== value; }); } @@ -118,7 +118,7 @@ export class FocusTrapZone extends BaseComponent implem } private _forceFocusInTrap(ev: FocusEvent) { - if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[FocusTrapZone._priorityStack.length - 1]) { + if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[FocusTrapZone._priorityStack.length - 1]) { const focusedElement = document.activeElement as HTMLElement; if (!elementContains(this.refs.root, focusedElement)) { From 139e444b0b3686c9f769ce8360a488832e4d21cb Mon Sep 17 00:00:00 2001 From: David Goff Date: Fri, 24 Feb 2017 15:08:54 -0800 Subject: [PATCH 5/7] Track clicks seperately --- .../FocusTrapZone/FocusTrapZone.tsx | 42 ++++++++++++------- 1 file changed, 28 insertions(+), 14 deletions(-) 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 11d9e4e5ec6bf9..1102e9accc915a 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -20,14 +20,20 @@ export class FocusTrapZone extends BaseComponent implem }; private _previouslyFocusedElement: HTMLElement; - private static _priorityStack: FocusTrapZone[] = []; - private _isInStack: boolean = false; + private static _focusStack: FocusTrapZone[] = []; + private _isInFocusStack: boolean = false; + private static _clickStack: FocusTrapZone[] = []; + private _isInClickStack: boolean = false; public componentWillMount() { - let { forceFocusInsideTrap = true } = this.props; + let { isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true } = this.props; if (forceFocusInsideTrap) { - this._isInStack = true; - FocusTrapZone._priorityStack.push(this); + this._isInFocusStack = true; + FocusTrapZone._focusStack.push(this); + } + if (!isClickableOutsideFocusTrap) { + this._isInClickStack = true; + FocusTrapZone._clickStack.push(this); } } @@ -50,10 +56,16 @@ export class FocusTrapZone extends BaseComponent implem let { ignoreExternalFocusing } = this.props; this._events.dispose(); - if (this._isInStack) { - FocusTrapZone._priorityStack = FocusTrapZone._priorityStack.filter((value: FocusTrapZone) => { + if (this._isInFocusStack || this._isInClickStack) { + let filter = (value: FocusTrapZone) => { return this !== value; - }); + }; + if (this._isInFocusStack) { + FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter(filter); + } + if (this._isInClickStack) { + FocusTrapZone._clickStack = FocusTrapZone._clickStack.filter(filter); + } } if (!ignoreExternalFocusing && this._previouslyFocusedElement) { @@ -118,7 +130,7 @@ export class FocusTrapZone extends BaseComponent implem } private _forceFocusInTrap(ev: FocusEvent) { - if (FocusTrapZone._priorityStack.length && this === FocusTrapZone._priorityStack[FocusTrapZone._priorityStack.length - 1]) { + if (FocusTrapZone._focusStack.length && this === FocusTrapZone._focusStack[FocusTrapZone._focusStack.length - 1]) { const focusedElement = document.activeElement as HTMLElement; if (!elementContains(this.refs.root, focusedElement)) { @@ -130,12 +142,14 @@ export class FocusTrapZone extends BaseComponent implem } private _forceClickInTrap(ev: MouseEvent) { - const clickedElement = ev.target as HTMLElement; + if (FocusTrapZone._clickStack.length && this === FocusTrapZone._clickStack[FocusTrapZone._clickStack.length - 1]) { + const clickedElement = ev.target as HTMLElement; - if (clickedElement && !elementContains(this.refs.root, clickedElement)) { - this.focus(); - ev.preventDefault(); - ev.stopPropagation(); + if (clickedElement && !elementContains(this.refs.root, clickedElement)) { + this.focus(); + ev.preventDefault(); + ev.stopPropagation(); + } } } } \ No newline at end of file From e5eebf319b52450d7a9f16c2451cd69389346a8f Mon Sep 17 00:00:00 2001 From: David Goff Date: Fri, 24 Feb 2017 17:43:47 -0800 Subject: [PATCH 6/7] Add FocusTrapZone demo showing off nested behavior --- .../FocusTrapZonePage/FocusTrapZonePage.tsx | 6 + .../examples/FocusTrapZone.Box.Example.scss | 5 + .../examples/FocusTrapZone.Nested.Example.tsx | 126 ++++++++++++++++++ 3 files changed, 137 insertions(+) create mode 100644 apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Nested.Example.tsx diff --git a/apps/fabric-examples/src/pages/FocusTrapZonePage/FocusTrapZonePage.tsx b/apps/fabric-examples/src/pages/FocusTrapZonePage/FocusTrapZonePage.tsx index 63adce4797a75d..16ea5b0d3d76ae 100644 --- a/apps/fabric-examples/src/pages/FocusTrapZonePage/FocusTrapZonePage.tsx +++ b/apps/fabric-examples/src/pages/FocusTrapZonePage/FocusTrapZonePage.tsx @@ -17,6 +17,9 @@ let FocusTrapZoneBoxExampleWithFocusableItemCode = require('./examples/FocusTrap import FocusTrapZoneBoxClickExample from './examples/FocusTrapZone.Box.Click.Example'; let FocusTrapZoneBoxClickExampleCode = require('./examples/FocusTrapZone.Box.Click.Example.tsx') as string; +import FocusTrapZoneNestedExample from './examples/FocusTrapZone.Nested.Example'; +let FocusTrapZoneNestedExampleCode = require('./examples/FocusTrapZone.Nested.Example.tsx') as string; + export class FocusTrapZonePage extends React.Component { public render() { return ( @@ -34,6 +37,9 @@ export class FocusTrapZonePage extends React.Component + + + } propertiesTables={ diff --git a/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Box.Example.scss b/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Box.Example.scss index 09b48341653ef9..1bd0f36c5abe57 100644 --- a/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Box.Example.scss +++ b/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Box.Example.scss @@ -3,3 +3,8 @@ .ms-FocusTrapZoneBoxExample { border: dashed 1px #ababab; } + +.ms-FocusTrapComponent { + border: black 2px solid; + padding: 5px; +} \ No newline at end of file diff --git a/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Nested.Example.tsx b/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Nested.Example.tsx new file mode 100644 index 00000000000000..9e73cbb0bfe009 --- /dev/null +++ b/apps/fabric-examples/src/pages/FocusTrapZonePage/examples/FocusTrapZone.Nested.Example.tsx @@ -0,0 +1,126 @@ +/* tslint:disable:no-unused-variable */ +import * as React from 'react'; +/* tslint:enable:no-unused-variable */ + +import * as ReactDOM from 'react-dom'; +import { Button } from 'office-ui-fabric-react/lib/Button'; +import { FocusTrapZone } from 'office-ui-fabric-react/lib/FocusTrapZone'; +import { Link } from 'office-ui-fabric-react/lib/Link'; +import { TextField } from 'office-ui-fabric-react/lib/TextField'; +import { Toggle } from 'office-ui-fabric-react/lib/Toggle'; +import { autobind } from 'office-ui-fabric-react/lib/Utilities'; +import './FocusTrapZone.Box.Example.scss'; + +interface IFocusTrapComponentProps { + name: string; + isActive: boolean; + setIsActive: (name: string, isActive: boolean) => void; +} + +interface IFocusTrapComponentState { +} + +class FocusTrapComponent extends React.Component { + + public refs: { + [key: string]: React.ReactInstance; + toggle: HTMLElement; + }; + + render() { + let contents = ( +
+ + + { + this.props.children + } +
+ ); + + if (this.props.isActive) { + return ( + + { + contents + } + + ); + } + return contents; + } + + @autobind + private _onStringButtonClicked() { + console.log(this.props.name); + } + + @autobind + private _onFocusTrapZoneToggleChanged(isChecked: boolean) { + this.props.setIsActive(this.props.name, isChecked); + } + +} + +export interface IFocusTrapZoneNestedExampleState { + stateMap: any; +} + +const NAMES: string[] = ['One', 'Two', 'Three', 'Four', 'Five']; + +export default class FocusTrapZoneNestedExample extends React.Component, IFocusTrapZoneNestedExampleState> { + + constructor() { + super(); + + this.state = { + stateMap: {} + }; + } + + public render() { + return ( +
+ + + + + + + + + + + +
+ ); + } + + @autobind + private _setIsActive(name: string, isActive: boolean): void { + this.state.stateMap[name] = isActive; + this.forceUpdate(); + } + + @autobind + private _randomize(): void { + for (let i in NAMES) { + let newVal: boolean = Math.random() < .5 ? false : true; + this.state.stateMap[NAMES[i]] = newVal; + } + this.forceUpdate(); + } + +} + + From 63bddbd6aab28f52dd43cf6c89258bfd760a2f47 Mon Sep 17 00:00:00 2001 From: David Zearing Date: Wed, 1 Mar 2017 19:00:21 -0800 Subject: [PATCH 7/7] Update dagoff-focusTrapZone_2017-02-23-22-47.json --- .../dagoff-focusTrapZone_2017-02-23-22-47.json | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json b/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json index 0b42eda517b3b9..ea94a8a365f750 100644 --- a/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json +++ b/common/changes/dagoff-focusTrapZone_2017-02-23-22-47.json @@ -2,19 +2,9 @@ "changes": [ { "packageName": "office-ui-fabric-react", - "comment": "Prevent multiple FocusTrapZones from fighting over focus", + "comment": "FocusTrapZone: Fixed a scenario where multiple instances would fight over focus.", "type": "patch" - }, - { - "comment": "", - "packageName": "@uifabric/utilities", - "type": "none" - }, - { - "comment": "", - "packageName": "@uifabric/example-app-base", - "type": "none" } ], "email": "dagoff@microsoft.com" -} \ No newline at end of file +}