Sticky ScrollablePane new props 7.x #9525

tharshada · 2019-06-20T14:58:02Z

Pull request checklist

Addresses an existing issue: Fixes #0000
Include a change request file using $ npm run change

Description of changes

ScrollablePane has three children: contentContainer, stickyAbove container, stickyBelow container.

<ScrollablePane>
  <Sticky><Breadcrumb {...breadcrumbProps} /></Sticky>
  <DetailsList {...detailsListProps} />
</ScrollablePane>

Initially, all the content is inside contentContainer. Depending on stickyPosition prop, a Sticky component inside ScrollablePane can stickyToTop or stickyToBottom or both.

When a Sticky component is non-sticky, its actual content (<Breadcrumb {...breadcrumbProps} /> in the example above), is inside contentContainer.

When it becomes sticky, a placeHolder of offsetheight & scrollWidth same as the actual content of the component is placed inside contentContainer and the actual content is removed from it & added inside stickyAbove container. The contentContainer is responsible for providing scrollbars to scroll in case of overflow. If the placeholder width is not set, and the actual content was only reason for horizontal overflow, there would be no horizontal scrollbar to scroll and see the overflow content of Sticky component.

When a Sticky component has become sticky, its actual content won't be in sync to contentContainer.scrollLeft on its own. To solve this, on contentContainer scroll event is attached whose handler is responsible for syncing scroll for Sticky components. The other reason for having this handler is, as user scrolls vertically, Sticky components would become sticky or non-sticky. To determine if a Sticky component has become sticky/non-sticky, some calculations are done. When a component becomes non-sticky from sticky, its actual content is removed from stickyAbove container and added back to its original place in DOM.

If there are many Sticky components, they must be added in a particular order in sticky container so that they appear in the same order as in non-sticky state. To ensure this, a sorting is done based on distanceFromTop (calculated for non-sticky div), in Sticky componentDidUpdate if this value gets changed.

The new props are being added with an intention of optimizing performance.

experimentalLayoutImprovements?: boolean - If true, enable the following performance optimizations:
- Don't rearrange/re-sort stickies in sticky container if distanceFromTop is changed. The sorting is done only one time for every Sticky componentDidMount. For the purpose of sorting, order?: number prop has been added to Sticky.
- Removing actual content from its actual place in DOM, calculating offsetHeight &
  scrollWidth for placeholder and adding actual content to stickyContainer is not done every time Sticky component becomes sticky from non-sticky and vice-versa.
- The other optimization is done to ensure the offsetHeight & scrollWidth calculations are not done and moving actual content to/from its actual place in DOM from/to stickyContainer is not done every time component becomes sticky/non-sticky. It is achieved by duplicating the actual content at its actual place in DOM and in the stickyContainer.
  - This also makes it possible to skip the offsetHeight calculations for stickyContainers.
stickyAbove container contains sticky content for all Sticky component(s) for which stickyPosition prop is StickyPosition.Header. It assumes StickyOnScroll behavior for whichstickyPosition prop is StickyPosition.Header
stickyBelow container contains sticky content for all Sticky component(s) for which stickyPosition prop is StickyPosition.Footer.
It assumes StickyAlways behavior for whichstickyPosition prop is StickyPosition.Footer.
StickyContainerBehaviorType options:
- Default: This behavior impacts PLT. It would calculate
  distance from top for nonSticky div even if user interaction has not started. If there is a large DetailsList and the DetailsFooter should be sticky, this calculation helps to ensure the footer is sticky and visible at the bottom without any user interaction (scrolling).
- StickyOnScroll: Sticky calculations are only started after the first vertical scroll, which means the calculations are skipped in PLT. After first scroll, the behavior is similar to Default. StickyOnScroll is most suitable for headers. If it was used on the DetailsFooter in the previous example, the footer would be visible only after first vertical scroll. Perf optimization assumes this behavior for all Sticky component having 'StickyPosition.Header'.
- StickyAlways is the least expensive behavior. It ensures the component is always sticky independent of vertical scroll and will always appear at its sticky position. However, StickyAlways is still different from css postion: fixed. Suppose there are multiple components which have sticky behavior StickyAlways and the same sticky position (header/footer). In this case, StickyAlways would not need top or bottom values for these components. StickyAlways is most suitable if sticky position is Header or Footer and there is no content above or below the component respectively. Perf optimization assumes this behavior for all Sticky component having 'StickyPosition.Footer'.
Height of horizontal scrollbar & width of vertical scrollbar are needed to correctly position stickyBelow container and stickyAbove container respectively. If scrollbarVisibility={ScrollbarVisibility.always}, height and width calculations for scrollbars are not done for every mutation callback. Scrollbars' height & width are calculated only once per mount phase.

