RoomList: fix room scroll when clicked in thread activity centre (#34179)

* Make room list handles `show_room_tile` in `Action.ViewRoom`

* Add e2e test to scroll the room list when a room in tac is clicked

apps/web/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts

@@ -11,6 +11,7 @@ import { rejectToast } from "@element-hq/element-web-playwright-common";
 import { expect, test } from ".";
 import { CommandOrControl } from "../../utils";
 import { isDendrite } from "../../../plugins/homeserver/dendrite";
+import { getRoomList, getRoomOptionsMenu } from "../../left-panel/room-list-panel/utils";
 
 test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
     test.skip(
@@ -23,13 +24,15 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         botCreateOpts: { displayName: "Other User" },
     });
 
+    test.beforeEach(async ({ page, app, user }) => {
+        await rejectToast(page, "Verify this device");
+        await rejectToast(page, "Notifications");
+    });
+
     test(
         "should have the button correctly aligned and displayed in the space panel when expanded",
         { tag: "@screenshot" },
         async ({ util, page }) => {
-            await rejectToast(page, "Verify this device");
-            await rejectToast(page, "Notifications");
-
             // Open the space panel
             await util.expandSpacePanel();
             // The buttons in the space panel should be aligned when expanded
@@ -41,9 +44,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         "should not show indicator when there is no thread",
         { tag: "@screenshot" },
         async ({ room1, util, app, page }) => {
-            await rejectToast(page, "Verify this device");
-            await rejectToast(page, "Notifications");
-
             // No indicator should be shown
             await util.assertNoTacIndicator();
 
@@ -62,9 +62,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         app,
         page,
     }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.goTo(room1);
         await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
 
@@ -80,9 +77,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         app,
         page,
     }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.goTo(room1);
         await util.receiveMessages(room1, [
             "Msg1",
@@ -104,9 +98,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         "should show the rooms with unread threads",
         { tag: "@screenshot" },
         async ({ room1, room2, util, msg, user, app, page }) => {
-            await rejectToast(page, "Verify this device");
-            await rejectToast(page, "Notifications");
-
             await util.goTo(room2);
             await util.populateThreads(room1, room2, msg, user);
             // The indicator should be shown
@@ -128,9 +119,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         "should update with a thread is read",
         { tag: "@screenshot" },
         async ({ room1, room2, util, msg, user, app, page }) => {
-            await rejectToast(page, "Verify this device");
-            await rejectToast(page, "Notifications");
-
             await util.goTo(room2);
             await util.populateThreads(room1, room2, msg, user);
 
@@ -155,9 +143,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
     );
 
     test("should order by recency after notification level", async ({ room1, room2, util, msg, user, app, page }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.goTo(room2);
         await util.populateThreads(room1, room2, msg, user, false);
 
@@ -169,9 +154,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
     });
 
     test("should block the Spotlight to open when the TAC is opened", async ({ util, page, app }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         const toggleSpotlight = () => page.keyboard.press(`${CommandOrControl}+k`);
 
         // Sanity check
@@ -187,9 +169,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
     });
 
     test("should have the correct hover state", { tag: "@screenshot" }, async ({ util, page, app }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.hoverTacButton();
         await expect(util.getSpacePanel()).toMatchScreenshot("tac-hovered.png");
 
@@ -200,9 +179,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
     });
 
     test("should mark all threads as read", { tag: "@screenshot" }, async ({ room1, room2, util, msg, page, app }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
 
         await util.assertNotificationTac();
@@ -223,9 +199,6 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
         app,
         page,
     }) => {
-        await rejectToast(page, "Verify this device");
-        await rejectToast(page, "Notifications");
-
         await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
 
         await util.openTac();
@@ -233,4 +206,43 @@ test.describe("Threads Activity Centre", { tag: "@no-firefox" }, () => {
 
         await util.assertThreadPanelIsOpened();
     });
+
+    // The target room is named so that A-Z sorting places it at the very bottom of the room list.
+    test.describe("scrolling the room list when a room is selected", () => {
+        test.use({ room1Name: "zzz thread room" });
+
+        test("should scroll the room list to reveal the room clicked in the TAC", async ({
+            room1,
+            util,
+            msg,
+            app,
+            page,
+        }) => {
+            // Give the target room an unread thread so it shows up in the TAC.
+            await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]);
+            await util.assertNotificationTac();
+
+            // Create filler rooms that sort before "zzz …" so A-Z sorting pushes the target below the fold.
+            for (let i = 0; i < 20; i++) {
+                await app.client.createRoom({ name: `room ${String(i).padStart(2, "0")}` });
+            }
+
+            // Sort A-Z so room positions are deterministic and the target sits at the bottom.
+            await getRoomOptionsMenu(page).click();
+            await page.getByRole("menuitemradio", { name: "A-Z" }).click();
+
+            const roomList = getRoomList(page);
+            const targetRow = roomList.getByRole("option", { name: `Open room ${room1.name}` });
+
+            await expect(roomList.getByRole("option", { name: "Open room room 00" })).toBeVisible();
+            await expect(targetRow).not.toBeInViewport();
+
+            // Clicking the room in the TAC dispatches view_room with show_room_tile…
+            await util.openTac();
+            await util.clickRoomInTac(room1.name);
+
+            // …which scrolls the room list to bring the room's tile into view.
+            await expect(targetRow).toBeInViewport();
+        });
+    });
 });

apps/web/src/viewmodels/room-list/RoomListViewModel.ts

@@ -461,7 +461,48 @@ export class RoomListViewModel
         if (target) this.scrollToIndex?.(target.index);
     };
 
