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 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..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 @@ -1294,7 +1294,7 @@ class FocusTrapZone extends BaseComponent, implements I // (undocumented) componentDidMount(): void; // (undocumented) - componentWillMount(): void; + componentDidUpdate(prevProps: IFocusTrapZoneProps): void; // (undocumented) componentWillReceiveProps(nextProps: IFocusTrapZoneProps): void; // (undocumented) 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..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,20 +21,8 @@ export class FocusTrapZone extends BaseComponent implem private _hasFocusHandler: boolean; private _hasClickHandler: boolean; - public componentWillMount(): void { - FocusTrapZone._focusStack.push(this); - } - 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,25 +35,24 @@ export class FocusTrapZone extends BaseComponent implem this._updateEventHandlers(nextProps); } - public componentWillUnmount(): void { - const { ignoreExternalFocusing } = this.props; - - this._events.dispose(); - FocusTrapZone._focusStack = FocusTrapZone._focusStack.filter((value: FocusTrapZone) => { - return this !== value; - }); + public componentDidUpdate(prevProps: IFocusTrapZoneProps) { + const prevForceFocusInsideTrap = prevProps.forceFocusInsideTrap !== undefined ? prevProps.forceFocusInsideTrap : true; + const newForceFocusInsideTrap = this.props.forceFocusInsideTrap !== undefined ? this.props.forceFocusInsideTrap : true; - 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); + if (!prevForceFocusInsideTrap && newForceFocusInsideTrap) { + // Transition from forceFocusInsideTrap disabled to enabled. Emulate what happens when a FocusTrapZone gets mounted + this._bringFocusIntoZone(); + } else if (prevForceFocusInsideTrap && !newForceFocusInsideTrap) { + // Transition from forceFocusInsideTrap enabled to disabled. Emulate what happens when a FocusTrapZone gets unmounted + this._returnFocusToInitiator(); } } + public componentWillUnmount(): void { + this._events.dispose(); + this._returnFocusToInitiator(); + } + public render(): JSX.Element { const { className, ariaLabelledBy } = this.props; const divProps = getNativeProps(this.props, divProperties); @@ -114,6 +101,37 @@ 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); + if (!elementContains(this._root.current, this._previouslyFocusedElementOutsideTrapZone) && !disableFirstFocus) { + this.focus(); + } + } + + 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 && + 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.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;