ScrollablePane: Optimization on how component works #4491
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…instead of root. Need to fix sticky bottom behavior still.
… setState once. Need to fix DetailsList behavior
… on StickyTop and StickyBottom after mounting/unmounting sticky.
…lsList example. Remove unused componentDidUpdate in Sticky.
…Top or stickyBottom
… stickyTopStyle and stickyBottomStyle
…dden for Top/Bottom sticky
… if props/state changes (preventing excessive setState with updateStickyRefHeights). Change this.root to this.contentContainer for check for initialScrollPosition prop. Move logic for appending child to this.stickyAbove/Below into sortSticky.
…. Fix _removeStickyFromContainers not removing stickies properly.
…hts. Change getScrollPosition to check for contentContainer.scrollTop
2 tasks
…ion and set distances once ScrollablePane has mounted
…c-react into edwl/fixScrollablePaneBehavior # Conflicts: # packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx # packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx
…sing duplicate props.children to placeholder divs
…c-react into edwl/fixScrollablePaneBehavior # Conflicts: # packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx # packages/office-ui-fabric-react/src/components/ScrollablePane/examples/ScrollablePane.Default.Example.tsx # packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx
…c-react into edwl/fixScrollablePaneBehavior # Conflicts: # packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx
|
@ThomasMichon @MaxLustig can you re-review at your earliest convenience please? |
…c-react into edwl/fixScrollablePaneBehavior # Conflicts: # packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.styles.ts
… stickytop/bottom. Add width on sticky component so element doesn't extend past to scrollbar.
ThomasMichon
reviewed
May 3, 2018
| zIndex: ZIndexes.ScrollablePane | ||
| }; | ||
|
|
||
| return ({ | ||
| root: [ | ||
| classNames.root, | ||
| { | ||
| WebkitOverflowScrolling: 'touch', |
There was a problem hiding this comment.
Should this be a lowercase w?
ThomasMichon
reviewed
May 3, 2018
| overflowY: 'hidden', | ||
| overflowX: 'auto' | ||
| }; | ||
|
|
There was a problem hiding this comment.
Can we get rid of the maxHeightStyles altogether now?
ThomasMichon
approved these changes
May 3, 2018
…StickyPlaceholder to use position absolute instead of display block/none.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull request checklist
$ npm run changeDescription of changes
position: absoluteto occupy parent spaceFocus areas to test
(optional)