Fix issues in scrollable pane (#4055)

* Fix infinite recursion in scrollable pane * Rush change * Fix root cause of infinite loop * Fixed issue with sticky headers not being added to stickyAbove * Remove ref for stickyContainer * Fix bad rebase
microsoft · Mar 1, 2018 · 5c53827 · 5c53827
1 parent fdab0a0
commit 5c53827
Show file tree

Hide file tree

Showing 6 changed files with 51 additions and 48 deletions.
diff --git a/common/changes/office-ui-fabric-react/maxlus-stickyContainer_2018-02-21-19-45.json b/common/changes/office-ui-fabric-react/maxlus-stickyContainer_2018-02-21-19-45.json
@@ -0,0 +1,11 @@
+{
+  "changes": [
+    {
+      "packageName": "office-ui-fabric-react",
+      "comment": "Fix infinite recursion in scrollable pane",
+      "type": "patch"
+    }
+  ],
+  "packageName": "office-ui-fabric-react",
+  "email": "maxlus@microsoft.com"
+}
diff --git a/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx b/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx
@@ -32,7 +32,6 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
   };
 
   public root: HTMLElement;
-  public stickyContainer: HTMLElement;
   public stickyAbove: HTMLElement;
   public stickyBelow: HTMLElement;
   private _subscribers: Set<Function>;
@@ -63,22 +62,11 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
   public componentDidMount() {
     this._events.on(this.root, 'scroll', this.notifySubscribers);
     this._events.on(window, 'resize', this._onWindowResize);
-    this._async.setTimeout(() => {
-      this._resizeContainer();
-      if (this.stickyContainer.parentElement && this.root.parentElement) {
-        this.stickyContainer.parentElement.removeChild(this.stickyContainer);
-        this.root.parentElement.insertBefore(this.stickyContainer, this.root.nextSibling);
-        this.notifySubscribers();
-      }
-    }, 500);
   }
 
   public componentWillUnmount() {
     this._events.off(this.root);
     this._events.off(window);
-    if (this.stickyContainer.parentElement) {
-      this.stickyContainer.parentElement.removeChild(this.stickyContainer);
-    }
   }
 
   public render() {
@@ -95,13 +83,12 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
         { ...getNativeProps(this.props, divProperties) }
         ref={ this._resolveRef('root') }
         className={ classNames.root }
-        data-is-scrollable={ true }
       >
-        <div ref={ this._resolveRef('stickyContainer') } className={ classNames.stickyContainer }>
-          <div ref={ this._resolveRef('stickyAbove') } className={ classNames.stickyAbove } />
-          <div ref={ this._resolveRef('stickyBelow') } className={ classNames.stickyBelow } />
+        <div ref={ this._resolveRef('stickyAbove') } className={ classNames.stickyAbove } />
+        <div ref={ this._resolveRef('stickyBelow') } className={ classNames.stickyBelow } />
+        <div data-is-scrollable={ true }>
+          { this.props.children }
         </div>
-        { this.props.children }
       </div>
     );
   }
@@ -171,7 +158,6 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
           }
         }, 1);
       }
-      this.notifySubscribers();
       this._setPlaceholderHeights(stickyList);
     }
   }
@@ -186,26 +172,12 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
 
   private _onWindowResize() {
     this._async.setTimeout(() => {
-      this._resizeContainer();
       this.notifySubscribers();
       this._setPlaceholderHeights(this._stickyAbove);
       this._setPlaceholderHeights(this._stickyBelow);
     }, 5);
   }
 
-  private _resizeContainer() {
-    const { stickyContainer, root } = this;
-    const { borderTopWidth, borderLeftWidth } = getComputedStyle(root);
-    stickyContainer.style.height = root.clientHeight + 'px';
-    stickyContainer.style.width = root.clientWidth + 'px';
-    if (borderTopWidth) {
-      stickyContainer.style.top = root.offsetTop + parseInt(borderTopWidth, 10) + 'px';
-    }
-    if (borderLeftWidth) {
-      stickyContainer.style.left = root.offsetLeft + parseInt(borderLeftWidth, 10) + 'px';
-    }
-  }
-
   @autobind
   private _setPlaceholderHeights(stickies: Set<Sticky>) {
     stickies.forEach((sticky, idx) => {
@@ -249,4 +221,4 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
     }
     return offset;
   }
-}
+}
diff --git a/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.scss b/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.scss
@@ -0,0 +1,32 @@
+@import '../../common/common';
+
+.root {
+  overflow-y: auto;
+  max-height: inherit;
+  height: inherit;
+  -webkit-overflow-scrolling: touch;
+}
+
+.stickyAbove,
+.stickyBelow {
+  position: absolute;
+  pointer-events: auto;
+  width: 100%;
+  z-index: 1;
+}
+
+.stickyAbove {
+  top: 0;
+
+  @include high-contrast {
+    border-bottom: 1px solid WindowText;
+  }
+}
+
+.stickyBelow {
+  bottom: 0;
+
+  @include high-contrast {
+    border-top: 1px solid WindowText;
+  }
+}
diff --git a/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.styles.ts b/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.styles.ts
@@ -33,14 +33,6 @@ export const getStyles = (
       },
       className
     ],
-    stickyContainer: [
-      {
-        overflow: 'hidden',
-        position: 'absolute',
-        pointerEvents: 'none',
-
-      }
-    ],
     stickyAbove: [
       {
         top: 0,

diff --git a/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.types.ts b/packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.types.ts
@@ -53,10 +53,6 @@ export interface IScrollablePaneStyles {
    * Style set for the root element.
    */
   root: IStyle;
-  /**
-   * Style set for the stickyContainer element.
-   */
-  stickyContainer: IStyle;
   /**
    * Style set for the stickyAbove element.
    */
@@ -65,4 +61,4 @@ export interface IScrollablePaneStyles {
    * Style set for the stickyAbove element.
    */
   stickyBelow: IStyle;
-}
+}
diff --git a/packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx b/packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx
@@ -145,8 +145,8 @@ export class Sticky extends BaseComponent<IStickyProps, IStickyState> {
     const canStickyFooter = stickyPosition === StickyPositionType.Both || stickyPosition === StickyPositionType.Footer;
 
     this.setState({
-      isStickyTop: canStickyHeader && ((!isStickyTop && top <= headerBound.bottom) || (isStickyTop && bottom < headerBound.bottom)),
-      isStickyBottom: canStickyFooter && ((!isStickyBottom && bottom >= footerBound.top) || (isStickyBottom && top > footerBound.top))
+      isStickyTop: canStickyHeader && ((top <= headerBound.bottom) || (isStickyTop && bottom < headerBound.bottom)),
+      isStickyBottom: canStickyFooter && ((bottom >= footerBound.top) || (isStickyBottom && top > footerBound.top))
     });
   }