From 1a6f81d2c6811c8b83e81c303f934e8f9fc3e2df Mon Sep 17 00:00:00 2001 From: Burney Jia Date: Tue, 11 Dec 2018 14:33:02 -0800 Subject: [PATCH 1/5] Bring focus into trap (and return to outside trap) when forceFocusInsideTrap changes --- .../FocusTrapZone/FocusTrapZone.tsx | 64 +++++++++++++------ .../src/components/Panel/Panel.base.tsx | 2 +- 2 files changed, 45 insertions(+), 21 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 b41470dfa7ba3..113eb7af210c2 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -26,15 +26,7 @@ export class FocusTrapZone extends BaseComponent implem } public componentDidMount(): void { - const { elementToFocusOnDismiss, disableFirstFocus = false } = this.props; - - this._previouslyFocusedElementOutsideTrapZone = elementToFocusOnDismiss - ? elementToFocusOnDismiss - : (document.activeElement as HTMLElement); - if (!elementContains(this._root.current, this._previouslyFocusedElementOutsideTrapZone) && !disableFirstFocus) { - this.focus(); - } - + this._bringFocusIntoZone(); this._updateEventHandlers(this.props); } @@ -47,23 +39,30 @@ export class FocusTrapZone extends BaseComponent implem this._updateEventHandlers(nextProps); } - public componentWillUnmount(): void { - const { ignoreExternalFocusing } = this.props; + public componentDidUpdate(prevProps: IFocusTrapZoneProps) { + const prevForceFocusInsideTrap = prevProps.forceFocusInsideTrap !== undefined ? prevProps.forceFocusInsideTrap : true; + const newForceFocusInsideTrap = this.props.forceFocusInsideTrap !== undefined ? this.props.forceFocusInsideTrap : true; + + if (!prevForceFocusInsideTrap && newForceFocusInsideTrap) { + // Transition from forceFocusInsideTrap disabled to enabled. Emulate what happens when a FocusTrapZone gets mounted + FocusTrapZone._focusStack.push(this); + this._bringFocusIntoZone(); + } else if (prevForceFocusInsideTrap && !newForceFocusInsideTrap) { + // Transition from forceFocusInsideTrap enabled to disabled. Emulate what happens when a FocusTrapZone gets unmounted + FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { + return this !== value; + }); + this._returnFocusToInitiator(); + } + } + public componentWillUnmount(): void { this._events.dispose(); FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { return this !== value; }); - const activeElement = document.activeElement as HTMLElement; - if ( - !ignoreExternalFocusing && - this._previouslyFocusedElementOutsideTrapZone && - typeof this._previouslyFocusedElementOutsideTrapZone.focus === 'function' && - (elementContains(this._root.current, activeElement) || activeElement === document.body) - ) { - focusAsync(this._previouslyFocusedElementOutsideTrapZone); - } + this._returnFocusToInitiator(); } public render(): JSX.Element { @@ -114,6 +113,31 @@ export class FocusTrapZone extends BaseComponent implem } } + private _bringFocusIntoZone(): void { + const { elementToFocusOnDismiss, disableFirstFocus = false } = this.props; + + this._previouslyFocusedElementOutsideTrapZone = elementToFocusOnDismiss + ? elementToFocusOnDismiss + : (document.activeElement as HTMLElement); + if (!elementContains(this._root.current, this._previouslyFocusedElementOutsideTrapZone) && !disableFirstFocus) { + this.focus(); + } + } + + private _returnFocusToInitiator(): void { + const { ignoreExternalFocusing } = this.props; + + const activeElement = document.activeElement as HTMLElement; + if ( + !ignoreExternalFocusing && + this._previouslyFocusedElementOutsideTrapZone && + typeof this._previouslyFocusedElementOutsideTrapZone.focus === 'function' && + (elementContains(this._root.current, activeElement) || activeElement === document.body) + ) { + focusAsync(this._previouslyFocusedElementOutsideTrapZone); + } + } + private _updateEventHandlers(newProps: IFocusTrapZoneProps): void { const { isClickableOutsideFocusTrap = false, forceFocusInsideTrap = true } = newProps; diff --git a/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx b/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx index 295ab6d3f46f6..3c7ffc1b12a8a 100644 --- a/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx +++ b/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx @@ -95,7 +95,7 @@ export class PanelBase extends BaseComponent implement elementToFocusOnDismiss, firstFocusableSelector, focusTrapZoneProps, - forceFocusInsideTrap, + forceFocusInsideTrap = true, hasCloseButton, headerText, headerClassName = '', From fc16eb0ca05a1d23a1f87f6c509d37a14affa8ad Mon Sep 17 00:00:00 2001 From: Burney Jia Date: Tue, 11 Dec 2018 14:54:34 -0800 Subject: [PATCH 2/5] Changes file --- .../yubojia-focusTrapZone_2018-12-11-22-54.json | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 common/changes/office-ui-fabric-react/yubojia-focusTrapZone_2018-12-11-22-54.json diff --git a/common/changes/office-ui-fabric-react/yubojia-focusTrapZone_2018-12-11-22-54.json b/common/changes/office-ui-fabric-react/yubojia-focusTrapZone_2018-12-11-22-54.json new file mode 100644 index 0000000000000..957f1fe6eb5e6 --- /dev/null +++ b/common/changes/office-ui-fabric-react/yubojia-focusTrapZone_2018-12-11-22-54.json @@ -0,0 +1,11 @@ +{ + "changes": [ + { + "packageName": "office-ui-fabric-react", + "comment": "FocusTrapZone - Make forceFocusInsideTrap prop changes modify focus", + "type": "patch" + } + ], + "packageName": "office-ui-fabric-react", + "email": "yubojia@microsoft.com" +} \ No newline at end of file From 9746e06431146f3f2c8fcab60213c209e35df7ef Mon Sep 17 00:00:00 2001 From: Burney Jia Date: Wed, 2 Jan 2019 11:47:31 -0800 Subject: [PATCH 3/5] API Changes File --- .../office-ui-fabric-react/etc/office-ui-fabric-react.api.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts b/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts index 60497cbe18258..e495878dd8c58 100644 --- a/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts +++ b/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts @@ -1294,6 +1294,8 @@ class FocusTrapZone extends BaseComponent, implements I // (undocumented) componentDidMount(): void; // (undocumented) + componentDidUpdate(prevProps: IFocusTrapZoneProps): void; + // (undocumented) componentWillMount(): void; // (undocumented) componentWillReceiveProps(nextProps: IFocusTrapZoneProps): void; From 263451fe0dfd86f1718304bf84d455b040817f48 Mon Sep 17 00:00:00 2001 From: Burney Jia Date: Mon, 7 Jan 2019 14:42:22 -0800 Subject: [PATCH 4/5] Code review comments --- .../components/FocusTrapZone/FocusTrapZone.tsx | 18 +++++++----------- .../src/components/Panel/Panel.base.tsx | 2 +- .../src/components/Panel/Panel.types.ts | 4 ++-- 3 files changed, 10 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 113eb7af210c2..30e680464c733 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -21,9 +21,7 @@ export class FocusTrapZone extends BaseComponent implem private _hasFocusHandler: boolean; private _hasClickHandler: boolean; - public componentWillMount(): void { - FocusTrapZone._focusStack.push(this); - } + public componentWillMount(): void {} public componentDidMount(): void { this._bringFocusIntoZone(); @@ -45,23 +43,15 @@ export class FocusTrapZone extends BaseComponent implem if (!prevForceFocusInsideTrap && newForceFocusInsideTrap) { // Transition from forceFocusInsideTrap disabled to enabled. Emulate what happens when a FocusTrapZone gets mounted - FocusTrapZone._focusStack.push(this); this._bringFocusIntoZone(); } else if (prevForceFocusInsideTrap && !newForceFocusInsideTrap) { // Transition from forceFocusInsideTrap enabled to disabled. Emulate what happens when a FocusTrapZone gets unmounted - FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { - return this !== value; - }); this._returnFocusToInitiator(); } } public componentWillUnmount(): void { this._events.dispose(); - FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { - return this !== value; - }); - this._returnFocusToInitiator(); } @@ -116,6 +106,8 @@ export class FocusTrapZone extends BaseComponent implem private _bringFocusIntoZone(): void { const { elementToFocusOnDismiss, disableFirstFocus = false } = this.props; + FocusTrapZone._focusStack.push(this); + this._previouslyFocusedElementOutsideTrapZone = elementToFocusOnDismiss ? elementToFocusOnDismiss : (document.activeElement as HTMLElement); @@ -127,6 +119,10 @@ export class FocusTrapZone extends BaseComponent implem private _returnFocusToInitiator(): void { const { ignoreExternalFocusing } = this.props; + FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { + return this !== value; + }); + const activeElement = document.activeElement as HTMLElement; if ( !ignoreExternalFocusing && diff --git a/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx b/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx index 3c7ffc1b12a8a..295ab6d3f46f6 100644 --- a/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx +++ b/packages/office-ui-fabric-react/src/components/Panel/Panel.base.tsx @@ -95,7 +95,7 @@ export class PanelBase extends BaseComponent implement elementToFocusOnDismiss, firstFocusableSelector, focusTrapZoneProps, - forceFocusInsideTrap = true, + forceFocusInsideTrap, hasCloseButton, headerText, headerClassName = '', diff --git a/packages/office-ui-fabric-react/src/components/Panel/Panel.types.ts b/packages/office-ui-fabric-react/src/components/Panel/Panel.types.ts index 3cf215d62fe92..764acea35d985 100644 --- a/packages/office-ui-fabric-react/src/components/Panel/Panel.types.ts +++ b/packages/office-ui-fabric-react/src/components/Panel/Panel.types.ts @@ -129,9 +129,9 @@ export interface IPanelProps extends React.HTMLAttributes { ignoreExternalFocusing?: boolean; /** - * Indicates whether Panel should force focus inside the focus trap zone + * Indicates whether Panel should force focus inside the focus trap zone. + * If not explicitly specified, behavior aligns with FocusTrapZone's default behavior. * Deprecated, use `focusTrapZoneProps`. - * @defaultvalue true * @deprecated Use `focusTrapZoneProps`. */ forceFocusInsideTrap?: boolean; From 3dfa2e4de01b7fabe9e29bea1b50211e1f663c50 Mon Sep 17 00:00:00 2001 From: Burney Jia Date: Mon, 7 Jan 2019 16:16:55 -0800 Subject: [PATCH 5/5] Remove componentWillMount empty block --- .../office-ui-fabric-react/etc/office-ui-fabric-react.api.ts | 2 -- .../src/components/FocusTrapZone/FocusTrapZone.tsx | 2 -- 2 files changed, 4 deletions(-) diff --git a/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts b/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts index e495878dd8c58..22900502f032b 100644 --- a/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts +++ b/packages/office-ui-fabric-react/etc/office-ui-fabric-react.api.ts @@ -1296,8 +1296,6 @@ class FocusTrapZone extends BaseComponent, implements I // (undocumented) componentDidUpdate(prevProps: IFocusTrapZoneProps): void; // (undocumented) - componentWillMount(): void; - // (undocumented) componentWillReceiveProps(nextProps: IFocusTrapZoneProps): void; // (undocumented) componentWillUnmount(): void; 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 30e680464c733..03d380bf1f8bc 100644 --- a/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx +++ b/packages/office-ui-fabric-react/src/components/FocusTrapZone/FocusTrapZone.tsx @@ -21,8 +21,6 @@ export class FocusTrapZone extends BaseComponent implem private _hasFocusHandler: boolean; private _hasClickHandler: boolean; - public componentWillMount(): void {} - public componentDidMount(): void { this._bringFocusIntoZone(); this._updateEventHandlers(this.props);