Callout/Positioning: perf improvement with hidden flag and improved repositioning logic

common/changes/office-ui-fabric-react/pref-improv_2018-03-30-22-12.json

@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Callout/Positioning: Improve callout perf with hidden flag and improve repositioning logic",
+      "type": "minor"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "joschect@microsoft.com"
+}

packages/office-ui-fabric-react/src/components/Callout/Callout.types.ts

@@ -198,9 +198,18 @@ export interface ICalloutProps {
   theme?: ITheme;
 
   /**
-  * Optional styles for the component.
-  */
+   * Optional styles for the component.
+   */
   getStyles?: IStyleFunction<ICalloutContentStyleProps, ICalloutContentStyles>;
+
+  /**
+   * If specified, renders the Callout in a hidden state.
+   * Use this flag, rather than rendering a callout conditionally based on visibility,
+   * to improve rendering performance when it becomes visible.
+   * Note: When callout is hidden its content will not be rendered. It will only render
+   * once the callout is visible.
+   */
+  hidden?: boolean;
 }
 
 export interface ICalloutContentStyleProps {

packages/office-ui-fabric-react/src/components/Callout/CalloutContent.base.tsx

@@ -78,6 +78,7 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
   private _positionAttempts: number;
   private _target: Element | MouseEvent | IPoint | null;
   private _setHeightOffsetTimer: number;
+  private _hasListeners = false;
 
   constructor(props: ICalloutProps) {
     super(props);
@@ -97,7 +98,16 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
 
   public componentDidUpdate() {
     this._setInitialFocus();
-    this._updateAsyncPosition();
+    if (!this.props.hidden) {
+      if (!this._hasListeners) {
+        this._addListeners();
+      }
+      this._updateAsyncPosition();
+    } else {
+      if (this._hasListeners) {
+        this._removeListeners();
+      }
+    }
   }
 
   public componentWillMount() {
@@ -119,10 +129,19 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
     if (newProps.finalHeight !== this.props.finalHeight) {
       this._setHeightOffsetEveryFrame();
     }
+
+    // if the callout becomes hidden, then remove any positions that were placed on it.
+    if (newProps.hidden && newProps.hidden !== this.props.hidden) {
+      this.setState({
+        positions: undefined
+      });
+    }
   }
 
   public componentDidMount() {
-    this._onComponentDidMount();
+    if (!this.props.hidden) {
+      this._onComponentDidMount();
+    }
   }
 
   public render() {
@@ -174,11 +193,13 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
     );
 
     const overflowStyle: React.CSSProperties = overflowYHidden ? { overflowY: 'hidden' } : {};
+    const visibilityStyle: React.CSSProperties | undefined = this.props.hidden ? { visibility: 'hidden' } : undefined;
     // React.CSSProperties does not understand IRawStyle, so the inline animations will need to be cast as any for now.
     const content = (
       <div
         ref={ this._hostElement }
         className={ this._classNames.container }
+        style={ visibilityStyle }
       >
         <div
           className={ css(this._classNames.root, positions && positions.targetEdge && ANIMATIONS[positions.targetEdge!]) }
@@ -195,7 +216,7 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
             />) }
           { beakVisible &&
             (<div className={ this._classNames.beakCurtain } />) }
-          <Popup
+          { !this.props.hidden && <Popup
             role={ role }
             ariaLabel={ ariaLabel }
             ariaDescribedBy={ ariaDescribedBy }
@@ -208,8 +229,9 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
           >
             { children }
           </Popup>
+          }
         </div>
-      </div>
+      </div >
     );
 
     return content;
@@ -247,11 +269,23 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
   protected _setInitialFocus = (): void => {
     if (this.props.setInitialFocus && !this._didSetInitialFocus && this.state.positions && this._calloutElement.value) {
       this._didSetInitialFocus = true;
-      focusFirstChild(this._calloutElement.value);
+      this._async.requestAnimationFrame(() => focusFirstChild(this._calloutElement.value!));
     }
   }
 
   protected _onComponentDidMount = (): void => {
+
+    this._addListeners();
+
+    if (this.props.onLayerMounted) {
+      this.props.onLayerMounted();
+    }
+
+    this._updateAsyncPosition();
+    this._setHeightOffsetEveryFrame();
+  }
+
+  private _addListeners() {
     // This is added so the callout will dismiss when the window is scrolled
     // but not when something inside the callout is scrolled. The delay seems
     // to be required to avoid React firing an async focus event in IE from
@@ -261,14 +295,16 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
       this._events.on(this._targetWindow, 'resize', this.dismiss, true);
       this._events.on(this._targetWindow.document.body, 'focus', this._dismissOnLostFocus, true);
       this._events.on(this._targetWindow.document.body, 'click', this._dismissOnLostFocus, true);
+      this._hasListeners = true;
     }, 0);
