Skip to content
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.

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

Fix timeline jumps #10001

Merged
merged 3 commits into from
Feb 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 8 additions & 2 deletions app/javascript/mastodon/components/media_gallery.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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) {
Expand All @@ -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,
});
Expand All @@ -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;

Expand Down
28 changes: 27 additions & 1 deletion app/javascript/mastodon/components/scrollable_list.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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) &&
Expand All @@ -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();
Expand Down Expand Up @@ -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,
})}
</IntersectionObserverArticleContainer>
))}

Expand Down
67 changes: 62 additions & 5 deletions app/javascript/mastodon/components/status.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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();
Expand Down Expand Up @@ -166,6 +207,10 @@ class Status extends ImmutablePureComponent {
}
}

handleRef = c => {
this.node = c;
}

render () {
let media = null;
let statusAvatar, prepend, rebloggedByText;
Expand All @@ -180,7 +225,7 @@ class Status extends ImmutablePureComponent {

if (hidden) {
return (
<div>
<div ref={this.handleRef}>
{status.getIn(['account', 'display_name']) || status.getIn(['account', 'username'])}
{status.get('content')}
</div>
Expand All @@ -195,7 +240,7 @@ class Status extends ImmutablePureComponent {

return (
<HotKeys handlers={minHandlers}>
<div className='status__wrapper status__wrapper--filtered focusable' tabIndex='0'>
<div className='status__wrapper status__wrapper--filtered focusable' tabIndex='0' ref={this.handleRef}>
<FormattedMessage id='status.filtered' defaultMessage='Filtered' />
</div>
</HotKeys>
Expand Down Expand Up @@ -243,19 +288,29 @@ 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}
/>
)}
</Bundle>
);
} else {
media = (
<Bundle fetchComponent={MediaGallery} loading={this.renderLoadingMediaGallery}>
{Component => <Component media={status.get('media_attachments')} sensitive={status.get('sensitive')} height={110} onOpenMedia={this.props.onOpenMedia} />}
{Component => (
<Component
media={status.get('media_attachments')}
sensitive={status.get('sensitive')}
height={110}
onOpenMedia={this.props.onOpenMedia}
cacheWidth={this.props.cacheMediaWidth}
defaultWidth={this.props.cachedMediaWidth}
/>
)}
</Bundle>
);
}
Expand All @@ -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}
/>
);
}
Expand All @@ -291,7 +348,7 @@ class Status extends ImmutablePureComponent {

return (
<HotKeys handlers={handlers}>
<div className={classNames('status__wrapper', `status__wrapper-${status.get('visibility')}`, { 'status__wrapper-reply': !!status.get('in_reply_to_id'), read: unread === false, focusable: !this.props.muted })} tabIndex={this.props.muted ? null : 0} data-featured={featured ? 'true' : null} aria-label={textForScreenReader(intl, status, rebloggedByText, !status.get('hidden'))}>
<div className={classNames('status__wrapper', `status__wrapper-${status.get('visibility')}`, { 'status__wrapper-reply': !!status.get('in_reply_to_id'), read: unread === false, focusable: !this.props.muted })} tabIndex={this.props.muted ? null : 0} data-featured={featured ? 'true' : null} aria-label={textForScreenReader(intl, status, rebloggedByText, !status.get('hidden'))} ref={this.handleRef}>
{prepend}

<div className={classNames('status', `status-${status.get('visibility')}`, { 'status-reply': !!status.get('in_reply_to_id'), muted: this.props.muted, read: unread === false })} data-id={status.get('id')}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {
Expand Down Expand Up @@ -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}
/>
);
}
Expand All @@ -149,7 +157,17 @@ class Notification extends ImmutablePureComponent {
</span>
</div>

<StatusContainer id={notification.get('status')} account={notification.get('account')} muted withDismiss hidden={!!this.props.hidden} />
<StatusContainer
id={notification.get('status')}
account={notification.get('account')}
muted
withDismiss
hidden={!!this.props.hidden}
getScrollPosition={this.props.getScrollPosition}
updateScrollBottom={this.props.updateScrollBottom}
cachedMediaWidth={this.props.cachedMediaWidth}
cacheMediaWidth={this.props.cacheMediaWidth}
/>
</div>
</HotKeys>
);
Expand All @@ -171,7 +189,17 @@ class Notification extends ImmutablePureComponent {
</span>
</div>

<StatusContainer id={notification.get('status')} account={notification.get('account')} muted withDismiss hidden={this.props.hidden} />
<StatusContainer
id={notification.get('status')}
account={notification.get('account')}
muted
withDismiss
hidden={this.props.hidden}
getScrollPosition={this.props.getScrollPosition}
updateScrollBottom={this.props.updateScrollBottom}
cachedMediaWidth={this.props.cachedMediaWidth}
cacheMediaWidth={this.props.cacheMediaWidth}
/>
</div>
</HotKeys>
);
Expand Down
5 changes: 4 additions & 1 deletion app/javascript/mastodon/features/status/components/card.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -69,7 +71,7 @@ export default class Card extends React.PureComponent {
};

state = {
width: 280,
width: this.props.defaultWidth || 280,
embedded: false,
};

Expand Down Expand Up @@ -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 });
}
}
Expand Down
4 changes: 3 additions & 1 deletion app/javascript/mastodon/features/video/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};

Expand All @@ -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,
Expand All @@ -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,
});
Expand Down