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

Fix thread list jumping back down while scrolling #9606

Merged
merged 8 commits into from
Nov 25, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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
4 changes: 2 additions & 2 deletions src/components/structures/MessagePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -347,8 +347,8 @@ export default class MessagePanel extends React.Component<IProps, IState> {
return this.eventTiles[eventId]?.ref?.current;
}

public getTileForEventId(eventId: string): UnwrappedEventTile {
if (!this.eventTiles) {
public getTileForEventId(eventId?: string): UnwrappedEventTile | undefined {
if (!this.eventTiles || eventId === undefined || eventId === null) {
justjanne marked this conversation as resolved.
Show resolved Hide resolved
return undefined;
}
return this.eventTiles[eventId];
Expand Down
14 changes: 1 addition & 13 deletions src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ limitations under the License.

import React, { useContext, useEffect, useRef, useState } from 'react';
import { EventTimelineSet } from 'matrix-js-sdk/src/models/event-timeline-set';
import { Thread, ThreadEvent } from 'matrix-js-sdk/src/models/thread';
import { Thread } from 'matrix-js-sdk/src/models/thread';
import { Room } from 'matrix-js-sdk/src/models/room';

import BaseCard from "../views/right_panel/BaseCard";
Expand Down Expand Up @@ -206,18 +206,6 @@ const ThreadPanel: React.FC<IProps> = ({
});
}, [mxClient, roomId]);

useEffect(() => {
function refreshTimeline() {
timelinePanel?.current.refreshTimeline();
}

room?.on(ThreadEvent.Update, refreshTimeline);

return () => {
room?.removeListener(ThreadEvent.Update, refreshTimeline);
};
}, [room, mxClient, timelineSet]);

useEffect(() => {
if (room) {
if (filterOption === ThreadFilterType.My) {
Expand Down
27 changes: 26 additions & 1 deletion src/components/structures/TimelinePanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import { RoomMember, RoomMemberEvent } from 'matrix-js-sdk/src/models/room-membe
import { debounce, throttle } from 'lodash';
import { logger } from "matrix-js-sdk/src/logger";
import { ClientEvent } from "matrix-js-sdk/src/client";
import { Thread } from 'matrix-js-sdk/src/models/thread';
import { Thread, ThreadEvent } from 'matrix-js-sdk/src/models/thread';
import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";
import { MatrixError } from 'matrix-js-sdk/src/http-api';
import { ReadReceipt } from 'matrix-js-sdk/src/models/read-receipt';
Expand Down Expand Up @@ -297,6 +297,8 @@ class TimelinePanel extends React.Component<IProps, IState> {
cli.on(MatrixEventEvent.Decrypted, this.onEventDecrypted);
cli.on(MatrixEventEvent.Replaced, this.onEventReplaced);
cli.on(ClientEvent.Sync, this.onSync);

this.props.timelineSet.room?.on(ThreadEvent.Update, this.onThreadUpdate);
}

public componentDidMount() {
Expand Down Expand Up @@ -325,6 +327,9 @@ class TimelinePanel extends React.Component<IProps, IState> {
logger.warn("Replacing timelineSet on a TimelinePanel - confusion may ensue");
}

this.props.timelineSet.room?.off(ThreadEvent.Update, this.onThreadUpdate);
this.props.timelineSet.room?.on(ThreadEvent.Update, this.onThreadUpdate);

const differentEventId = prevProps.eventId != this.props.eventId;
const differentHighlightedEventId = prevProps.highlightedEventId != this.props.highlightedEventId;
const differentAvoidJump = prevProps.eventScrollIntoView && !this.props.eventScrollIntoView;
Expand Down Expand Up @@ -366,6 +371,7 @@ class TimelinePanel extends React.Component<IProps, IState> {
client.removeListener(MatrixEventEvent.Replaced, this.onEventReplaced);
client.removeListener(MatrixEventEvent.VisibilityChange, this.onEventVisibilityChange);
client.removeListener(ClientEvent.Sync, this.onSync);
this.props.timelineSet.room?.removeListener(ThreadEvent.Update, this.onThreadUpdate);
}
}

Expand Down Expand Up @@ -742,6 +748,25 @@ class TimelinePanel extends React.Component<IProps, IState> {
this.forceUpdate();
};

private onThreadUpdate = (thread: Thread): void => {
if (this.unmounted) {
return;
}

// ignore events for other rooms
const roomId = thread.roomId;
if (roomId !== this.props.timelineSet.room?.roomId) {
return;
}

// we could skip an update if the event isn't in our timeline,
// but that's probably an early optimisation.
const tile = this.messagePanel.current?.getTileForEventId(thread.id);
if (tile) {
tile.forceUpdate();
}
};

// Called whenever the visibility of an event changes, as per
// MSC3531. We typically need to re-render the tile.
private onEventVisibilityChange = (ev: MatrixEvent): void => {
Expand Down
98 changes: 98 additions & 0 deletions test/components/structures/TimelinePanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ import {
} from 'matrix-js-sdk/src/matrix';
import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts";
import { render, RenderResult } from "@testing-library/react";
import { FeatureSupport, THREAD_RELATION_TYPE, ThreadFilterType, Thread } from "matrix-js-sdk/src/models/thread";

import MatrixClientContext from "../../../src/contexts/MatrixClientContext";
import { mkRoom, stubClient } from "../../test-utils";
import TimelinePanel from '../../../src/components/structures/TimelinePanel';
import { MatrixClientPeg } from '../../../src/MatrixClientPeg';
Expand Down Expand Up @@ -192,4 +194,100 @@ describe('TimelinePanel', () => {
rerender(<TimelinePanel {...props} />);
expect(props.onEventScrolledIntoView).toHaveBeenCalledWith(events[1].getId());
});

it('updates thread previews', async () => {
const client = MatrixClientPeg.get();

Thread.hasServerSideSupport = FeatureSupport.Stable;
client.supportsExperimentalThreads = () => true;
const getValueCopy = SettingsStore.getValue;
SettingsStore.getValue = jest.fn().mockImplementation((name: string) => {
if (name === "feature_thread") return true;
return getValueCopy(name);
});

const room = new Room("roomId", client, "userId");
const allThreads = new EventTimelineSet(room, {
pendingEvents: false,
}, undefined, undefined, ThreadFilterType.All);
const timeline = new EventTimeline(allThreads);
allThreads.getLiveTimeline = () => timeline;
allThreads.getTimelineForEvent = () => timeline;

const reply1 = new MatrixEvent({
room_id: room.roomId,
event_id: 'event_reply_1',
type: EventType.RoomMessage,
user_id: "userId",
content: MessageEvent.from(`ReplyEvent1`).serialize().content,
});

const reply2 = new MatrixEvent({
room_id: room.roomId,
event_id: 'event_reply_2',
type: EventType.RoomMessage,
user_id: "userId",
content: MessageEvent.from(`ReplyEvent2`).serialize().content,
});

const rootEvent = new MatrixEvent({
room_id: room.roomId,
event_id: 'event_root',
type: EventType.RoomMessage,
user_id: "userId",
content: MessageEvent.from(`RootEvent`).serialize().content,
unsigned: {
"m.relations": {
[THREAD_RELATION_TYPE.name]: {
"latest_event": reply1.event,
"count": 1,
"current_user_participated": true,
},
},
},
});

const eventMap: { [key: string]: MatrixEvent } = {
[rootEvent.getId()!]: rootEvent,
[reply1.getId()!]: reply1,
[reply2.getId()!]: reply2,
};

room.findEventById = (eventId: string) => eventMap[eventId];
client.fetchRoomEvent = async (roomId: string, eventId: string) =>
roomId === room.roomId ? eventMap[eventId]?.event : {};

const thread = room.createThread(rootEvent.getId()!, rootEvent, [], true);
// So that we do not have to mock the thread loading
thread.initialEventsFetched = true;
// @ts-ignore
thread.fetchEditsWhereNeeded = () => Promise.resolve();
await thread.addEvent(reply1, true);
await timeline.addEvent(thread.rootEvent!, true);

const dom = render(
<MatrixClientContext.Provider value={client}>
<TimelinePanel
timelineSet={allThreads}
manageReadReceipts
sendReadReceiptOnLoad
/>
</MatrixClientContext.Provider>,
);
await dom.findByText("RootEvent");
await dom.findByText("ReplyEvent1");

rootEvent.setUnsigned({
"m.relations": {
[THREAD_RELATION_TYPE.name]: {
"latest_event": reply2.event,
"count": 2,
"current_user_participated": true,
},
},
});

await thread.addEvent(reply2, false, true);
await dom.findByText("ReplyEvent2");
});
});