Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

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

Merged
merged 3 commits into from
Apr 8, 2022
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccf36e5
Fix: Remove jittery timeline scrolling after jumping to an event
justjanne Apr 8, 2022
00e8904
Fix: Remove onUserScroll handler and merge it with onScroll
justjanne Apr 8, 2022
48bea9b
Fix: Reset scrollIntoView state earlier
justjanne Apr 8, 2022
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
6 changes: 1 addition & 5 deletions src/components/structures/MessagePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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>;

Expand Down Expand Up @@ -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}
Expand Down
1 change: 1 addition & 0 deletions src/components/structures/RightPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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}
/>;
Expand Down
44 changes: 29 additions & 15 deletions src/components/structures/RoomView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
Expand Down Expand Up @@ -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(),
});
}
}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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' });
Expand Down Expand Up @@ -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}
Expand Down
15 changes: 1 addition & 14 deletions src/components/structures/ScrollPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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}
>
Expand Down
12 changes: 8 additions & 4 deletions src/components/structures/ThreadView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ interface IProps {
e2eStatus?: E2EStatus;
initialEvent?: MatrixEvent;
isInitialEventHighlighted?: boolean;
initialEventScrollIntoView?: boolean;
}

interface IState {
Expand Down Expand Up @@ -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
});
Expand Down Expand Up @@ -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> }
Expand Down
86 changes: 51 additions & 35 deletions src/components/structures/TimelinePanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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();
});
}

/**
Expand All @@ -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 });
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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}
Expand Down
Loading