RoomList: fix alt+arrow up/down navigation

apps/web/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts

@@ -11,7 +11,7 @@ import { closeReleaseAnnouncement, rejectToast } from "@element-hq/element-web-p
 import { expect, test } from "../../../element-web-test";
 import { type Bot } from "../../../pages/bot";
 import { type ElementAppPage } from "../../../pages/ElementAppPage";
-import { getRoomList } from "./utils";
+import { assertRoomListOrder, getRoomList } from "./utils";
 
 test.describe("Room list", () => {
     test.use({
@@ -313,6 +313,44 @@ test.describe("Room list", () => {
         });
     });
 
+    test.describe("Room navigation with the keyboard", () => {
+        // Alt+ArrowUp/Down steps through rooms in their displayed order. Selecting a room sends a read
+        // receipt that re-emits the (unchanged) list, which used to reshuffle it under the cursor so
+        // the next press opened the wrong room.
+        test("should step through the rooms in their displayed order", async ({ page, app, user, bot }) => {
+            const roomListView = getRoomList(page);
+
+            // Creation order sets recency (newest on top): roomC, roomB, roomA.
+            const roomAId = await app.client.createRoom({ name: "roomA", invite: [bot.credentials.userId] });
+            await bot.joinRoom(roomAId);
+            await app.client.createRoom({ name: "roomB" });
+            await app.client.createRoom({ name: "roomC" });
+
+            const roomA = roomListView.getByRole("option", { name: "Open room roomA" });
+            const roomB = roomListView.getByRole("option", { name: "Open room roomB" });
+            const roomC = roomListView.getByRole("option", { name: "Open room roomC" });
+
+            // Open roomB, then bump roomA to the top. roomB is active so it stays put (sticky), giving
+            // a displayed order of roomA, roomB, roomC that differs from the recency order.
+            await roomB.click();
+            await expect(roomB).toHaveAttribute("aria-selected", "true");
+            await bot.sendMessage(roomAId, "bump");
+            await assertRoomListOrder(page, ["roomA", "roomB", "roomC"]);
+
+            // Up from roomB selects roomA (shown above it) in the room list.
+            await page.keyboard.press("Alt+ArrowUp");
+            await expect(roomA).toHaveAttribute("aria-selected", "true");
+
+            // Down must select the next visible room (roomB), not roomC as it did when the list reshuffled.
+            await page.keyboard.press("Alt+ArrowDown");
+            await expect(roomB).toHaveAttribute("aria-selected", "true");
+
+            // Once more selects roomC.
+            await page.keyboard.press("Alt+ArrowDown");
+            await expect(roomC).toHaveAttribute("aria-selected", "true");
+        });
+    });
+
     test.describe("Avatar decoration", () => {
         test.use({ labsFlags: ["feature_video_rooms"] });
 

apps/web/playwright/e2e/left-panel/room-list-panel/utils.ts

@@ -160,6 +160,25 @@ export async function assertSectionsOrder(page: Page, expectedOrder: string[]):
     }
 }
 
+/**
+ * Assert that the given rooms appear in the room list in the given top-to-bottom order.
+ * Retries until the list settles so it is robust to async re-sorts.
+ * @param page
+ * @param roomNames the room names, in the expected top-to-bottom order
+ */
+export async function assertRoomListOrder(page: Page, roomNames: string[]): Promise<void> {
+    const roomList = getRoomList(page);
+    await expect(async () => {
+        let previousY = -Infinity;
+        for (const name of roomNames) {
+            const box = await roomList.getByRole("option", { name: `Open room ${name}` }).boundingBox();
+            expect(box, `expected room "${name}" to be visible in the room list`).not.toBeNull();
+            expect(box!.y, `expected room "${name}" to be below the previous room`).toBeGreaterThan(previousY);
+            previousY = box!.y;
+        }
+    }).toPass();
+}
+
 /**
  * Get the primary filters container
  * @param page

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

@@ -137,6 +137,13 @@ export class RoomListViewModel
      */
     private scrollToIndex?: (index: number) => void;
 
+    /**
+     * Signature of the store's room list ({@link roomListSignature}) as of the last update, used to
+     * detect no-op list updates: selecting a room sends a read receipt that re-emits the same list,
+     * and we must not re-sort then or rooms shuffle around just from being selected.
+     */
+    private lastRoomListSignature: number = 0;
+
     public constructor(props: RoomListViewModelProps) {
         const activeSpace = SpaceStore.instance.activeSpaceRoom;
 
@@ -167,6 +174,7 @@ export class RoomListViewModel
         });
 
         this.roomsResult = roomsResult;
+        this.lastRoomListSignature = roomListSignature(roomsResult);
         this.sections = sections;
 
         // Build initial roomsMap from roomsResult
@@ -242,6 +250,7 @@ export class RoomListViewModel
         // Update rooms result with new filter
         const filterKeys = this.activeFilter !== undefined ? [this.activeFilter] : undefined;
         this.roomsResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
+        this.lastRoomListSignature = roomListSignature(this.roomsResult);
 
         // Update roomsMap immediately before clearing VMs
         this.updateRoomsMap(this.roomsResult);
@@ -272,10 +281,11 @@ export class RoomListViewModel
     }
 
     /**
-     * Get the ordered list of room IDs.
+     * The room IDs in displayed order (sticky-adjusted, collapse-filtered), matching the indices the
+     * view reports to {@link updateVisibleRooms}.
      */
     public get roomIds(): string[] {
-        return this.roomsResult.sections.flatMap((section) => section.rooms).map((room) => room.roomId);
+        return this.sections.flatMap((section) => section.rooms).map((room) => room.roomId);
     }
 
     /**
@@ -483,6 +493,25 @@ export class RoomListViewModel
         if (index !== undefined) this.scrollToIndex?.(index);
     }
 
+    /**
+     * Settle the list to the store's real order after a room is clicked, keeping the clicked room at
+     * its current displayed position (making it the new sticky room) so the previously sticky room
+     * falls back into place. No-op if the room is not in the list.
+     */
+    private async settleRoomOrder(roomId: string): Promise<void> {
+        const position = this.findRoomPosition(this.roomsResult.sections, roomId);
+        if (!position) return;
+
+        // Anchor the sticky room to the clicked room at its current position, then re-sort from the
+        // store's real order around it.
+        this.lastActiveRoomPosition = position;
+        const filterKeys = this.activeFilter !== undefined ? [this.activeFilter] : undefined;
+        this.roomsResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
+        this.lastRoomListSignature = roomListSignature(this.roomsResult);
+        this.updateRoomsMap(this.roomsResult);
+        await this.updateRoomListData(false, roomId);
+    }
+
     /**
      * 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
@@ -511,8 +540,12 @@ 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.ViewRoom && payload.room_id) {
+            if (payload.show_room_tile) {
+                await this.scrollRoomIntoView(payload.room_id);
+            } else {
+                await this.settleRoomOrder(payload.room_id);
+            }
         } else if (payload.action === Action.RoomListCollapseAllSections) {
             this.onCollapseAllSections(false);
         } else if (payload.action === Action.RoomListExpandAllSections) {
@@ -588,11 +621,14 @@ export class RoomListViewModel
         const oldSpaceId = this.roomsResult.spaceId;
 
         // Refresh room data from store
-        this.roomsResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
-        const newSpaceId = this.roomsResult.spaceId;
+        const freshResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
+        const newSpaceId = freshResult.spaceId;
 
         // Detect space change
         if (oldSpaceId !== newSpaceId) {
+            this.roomsResult = freshResult;
+            this.lastRoomListSignature = roomListSignature(freshResult);
+
             // Clear view models when the space changes
             // We only want to do this on space changes, not on regular list updates, to preserve view models when possible
             // The view models are disposed when scrolling out of view (handled by updateVisibleRooms)
@@ -614,6 +650,17 @@ export class RoomListViewModel
             return;
         }
 
+        // No-op update: the store returned the same list as last time (e.g. selecting a room sent a
+        // read receipt, which the recency sort re-inserts in place). Pass isRoomChange to keep the
+        // displayed order so rooms don't shuffle around, while still recomputing derived state.
+        const newSignature = roomListSignature(freshResult);
+        if (newSignature === this.lastRoomListSignature) {
+            this.updateRoomListData(true);
+            return;
+        }
+
+        this.roomsResult = freshResult;
+        this.lastRoomListSignature = newSignature;
         this.updateRoomsMap(this.roomsResult);
 
         // Normal room list update (not a space change)
@@ -658,7 +705,7 @@ export class RoomListViewModel
      * Apply sticky room logic to keep the active room at the same position within its section.
      * When the room list updates, this prevents the selected room from jumping around in the UI.
      *
-     * @param isRoomChange - Whether this update is due to a room change (not a list update)
+     * @param isRoomChange - When true, keep the current order (skip sticky re-sorting)
      * @param roomId - The room ID to apply sticky logic for (can be null/undefined)
      * @returns The modified sections array with sticky positioning applied
      */