How new props are to be used

 <ScrollablePane 
   scrollbarVisibility={ScrollbarVisibility.always} 
   experimentalLayoutImprovements={true}
>
  <Sticky 
    order={1}
    stickyPosition={StickyPositionType.Header} 
    stickyBackgroundColor={getTheme().palette.white}
  >
    …
  </Sticky>

  <Sticky 
    order={2}
    stickyPosition={StickyPositionType.Header} 
    stickyBackgroundColor={getTheme().palette.white}
  >
    …
  </Sticky>

  <Sticky 
    order={1}
    stickyPosition={StickyPositionType.Footer} 
    stickyBackgroundColor={getTheme().palette.white}
  >
    …
  </Sticky>
</ScrollablePane>

#9060

Microsoft Reviewers: Open in CodeFlow

…icky

is non-sticky

calculation if scrollbar visibility is set to 'always'

…verflow if scrollbar visibility is set to 'always'

removing example

msft-github-bot · 2019-06-20T15:23:00Z

Component Perf Analysis

No significant results to display.

All results

Scenario	Master Ticks	PR Ticks
BaseButton	828	848
BaseButton (experiments)	1125	1121
DefaultButton	1157	1107
DefaultButton (experiments)	2153	2234
DetailsRow	3684	3666
DetailsRow (fast icons)	3553	3558
DetailsRow without styles	3380	3347
DocumentCardTitle with truncation	34189	34268
MenuButton	1471	1477
MenuButton (experiments)	3957	3935
PrimaryButton	1317	1274
PrimaryButton (experiments)	2162	2203
SplitButton	3155	3107
SplitButton (experiments)	7678	7770
Stack	510	522
Stack with Intrinsic children	1254	1269
Stack with Text children	4686	4933
Text	448	438
Toggle	987	951
Toggle (experiments)	2533	2523
button	60	84

size-auditor · 2019-06-20T15:24:39Z

Bundle test	Size (minified)	Diff from master
ScrollablePane	66.249 kB	3.828 kB
Sticky	24.391 kB	1.869 kB

Exceeds Tolerance Exceeds Baseline Below Baseline 1 kB = 1000 bytes

tharshada · 2019-06-20T16:50:20Z

@shagra-ms @KevinTCoughlin @leddie24 @ThomasMichon please review

…c-react into sticky-scrollablePane-newProps-7.x

natalieethell · 2019-06-21T20:34:01Z

@tharshada and I were speaking offline about some snapshot test failures following the addition of a new ScrollablePane example (not yet added to this PR). It looks like the existing ScrollablePane example pages are included in the excludedExampleFiles of ComponentExamples.test.tsx.

https://github.com/OfficeDev/office-ui-fabric-react/blob/e18c895f03ee39ff1a3b964bc9a20ec42f64d53b/packages/office-ui-fabric-react/src/components/ComponentExamples.test.tsx#L80-L81

I think this is because of an issue with ref not being supported by react-test-renderer, as mentioned in more detail here: #6681. ComponentExamples.test is using react-test-rendered to test the example files, which I think is the issue.

@JasonGore it looks like you may have some more context here. If another ScrollablePane example is added, should the example file name also be added to excludedExampleFiles?

JasonGore · 2019-06-21T21:03:02Z

That list is there for any tests causing issues.

natalieethell · 2019-06-21T22:22:31Z

@JasonGore alright, thanks! @tharshada try adding it there and let me know if that helps

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

size-auditor · 2019-09-04T06:02:08Z

Asset size changes

Project	Bundle	Baseline Size	New Size	Difference
office-ui-fabric-react	ScrollablePane	59.489 kB	59.508 kB	19 bytes
office-ui-fabric-react	Sticky	22.533 kB	22.297 kB	-236 bytes

Over Tolerance (1024 B) Over Baseline Below Baseline New Removed 1 kB = 1000 B

Baseline commit: 30384d21f414d50394e8ce1657c9bbc986f93c4c (build)

…om/tharshada/office-ui-fabric-react into sticky-scrollablePane-newProps-7.x

