New room list: fix incorrect decoration (#29770)

* fix(call): reset call value when the roomId changes

* fix(call): reset presence indicator when the room changes

* refactor: use existing `usePresence`

* test: fix room avatar view test

* test: update snapshots

src/components/viewmodels/avatars/RoomAvatarViewModel.tsx

@@ -5,32 +5,11 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-import {
+import { EventType, JoinRule, type MatrixEvent, type Room, RoomEvent } from "matrix-js-sdk/src/matrix";
-    EventType,
-    JoinRule,
-    type MatrixEvent,
-    type Room,
-    RoomEvent,
-    type User,
-    UserEvent,
-} from "matrix-js-sdk/src/matrix";
 import { useEffect, useState } from "react";
 
 import { useTypedEventEmitter } from "../../../hooks/useEventEmitter";
-import DMRoomMap from "../../../utils/DMRoomMap";
+import { useDmMember, usePresence, type Presence } from "../../views/avatars/WithPresenceIndicator";
-import { getJoinedNonFunctionalMembers } from "../../../utils/room/getJoinedNonFunctionalMembers";
-import { BUSY_PRESENCE_NAME } from "../../views/rooms/PresenceLabel";
-import { isPresenceEnabled } from "../../../utils/presence";
-
-/**
- * The presence of a user in a DM room.
- * - "online": The user is online.
- * - "offline": The user is offline.
- * - "busy": The user is busy.
- * - "unavailable": the presence is unavailable.
- * - null: the user is not in a DM room or presence is not enabled.
- */
-export type Presence = "online" | "offline" | "busy" | "unavailable" | null;
 
 export interface RoomAvatarViewState {
     /**
@@ -50,7 +29,7 @@ export interface RoomAvatarViewState {
      * The presence of the user in the DM room.
      * If null, the user is not in a DM room or presence is not enabled.
      */
-    presence: Presence;
+    presence: Presence | null;
 }
 
 /**
@@ -59,7 +38,8 @@ export interface RoomAvatarViewState {
  */
 export function useRoomAvatarViewModel(room: Room): RoomAvatarViewState {
     const isVideoRoom = room.isElementVideoRoom() || room.isCallRoom();
-    const presence = useDMPresence(room);
+    const roomMember = useDmMember(room);
+    const presence = usePresence(room, roomMember);
     const isPublic = useIsPublic(room);
 
     const hasDecoration = isPublic || isVideoRoom || presence !== null;
@@ -97,48 +77,3 @@ function useIsPublic(room: Room): boolean {
 function isRoomPublic(room: Room): boolean {
     return room.getJoinRule() === JoinRule.Public;
 }
-
-/**
- * Hook listening to the presence of the DM user.
- * @param room
- */
-function useDMPresence(room: Room): Presence {
-    const dmUser = getDMUser(room);
-    const [presence, setPresence] = useState<Presence>(getPresence(dmUser));
-    useTypedEventEmitter(dmUser, UserEvent.Presence, () => setPresence(getPresence(dmUser)));
-    useTypedEventEmitter(dmUser, UserEvent.CurrentlyActive, () => setPresence(getPresence(dmUser)));
-
-    return presence;
-}
-
-/**
- * Get the DM user of the room.
- * Return undefined if the room is not a DM room, if we can't find the user or if the presence is not enabled.
- * @param room
- * @returns found user
- */
-function getDMUser(room: Room): User | undefined {
-    const otherUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId);
-    if (!otherUserId) return;
-    if (getJoinedNonFunctionalMembers(room).length !== 2) return;
-    if (!isPresenceEnabled(room.client)) return;
-
-    return room.client.getUser(otherUserId) || undefined;
-}
-
-/**
- * Get the presence of the DM user.
- * @param dmUser
- */
-function getPresence(dmUser: User | undefined): Presence {
-    if (!dmUser) return null;
-    if (BUSY_PRESENCE_NAME.matches(dmUser.presence)) return "busy";
-
-    const isOnline = dmUser.currentlyActive || dmUser.presence === "online";
-    if (isOnline) return "online";
-
-    if (dmUser.presence === "offline") return "offline";
-    if (dmUser.presence === "unavailable") return "unavailable";
-
-    return null;
-}

src/components/views/avatars/RoomAvatarView.tsx

@@ -15,8 +15,9 @@ import BusyIcon from "@vector-im/compound-design-tokens/assets/web/icons/presenc
 import classNames from "classnames";
 
 import RoomAvatar from "./RoomAvatar";
-import { useRoomAvatarViewModel, type Presence } from "../../viewmodels/avatars/RoomAvatarViewModel";
+import { useRoomAvatarViewModel } from "../../viewmodels/avatars/RoomAvatarViewModel";
 import { _t } from "../../../languageHandler";
+import { Presence } from "./WithPresenceIndicator";
 
 interface RoomAvatarViewProps {
     /**
@@ -83,7 +84,7 @@ type PresenceDecorationProps = {
  */
 function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element {
     switch (presence) {
-        case "online":
+        case Presence.Online:
             return (
                 <OnlineOrUnavailableIcon
                     width="8px"
@@ -93,7 +94,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
                     aria-label={_t("presence|online")}
                 />
             );
-        case "unavailable":
+        case Presence.Away:
             return (
                 <OnlineOrUnavailableIcon
                     width="8px"
@@ -103,7 +104,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
                     aria-label={_t("presence|away")}
                 />
             );
-        case "offline":
+        case Presence.Offline:
             return (
                 <OfflineIcon
                     width="8px"
@@ -113,7 +114,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
                     aria-label={_t("presence|offline")}
                 />
             );
-        case "busy":
+        case Presence.Busy:
             return (
                 <BusyIcon
                     width="8px"

src/components/views/avatars/WithPresenceIndicator.tsx

@@ -26,7 +26,7 @@ interface Props {
     children: ReactNode;
 }
 
-enum Presence {
+export enum Presence {
     // Note: the names here are used in CSS class names
     Online = "ONLINE",
     Away = "AWAY",
@@ -86,7 +86,7 @@ function getPresence(member: RoomMember | null): Presence | null {
     return null;
 }
 
-const usePresence = (room: Room, member: RoomMember | null): Presence | null => {
+export const usePresence = (room: Room, member: RoomMember | null): Presence | null => {
     const [presence, setPresence] = useState<Presence | null>(getPresence(member));
     const updatePresence = (): void => {
         setPresence(getPresence(member));

src/hooks/useCall.ts

@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
 Please see LICENSE files in the repository root for full details.
 */
 
-import { useState, useCallback, useMemo } from "react";
+import { useState, useCallback, useMemo, useEffect } from "react";
 
 import type { RoomMember } from "matrix-js-sdk/src/matrix";
 import { type Call, ConnectionState, CallEvent } from "../models/Call";
@@ -20,6 +20,12 @@ export const useCall = (roomId: string): Call | null => {
     useEventEmitter(CallStore.instance, CallStoreEvent.Call, (call: Call | null, forRoomId: string) => {
         if (forRoomId === roomId) setCall(call);
     });
+
+    // Reset the value when the roomId changes
+    useEffect(() => {
+        setCall(CallStore.instance.getCall(roomId));
+    }, [roomId]);
+
     return call;
 };
 

test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx

@@ -5,32 +5,18 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-import { renderHook, waitFor, act } from "jest-matrix-react";
+import { renderHook, waitFor } from "jest-matrix-react";
-import {
+import { JoinRule, type MatrixClient, type Room, RoomMember, User } from "matrix-js-sdk/src/matrix";
-    JoinRule,
-    type MatrixClient,
-    MatrixEvent,
-    type Room,
-    type RoomMember,
-    User,
-    UserEvent,
-} from "matrix-js-sdk/src/matrix";
-import { mocked } from "jest-mock";
 
 import { useRoomAvatarViewModel } from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel";
 import { createTestClient, mkStubRoom } from "../../../../test-utils";
 import DMRoomMap from "../../../../../src/utils/DMRoomMap";
-import { getJoinedNonFunctionalMembers } from "../../../../../src/utils/room/getJoinedNonFunctionalMembers";
+import * as PresenceIndicatorModule from "../../../../../src/components/views/avatars/WithPresenceIndicator";
-import { isPresenceEnabled } from "../../../../../src/utils/presence";
 
 jest.mock("../../../../../src/utils/room/getJoinedNonFunctionalMembers", () => ({
     getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([]),
 }));
 
-jest.mock("../../../../../src/utils/presence", () => ({
-    isPresenceEnabled: jest.fn().mockReturnValue(false),
-}));
-
 describe("RoomAvatarViewModel", () => {
     let matrixClient: MatrixClient;
     let room: Room;
@@ -41,6 +27,9 @@ describe("RoomAvatarViewModel", () => {
 
         DMRoomMap.makeShared(matrixClient);
         jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue(null);
+
+        jest.spyOn(PresenceIndicatorModule, "useDmMember").mockReturnValue(null);
+        jest.spyOn(PresenceIndicatorModule, "usePresence").mockReturnValue(null);
     });
 
     it("should has hasDecoration to false", async () => {
@@ -74,62 +63,14 @@ describe("RoomAvatarViewModel", () => {
         await waitFor(() => expect(vm.current.isPublic).toBe(true));
     });
 
-    describe("presence", () => {
+    it("should return presence", async () => {
-        let user: User;
+        const user = User.createUser("userId", matrixClient);
+        const roomMember = new RoomMember(room.roomId, "userId");
+        roomMember.user = user;
+        jest.spyOn(PresenceIndicatorModule, "useDmMember").mockReturnValue(roomMember);
+        jest.spyOn(PresenceIndicatorModule, "usePresence").mockReturnValue(PresenceIndicatorModule.Presence.Online);
 
-        beforeEach(() => {
+        const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue("userId");
+        expect(vm.current.presence).toBe(PresenceIndicatorModule.Presence.Online);
-            mocked(getJoinedNonFunctionalMembers).mockReturnValue([{}, {}] as RoomMember[]);
-            mocked(isPresenceEnabled).mockReturnValue(true);
-
-            user = User.createUser("userId", matrixClient);
-            jest.spyOn(matrixClient, "getUser").mockReturnValue(user);
-        });
-
-        it("should has presence set to null", () => {
-            jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue(null);
-
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.presence).toBe(null);
-        });
-
-        it("should has online presence", async () => {
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.presence).toBe("offline");
-
-            user.presence = "online";
-
-            await act(() => user.emit(UserEvent.Presence, new MatrixEvent(), user));
-            await waitFor(() => expect(vm.current.presence).toBe("online"));
-
-            user.currentlyActive = true;
-            user.presence = "offline";
-
-            await act(() => user.emit(UserEvent.CurrentlyActive, new MatrixEvent(), user));
-            await waitFor(() => expect(vm.current.presence).toBe("online"));
-        });
-
-        it("should has busy presence", async () => {
-            user.presence = "busy";
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.presence).toBe("busy");
-        });
-
-        it("should has offline presence", async () => {
-            user.presence = "offline";
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.presence).toBe("offline");
-        });
-
-        it("should has unavailable presence", async () => {
-            user.presence = "unavailable";
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.presence).toBe("unavailable");
-        });
-
-        it("should has hasDecoration to true", async () => {
-            const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
-            expect(vm.current.hasDecoration).toBe(true);
-        });
     });
 });

test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx

@@ -12,11 +12,11 @@ import { mocked } from "jest-mock";
 import { RoomAvatarView } from "../../../../../src/components/views/avatars/RoomAvatarView";
 import { mkStubRoom, stubClient } from "../../../../test-utils";
 import {
-    type Presence,
     type RoomAvatarViewState,
     useRoomAvatarViewModel,
 } from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel";
 import DMRoomMap from "../../../../../src/utils/DMRoomMap";
+import { Presence } from "../../../../../src/components/views/avatars/WithPresenceIndicator";
 
 jest.mock("../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel", () => ({
     useRoomAvatarViewModel: jest.fn(),
@@ -83,10 +83,10 @@ describe("<RoomAvatarView />", () => {
     });
 
     it.each([
-        { presence: "online" as Presence, label: "Online" },
+        { presence: Presence.Online, label: "Online" },
-        { presence: "offline" as Presence, label: "Offline" },
+        { presence: Presence.Offline, label: "Offline" },
-        { presence: "busy" as Presence, label: "Busy" },
+        { presence: Presence.Busy, label: "Busy" },
-        { presence: "unavailable" as Presence, label: "Away" },
+        { presence: Presence.Away, label: "Away" },
     ])("should render the $presence presence", ({ presence, label }) => {
         mocked(useRoomAvatarViewModel).mockReturnValue({
             ...defaultValue,

test/unit-tests/components/views/avatars/__snapshots__/RoomAvatarView-test.tsx.snap

@@ -108,7 +108,76 @@ exports[`<RoomAvatarView /> should render a video room decoration 1`] = `
 </DocumentFragment>
 `;
 
-exports[`<RoomAvatarView /> should render the busy presence 1`] = `
+exports[`<RoomAvatarView /> should render the AWAY presence 1`] = `
+<DocumentFragment>
+  <div
+    class="mx_RoomAvatarView"
+  >
+    <span
+      aria-label="Avatar"
+      class="_avatar_1qbcf_8 mx_BaseAvatar mx_RoomAvatarView_RoomAvatar mx_RoomAvatarView_RoomAvatar_icon mx_RoomAvatarView_RoomAvatar_presence"
+      data-color="1"
+      data-testid="avatar-img"
+      data-type="round"
+      style="--cpd-avatar-size: 32px;"
+    >
+      <img
+        alt=""
+        class="_image_1qbcf_41"
+        data-type="round"
+        height="32px"
+        loading="lazy"
+        referrerpolicy="no-referrer"
+        src="http://this.is.a.url/avatar.url/room.png"
+        width="32px"
+      />
+    </span>
+    <svg
+      aria-label="This room is a video room"
+      class="mx_RoomAvatarView_icon"
+      color="var(--cpd-color-icon-tertiary)"
+      fill="currentColor"
+      height="16px"
+      viewBox="0 0 24 24"
+      width="16px"
+      xmlns="http://www.w3.org/2000/svg"
+    >
+      <path
+        d="M6 4h10a2 2 0 0 1 2 2v4.286l3.35-2.871a1 1 0 0 1 1.65.76v7.65a1 1 0 0 1-1.65.76L18 13.715V18a2 2 0 0 1-2 2H6a4 4 0 0 1-4-4V8a4 4 0 0 1 4-4"
+      />
+    </svg>
+    <svg
+      aria-label="Away"
+      class="mx_RoomAvatarView_PresenceDecoration"
+      color="var(--cpd-color-icon-quaternary)"
+      fill="currentColor"
+      height="8px"
+      viewBox="0 0 8 8"
+      width="8px"
+      xmlns="http://www.w3.org/2000/svg"
+    >
+      <g
+        clip-path="url(#a)"
+      >
+        <path
+          d="M8 4a4 4 0 1 1-8 0 4 4 0 0 1 8 0"
+        />
+      </g>
+      <defs>
+        <clippath
+          id="a"
+        >
+          <path
+            d="M0 0h8v8H0z"
+          />
+        </clippath>
+      </defs>
+    </svg>
+  </div>
+</DocumentFragment>
+`;
+
+exports[`<RoomAvatarView /> should render the BUSY presence 1`] = `
 <DocumentFragment>
   <div
     class="mx_RoomAvatarView"
@@ -179,7 +248,7 @@ exports[`<RoomAvatarView /> should render the busy presence 1`] = `
 </DocumentFragment>
 `;
 
-exports[`<RoomAvatarView /> should render the offline presence 1`] = `
+exports[`<RoomAvatarView /> should render the OFFLINE presence 1`] = `
 <DocumentFragment>
   <div
     class="mx_RoomAvatarView"
@@ -250,7 +319,7 @@ exports[`<RoomAvatarView /> should render the offline presence 1`] = `
 </DocumentFragment>
 `;
 
-exports[`<RoomAvatarView /> should render the online presence 1`] = `
+exports[`<RoomAvatarView /> should render the ONLINE presence 1`] = `
 <DocumentFragment>
   <div
     class="mx_RoomAvatarView"
@@ -318,72 +387,3 @@ exports[`<RoomAvatarView /> should render the online presence 1`] = `
   </div>
 </DocumentFragment>
 `;
-
-exports[`<RoomAvatarView /> should render the unavailable presence 1`] = `
-<DocumentFragment>
-  <div
-    class="mx_RoomAvatarView"
-  >
-    <span
-      aria-label="Avatar"
-      class="_avatar_1qbcf_8 mx_BaseAvatar mx_RoomAvatarView_RoomAvatar mx_RoomAvatarView_RoomAvatar_icon mx_RoomAvatarView_RoomAvatar_presence"
-      data-color="1"
-      data-testid="avatar-img"
-      data-type="round"
-      style="--cpd-avatar-size: 32px;"
-    >
-      <img
-        alt=""
-        class="_image_1qbcf_41"
-        data-type="round"
-        height="32px"
-        loading="lazy"
-        referrerpolicy="no-referrer"
-        src="http://this.is.a.url/avatar.url/room.png"
-        width="32px"
-      />
-    </span>
-    <svg
-      aria-label="This room is a video room"
-      class="mx_RoomAvatarView_icon"
-      color="var(--cpd-color-icon-tertiary)"
-      fill="currentColor"
-      height="16px"
-      viewBox="0 0 24 24"
-      width="16px"
-      xmlns="http://www.w3.org/2000/svg"
-    >
-      <path
-        d="M6 4h10a2 2 0 0 1 2 2v4.286l3.35-2.871a1 1 0 0 1 1.65.76v7.65a1 1 0 0 1-1.65.76L18 13.715V18a2 2 0 0 1-2 2H6a4 4 0 0 1-4-4V8a4 4 0 0 1 4-4"
-      />
-    </svg>
-    <svg
-      aria-label="Away"
-      class="mx_RoomAvatarView_PresenceDecoration"
-      color="var(--cpd-color-icon-quaternary)"
-      fill="currentColor"
-      height="8px"
-      viewBox="0 0 8 8"
-      width="8px"
-      xmlns="http://www.w3.org/2000/svg"
-    >
-      <g
-        clip-path="url(#a)"
-      >
-        <path
-          d="M8 4a4 4 0 1 1-8 0 4 4 0 0 1 8 0"
-        />
-      </g>
-      <defs>
-        <clippath
-          id="a"
-        >
-          <path
-            d="M0 0h8v8H0z"
-          />
-        </clippath>
-      </defs>
-    </svg>
-  </div>
-</DocumentFragment>
-`;