-    private onDispatch = (payload: any): void => {
+    /**
+     * Scroll a room into view, expanding its section first if it is collapsed so the tile can
+     * actually be shown.
+     */
+    private async scrollRoomIntoView(roomId: string): Promise<void> {
+        // Look in the full (pre-collapse) sections so we can find rooms hidden in collapsed sections.
+        const section = this.roomsResult.sections.find((s) => s.rooms.some((room) => room.roomId === roomId));
+        // Room not found
+        if (!section) return;
+
+        const headerViewModel = this.roomSectionHeaderViewModels.get(section.tag);
+        // Expand and rebuild the section
+        if (headerViewModel && !headerViewModel.isExpanded) {
+            headerViewModel.isExpanded = true;
+            await this.updateRoomListData();
+        }
+
+        // Scroll to the room
+        const index = this.getRoomEntryIndex(roomId);
+        if (index !== undefined) this.scrollToIndex?.(index);
+    }
+
+    /**
+     * Compute a room's index in the list's entry space, or undefined if it is not in the displayed
+     * sections (e.g. collapsed or filtered out). Flat list: the room index; grouped list: includes
+     * one slot per section header (matching {@link firstUnreadRoomBelowFold}).
+     */
+    private getRoomEntryIndex(roomId: string): number | undefined {
+        // A grouped list renders a header entry before each section's rooms; a flat list does not.
+        const hasSectionHeaders = !this.snapshot.current.isFlatList;
+
+        let entryIndex = 0;
+        for (const section of this.sections) {
+            if (hasSectionHeaders) entryIndex++; // section header entry
+            const indexInSection = section.rooms.findIndex((room) => room.roomId === roomId);
+            if (indexInSection !== -1) return entryIndex + indexInSection;
+            entryIndex += section.rooms.length;
+        }
+        return undefined;
+    }
+
+    private onDispatch = async (payload: any): Promise<void> => {
         if (payload.action === Action.ActiveRoomChanged) {
             // When the active room changes, update the room list data to reflect the new selected room
             // Pass isRoomChange=true so sticky logic doesn't prevent the index from updating
@@ -470,6 +511,8 @@ export class RoomListViewModel
             // Handle keyboard navigation shortcuts (Alt+ArrowUp/Down)
             // This was previously handled by useRoomListNavigation hook
             this.handleViewRoomDelta(payload as ViewRoomDeltaPayload);
+        } else if (payload.action === Action.ViewRoom && payload.show_room_tile && payload.room_id) {
+            await this.scrollRoomIntoView(payload.room_id);
         } else if (payload.action === Action.RoomListCollapseAllSections) {
             this.onCollapseAllSections(false);
         } else if (payload.action === Action.RoomListExpandAllSections) {

apps/web/test/viewmodels/room-list/RoomListViewModel-test.tsx

@@ -1328,4 +1328,70 @@ describe("RoomListViewModel", () => {
             expect(tagRoom).not.toHaveBeenCalled();
         });
     });
+
+    describe("show_room_tile scroll", () => {
+        beforeEach(() => {
+            // Dispatching ViewRoom is also handled by the global RoomViewStore, which calls
+            // MatrixClientPeg.safeGet(); stubClient sets up the peg so that doesn't throw.
+            stubClient();
+        });
+
+        it("should scroll a room into view in a flat list", async () => {
+            viewModel = new RoomListViewModel({ client: matrixClient });
+            const scrollSpy = jest.fn();
+            viewModel.setScrollToIndex(scrollSpy);
+
+            dispatcher.dispatch({
+                action: Action.ViewRoom,
+                room_id: "!room2:server",
+                show_room_tile: true,
+                metricsTrigger: undefined,
+            });
+
+            // Flat list: entry index == room index.
+            await waitFor(() => expect(scrollSpy).toHaveBeenCalledWith(1));
+        });
+
+        it("should scroll a room into view in a grouped list, accounting for section headers", async () => {
+            const favRoom1 = mkStubRoom("!fav1:server", "Fav 1", matrixClient);
+            const favRoom2 = mkStubRoom("!fav2:server", "Fav 2", matrixClient);
+            const regularRoom1 = mkStubRoom("!reg1:server", "Reg 1", matrixClient);
+            jest.spyOn(RoomListStoreV3.instance, "getSortedRoomsInActiveSpace").mockReturnValue({
+                spaceId: "home",
+                sections: [
+                    { tag: DefaultTagID.Favourite, rooms: [favRoom1, favRoom2] },
+                    { tag: CHATS_TAG, rooms: [regularRoom1] },
+                ],
+            });
+            viewModel = new RoomListViewModel({ client: matrixClient });
+            const scrollSpy = jest.fn();
+            viewModel.setScrollToIndex(scrollSpy);
+
+            dispatcher.dispatch({
+                action: Action.ViewRoom,
+                room_id: "!reg1:server",
+                show_room_tile: true,
+                metricsTrigger: undefined,
+            });
+
+            // Entry space: [Fav header(0), fav1(1), fav2(2), Chats header(3), reg1(4)]
+            await waitFor(() => expect(scrollSpy).toHaveBeenCalledWith(4));
+        });
+
+        it("should not scroll when the room is not in the current list", async () => {
+            viewModel = new RoomListViewModel({ client: matrixClient });
+            const scrollSpy = jest.fn();
+            viewModel.setScrollToIndex(scrollSpy);
+
+            dispatcher.dispatch({
+                action: Action.ViewRoom,
+                room_id: "!room3:server",
+                show_room_tile: true,
+                metricsTrigger: undefined,
+            });
+
+            await waitFor(() => expect(scrollSpy).toHaveBeenCalledWith(2));
+            expect(scrollSpy).toHaveBeenCalledTimes(1);
+        });
+    });
 });