From aee93bfc9c7169f2e784de8572263de911922760 Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 11 Feb 2019 13:19:59 +0100 Subject: [PATCH] Fix timeline jumps (#10001) * Avoid two-step rendering of statuses as much as possible Cache width shared by Video player, MediaGallery and Cards at the ScrollableList level, pass it down through StatusList and Notifications. * Adjust scroll when new preview cards appear * Adjust scroll when statuses above the current scroll position are deleted --- .../mastodon/components/media_gallery.js | 10 ++- .../mastodon/components/scrollable_list.js | 28 +++++++- app/javascript/mastodon/components/status.js | 67 +++++++++++++++++-- .../notifications/components/notification.js | 32 ++++++++- .../features/status/components/card.js | 5 +- .../mastodon/features/video/index.js | 4 +- 6 files changed, 134 insertions(+), 12 deletions(-) diff --git a/app/javascript/mastodon/components/media_gallery.js b/app/javascript/mastodon/components/media_gallery.js index c507920d0..a2bc95255 100644 --- a/app/javascript/mastodon/components/media_gallery.js +++ b/app/javascript/mastodon/components/media_gallery.js @@ -194,6 +194,8 @@ class MediaGallery extends React.PureComponent { height: PropTypes.number.isRequired, onOpenMedia: PropTypes.func.isRequired, intl: PropTypes.object.isRequired, + defaultWidth: PropTypes.number, + cacheWidth: PropTypes.func, }; static defaultProps = { @@ -202,6 +204,7 @@ class MediaGallery extends React.PureComponent { state = { visible: displayMedia !== 'hide_all' && !this.props.sensitive || displayMedia === 'show_all', + width: this.props.defaultWidth, }; componentWillReceiveProps (nextProps) { @@ -221,6 +224,7 @@ class MediaGallery extends React.PureComponent { handleRef = (node) => { if (node /*&& this.isStandaloneEligible()*/) { // offsetWidth triggers a layout, so only calculate when we need to + if (this.props.cacheWidth) this.props.cacheWidth(node.offsetWidth); this.setState({ width: node.offsetWidth, }); @@ -233,8 +237,10 @@ class MediaGallery extends React.PureComponent { } render () { - const { media, intl, sensitive, height } = this.props; - const { width, visible } = this.state; + const { media, intl, sensitive, height, defaultWidth } = this.props; + const { visible } = this.state; + + const width = this.state.width || defaultWidth; let children; diff --git a/app/javascript/mastodon/components/scrollable_list.js b/app/javascript/mastodon/components/scrollable_list.js index fec06e263..0376cf85a 100644 --- a/app/javascript/mastodon/components/scrollable_list.js +++ b/app/javascript/mastodon/components/scrollable_list.js @@ -40,6 +40,7 @@ export default class ScrollableList extends PureComponent { state = { fullscreen: null, + cachedMediaWidth: 250, // Default media/card width using default Mastodon theme }; intersectionObserverWrapper = new IntersectionObserverWrapper(); @@ -130,6 +131,20 @@ export default class ScrollableList extends PureComponent { this.handleScroll(); } + getScrollPosition = () => { + if (this.node && (this.node.scrollTop > 0 || this.mouseMovedRecently)) { + return { height: this.node.scrollHeight, top: this.node.scrollTop }; + } else { + return null; + } + } + + updateScrollBottom = (snapshot) => { + const newScrollTop = this.node.scrollHeight - snapshot; + + this.setScrollTop(newScrollTop); + } + getSnapshotBeforeUpdate (prevProps) { const someItemInserted = React.Children.count(prevProps.children) > 0 && React.Children.count(prevProps.children) < React.Children.count(this.props.children) && @@ -150,6 +165,12 @@ export default class ScrollableList extends PureComponent { } } + cacheMediaWidth = (width) => { + if (width && this.state.cachedMediaWidth !== width) { + this.setState({ cachedMediaWidth: width }); + } + } + componentWillUnmount () { this.clearMouseIdleTimer(); this.detachScrollListener(); @@ -239,7 +260,12 @@ 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, + cachedMediaWidth: this.state.cachedMediaWidth, + cacheMediaWidth: this.cacheMediaWidth, + })} ))} diff --git a/app/javascript/mastodon/components/status.js b/app/javascript/mastodon/components/status.js index 2be6c4b36..386404b57 100644 --- a/app/javascript/mastodon/components/status.js +++ b/app/javascript/mastodon/components/status.js @@ -69,6 +69,10 @@ class Status extends ImmutablePureComponent { onMoveUp: PropTypes.func, onMoveDown: PropTypes.func, showThread: PropTypes.bool, + getScrollPosition: PropTypes.func, + updateScrollBottom: PropTypes.func, + cacheMediaWidth: PropTypes.func, + cachedMediaWidth: PropTypes.number, }; // Avoid checking props that are functions (and whose equality will always @@ -80,6 +84,43 @@ class Status extends ImmutablePureComponent { 'hidden', ]; + // Track height changes we know about to compensate scrolling + componentDidMount () { + this.didShowCard = !this.props.muted && !this.props.hidden && this.props.status.get('card'); + } + + getSnapshotBeforeUpdate () { + if (this.props.getScrollPosition) { + return this.props.getScrollPosition(); + } else { + return null; + } + } + + // Compensate height changes + componentDidUpdate (prevProps, prevState, snapshot) { + const doShowCard = !this.props.muted && !this.props.hidden && this.props.status.get('card'); + if (doShowCard && !this.didShowCard) { + this.didShowCard = true; + if (snapshot !== null && this.props.updateScrollBottom) { + if (this.node && this.node.offsetTop < snapshot.top) { + this.props.updateScrollBottom(snapshot.height - snapshot.top); + } + } + } + } + + componentWillUnmount() { + if (this.node && this.props.getScrollPosition) { + const position = this.props.getScrollPosition(); + if (position !== null && this.node.offsetTop < position.top) { + requestAnimationFrame(() => { + this.props.updateScrollBottom(position.height - position.top); + }); + } + } + } + handleClick = () => { if (this.props.onClick) { this.props.onClick(); @@ -166,6 +207,10 @@ class Status extends ImmutablePureComponent { } } + handleRef = c => { + this.node = c; + } + render () { let media = null; let statusAvatar, prepend, rebloggedByText; @@ -180,7 +225,7 @@ class Status extends ImmutablePureComponent { if (hidden) { return ( -
+
{status.getIn(['account', 'display_name']) || status.getIn(['account', 'username'])} {status.get('content')}
@@ -195,7 +240,7 @@ class Status extends ImmutablePureComponent { return ( -
+
@@ -243,11 +288,12 @@ class Status extends ImmutablePureComponent { preview={video.get('preview_url')} src={video.get('url')} alt={video.get('description')} - width={239} + width={this.props.cachedMediaWidth} height={110} inline sensitive={status.get('sensitive')} onOpenVideo={this.handleOpenVideo} + cacheWidth={this.props.cacheMediaWidth} /> )} @@ -255,7 +301,16 @@ class Status extends ImmutablePureComponent { } else { media = ( - {Component => } + {Component => ( + + )} ); } @@ -265,6 +320,8 @@ class Status extends ImmutablePureComponent { onOpenMedia={this.props.onOpenMedia} card={status.get('card')} compact + cacheWidth={this.props.cacheMediaWidth} + defaultWidth={this.props.cachedMediaWidth} /> ); } @@ -291,7 +348,7 @@ class Status extends ImmutablePureComponent { return ( -
+
{prepend}
diff --git a/app/javascript/mastodon/features/notifications/components/notification.js b/app/javascript/mastodon/features/notifications/components/notification.js index 0bd8d47c3..9669b6e7d 100644 --- a/app/javascript/mastodon/features/notifications/components/notification.js +++ b/app/javascript/mastodon/features/notifications/components/notification.js @@ -35,6 +35,10 @@ class Notification extends ImmutablePureComponent { onToggleHidden: PropTypes.func.isRequired, status: PropTypes.option, intl: PropTypes.object.isRequired, + getScrollPosition: PropTypes.func, + updateScrollBottom: PropTypes.func, + cacheMediaWidth: PropTypes.func, + cachedMediaWidth: PropTypes.number, }; handleMoveUp = () => { @@ -129,6 +133,10 @@ class Notification extends ImmutablePureComponent { onMoveDown={this.handleMoveDown} onMoveUp={this.handleMoveUp} contextType='notifications' + getScrollPosition={this.props.getScrollPosition} + updateScrollBottom={this.props.updateScrollBottom} + cachedMediaWidth={this.props.cachedMediaWidth} + cacheMediaWidth={this.props.cacheMediaWidth} /> ); } @@ -149,7 +157,17 @@ class Notification extends ImmutablePureComponent {
-
); @@ -171,7 +189,17 @@ class Notification extends ImmutablePureComponent {
-
); diff --git a/app/javascript/mastodon/features/status/components/card.js b/app/javascript/mastodon/features/status/components/card.js index 8237de84d..0eff54411 100644 --- a/app/javascript/mastodon/features/status/components/card.js +++ b/app/javascript/mastodon/features/status/components/card.js @@ -61,6 +61,8 @@ export default class Card extends React.PureComponent { maxDescription: PropTypes.number, onOpenMedia: PropTypes.func.isRequired, compact: PropTypes.bool, + defaultWidth: PropTypes.number, + cacheWidth: PropTypes.func, }; static defaultProps = { @@ -69,7 +71,7 @@ export default class Card extends React.PureComponent { }; state = { - width: 280, + width: this.props.defaultWidth || 280, embedded: false, }; @@ -112,6 +114,7 @@ export default class Card extends React.PureComponent { setRef = c => { if (c) { + if (this.props.cacheWidth) this.props.cacheWidth(c.offsetWidth); this.setState({ width: c.offsetWidth }); } } diff --git a/app/javascript/mastodon/features/video/index.js b/app/javascript/mastodon/features/video/index.js index 15a5de4b8..894fe78d9 100644 --- a/app/javascript/mastodon/features/video/index.js +++ b/app/javascript/mastodon/features/video/index.js @@ -100,6 +100,7 @@ class Video extends React.PureComponent { onCloseVideo: PropTypes.func, detailed: PropTypes.bool, inline: PropTypes.bool, + cacheWidth: PropTypes.func, intl: PropTypes.object.isRequired, }; @@ -109,7 +110,7 @@ class Video extends React.PureComponent { volume: 0.5, paused: true, dragging: false, - containerWidth: false, + containerWidth: this.props.width, fullscreen: false, hovered: false, muted: false, @@ -129,6 +130,7 @@ class Video extends React.PureComponent { this.player = c; if (c) { + if (this.props.cacheWidth) this.props.cacheWidth(this.player.offsetWidth); this.setState({ containerWidth: c.offsetWidth, });