+  }
 
-    if (this.props.onLayerMounted) {
-      this.props.onLayerMounted();
-    }
-
-    this._updateAsyncPosition();
-    this._setHeightOffsetEveryFrame();
+  private _removeListeners() {
+    this._events.off(this._targetWindow, 'scroll', this._dismissOnScroll, true);
+    this._events.off(this._targetWindow, 'resize', this.dismiss, true);
+    this._events.off(this._targetWindow.document.body, 'focus', this._dismissOnLostFocus, true);
+    this._events.off(this._targetWindow.document.body, 'click', this._dismissOnLostFocus, true);
+    this._hasListeners = false;
   }
 
   private _updateAsyncPosition() {
@@ -299,7 +335,7 @@ export class CalloutContentBase extends BaseComponent<ICalloutProps, ICalloutSta
       currentProps = assign(currentProps, this.props);
       currentProps!.bounds = this._getBounds();
       currentProps!.target = this._target!;
-      const newPositions: ICalloutPositionedInfo = positionCallout(currentProps!, hostElement, calloutElement);
+      const newPositions: ICalloutPositionedInfo = positionCallout(currentProps!, hostElement, calloutElement, positions);
 
       // Set the new position only when the positions are not exists or one of the new callout positions are different.
       // The position should not change if the position is within 2 decimal places.

packages/office-ui-fabric-react/src/components/Callout/__snapshots__/Callout.test.tsx.snap

@@ -7,6 +7,7 @@ exports[`Callout renders Callout correctly 1`] = `
       {
         position: relative;
       }
+  style={undefined}
 >
   <div
     className=

packages/office-ui-fabric-react/src/components/Callout/examples/Callout.Basic.Example.tsx

@@ -33,34 +33,33 @@ export class CalloutBasicExample extends React.Component<{}, ICalloutBaiscExampl
             text={ isCalloutVisible ? 'Hide callout' : 'Show callout' }
           />
         </div>
-        { isCalloutVisible && (
-          <Callout
-            className='ms-CalloutExample-callout'
-            ariaLabelledBy={ 'callout-label-1' }
-            ariaDescribedBy={ 'callout-description-1' }
-            role={ 'alertdialog' }
-            gapSpace={ 0 }
-            target={ this._menuButtonElement }
-            onDismiss={ this._onCalloutDismiss }
-            setInitialFocus={ true }
-          >
-            <div className='ms-CalloutExample-header'>
-              <p className='ms-CalloutExample-title' id={ 'callout-label-1' }>
-                All of your favorite people
+        <Callout
+          className='ms-CalloutExample-callout'
+          ariaLabelledBy={ 'callout-label-1' }
+          ariaDescribedBy={ 'callout-description-1' }
+          role={ 'alertdialog' }
+          gapSpace={ 0 }
+          target={ this._menuButtonElement }
+          onDismiss={ this._onCalloutDismiss }
+          setInitialFocus={ true }
+          hidden={ !this.state.isCalloutVisible }
+        >
+          <div className='ms-CalloutExample-header'>
+            <p className='ms-CalloutExample-title' id={ 'callout-label-1' }>
+              All of your favorite people
               </p>
-            </div>
-            <div className='ms-CalloutExample-inner'>
-              <div className='ms-CalloutExample-content'>
-                <p className='ms-CalloutExample-subText' id={ 'callout-description-1' }>
-                  Message body is optional. If help documentation is available, consider adding a link to learn more at the bottom.
+          </div>
+          <div className='ms-CalloutExample-inner'>
+            <div className='ms-CalloutExample-content'>
+              <p className='ms-CalloutExample-subText' id={ 'callout-description-1' }>
+                Message body is optional. If help documentation is available, consider adding a link to learn more at the bottom.
                 </p>
-              </div>
-              <div className='ms-CalloutExample-actions'>
-                <Link className='ms-CalloutExample-link' href='http://microsoft.com'>Go to microsoft</Link>
-              </div>
             </div>
-          </Callout>
-        ) }
+            <div className='ms-CalloutExample-actions'>
+              <Link className='ms-CalloutExample-link' href='http://microsoft.com'>Go to microsoft</Link>
+            </div>
+          </div>
+        </Callout>
       </div>
     );
   }

packages/office-ui-fabric-react/src/components/Dropdown/Dropdown.tsx

@@ -549,7 +549,9 @@ export class Dropdown extends BaseComponent<IDropdownInternalProps, IDropdownSta
 
   private _onPositioned = (): void => {
     if (this._focusZone.value) {
-      this._focusZone.value.focus();
+      // Focusing an element can trigger a reflow. Making this wait until there is an animation
+      // frame can improve perf significantly.
+      this._async.requestAnimationFrame(() => this._focusZone.value!.focus());
     }
   }
 

packages/office-ui-fabric-react/src/components/Popup/Popup.tsx

@@ -10,10 +10,14 @@ import {
 } from '../../Utilities';
 import { IPopupProps } from './Popup.types';
 
+export interface IPopupState {
+  needsVerticalScrollBar?: boolean;
+}
+
 /**
  * This adds accessibility to Dialog and Panel controls
  */
-export class Popup extends BaseComponent<IPopupProps, {}> {
+export class Popup extends BaseComponent<IPopupProps, IPopupState> {
 
   public static defaultProps: IPopupProps = {
     shouldRestoreFocus: true
@@ -24,6 +28,11 @@ export class Popup extends BaseComponent<IPopupProps, {}> {
   private _originalFocusedElement: HTMLElement;
   private _containsFocus: boolean;
 
+  public constructor(props: IPopupProps) {
+    super(props);
+    this.state = { needsVerticalScrollBar: false };
+  }
+
   public componentWillMount() {
     this._originalFocusedElement = getDocument()!.activeElement as HTMLElement;
   }
@@ -39,6 +48,12 @@ export class Popup extends BaseComponent<IPopupProps, {}> {
     if (doesElementContainFocus(this._root.value)) {
       this._containsFocus = true;
     }
+
+    this._updateScrollBarAsync();
+  }
+
+  public componentDidUpdate() {
+    this._updateScrollBarAsync();
   }
 
   public componentWillUnmount(): void {
@@ -59,12 +74,6 @@ export class Popup extends BaseComponent<IPopupProps, {}> {
   public render() {
     const { role, className, ariaLabel, ariaLabelledBy, ariaDescribedBy, style } = this.props;
 
-    let needsVerticalScrollBar = false;
-    if (this._root.value && this._root.value.firstElementChild) {
-      needsVerticalScrollBar = this._root.value.clientHeight > 0
-        && this._root.value.firstElementChild.clientHeight > this._root.value.clientHeight;
-    }
-
     return (
       <div
         ref={ this._root }
@@ -75,7 +84,7 @@ export class Popup extends BaseComponent<IPopupProps, {}> {
         aria-labelledby={ ariaLabelledBy }
         aria-describedby={ ariaDescribedBy }
         onKeyDown={ this._onKeyDown }
-        style={ { overflowY: needsVerticalScrollBar ? 'scroll' : 'auto', ...style } }
+        style={ { overflowY: this.state.needsVerticalScrollBar ? 'scroll' : 'auto', ...style } }
       >
         { this.props.children }
       </div>
@@ -97,6 +106,25 @@ export class Popup extends BaseComponent<IPopupProps, {}> {
     }
   }
 
+  private _updateScrollBarAsync() {
+    this._async.requestAnimationFrame(() => {
+      this._getScrollBar();
+    });
+  }
+
+  private _getScrollBar() {
+    let needsVerticalScrollBar = false;
+    if (this._root && this._root.value && this._root.value.firstElementChild) {
+      needsVerticalScrollBar = this._root.value.clientHeight > 0
+          && this._root.value.firstElementChild.clientHeight > this._root.value.clientHeight;
+    }
+    if (this.state.needsVerticalScrollBar !== needsVerticalScrollBar) {
+      this.setState({
+        needsVerticalScrollBar: needsVerticalScrollBar
+      });
+    }
+  }
+
   private _onFocus() {
     this._containsFocus = true;
   }