Skip to content
This repository was archived by the owner on Dec 7, 2024. It is now read-only.

Commit 579a166

Browse files
justjannejustjanne
authored
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]>
1 parent 285dc25 commit 579a166

File tree

11 files changed

+118
-87
lines changed

11 files changed

+118
-87
lines changed

Diff for: ‎src/components/structures/MessagePanel.tsx

+1-5
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
import React, { createRef, KeyboardEvent, ReactNode, SyntheticEvent, TransitionEvent } from 'react';
17+
import React, { createRef, KeyboardEvent, ReactNode, TransitionEvent } from 'react';
1818
import ReactDOM from 'react-dom';
1919
import classNames from 'classnames';
2020
import { Room } from 'matrix-js-sdk/src/models/room';
@@ -170,9 +170,6 @@ interface IProps {
170170
// callback which is called when the panel is scrolled.
171171
onScroll?(event: Event): void;
172172

173-
// callback which is called when the user interacts with the room timeline
174-
onUserScroll(event: SyntheticEvent): void;
175-
176173
// callback which is called when more content is needed.
177174
onFillRequest?(backwards: boolean): Promise<boolean>;
178175

@@ -1030,7 +1027,6 @@ export default class MessagePanel extends React.Component<IProps, IState> {
10301027
ref={this.scrollPanel}
10311028
className={classes}
10321029
onScroll={this.props.onScroll}
1033-
onUserScroll={this.props.onUserScroll}
10341030
onFillRequest={this.props.onFillRequest}
10351031
onUnfillRequest={this.props.onUnfillRequest}
10361032
style={style}

Diff for: ‎src/components/structures/RightPanel.tsx

+1
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,7 @@ export default class RightPanel extends React.Component<IProps, IState> {
231231
mxEvent={cardState.threadHeadEvent}
232232
initialEvent={cardState.initialEvent}
233233
isInitialEventHighlighted={cardState.isInitialEventHighlighted}
234+
initialEventScrollIntoView={cardState.initialEventScrollIntoView}
234235
permalinkCreator={this.props.permalinkCreator}
235236
e2eStatus={this.props.e2eStatus}
236237
/>;

Diff for: ‎src/components/structures/RoomView.tsx

+29-15
Original file line numberDiff line numberDiff line change
@@ -155,6 +155,8 @@ export interface IRoomState {
155155
initialEventPixelOffset?: number;
156156
// Whether to highlight the event scrolled to
157157
isInitialEventHighlighted?: boolean;
158+
// Whether to scroll the event into view
159+
initialEventScrollIntoView?: boolean;
158160
replyToEvent?: MatrixEvent;
159161
numUnreadMessages: number;
160162
searchTerm?: string;
@@ -404,7 +406,8 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
404406

405407
const roomId = RoomViewStore.instance.getRoomId();
406408

407-
const newState: Pick<IRoomState, any> = {
409+
// This convoluted type signature ensures we get IntelliSense *and* correct typing
410+
const newState: Partial<IRoomState> & Pick<IRoomState, any> = {
408411
roomId,
409412
roomAlias: RoomViewStore.instance.getRoomAlias(),
410413
roomLoading: RoomViewStore.instance.isRoomLoading(),
@@ -443,22 +446,29 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
443446
);
444447
}
445448

449+
// If we have an initial event, we want to reset the event pixel offset to ensure it ends up
450+
// visible
451+
newState.initialEventPixelOffset = null;
452+
446453
const thread = initialEvent?.getThread();
447454
if (thread && !initialEvent?.isThreadRoot) {
448455
showThread({
449456
rootEvent: thread.rootEvent,
450457
initialEvent,
451458
highlighted: RoomViewStore.instance.isInitialEventHighlighted(),
459+
scroll_into_view: RoomViewStore.instance.initialEventScrollIntoView(),
452460
});
453461
} else {
454462
newState.initialEventId = initialEventId;
455463
newState.isInitialEventHighlighted = RoomViewStore.instance.isInitialEventHighlighted();
464+
newState.initialEventScrollIntoView = RoomViewStore.instance.initialEventScrollIntoView();
456465

457466
if (thread && initialEvent?.isThreadRoot) {
458467
showThread({
459468
rootEvent: thread.rootEvent,
460469
initialEvent,
461470
highlighted: RoomViewStore.instance.isInitialEventHighlighted(),
471+
scroll_into_view: RoomViewStore.instance.initialEventScrollIntoView(),
462472
});
463473
}
464474
}
@@ -758,19 +768,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
758768
}
759769
}
760770

761-
private onUserScroll = () => {
762-
if (this.state.initialEventId && this.state.isInitialEventHighlighted) {
763-
dis.dispatch<ViewRoomPayload>({
764-
action: Action.ViewRoom,
765-
room_id: this.state.room.roomId,
766-
event_id: this.state.initialEventId,
767-
highlighted: false,
768-
replyingToEvent: this.state.replyToEvent,
769-
metricsTrigger: undefined, // room doesn't change
770-
});
771-
}
772-
};
773-
774771
private onRightPanelStoreUpdate = () => {
775772
this.setState({
776773
showRightPanel: RightPanelStore.instance.isOpenForRoom(this.state.roomId),
@@ -1301,6 +1298,22 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
13011298
this.updateTopUnreadMessagesBar();
13021299
};
13031300

1301+
private resetJumpToEvent = (eventId?: string) => {
1302+
if (this.state.initialEventId && this.state.initialEventScrollIntoView &&
1303+
this.state.initialEventId === eventId) {
1304+
debuglog("Removing scroll_into_view flag from initial event");
1305+
dis.dispatch<ViewRoomPayload>({
1306+
action: Action.ViewRoom,
1307+
room_id: this.state.room.roomId,
1308+
event_id: this.state.initialEventId,
1309+
highlighted: this.state.isInitialEventHighlighted,
1310+
scroll_into_view: false,
1311+
replyingToEvent: this.state.replyToEvent,
1312+
metricsTrigger: undefined, // room doesn't change
1313+
});
1314+
}
1315+
};
1316+
13041317
private injectSticker(url: string, info: object, text: string, threadId: string | null) {
13051318
if (this.context.isGuest()) {
13061319
dis.dispatch({ action: 'require_registration' });
@@ -2051,9 +2064,10 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
20512064
hidden={hideMessagePanel}
20522065
highlightedEventId={highlightedEventId}
20532066
eventId={this.state.initialEventId}
2067+
eventScrollIntoView={this.state.initialEventScrollIntoView}
20542068
eventPixelOffset={this.state.initialEventPixelOffset}
20552069
onScroll={this.onMessageListScroll}
2056-
onUserScroll={this.onUserScroll}
2070+
onEventScrolledIntoView={this.resetJumpToEvent}
20572071
onReadMarkerUpdated={this.updateTopUnreadMessagesBar}
20582072
showUrlPreview={this.state.showUrlPreview}
20592073
className={this.messagePanelClassNames}

Diff for: ‎src/components/structures/ScrollPanel.tsx

+1-14
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
import React, { createRef, CSSProperties, ReactNode, SyntheticEvent, KeyboardEvent } from "react";
17+
import React, { createRef, CSSProperties, ReactNode, KeyboardEvent } from "react";
1818
import { logger } from "matrix-js-sdk/src/logger";
1919

2020
import Timer from '../../utils/Timer';
@@ -109,10 +109,6 @@ interface IProps {
109109
/* onScroll: a callback which is called whenever any scroll happens.
110110
*/
111111
onScroll?(event: Event): void;
112-
113-
/* onUserScroll: callback which is called when the user interacts with the room timeline
114-
*/
115-
onUserScroll?(event: SyntheticEvent): void;
116112
}
117113

118114
/* This component implements an intelligent scrolling list.
@@ -593,29 +589,21 @@ export default class ScrollPanel extends React.Component<IProps> {
593589
* @param {object} ev the keyboard event
594590
*/
595591
public handleScrollKey = (ev: KeyboardEvent) => {
596-
let isScrolling = false;
597592
const roomAction = getKeyBindingsManager().getRoomAction(ev);
598593
switch (roomAction) {
599594
case KeyBindingAction.ScrollUp:
600595
this.scrollRelative(-1);
601-
isScrolling = true;
602596
break;
603597
case KeyBindingAction.ScrollDown:
604598
this.scrollRelative(1);
605-
isScrolling = true;
606599
break;
607600
case KeyBindingAction.JumpToFirstMessage:
608601
this.scrollToTop();
609-
isScrolling = true;
610602
break;
611603
case KeyBindingAction.JumpToLatestMessage:
612604
this.scrollToBottom();
613-
isScrolling = true;
614605
break;
615606
}
616-
if (isScrolling && this.props.onUserScroll) {
617-
this.props.onUserScroll(ev);
618-
}
619607
};
620608

621609
/* Scroll the panel to bring the DOM node with the scroll token
@@ -965,7 +953,6 @@ export default class ScrollPanel extends React.Component<IProps> {
965953
<AutoHideScrollbar
966954
wrappedRef={this.collectScroll}
967955
onScroll={this.onScroll}
968-
onWheel={this.props.onUserScroll}
969956
className={`mx_ScrollPanel ${this.props.className}`}
970957
style={this.props.style}
971958
>

Diff for: ‎src/components/structures/ThreadView.tsx

+8-4
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ interface IProps {
6161
e2eStatus?: E2EStatus;
6262
initialEvent?: MatrixEvent;
6363
isInitialEventHighlighted?: boolean;
64+
initialEventScrollIntoView?: boolean;
6465
}
6566

6667
interface IState {
@@ -215,13 +216,15 @@ export default class ThreadView extends React.Component<IProps, IState> {
215216
}
216217
};
217218

218-
private resetHighlightedEvent = (): void => {
219-
if (this.props.initialEvent && this.props.isInitialEventHighlighted) {
219+
private resetJumpToEvent = (event?: string): void => {
220+
if (this.props.initialEvent && this.props.initialEventScrollIntoView &&
221+
event === this.props.initialEvent?.getId()) {
220222
dis.dispatch<ViewRoomPayload>({
221223
action: Action.ViewRoom,
222224
room_id: this.props.room.roomId,
223225
event_id: this.props.initialEvent?.getId(),
224-
highlighted: false,
226+
highlighted: this.props.isInitialEventHighlighted,
227+
scroll_into_view: false,
225228
replyingToEvent: this.state.replyToEvent,
226229
metricsTrigger: undefined, // room doesn't change
227230
});
@@ -372,7 +375,8 @@ export default class ThreadView extends React.Component<IProps, IState> {
372375
editState={this.state.editState}
373376
eventId={this.props.initialEvent?.getId()}
374377
highlightedEventId={highlightedEventId}
375-
onUserScroll={this.resetHighlightedEvent}
378+
eventScrollIntoView={this.props.initialEventScrollIntoView}
379+
onEventScrolledIntoView={this.resetJumpToEvent}
376380
onPaginationRequest={this.onPaginationRequest}
377381
/>
378382
</div> }

Diff for: ‎src/components/structures/TimelinePanel.tsx

+51-35
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
1414
limitations under the License.
1515
*/
1616

17-
import React, { createRef, ReactNode, SyntheticEvent } from 'react';
17+
import React, { createRef, ReactNode } from 'react';
1818
import ReactDOM from "react-dom";
1919
import { NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room";
2020
import { MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/models/event";
@@ -91,6 +91,9 @@ interface IProps {
9191
// id of an event to jump to. If not given, will go to the end of the live timeline.
9292
eventId?: string;
9393

94+
// whether we should scroll the event into view
95+
eventScrollIntoView?: boolean;
96+
9497
// where to position the event given by eventId, in pixels from the bottom of the viewport.
9598
// If not given, will try to put the event half way down the viewport.
9699
eventPixelOffset?: number;
@@ -124,8 +127,7 @@ interface IProps {
124127
// callback which is called when the panel is scrolled.
125128
onScroll?(event: Event): void;
126129

127-
// callback which is called when the user interacts with the room timeline
128-
onUserScroll?(event: SyntheticEvent): void;
130+
onEventScrolledIntoView?(eventId?: string): void;
129131

130132
// callback which is called when the read-up-to mark is updated.
131133
onReadMarkerUpdated?(): void;
@@ -327,9 +329,11 @@ class TimelinePanel extends React.Component<IProps, IState> {
327329

328330
const differentEventId = newProps.eventId != this.props.eventId;
329331
const differentHighlightedEventId = newProps.highlightedEventId != this.props.highlightedEventId;
330-
if (differentEventId || differentHighlightedEventId) {
331-
logger.log("TimelinePanel switching to eventId " + newProps.eventId +
332-
" (was " + this.props.eventId + ")");
332+
const differentAvoidJump = newProps.eventScrollIntoView && !this.props.eventScrollIntoView;
333+
if (differentEventId || differentHighlightedEventId || differentAvoidJump) {
334+
logger.log("TimelinePanel switching to " +
335+
"eventId " + newProps.eventId + " (was " + this.props.eventId + "), " +
336+
"scrollIntoView: " + newProps.eventScrollIntoView + " (was " + this.props.eventScrollIntoView + ")");
333337
return this.initTimeline(newProps);
334338
}
335339
}
@@ -1123,7 +1127,41 @@ class TimelinePanel extends React.Component<IProps, IState> {
11231127
offsetBase = 0.5;
11241128
}
11251129

1126-
return this.loadTimeline(initialEvent, pixelOffset, offsetBase);
1130+
return this.loadTimeline(initialEvent, pixelOffset, offsetBase, props.eventScrollIntoView);
1131+
}
1132+
1133+
private scrollIntoView(eventId?: string, pixelOffset?: number, offsetBase?: number): void {
1134+
const doScroll = () => {
1135+
if (eventId) {
1136+
debuglog("TimelinePanel scrolling to eventId " + eventId +
1137+
" at position " + (offsetBase * 100) + "% + " + pixelOffset);
1138+
this.messagePanel.current.scrollToEvent(
1139+
eventId,
1140+
pixelOffset,
1141+
offsetBase,
1142+
);
1143+
} else {
1144+
debuglog("TimelinePanel scrolling to bottom");
1145+
this.messagePanel.current.scrollToBottom();
1146+
}
1147+
};
1148+
1149+
debuglog("TimelinePanel scheduling scroll to event");
1150+
this.props.onEventScrolledIntoView?.(eventId);
1151+
// Ensure the correct scroll position pre render, if the messages have already been loaded to DOM,
1152+
// to avoid it jumping around
1153+
doScroll();
1154+
1155+
// Ensure the correct scroll position post render for correct behaviour.
1156+
//
1157+
// requestAnimationFrame runs our code immediately after the DOM update but before the next repaint.
1158+
//
1159+
// If the messages have just been loaded for the first time, this ensures we'll repeat setting the
1160+
// correct scroll position after React has re-rendered the TimelinePanel and MessagePanel and
1161+
// updated the DOM.
1162+
window.requestAnimationFrame(() => {
1163+
doScroll();
1164+
});
11271165
}
11281166

11291167
/**
@@ -1139,8 +1177,10 @@ class TimelinePanel extends React.Component<IProps, IState> {
11391177
* @param {number?} offsetBase the reference point for the pixelOffset. 0
11401178
* means the top of the container, 1 means the bottom, and fractional
11411179
* values mean somewhere in the middle. If omitted, it defaults to 0.
1180+
*
1181+
* @param {boolean?} scrollIntoView whether to scroll the event into view.
11421182
*/
1143-
private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number): void {
1183+
private loadTimeline(eventId?: string, pixelOffset?: number, offsetBase?: number, scrollIntoView = true): void {
11441184
this.timelineWindow = new TimelineWindow(
11451185
MatrixClientPeg.get(), this.props.timelineSet,
11461186
{ windowLimit: this.props.timelineCap });
@@ -1176,32 +1216,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
11761216
return;
11771217
}
11781218

1179-
const doScroll = () => {
1180-
if (eventId) {
1181-
debuglog("TimelinePanel scrolling to eventId " + eventId);
1182-
this.messagePanel.current.scrollToEvent(
1183-
eventId,
1184-
pixelOffset,
1185-
offsetBase,
1186-
);
1187-
} else {
1188-
debuglog("TimelinePanel scrolling to bottom");
1189-
this.messagePanel.current.scrollToBottom();
1190-
}
1191-
};
1192-
1193-
// Ensure the correct scroll position pre render, if the messages have already been loaded to DOM, to
1194-
// avoid it jumping around
1195-
doScroll();
1196-
1197-
// Ensure the correct scroll position post render for correct behaviour.
1198-
//
1199-
// requestAnimationFrame runs our code immediately after the DOM update but before the next repaint.
1200-
//
1201-
// If the messages have just been loaded for the first time, this ensures we'll repeat setting the
1202-
// correct scroll position after React has re-rendered the TimelinePanel and MessagePanel and updated
1203-
// the DOM.
1204-
window.requestAnimationFrame(doScroll);
1219+
if (scrollIntoView) {
1220+
this.scrollIntoView(eventId, pixelOffset, offsetBase);
1221+
}
12051222

12061223
if (this.props.sendReadReceiptOnLoad) {
12071224
this.sendReadReceipt();
@@ -1651,7 +1668,6 @@ class TimelinePanel extends React.Component<IProps, IState> {
16511668
ourUserId={MatrixClientPeg.get().credentials.userId}
16521669
stickyBottom={stickyBottom}
16531670
onScroll={this.onMessageListScroll}
1654-
onUserScroll={this.props.onUserScroll}
16551671
onFillRequest={this.onMessageListFillRequest}
16561672
onUnfillRequest={this.onMessageListUnfillRequest}
16571673
isTwelveHour={this.context?.showTwelveHourTimestamps ?? this.state.isTwelveHour}

0 commit comments

Comments
 (0)