From 18a7250cf9bf0fbe23f40f661123d9465659e003 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 15 Apr 2025 18:35:37 +0200 Subject: [PATCH] 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 --- .../avatars/RoomAvatarViewModel.tsx | 75 +-------- .../views/avatars/RoomAvatarView.tsx | 11 +- .../views/avatars/WithPresenceIndicator.tsx | 4 +- src/hooks/useCall.ts | 8 +- .../avatars/RoomAvatarViewModel-test.tsx | 87 ++--------- .../views/avatars/RoomAvatarView-test.tsx | 10 +- .../RoomAvatarView-test.tsx.snap | 144 +++++++++--------- 7 files changed, 111 insertions(+), 228 deletions(-) diff --git a/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx b/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx index 409bacf6ac..8879f5ae69 100644 --- a/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx +++ b/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx @@ -5,32 +5,11 @@ * Please see LICENSE files in the repository root for full details. */ -import { - EventType, - JoinRule, - type MatrixEvent, - type Room, - RoomEvent, - type User, - UserEvent, -} from "matrix-js-sdk/src/matrix"; +import { EventType, JoinRule, type MatrixEvent, type Room, RoomEvent } from "matrix-js-sdk/src/matrix"; import { useEffect, useState } from "react"; import { useTypedEventEmitter } from "../../../hooks/useEventEmitter"; -import DMRoomMap from "../../../utils/DMRoomMap"; -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; +import { useDmMember, usePresence, type Presence } from "../../views/avatars/WithPresenceIndicator"; 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(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; -} diff --git a/src/components/views/avatars/RoomAvatarView.tsx b/src/components/views/avatars/RoomAvatarView.tsx index 96bfbaf198..8810d073c5 100644 --- a/src/components/views/avatars/RoomAvatarView.tsx +++ b/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 ( ); - case "unavailable": + case Presence.Away: return ( ); - case "offline": + case Presence.Offline: return ( ); - case "busy": + case Presence.Busy: return ( { +export const usePresence = (room: Room, member: RoomMember | null): Presence | null => { const [presence, setPresence] = useState(getPresence(member)); const updatePresence = (): void => { setPresence(getPresence(member)); diff --git a/src/hooks/useCall.ts b/src/hooks/useCall.ts index 687b7c014e..db0660df2b 100644 --- a/src/hooks/useCall.ts +++ b/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; }; diff --git a/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx b/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx index 591b9f928b..ffaa152a53 100644 --- a/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx +++ b/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 { - JoinRule, - type MatrixClient, - MatrixEvent, - type Room, - type RoomMember, - User, - UserEvent, -} from "matrix-js-sdk/src/matrix"; -import { mocked } from "jest-mock"; +import { renderHook, waitFor } from "jest-matrix-react"; +import { JoinRule, type MatrixClient, type Room, RoomMember, User } from "matrix-js-sdk/src/matrix"; 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 { isPresenceEnabled } from "../../../../../src/utils/presence"; +import * as PresenceIndicatorModule from "../../../../../src/components/views/avatars/WithPresenceIndicator"; 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", () => { - let user: User; + it("should return presence", async () => { + 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(() => { - jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue("userId"); - 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); - }); + const { result: vm } = renderHook(() => useRoomAvatarViewModel(room)); + expect(vm.current.presence).toBe(PresenceIndicatorModule.Presence.Online); }); }); diff --git a/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx b/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx index f86c6b949f..020b92227d 100644 --- a/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx +++ b/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("", () => { }); it.each([ - { presence: "online" as Presence, label: "Online" }, - { presence: "offline" as Presence, label: "Offline" }, - { presence: "busy" as Presence, label: "Busy" }, - { presence: "unavailable" as Presence, label: "Away" }, + { presence: Presence.Online, label: "Online" }, + { presence: Presence.Offline, label: "Offline" }, + { presence: Presence.Busy, label: "Busy" }, + { presence: Presence.Away, label: "Away" }, ])("should render the $presence presence", ({ presence, label }) => { mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue, diff --git a/test/unit-tests/components/views/avatars/__snapshots__/RoomAvatarView-test.tsx.snap b/test/unit-tests/components/views/avatars/__snapshots__/RoomAvatarView-test.tsx.snap index 744d4cf4ad..1a4263a0c9 100644 --- a/test/unit-tests/components/views/avatars/__snapshots__/RoomAvatarView-test.tsx.snap +++ b/test/unit-tests/components/views/avatars/__snapshots__/RoomAvatarView-test.tsx.snap @@ -108,7 +108,76 @@ exports[` should render a video room decoration 1`] = ` `; -exports[` should render the busy presence 1`] = ` +exports[` should render the AWAY presence 1`] = ` + +
+ + + + + + + + + + + + + + + + +
+ +`; + +exports[` should render the BUSY presence 1`] = `
should render the busy presence 1`] = ` `; -exports[` should render the offline presence 1`] = ` +exports[` should render the OFFLINE presence 1`] = `
should render the offline presence 1`] = ` `; -exports[` should render the online presence 1`] = ` +exports[` should render the ONLINE presence 1`] = `
should render the online presence 1`] = `
`; - -exports[` should render the unavailable presence 1`] = ` - -
- - - - - - - - - - - - - - - - -
- -`;