…c-react into sticky-scrollablePane-newProps-7.x

tharshada · 2019-09-04T19:06:43Z

@betrue-final-final there has been some changes in DetailsHeader component, the screeners for ScrollablePane component 've to be updated. Also, accept the new UI states for ScrollablePane component.

natalieethell · 2019-09-19T22:35:38Z

@tharshada It looks like there's a merge conflict - want to try merging with the latest master and pushing to your branch?

tharshada · 2019-09-20T06:41:49Z

@tharshada It looks like there's a merge conflict - want to try merging with the latest master and pushing to your branch?

I am working on minimizing the bundle size by making code changes, it 'll take time...

…c-react into sticky-scrollablePane-newProps-7.x

ecraig12345 · 2019-10-17T23:50:27Z

Now that @tharshada has added a PR description (I further updated the markdown to be easier to read) and all build steps are passing, can someone familiar with the area take another look at this PR? @dzearing @KevinTCoughlin @leddie24 @ThomasMichon

tharshada · 2019-10-22T04:47:11Z

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

+    if (
+      _getStateStickyTopHeight(prevState) !== _getStateStickyTopHeight(state) ||
+      _getStateStickyBottomHeight(prevState) !== _getStateStickyBottomHeight(state)
+    ) {
      this.notifySubscribers();
    }

    this._async.setTimeout(this._onWindowResize, 0);


!this._perf() && this._async.setTimeout(this._onWindowResize, 0);

tharshada · 2019-10-22T04:53:37Z

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

+   * for no Sticky component, prop 'stickyPosition' is 'StickyPositionType.Both'.
+   *
+   * It assumes:
+   * 1. 'OnScroll' sticky behavior for all Sticky components, props


modify comment to- 'OnScroll' sticky behavior for all Sticky components, having 'stickyPosition' prop equal to StickyPosition.Header

tharshada · 2019-10-22T04:54:02Z

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

+   * It assumes:
+   * 1. 'OnScroll' sticky behavior for all Sticky components, props
+   * 'stickyPosition' is 'StickyPositionType.Header',
+   * 2. 'Always' sticky behavior for all Sticky components, prop


modify comment same as above

tharshada · 2019-10-22T04:55:01Z

...c/components/ScrollablePane/examples/ScrollablePane.Sticky.Optimized.DetailsList.Example.tsx

+
+  public render(): JSX.Element {
+    const { items } = this.state;
+    const stickyBackgroundColor = getTheme().palette.red;


getTheme().palette.white;

tharshada · 2019-10-22T04:55:40Z

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

+ * Returns elem.offsetParent
+ * @param elem
+ */
+function _getoffsetParent(elem: HTMLElement): Element | null {


change to _getOffsetParent

tharshada · 2019-10-22T04:59:12Z

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

+    backgroundColor: stickyBackgroundColor
+  };
+}
+// debatable


remove this

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

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

micahgodbolt · 2019-12-05T17:31:53Z

@tharshada is this PR still active? Can we close it down until you're ready to work on it again?

tharshada · 2019-12-05T17:35:17Z

@tharshada is this PR still active? Can we close it down until you're ready to work on it again?

done

tharshada added 14 commits June 19, 2019 20:28

prop to opt for placeholder vs. actual element

9b0ff4b

ScrollablePane doesn't listen to events or observe mutations if no St…

d1ee070

…icky

caching scroll values; no scroll sync needed if component

c243fdb

is non-sticky

fix non sticky placeholder logic

b4c4979

minimize reflow due to scrollbar height/width

c63cbf8

calculation if scrollbar visibility is set to 'always'

horizontal & vertical scrollbars are visible independent of content o…

f85060f

…verflow if scrollbar visibility is set to 'always'

adding prop 'order' for Sticky to arrange stickies (top to bottom)

62383a6

adding containerBehavior prop

53028e4

adding example with new sticky/scrollablePane props

0b3a8f1

updating api

3a99a03

updating snapshot for ScrollablePane and

3a992f0

removing example

adding screener test

0482462

change file

8b82da3

updating test

de5093c

msft-github-bot added the PR: API Modified label Jun 20, 2019

msft-github-bot assigned natalieethell Jun 20, 2019

tharshada mentioned this pull request Jun 20, 2019

Scrollable pane sticky new props #9060

Closed

2 tasks

tharshada added 2 commits June 20, 2019 22:50

updating screener

e63a6dc

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

