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

Author: florianduros

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();
+    });
 });

src/components/structures/RoomView.tsx

@@ -1407,8 +1407,6 @@ export class RoomView extends React.Component<IRoomProps, IRoomState> {
             tombstone: this.getRoomTombstone(room),
             liveTimeline: room.getLiveTimeline(),
         });
-
-        dis.dispatch<ActionPayload>({ action: Action.RoomLoaded });
     };
 
     private onRoomTimelineReset = (room?: Room): void => {

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
      */

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<State> = {
                 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 });
-    }
 }

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 });
-    });
 });

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 });
         });
     });