From 3acd648ed3ee171f650577946890a60ccb313003 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 19 Dec 2023 14:21:44 +0100 Subject: [PATCH] Fix timeline position when moving to a room and coming back (#12055) * Force `initialEventId` in `RoomView` * Remove Force `initialEventId` in `RoomView` * Add e2e tests to verify we memorize the timeline position * Fill `viewRoomOpts` in `viewRoom` * Reset jest mock in sliding sync test * Add comments --- playwright/e2e/room/room.spec.ts | 39 ++++++++++++++++++++ src/components/structures/RoomView.tsx | 2 - src/dispatcher/actions.ts | 5 --- src/stores/RoomViewStore.tsx | 20 +++------- test/components/structures/RoomView-test.tsx | 6 --- test/stores/RoomViewStore-test.ts | 14 ++++--- 6 files changed, 52 insertions(+), 34 deletions(-) diff --git a/playwright/e2e/room/room.spec.ts b/playwright/e2e/room/room.spec.ts index 2bd9e06c07..43edeaab38 100644 --- a/playwright/e2e/room/room.spec.ts +++ b/playwright/e2e/room/room.spec.ts @@ -60,4 +60,43 @@ test.describe("Room Directory", () => { // confirm the room was loaded await expect(page.getByText("Charlie joined the room")).toBeVisible(); }); + + test("should memorize the timeline position when switch Room A -> Room B -> Room A", async ({ + page, + app, + user, + }) => { + // Create the two rooms + const roomAId = await app.client.createRoom({ name: "Room A" }); + const roomBId = await app.client.createRoom({ name: "Room B" }); + // Display Room A + await app.viewRoomById(roomAId); + + // Send the first message and get the event ID + const { event_id: eventId } = await app.client.sendMessage(roomAId, { body: "test0", msgtype: "m.text" }); + // Send 49 more messages + for (let i = 1; i < 50; i++) { + await app.client.sendMessage(roomAId, { body: `test${i}`, msgtype: "m.text" }); + } + + // Wait for all the messages to be displayed + await expect( + page.locator(".mx_EventTile_last .mx_MTextBody .mx_EventTile_body").getByText("test49"), + ).toBeVisible(); + + // Display the first message + await page.goto(`/#/room/${roomAId}/${eventId}`); + + // Wait for the first message to be displayed + await expect(page.locator(".mx_MTextBody .mx_EventTile_body").getByText("test0")).toBeInViewport(); + + // Display Room B + await app.viewRoomById(roomBId); + // Display Room A + await app.viewRoomById(roomAId); + + // The timeline should display the first message + // The previous position before switching to Room B should be remembered + await expect(page.locator(".mx_MTextBody .mx_EventTile_body").getByText("test0")).toBeInViewport(); + }); }); diff --git a/src/components/structures/RoomView.tsx b/src/components/structures/RoomView.tsx index 364ec3a32f..7e36a5aa06 100644 --- a/src/components/structures/RoomView.tsx +++ b/src/components/structures/RoomView.tsx @@ -1407,8 +1407,6 @@ export class RoomView extends React.Component { tombstone: this.getRoomTombstone(room), liveTimeline: room.getLiveTimeline(), }); - - dis.dispatch({ action: Action.RoomLoaded }); }; private onRoomTimelineReset = (room?: Room): void => { diff --git a/src/dispatcher/actions.ts b/src/dispatcher/actions.ts index fb1d360a6c..5783e78331 100644 --- a/src/dispatcher/actions.ts +++ b/src/dispatcher/actions.ts @@ -372,11 +372,6 @@ export enum Action { */ OpenSpotlight = "open_spotlight", - /** - * Fired when the room loaded. - */ - RoomLoaded = "room_loaded", - /** * Opens right panel with 3pid invite information */ diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index c47114ecd4..78b6660b3d 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -375,10 +375,6 @@ export class RoomViewStore extends EventEmitter { this.cancelAskToJoin(payload as CancelAskToJoinPayload); break; } - case Action.RoomLoaded: { - this.setViewRoomOpts(); - break; - } } } @@ -443,6 +439,10 @@ export class RoomViewStore extends EventEmitter { return; } + const viewRoomOpts: ViewRoomOpts = { buttons: [] }; + // Allow modules to update the list of buttons for the room by updating `viewRoomOpts`. + ModuleRunner.instance.invoke(RoomViewLifecycle.ViewRoom, viewRoomOpts, this.getRoomId()); + const newState: Partial = { roomId: payload.room_id, roomAlias: payload.room_alias ?? null, @@ -464,6 +464,7 @@ export class RoomViewStore extends EventEmitter { (payload.room_id === this.state.roomId ? this.state.viewingCall : CallStore.instance.getActiveCall(payload.room_id) !== null), + viewRoomOpts, }; // Allow being given an event to be replied to when switching rooms but sanity check its for this room @@ -823,15 +824,4 @@ export class RoomViewStore extends EventEmitter { public getViewRoomOpts(): ViewRoomOpts { return this.state.viewRoomOpts; } - - /** - * Invokes the view room lifecycle to set the view room options. - * - * @returns {void} - */ - private setViewRoomOpts(): void { - const viewRoomOpts: ViewRoomOpts = { buttons: [] }; - ModuleRunner.instance.invoke(RoomViewLifecycle.ViewRoom, viewRoomOpts, this.getRoomId()); - this.setState({ viewRoomOpts }); - } } diff --git a/test/components/structures/RoomView-test.tsx b/test/components/structures/RoomView-test.tsx index 29a0f51c03..8f6034d2e1 100644 --- a/test/components/structures/RoomView-test.tsx +++ b/test/components/structures/RoomView-test.tsx @@ -585,10 +585,4 @@ describe("RoomView", () => { expect(dis.dispatch).toHaveBeenCalledWith({ action: "cancel_ask_to_join", roomId: room.roomId }); }); }); - - it("fires Action.RoomLoaded", async () => { - jest.spyOn(dis, "dispatch"); - await mountRoomView(); - expect(dis.dispatch).toHaveBeenCalledWith({ action: Action.RoomLoaded }); - }); }); diff --git a/test/stores/RoomViewStore-test.ts b/test/stores/RoomViewStore-test.ts index a9b63273fe..8ad55e6fc1 100644 --- a/test/stores/RoomViewStore-test.ts +++ b/test/stores/RoomViewStore-test.ts @@ -134,11 +134,6 @@ describe("RoomViewStore", function () { await untilDispatch(Action.CancelAskToJoin, dis); }; - const dispatchRoomLoaded = async () => { - dis.dispatch({ action: Action.RoomLoaded }); - await untilDispatch(Action.RoomLoaded, dis); - }; - let roomViewStore: RoomViewStore; let slidingSyncManager: SlidingSyncManager; let dis: MatrixDispatcher; @@ -425,6 +420,10 @@ describe("RoomViewStore", function () { }); }); + afterEach(() => { + jest.spyOn(SettingsStore, "getValue").mockReset(); + }); + it("subscribes to the room", async () => { const setRoomVisible = jest .spyOn(slidingSyncManager, "setRoomVisible") @@ -598,7 +597,10 @@ describe("RoomViewStore", function () { opts.buttons = buttons; } }); - await dispatchRoomLoaded(); + + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ViewRoom, dis); + expect(roomViewStore.getViewRoomOpts()).toEqual({ buttons }); }); });