@@ -708,6 +755,13 @@ export class RoomListViewModel
         });
     }
 
+    /**
+     * Updates the room list data.
+     * @param isRoomChange - Keep the current displayed order instead of re-sorting. Set for a room
+     *     change (selecting a room must not reshuffle the list) or a no-op list update.
+     * @param roomIdOverride - Optional room ID to use for calculations.
+     * @param scrollToSectionTag - Optional section tag to scroll to.
+     */
     private async updateRoomListData(
         isRoomChange: boolean = false,
         roomIdOverride: string | null = null,
@@ -837,6 +891,7 @@ export class RoomListViewModel
         // Refresh roomsResult so the new section lands in the same snapshot as the scroll-to.
         const filterKeys = this.activeFilter !== undefined ? [this.activeFilter] : undefined;
         this.roomsResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
+        this.lastRoomListSignature = roomListSignature(this.roomsResult);
         this.updateRoomsMap(this.roomsResult);
         this.updateRoomListData(false, null, tag);
         this.showToast("section_created");
@@ -884,6 +939,7 @@ export class RoomListViewModel
         // Scroll to the section after it moved
         const filterKeys = this.activeFilter !== undefined ? [this.activeFilter] : undefined;
         this.roomsResult = RoomListStoreV3.instance.getSortedRoomsInActiveSpace(filterKeys);
+        this.lastRoomListSignature = roomListSignature(this.roomsResult);
         this.updateRoomsMap(this.roomsResult);
         this.updateRoomListData(false, null, sourceTag);
     };