e6c2f16

…c-react into sticky-scrollablePane-newProps-7.x

tharshada mentioned this pull request Jun 20, 2019

Sticky scrollable pane new props 6.x #9527

Closed

2 tasks

tharshada added 2 commits June 21, 2019 18:26

read scrollbar height/width from local storage

01637c3

read scrollbar height/width if available before mounting

1b70cd3

adding example

a2da775

shagra-ms reviewed Jun 25, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/Sticky/Sticky.types.ts Show resolved Hide resolved

Update ScrollablePane.test.tsx

60c1e27

tharshada added 7 commits September 4, 2019 14:33

adding uts

ca65dab

Merge branch 'sticky-scrollablePane-newProps-7.x' of https://github.c…

d56093a

…om/tharshada/office-ui-fabric-react into sticky-scrollablePane-newProps-7.x

update setStickyContainerHeight

e42a6b4

updating unit test

3ab11e9

fixing import

20f89f4

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

9dd2983

…c-react into sticky-scrollablePane-newProps-7.x

adding UI state

3ffa093

tharshada added 2 commits October 16, 2019 20:10

Merge branch 'master' of https://github.com/OfficeDev/office-ui-fabri…

6d84fd3

…c-react into sticky-scrollablePane-newProps-7.x

refactoring code to minimize bundle size

9959ae5

tharshada requested review from jdhuntington, dzearing, KevinTCoughlin and leddie24 October 16, 2019 18:17

tharshada commented Oct 22, 2019

View reviewed changes

ecraig12345 reviewed Oct 22, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx Show resolved Hide resolved

ecraig12345 reviewed Oct 22, 2019

View reviewed changes

packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx Show resolved Hide resolved

tharshada closed this Dec 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sticky ScrollablePane new props 7.x #9525

Sticky ScrollablePane new props 7.x #9525

tharshada commented Jun 20, 2019 •

edited

Loading

msft-github-bot commented Jun 20, 2019 •

edited

Loading

size-auditor bot commented Jun 20, 2019 •

edited

Loading

tharshada commented Jun 20, 2019

natalieethell commented Jun 21, 2019

JasonGore commented Jun 21, 2019

natalieethell commented Jun 21, 2019

size-auditor bot commented Sep 4, 2019 •

edited

Loading

tharshada commented Sep 4, 2019

natalieethell commented Sep 19, 2019

tharshada commented Sep 20, 2019

ecraig12345 commented Oct 17, 2019

tharshada Oct 22, 2019

tharshada Oct 22, 2019

tharshada Oct 22, 2019

tharshada Oct 22, 2019

tharshada Oct 22, 2019

tharshada Oct 22, 2019

micahgodbolt commented Dec 5, 2019

tharshada commented Dec 5, 2019

Sticky ScrollablePane new props 7.x #9525

Sticky ScrollablePane new props 7.x #9525

Conversation

tharshada commented Jun 20, 2019 • edited Loading

Pull request checklist

Description of changes

How new props are to be used

Microsoft Reviewers: Open in CodeFlow

msft-github-bot commented Jun 20, 2019 • edited Loading

Component Perf Analysis

size-auditor bot commented Jun 20, 2019 • edited Loading

tharshada commented Jun 20, 2019

natalieethell commented Jun 21, 2019

JasonGore commented Jun 21, 2019

natalieethell commented Jun 21, 2019

size-auditor bot commented Sep 4, 2019 • edited Loading

Asset size changes

tharshada commented Sep 4, 2019

natalieethell commented Sep 19, 2019

tharshada commented Sep 20, 2019

ecraig12345 commented Oct 17, 2019

tharshada Oct 22, 2019

Choose a reason for hiding this comment

tharshada Oct 22, 2019

Choose a reason for hiding this comment

tharshada Oct 22, 2019

Choose a reason for hiding this comment

tharshada Oct 22, 2019

Choose a reason for hiding this comment

tharshada Oct 22, 2019

Choose a reason for hiding this comment

tharshada Oct 22, 2019

Choose a reason for hiding this comment

micahgodbolt commented Dec 5, 2019

tharshada commented Dec 5, 2019

tharshada commented Jun 20, 2019 •

edited

Loading

msft-github-bot commented Jun 20, 2019 •

edited

Loading

size-auditor bot commented Jun 20, 2019 •

edited

Loading

size-auditor bot commented Sep 4, 2019 •

edited

Loading