Skip to content

Fix issues in scrollable pane #4055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ThomasMichon merged 6 commits into from
Mar 1, 2018
Merged

Fix issues in scrollable pane #4055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f49dc4b
Fix infinite recursion in scrollable pane
Feb 21, 2018
46715b7
Rush change
Feb 21, 2018
e02c3cf
Fix root cause of infinite loop
Feb 22, 2018
af2f883
Fixed issue with sticky headers not being added to stickyAbove
Feb 23, 2018
d4281c7
Remove ref for stickyContainer
Feb 23, 2018
9176014
Fix bad rebase
Feb 27, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions common/changes/office-ui-fabric-react/maxlus-stickyContainer_2018-02-21-19-45.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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"
}
38 changes: 5 additions & 33 deletions packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.base.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
Expand Down Expand Up @@ -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() {
Expand All @@ -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>
);
}
Expand Down Expand Up @@ -171,7 +158,6 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
}
}, 1);
}
this.notifySubscribers();

@ThomasMichon ThomasMichon Feb 21, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you should just move this call into the timeout.

this._setPlaceholderHeights(stickyList);
}
}
Expand All @@ -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) => {
Expand Down Expand Up @@ -249,4 +221,4 @@ export class ScrollablePaneBase extends BaseComponent<IScrollablePaneProps, {}>
}
return offset;
}
}
}
32 changes: 32 additions & 0 deletions packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}
}
8 changes: 0 additions & 8 deletions packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.styles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,6 @@ export const getStyles = (
},
className
],
stickyContainer: [
{
overflow: 'hidden',
position: 'absolute',
pointerEvents: 'none',

}
],
stickyAbove: [
{
top: 0,
Expand Down
6 changes: 1 addition & 5 deletions packages/office-ui-fabric-react/src/components/ScrollablePane/ScrollablePane.types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -65,4 +61,4 @@ export interface IScrollablePaneStyles {
* Style set for the stickyAbove element.
*/
stickyBelow: IStyle;
}
}
4 changes: 2 additions & 2 deletions packages/office-ui-fabric-react/src/components/Sticky/Sticky.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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))
});
}

Expand Down