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 <[email protected]>

Author: justjanne

src/components/structures/MessagePanel.tsx

@@ -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}

src/components/structures/RightPanel.tsx

@@ -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}
                 />;

src/components/structures/RoomView.tsx

@@ -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}

src/components/structures/ScrollPanel.tsx

@@ -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}
             >

src/components/structures/ThreadView.tsx

@@ -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> }

src/components/structures/TimelinePanel.tsx

@@ -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}