From 8f8b71a27846cb0960ab9787ef7dfcc4f6ff4128 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 15 Apr 2018 18:23:11 +0200 Subject: [PATCH 1/3] Use React's new lifecycles for scrollable lists --- .../glitch/components/scrollable_list.js | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/app/javascript/flavours/glitch/components/scrollable_list.js b/app/javascript/flavours/glitch/components/scrollable_list.js index 8b1e3c93dd9..df31eb10cc8 100644 --- a/app/javascript/flavours/glitch/components/scrollable_list.js +++ b/app/javascript/flavours/glitch/components/scrollable_list.js @@ -43,7 +43,6 @@ export default class ScrollableList extends PureComponent { if (this.node) { const { scrollTop, scrollHeight, clientHeight } = this.node; const offset = scrollHeight - scrollTop - clientHeight; - this._oldScrollPosition = scrollHeight - scrollTop; if (400 > offset && this.props.onScrollToBottom && !this.props.isLoading) { this.props.onScrollToBottom(); @@ -74,21 +73,26 @@ export default class ScrollableList extends PureComponent { this.handleScroll(); } - componentDidUpdate (prevProps) { + getSnapshotBeforeUpdate (prevProps, prevState) { const someItemInserted = React.Children.count(prevProps.children) > 0 && React.Children.count(prevProps.children) < React.Children.count(this.props.children) && this.getFirstChildKey(prevProps) !== this.getFirstChildKey(this.props); + if (someItemInserted && this.node.scrollTop > 0) { + return this.node.scrollHeight - this.node.scrollTop; + } else { + return null; + } + } + componentDidUpdate (prevProps, prevState, snapshot) { // Reset the scroll position when a new child comes in in order not to // jerk the scrollbar around if you're already scrolled down the page. - if (someItemInserted && this._oldScrollPosition && this.node.scrollTop > 0) { - const newScrollTop = this.node.scrollHeight - this._oldScrollPosition; + if (snapshot !== null) { + const newScrollTop = this.node.scrollHeight - snapshot; if (this.node.scrollTop !== newScrollTop) { this.node.scrollTop = newScrollTop; } - } else { - this._oldScrollPosition = this.node.scrollHeight - this.node.scrollTop; } } From 931a4d1ebf24c3f2b92782f2ee26558ead866f16 Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 15 Apr 2018 18:46:23 +0200 Subject: [PATCH 2/3] Clean up dead code --- .../glitch/components/scrollable_list.js | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/app/javascript/flavours/glitch/components/scrollable_list.js b/app/javascript/flavours/glitch/components/scrollable_list.js index df31eb10cc8..928c49737ab 100644 --- a/app/javascript/flavours/glitch/components/scrollable_list.js +++ b/app/javascript/flavours/glitch/components/scrollable_list.js @@ -34,7 +34,7 @@ export default class ScrollableList extends PureComponent { }; state = { - lastMouseMove: null, + fullscreen: null, }; intersectionObserverWrapper = new IntersectionObserverWrapper(); @@ -56,14 +56,6 @@ export default class ScrollableList extends PureComponent { trailing: true, }); - handleMouseMove = throttle(() => { - this._lastMouseMove = new Date(); - }, 300); - - handleMouseLeave = () => { - this._lastMouseMove = null; - } - componentDidMount () { this.attachScrollListener(); this.attachIntersectionObserver(); @@ -145,10 +137,6 @@ export default class ScrollableList extends PureComponent { this.props.onScrollToBottom(); } - _recentlyMoved () { - return this._lastMouseMove !== null && ((new Date()) - this._lastMouseMove < 600); - } - render () { const { children, scrollKey, trackScroll, shouldUpdateScroll, isLoading, hasMore, prepend, emptyMessage } = this.props; const { fullscreen } = this.state; @@ -159,7 +147,7 @@ export default class ScrollableList extends PureComponent { if (isLoading || childrenCount > 0 || !emptyMessage) { scrollableArea = ( -
+
{prepend} From 694337d9bbaa2505f659d9b9980d393bd317679a Mon Sep 17 00:00:00 2001 From: Thibaut Girka Date: Sun, 15 Apr 2018 21:29:03 +0200 Subject: [PATCH 3/3] Fix auto-collapsed toots making the TL jump (fixes #417) --- .../glitch/components/scrollable_list.js | 26 ++++++++++++----- .../flavours/glitch/components/status.js | 29 ++++++++++++++++++- .../notifications/components/notification.js | 10 +++++++ 3 files changed, 56 insertions(+), 9 deletions(-) diff --git a/app/javascript/flavours/glitch/components/scrollable_list.js b/app/javascript/flavours/glitch/components/scrollable_list.js index 928c49737ab..df3ace4c19a 100644 --- a/app/javascript/flavours/glitch/components/scrollable_list.js +++ b/app/javascript/flavours/glitch/components/scrollable_list.js @@ -65,6 +65,22 @@ export default class ScrollableList extends PureComponent { this.handleScroll(); } + getScrollPosition = () => { + if (this.node && this.node.scrollTop > 0) { + return {height: this.node.scrollHeight, top: this.node.scrollTop}; + } else { + return null; + } + } + + updateScrollBottom = (snapshot) => { + const newScrollTop = this.node.scrollHeight - snapshot; + + if (this.node.scrollTop !== newScrollTop) { + this.node.scrollTop = newScrollTop; + } + } + getSnapshotBeforeUpdate (prevProps, prevState) { const someItemInserted = React.Children.count(prevProps.children) > 0 && React.Children.count(prevProps.children) < React.Children.count(this.props.children) && @@ -79,13 +95,7 @@ export default class ScrollableList extends PureComponent { componentDidUpdate (prevProps, prevState, snapshot) { // Reset the scroll position when a new child comes in in order not to // jerk the scrollbar around if you're already scrolled down the page. - if (snapshot !== null) { - const newScrollTop = this.node.scrollHeight - snapshot; - - if (this.node.scrollTop !== newScrollTop) { - this.node.scrollTop = newScrollTop; - } - } + if (snapshot !== null) this.updateScrollBottom(snapshot); } componentWillUnmount () { @@ -160,7 +170,7 @@ export default class ScrollableList extends PureComponent { intersectionObserverWrapper={this.intersectionObserverWrapper} saveHeightKey={trackScroll ? `${this.context.router.route.location.key}:${scrollKey}` : null} > - {child} + {React.cloneElement(child, {getScrollPosition: this.getScrollPosition, updateScrollBottom: this.updateScrollBottom})} ))} diff --git a/app/javascript/flavours/glitch/components/status.js b/app/javascript/flavours/glitch/components/status.js index eb621d5d744..cf82c9ac6bb 100644 --- a/app/javascript/flavours/glitch/components/status.js +++ b/app/javascript/flavours/glitch/components/status.js @@ -45,10 +45,13 @@ export default class Status extends ImmutablePureComponent { withDismiss: PropTypes.bool, onMoveUp: PropTypes.func, onMoveDown: PropTypes.func, + getScrollPosition: PropTypes.func, + updateScrollBottom: PropTypes.func, }; state = { isExpanded: null, + autoCollapsed: false, } // Avoid checking props that are functions (and whose equality will always @@ -134,7 +137,31 @@ export default class Status extends ImmutablePureComponent { default: return false; } - }()) this.setExpansion(false); + }()) { + this.setExpansion(false); + // Hack to fix timeline jumps on second rendering when auto-collapsing + this.setState({ autoCollapsed: true }); + } + } + + getSnapshotBeforeUpdate (prevProps, prevState) { + if (this.props.getScrollPosition) { + return this.props.getScrollPosition(); + } else { + return null; + } + } + + // Hack to fix timeline jumps on second rendering when auto-collapsing + componentDidUpdate (prevProps, prevState, snapshot) { + if (this.state.autoCollapsed) { + this.setState({ autoCollapsed: false }); + if (snapshot !== null && this.props.updateScrollBottom) { + if (this.node.offsetTop < snapshot.top) { + this.props.updateScrollBottom(snapshot.height - snapshot.top); + } + } + } } // `setExpansion()` sets the value of `isExpanded` in our state. It takes diff --git a/app/javascript/flavours/glitch/features/notifications/components/notification.js b/app/javascript/flavours/glitch/features/notifications/components/notification.js index cc77426d3b0..6fc7173ed7c 100644 --- a/app/javascript/flavours/glitch/features/notifications/components/notification.js +++ b/app/javascript/flavours/glitch/features/notifications/components/notification.js @@ -16,6 +16,8 @@ export default class Notification extends ImmutablePureComponent { onMoveUp: PropTypes.func.isRequired, onMoveDown: PropTypes.func.isRequired, onMention: PropTypes.func.isRequired, + getScrollPosition: PropTypes.func, + updateScrollBottom: PropTypes.func, }; render () { @@ -25,6 +27,8 @@ export default class Notification extends ImmutablePureComponent { onMoveDown, onMoveUp, onMention, + getScrollPosition, + updateScrollBottom, } = this.props; switch(notification.get('type')) { @@ -50,6 +54,8 @@ export default class Notification extends ImmutablePureComponent { onMoveDown={onMoveDown} onMoveUp={onMoveUp} onMention={onMention} + getScrollPosition={getScrollPosition} + updateScrollBottom={updateScrollBottom} withDismiss /> ); @@ -66,6 +72,8 @@ export default class Notification extends ImmutablePureComponent { onMoveDown={onMoveDown} onMoveUp={onMoveUp} onMention={onMention} + getScrollPosition={getScrollPosition} + updateScrollBottom={updateScrollBottom} withDismiss /> ); @@ -82,6 +90,8 @@ export default class Notification extends ImmutablePureComponent { onMoveDown={onMoveDown} onMoveUp={onMoveUp} onMention={onMention} + getScrollPosition={getScrollPosition} + updateScrollBottom={updateScrollBottom} withDismiss /> );