Fix: Remove jittery timeline scrolling after jumping to an event (#8263)

* Fix: Remove jittery timeline scrolling after jumping to an event

* Fix: Remove onUserScroll handler and merge it with onScroll

* Fix: Reset scrollIntoView state earlier

Co-authored-by: Janne Mareike Koschinski <jannemk@element.io>
This commit is contained in:
Janne Mareike Koschinski 2022-04-08 20:48:57 +02:00 committed by GitHub
parent 285dc25b3e
commit 579a166113
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 118 additions and 87 deletions

View file

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React, { createRef, KeyboardEvent, ReactNode, SyntheticEvent, TransitionEvent } from 'react';
import React, { createRef, KeyboardEvent, ReactNode, TransitionEvent } from 'react';
import ReactDOM from 'react-dom';
import classNames from 'classnames';
import { Room } from 'matrix-js-sdk/src/models/room';
@ -170,9 +170,6 @@ interface IProps {
// callback which is called when the panel is scrolled.
onScroll?(event: Event): void;
// callback which is called when the user interacts with the room timeline
onUserScroll(event: SyntheticEvent): void;
// callback which is called when more content is needed.
onFillRequest?(backwards: boolean): Promise<boolean>;
@ -1030,7 +1027,6 @@ export default class MessagePanel extends React.Component<IProps, IState> {
ref={this.scrollPanel}
className={classes}
onScroll={this.props.onScroll}
onUserScroll={this.props.onUserScroll}
onFillRequest={this.props.onFillRequest}
onUnfillRequest={this.props.onUnfillRequest}
style={style}

View file

@ -231,6 +231,7 @@ export default class RightPanel extends React.Component<IProps, IState> {
mxEvent={cardState.threadHeadEvent}
initialEvent={cardState.initialEvent}
isInitialEventHighlighted={cardState.isInitialEventHighlighted}
initialEventScrollIntoView={cardState.initialEventScrollIntoView}
permalinkCreator={this.props.permalinkCreator}
e2eStatus={this.props.e2eStatus}
/>;

View file

@ -155,6 +155,8 @@ export interface IRoomState {
initialEventPixelOffset?: number;
// Whether to highlight the event scrolled to
isInitialEventHighlighted?: boolean;
// Whether to scroll the event into view
initialEventScrollIntoView?: boolean;
replyToEvent?: MatrixEvent;
numUnreadMessages: number;
searchTerm?: string;
@ -404,7 +406,8 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
const roomId = RoomViewStore.instance.getRoomId();
const newState: Pick<IRoomState, any> = {
// This convoluted type signature ensures we get IntelliSense *and* correct typing
const newState: Partial<IRoomState> & Pick<IRoomState, any> = {
roomId,
roomAlias: RoomViewStore.instance.getRoomAlias(),
roomLoading: RoomViewStore.instance.isRoomLoading(),
@ -443,22 +446,29 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
);
}
// If we have an initial event, we want to reset the event pixel offset to ensure it ends up
// visible
newState.initialEventPixelOffset = null;
const thread = initialEvent?.getThread();
if (thread && !initialEvent?.isThreadRoot) {
showThread({
rootEvent: thread.rootEvent,
initialEvent,
highlighted: RoomViewStore.instance.isInitialEventHighlighted(),
scroll_into_view: RoomViewStore.instance.initialEventScrollIntoView(),
});
} else {
newState.initialEventId = initialEventId;
newState.isInitialEventHighlighted = RoomViewStore.instance.isInitialEventHighlighted();
newState.initialEventScrollIntoView = RoomViewStore.instance.initialEventScrollIntoView();
if (thread && initialEvent?.isThreadRoot) {
showThread({
rootEvent: thread.rootEvent,
initialEvent,
highlighted: RoomViewStore.instance.isInitialEventHighlighted(),
scroll_into_view: RoomViewStore.instance.initialEventScrollIntoView(),
});
}
}
@ -758,19 +768,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
}
}
private onUserScroll = () => {
if (this.state.initialEventId && this.state.isInitialEventHighlighted) {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: this.state.room.roomId,
event_id: this.state.initialEventId,
highlighted: false,
replyingToEvent: this.state.replyToEvent,
metricsTrigger: undefined, // room doesn't change
});
}
};
private onRightPanelStoreUpdate = () => {
this.setState({
showRightPanel: RightPanelStore.instance.isOpenForRoom(this.state.roomId),
@ -1301,6 +1298,22 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
this.updateTopUnreadMessagesBar();
};
private resetJumpToEvent = (eventId?: string) => {
if (this.state.initialEventId && this.state.initialEventScrollIntoView &&
this.state.initialEventId === eventId) {
debuglog("Removing scroll_into_view flag from initial event");
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: this.state.room.roomId,
event_id: this.state.initialEventId,
highlighted: this.state.isInitialEventHighlighted,
scroll_into_view: false,
replyingToEvent: this.state.replyToEvent,
metricsTrigger: undefined, // room doesn't change
});
}
};
private injectSticker(url: string, info: object, text: string, threadId: string | null) {
if (this.context.isGuest()) {
dis.dispatch({ action: 'require_registration' });
@ -2051,9 +2064,10 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
hidden={hideMessagePanel}
highlightedEventId={highlightedEventId}
eventId={this.state.initialEventId}
eventScrollIntoView={this.state.initialEventScrollIntoView}
eventPixelOffset={this.state.initialEventPixelOffset}
onScroll={this.onMessageListScroll}
onUserScroll={this.onUserScroll}
onEventScrolledIntoView={this.resetJumpToEvent}
onReadMarkerUpdated={this.updateTopUnreadMessagesBar}
showUrlPreview={this.state.showUrlPreview}
className={this.messagePanelClassNames}

View file

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React, { createRef, CSSProperties, ReactNode, SyntheticEvent, KeyboardEvent } from "react";
import React, { createRef, CSSProperties, ReactNode, KeyboardEvent } from "react";
import { logger } from "matrix-js-sdk/src/logger";
import Timer from '../../utils/Timer';
@ -109,10 +109,6 @@ interface IProps {
/* onScroll: a callback which is called whenever any scroll happens.
*/
onScroll?(event: Event): void;
/* onUserScroll: callback which is called when the user interacts with the room timeline
*/
onUserScroll?(event: SyntheticEvent): void;
}
/* This component implements an intelligent scrolling list.
@ -593,29 +589,21 @@ export default class ScrollPanel extends React.Component<IProps> {
* @param {object} ev the keyboard event
*/
public handleScrollKey = (ev: KeyboardEvent) => {
let isScrolling = false;
const roomAction = getKeyBindingsManager().getRoomAction(ev);
switch (roomAction) {
case KeyBindingAction.ScrollUp:
this.scrollRelative(-1);
isScrolling = true;
break;
case KeyBindingAction.ScrollDown:
this.scrollRelative(1);
isScrolling = true;
break;
case KeyBindingAction.JumpToFirstMessage:
this.scrollToTop();
isScrolling = true;
break;
case KeyBindingAction.JumpToLatestMessage:
this.scrollToBottom();
isScrolling = true;
break;
}
if (isScrolling && this.props.onUserScroll) {
this.props.onUserScroll(ev);
}
};
/* Scroll the panel to bring the DOM node with the scroll token
@ -965,7 +953,6 @@ export default class ScrollPanel extends React.Component<IProps> {
<AutoHideScrollbar
wrappedRef={this.collectScroll}
onScroll={this.onScroll}
onWheel={this.props.onUserScroll}
className={`mx_ScrollPanel ${this.props.className}`}
style={this.props.style}
>

View file

@ -61,6 +61,7 @@ interface IProps {
e2eStatus?: E2EStatus;
initialEvent?: MatrixEvent;
isInitialEventHighlighted?: boolean;
initialEventScrollIntoView?: boolean;
}
interface IState {
@ -215,13 +216,15 @@ export default class ThreadView extends React.Component<IProps, IState> {
}
};
private resetHighlightedEvent = (): void => {
if (this.props.initialEvent && this.props.isInitialEventHighlighted) {
private resetJumpToEvent = (event?: string): void => {
if (this.props.initialEvent && this.props.initialEventScrollIntoView &&
event === this.props.initialEvent?.getId()) {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: this.props.room.roomId,
event_id: this.props.initialEvent?.getId(),
highlighted: false,
highlighted: this.props.isInitialEventHighlighted,
scroll_into_view: false,
replyingToEvent: this.state.replyToEvent,
metricsTrigger: undefined, // room doesn't change
});
@ -372,7 +375,8 @@ export default class ThreadView extends React.Component<IProps, IState> {
editState={this.state.editState}
eventId={this.props.initialEvent?.getId()}
highlightedEventId={highlightedEventId}
onUserScroll={this.resetHighlightedEvent}
eventScrollIntoView={this.props.initialEventScrollIntoView}
onEventScrolledIntoView={this.resetJumpToEvent}
onPaginationRequest={this.onPaginationRequest}
/>
</div> }

View file

@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/
import React, { createRef, ReactNode, SyntheticEvent } from 'react';
import React, { createRef, ReactNode } from 'react';
import ReactDOM from "react-dom";
import { NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room";
import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event";
@ -91,6 +91,9 @@ interface IProps {
// id of an event to jump to. If not given, will go to the end of the live timeline.
eventId?: string;
// whether we should scroll the event into view
eventScrollIntoView?: boolean;
// where to position the event given by eventId, in pixels from the bottom of the viewport.
// If not given, will try to put the event half way down the viewport.
eventPixelOffset?: number;
@ -124,8 +127,7 @@ interface IProps {
// callback which is called when the panel is scrolled.
onScroll?(event: Event): void;
// callback which is called when the user interacts with the room timeline
onUserScroll?(event: SyntheticEvent): void;
onEventScrolledIntoView?(eventId?: string): void;
// callback which is called when the read-up-to mark is updated.
onReadMarkerUpdated?(): void;
@ -327,9 +329,11 @@ class TimelinePanel extends React.Component<IProps, IState> {
const differentEventId = newProps.eventId != this.props.eventId;
const differentHighlightedEventId = newProps.highlightedEventId != this.props.highlightedEventId;
if (differentEventId || differentHighlightedEventId) {
logger.log("TimelinePanel switching to eventId " + newProps.eventId +
" (was " + this.props.eventId + ")");
const differentAvoidJump = newProps.eventScrollIntoView && !this.props.eventScrollIntoView;
if (differentEventId || differentHighlightedEventId || differentAvoidJump) {
logger.log("TimelinePanel switching to " +
"eventId " + newProps.eventId + " (was " + this.props.eventId + "), " +
"scrollIntoView: " + newProps.eventScrollIntoView + " (was " + this.props.eventScrollIntoView + ")");
return this.initTimeline(newProps);
}
}
@ -1123,7 +1127,41 @@ class TimelinePanel extends React.Component<IProps, IState> {
offsetBase = 0.5;
}
return this.loadTimeline(initialEvent, pixelOffset, offsetBase);
return this.loadTimeline(initialEvent, pixelOffset, offsetBase, props.eventScrollIntoView);
}
private scrollIntoView(eventId?: string, pixelOffset?: number, offsetBase?: number): void {
const doScroll = () => {
if (eventId) {
debuglog("TimelinePanel scrolling to eventId " + eventId +
" at position " + (offsetBase * 100) + "% + " + pixelOffset);
this.messagePanel.current.scrollToEvent(
eventId,
pixelOffset,
offsetBase,
);
} else {
debuglog("TimelinePanel scrolling to bottom");
this.messagePanel.current.scrollToBottom();
}
};
debuglog("TimelinePanel scheduling scroll to event");
this.props.onEventScrolledIntoView?.(eventId);
// Ensure the correct scroll position pre render, if the messages have already been loaded to DOM,
// to avoid it jumping around
doScroll();
// Ensure the correct scroll position post render for correct behaviour.
//
// requestAnimationFrame runs our code immediately after the DOM update but before the next repaint.
//
// If the messages have just been loaded for the first time, this ensures we'll repeat setting the
// correct scroll position after React has re-rendered the TimelinePanel and MessagePanel and
// updated the DOM.
window.requestAnimationFrame(() => {
doScroll();
});
}
/**
@ -1139,8 +1177,10 @@ class TimelinePanel extends React.Component<IProps, IState> {
* @param {number?} offsetBase the reference point for the pixelOffset. 0
* means the top of the container, 1 means the bottom, and fractional
* values mean somewhere in the middle. If omitted, it defaults to 0.
*
* @param {boolean?} scrollIntoView whether to scroll the event into view.
*/
private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number): void {
private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number, scrollIntoView = true): void {
this.timelineWindow = new TimelineWindow(
MatrixClientPeg.get(), this.props.timelineSet,
{ windowLimit: this.props.timelineCap });
@ -1176,32 +1216,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
return;
}
const doScroll = () => {
if (eventId) {
debuglog("TimelinePanel scrolling to eventId " + eventId);
this.messagePanel.current.scrollToEvent(
eventId,
pixelOffset,
offsetBase,
);
} else {
debuglog("TimelinePanel scrolling to bottom");
this.messagePanel.current.scrollToBottom();
}
};
// Ensure the correct scroll position pre render, if the messages have already been loaded to DOM, to
// avoid it jumping around
doScroll();
// Ensure the correct scroll position post render for correct behaviour.
//
// requestAnimationFrame runs our code immediately after the DOM update but before the next repaint.
//
// If the messages have just been loaded for the first time, this ensures we'll repeat setting the
// correct scroll position after React has re-rendered the TimelinePanel and MessagePanel and updated
// the DOM.
window.requestAnimationFrame(doScroll);
if (scrollIntoView) {
this.scrollIntoView(eventId, pixelOffset, offsetBase);
}
if (this.props.sendReadReceiptOnLoad) {
this.sendReadReceipt();
@ -1651,7 +1668,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
ourUserId={MatrixClientPeg.get().credentials.userId}
stickyBottom={stickyBottom}
onScroll={this.onMessageListScroll}
onUserScroll={this.props.onUserScroll}
onFillRequest={this.onMessageListFillRequest}
onUnfillRequest={this.onMessageListUnfillRequest}
isTwelveHour={this.context?.showTwelveHourTimestamps ?? this.state.isTwelveHour}

View file

@ -146,19 +146,6 @@ export default class TimelineCard extends React.Component<IProps, IState> {
}
};
private onUserScroll = (): void => {
if (this.state.initialEventId && this.state.isInitialEventHighlighted) {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: this.props.room.roomId,
event_id: this.state.initialEventId,
highlighted: false,
replyingToEvent: this.state.replyToEvent,
metricsTrigger: undefined, // room doesn't change
});
}
};
private onScroll = (): void => {
const timelinePanel = this.timelinePanel.current;
if (!timelinePanel) return;
@ -171,6 +158,17 @@ export default class TimelineCard extends React.Component<IProps, IState> {
atEndOfLiveTimeline: false,
});
}
if (this.state.initialEventId && this.state.isInitialEventHighlighted) {
dis.dispatch<ViewRoomPayload>({
action: Action.ViewRoom,
room_id: this.props.room.roomId,
event_id: this.state.initialEventId,
highlighted: false,
replyingToEvent: this.state.replyToEvent,
metricsTrigger: undefined, // room doesn't change
});
}
};
private onMeasurement = (narrow: boolean): void => {
@ -263,7 +261,6 @@ export default class TimelineCard extends React.Component<IProps, IState> {
resizeNotifier={this.props.resizeNotifier}
highlightedEventId={highlightedEventId}
onScroll={this.onScroll}
onUserScroll={this.onUserScroll}
/>
</div>

View file

@ -26,6 +26,7 @@ export const showThread = (props: {
rootEvent: MatrixEvent;
initialEvent?: MatrixEvent;
highlighted?: boolean;
scroll_into_view?: boolean;
push?: boolean;
}) => {
const push = props.push ?? false;
@ -35,6 +36,7 @@ export const showThread = (props: {
threadHeadEvent: props.rootEvent,
initialEvent: props.initialEvent,
isInitialEventHighlighted: props.highlighted,
initialEventScrollIntoView: props.scroll_into_view,
},
};
if (push) {

View file

@ -32,6 +32,7 @@ export interface ViewRoomPayload extends Pick<ActionPayload, "action"> {
event_id?: string; // the event to ensure is in view if any
highlighted?: boolean; // whether to highlight `event_id`
scroll_into_view?: boolean; // whether to scroll `event_id` into view
should_peek?: boolean; // whether we should peek the room if we are not yet joined
joining?: boolean; // whether we have already sent a join request for this room
via_servers?: string[]; // the list of servers to join via if no room_alias is provided

View file

@ -62,6 +62,8 @@ const INITIAL_STATE = {
initialEventPixelOffset: null,
// Whether to highlight the initial event
isInitialEventHighlighted: false,
// whether to scroll `event_id` into view
initialEventScrollIntoView: true,
// The room alias of the room (or null if not originally specified in view_room)
roomAlias: null,
@ -291,6 +293,7 @@ export class RoomViewStore extends Store<ActionPayload> {
roomAlias: payload.room_alias,
initialEventId: payload.event_id,
isInitialEventHighlighted: payload.highlighted,
initialEventScrollIntoView: payload.scroll_into_view ?? true,
roomLoading: false,
roomLoadError: null,
// should peek by default
@ -333,6 +336,7 @@ export class RoomViewStore extends Store<ActionPayload> {
initialEventId: null,
initialEventPixelOffset: null,
isInitialEventHighlighted: null,
initialEventScrollIntoView: true,
roomAlias: payload.room_alias,
roomLoading: true,
roomLoadError: null,
@ -475,6 +479,11 @@ export class RoomViewStore extends Store<ActionPayload> {
return this.state.isInitialEventHighlighted;
}
// Whether to avoid jumping to the initial event
public initialEventScrollIntoView() {
return this.state.initialEventScrollIntoView;
}
// The room alias of the room (or null if not originally specified in view_room)
public getRoomAlias() {
return this.state.roomAlias;

View file

@ -34,6 +34,7 @@ export interface IRightPanelCardState {
threadHeadEvent?: MatrixEvent;
initialEvent?: MatrixEvent;
isInitialEventHighlighted?: boolean;
initialEventScrollIntoView?: boolean;
}
export interface IRightPanelCardStateStored {
@ -47,6 +48,7 @@ export interface IRightPanelCardStateStored {
threadHeadEventId?: string;
initialEventId?: string;
isInitialEventHighlighted?: boolean;
initialEventScrollIntoView?: boolean;
}
export interface IRightPanelCard {
@ -87,6 +89,7 @@ export function convertCardToStore(panelState: IRightPanelCard): IRightPanelCard
widgetId: state.widgetId,
spaceId: state.spaceId,
isInitialEventHighlighted: state.isInitialEventHighlighted,
initialEventScrollIntoView: state.initialEventScrollIntoView,
threadHeadEventId: !!state?.threadHeadEvent?.getId() ?
panelState.state.threadHeadEvent.getId() : undefined,
memberInfoEventId: !!state?.memberInfoEvent?.getId() ?
@ -106,6 +109,7 @@ function convertStoreToCard(panelStateStore: IRightPanelCardStored, room: Room):
widgetId: stateStored.widgetId,
spaceId: stateStored.spaceId,
isInitialEventHighlighted: stateStored.isInitialEventHighlighted,
initialEventScrollIntoView: stateStored.initialEventScrollIntoView,
threadHeadEvent: !!stateStored?.threadHeadEventId ?
room.findEventById(stateStored.threadHeadEventId) : undefined,
memberInfoEvent: !!stateStored?.memberInfoEventId ?