@@ -918,6 +974,22 @@ export class RoomListViewModel
     };
 }
 
+/**
+ * A signature of the store's room list capturing section order, section membership and room order,
+ * hashed to a fixed-size number so it doesn't grow with the number of rooms. Two results with the
+ * same signature render to the same list, so an update between them is a no-op that we can skip
+ * re-sorting for (see {@link RoomListViewModel.lastRoomListSignature}).
+ */
+function roomListSignature(roomsResult: RoomsResult): number {
+    const signature = roomsResult.sections.map((s) => `${s.tag}:${s.rooms.map((r) => r.roomId).join(",")}`).join("|");
+    // FNV-1a 32-bit hash.
+    let hash = 0x811c9dc5;
+    for (let i = 0; i < signature.length; i++) {
+        hash = Math.imul(hash ^ (signature.codePointAt(i) || 0), 0x01000193);
+    }
+    return hash >>> 0;
+}
+
 /**
  * Compute the sections to display in the room list based on the rooms result and section expansion state.
  * @param roomsResult - The current rooms result containing sections and rooms

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

@@ -303,6 +303,81 @@ describe("RoomListViewModel", () => {
             await flushPromises();
             expect(viewModel.getSnapshot().roomListState.activeRoomIndex).toBe(2);
         });
+
+        /**
+         * Set up a displayed order of room1, room2, room3 where room3 is the active room kept sticky
+         * at the bottom even though the store bumped it to the top (so the displayed order differs
+         * from the store's real order of room3, room1, room2).
+         */
+        const setUpStickyRoom3 = async (): Promise<void> => {
+            jest.spyOn(SDKContextClass.instance.roomViewStore, "getRoomId").mockReturnValue("!room3:server");
+            dispatcher.dispatch({ action: Action.ActiveRoomChanged, newRoomId: "!room3:server" });
+            await flushPromises();
+
+            jest.spyOn(RoomListStoreV3.instance, "getSortedRoomsInActiveSpace").mockReturnValue({
+                spaceId: "home",
+                sections: [{ tag: CHATS_TAG, rooms: [room3, room1, room2] }],
+            });
+            RoomListStoreV3.instance.emit(RoomListStoreV3Event.ListsUpdate);
+            expect(viewModel.getSnapshot().sections[0].roomIds).toEqual([
+                "!room1:server",
+                "!room2:server",
+                "!room3:server",
+            ]);
+        };
+
+        it("should not reshuffle when navigating with the keyboard (show_room_tile)", async () => {
+            stubClient();
+            viewModel = new RoomListViewModel({ client: matrixClient });
+            await setUpStickyRoom3();
+
+            // Navigate to room1 with the keyboard: dispatches ViewRoom with show_room_tile.
+            jest.spyOn(SDKContextClass.instance.roomViewStore, "getRoomId").mockReturnValue("!room1:server");
+            dispatcher.dispatch({
+                action: Action.ViewRoom,
+                room_id: "!room1:server",
+                show_room_tile: true,
+                metricsTrigger: undefined,
+            });
+            dispatcher.dispatch({
+                action: Action.ActiveRoomChanged,
+                oldRoomId: "!room3:server",
+                newRoomId: "!room1:server",
+            });
+            await flushPromises();
+
+            // A no-op list update (e.g. a read receipt from selecting the room) must not move any room.
+            RoomListStoreV3.instance.emit(RoomListStoreV3Event.ListsUpdate);
+            expect(viewModel.getSnapshot().sections[0].roomIds).toEqual([
+                "!room1:server",
+                "!room2:server",
+                "!room3:server",
+            ]);
+        });
+
+        it("should reshuffle to the real order when switching rooms with a click", async () => {
+            stubClient();
+            viewModel = new RoomListViewModel({ client: matrixClient });
+            await setUpStickyRoom3();
+
+            // Click room1: dispatches ViewRoom without show_room_tile.
+            jest.spyOn(SDKContextClass.instance.roomViewStore, "getRoomId").mockReturnValue("!room1:server");
+            dispatcher.dispatch({ action: Action.ViewRoom, room_id: "!room1:server", metricsTrigger: undefined });
+            dispatcher.dispatch({
+                action: Action.ActiveRoomChanged,
+                oldRoomId: "!room3:server",
+                newRoomId: "!room1:server",
+            });
+            await flushPromises();
+
+            // The no-op list update now lets the list settle to the store's real order (room1 sticky).
+            RoomListStoreV3.instance.emit(RoomListStoreV3Event.ListsUpdate);
+            expect(viewModel.getSnapshot().sections[0].roomIds).toEqual([
+                "!room1:server",
+                "!room3:server",
+                "!room2:server",
+            ]);
+        });
     });
 
     describe("Filters", () => {