Skip to content

Commit 18a7250

Browse files
authored
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
1 parent 7e5f96c commit 18a7250

File tree

7 files changed

+55
-172
lines changed

7 files changed

+55
-172
lines changed

src/components/viewmodels/avatars/RoomAvatarViewModel.tsx

Lines changed: 5 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -5,32 +5,11 @@
55
* Please see LICENSE files in the repository root for full details.
66
*/
77

8-
import {
9-
EventType,
10-
JoinRule,
11-
type MatrixEvent,
12-
type Room,
13-
RoomEvent,
14-
type User,
15-
UserEvent,
16-
} from "matrix-js-sdk/src/matrix";
8+
import { EventType, JoinRule, type MatrixEvent, type Room, RoomEvent } from "matrix-js-sdk/src/matrix";
179
import { useEffect, useState } from "react";
1810

1911
import { useTypedEventEmitter } from "../../../hooks/useEventEmitter";
20-
import DMRoomMap from "../../../utils/DMRoomMap";
21-
import { getJoinedNonFunctionalMembers } from "../../../utils/room/getJoinedNonFunctionalMembers";
22-
import { BUSY_PRESENCE_NAME } from "../../views/rooms/PresenceLabel";
23-
import { isPresenceEnabled } from "../../../utils/presence";
24-
25-
/**
26-
* The presence of a user in a DM room.
27-
* - "online": The user is online.
28-
* - "offline": The user is offline.
29-
* - "busy": The user is busy.
30-
* - "unavailable": the presence is unavailable.
31-
* - null: the user is not in a DM room or presence is not enabled.
32-
*/
33-
export type Presence = "online" | "offline" | "busy" | "unavailable" | null;
12+
import { useDmMember, usePresence, type Presence } from "../../views/avatars/WithPresenceIndicator";
3413

3514
export interface RoomAvatarViewState {
3615
/**
@@ -50,7 +29,7 @@ export interface RoomAvatarViewState {
5029
* The presence of the user in the DM room.
5130
* If null, the user is not in a DM room or presence is not enabled.
5231
*/
53-
presence: Presence;
32+
presence: Presence | null;
5433
}
5534

5635
/**
@@ -59,7 +38,8 @@ export interface RoomAvatarViewState {
5938
*/
6039
export function useRoomAvatarViewModel(room: Room): RoomAvatarViewState {
6140
const isVideoRoom = room.isElementVideoRoom() || room.isCallRoom();
62-
const presence = useDMPresence(room);
41+
const roomMember = useDmMember(room);
42+
const presence = usePresence(room, roomMember);
6343
const isPublic = useIsPublic(room);
6444

6545
const hasDecoration = isPublic || isVideoRoom || presence !== null;
@@ -97,48 +77,3 @@ function useIsPublic(room: Room): boolean {
9777
function isRoomPublic(room: Room): boolean {
9878
return room.getJoinRule() === JoinRule.Public;
9979
}
100-
101-
/**
102-
* Hook listening to the presence of the DM user.
103-
* @param room
104-
*/
105-
function useDMPresence(room: Room): Presence {
106-
const dmUser = getDMUser(room);
107-
const [presence, setPresence] = useState<Presence>(getPresence(dmUser));
108-
useTypedEventEmitter(dmUser, UserEvent.Presence, () => setPresence(getPresence(dmUser)));
109-
useTypedEventEmitter(dmUser, UserEvent.CurrentlyActive, () => setPresence(getPresence(dmUser)));
110-
111-
return presence;
112-
}
113-
114-
/**
115-
* Get the DM user of the room.
116-
* Return undefined if the room is not a DM room, if we can't find the user or if the presence is not enabled.
117-
* @param room
118-
* @returns found user
119-
*/
120-
function getDMUser(room: Room): User | undefined {
121-
const otherUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId);
122-
if (!otherUserId) return;
123-
if (getJoinedNonFunctionalMembers(room).length !== 2) return;
124-
if (!isPresenceEnabled(room.client)) return;
125-
126-
return room.client.getUser(otherUserId) || undefined;
127-
}
128-
129-
/**
130-
* Get the presence of the DM user.
131-
* @param dmUser
132-
*/
133-
function getPresence(dmUser: User | undefined): Presence {
134-
if (!dmUser) return null;
135-
if (BUSY_PRESENCE_NAME.matches(dmUser.presence)) return "busy";
136-
137-
const isOnline = dmUser.currentlyActive || dmUser.presence === "online";
138-
if (isOnline) return "online";
139-
140-
if (dmUser.presence === "offline") return "offline";
141-
if (dmUser.presence === "unavailable") return "unavailable";
142-
143-
return null;
144-
}

src/components/views/avatars/RoomAvatarView.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ import BusyIcon from "@vector-im/compound-design-tokens/assets/web/icons/presenc
1515
import classNames from "classnames";
1616

1717
import RoomAvatar from "./RoomAvatar";
18-
import { useRoomAvatarViewModel, type Presence } from "../../viewmodels/avatars/RoomAvatarViewModel";
18+
import { useRoomAvatarViewModel } from "../../viewmodels/avatars/RoomAvatarViewModel";
1919
import { _t } from "../../../languageHandler";
20+
import { Presence } from "./WithPresenceIndicator";
2021

2122
interface RoomAvatarViewProps {
2223
/**
@@ -83,7 +84,7 @@ type PresenceDecorationProps = {
8384
*/
8485
function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element {
8586
switch (presence) {
86-
case "online":
87+
case Presence.Online:
8788
return (
8889
<OnlineOrUnavailableIcon
8990
width="8px"
@@ -93,7 +94,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
9394
aria-label={_t("presence|online")}
9495
/>
9596
);
96-
case "unavailable":
97+
case Presence.Away:
9798
return (
9899
<OnlineOrUnavailableIcon
99100
width="8px"
@@ -103,7 +104,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
103104
aria-label={_t("presence|away")}
104105
/>
105106
);
106-
case "offline":
107+
case Presence.Offline:
107108
return (
108109
<OfflineIcon
109110
width="8px"
@@ -113,7 +114,7 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element
113114
aria-label={_t("presence|offline")}
114115
/>
115116
);
116-
case "busy":
117+
case Presence.Busy:
117118
return (
118119
<BusyIcon
119120
width="8px"

src/components/views/avatars/WithPresenceIndicator.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ interface Props {
2626
children: ReactNode;
2727
}
2828

29-
enum Presence {
29+
export enum Presence {
3030
// Note: the names here are used in CSS class names
3131
Online = "ONLINE",
3232
Away = "AWAY",
@@ -86,7 +86,7 @@ function getPresence(member: RoomMember | null): Presence | null {
8686
return null;
8787
}
8888

89-
const usePresence = (room: Room, member: RoomMember | null): Presence | null => {
89+
export const usePresence = (room: Room, member: RoomMember | null): Presence | null => {
9090
const [presence, setPresence] = useState<Presence | null>(getPresence(member));
9191
const updatePresence = (): void => {
9292
setPresence(getPresence(member));

src/hooks/useCall.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Com
66
Please see LICENSE files in the repository root for full details.
77
*/
88

9-
import { useState, useCallback, useMemo } from "react";
9+
import { useState, useCallback, useMemo, useEffect } from "react";
1010

1111
import type { RoomMember } from "matrix-js-sdk/src/matrix";
1212
import { type Call, ConnectionState, CallEvent } from "../models/Call";
@@ -20,6 +20,12 @@ export const useCall = (roomId: string): Call | null => {
2020
useEventEmitter(CallStore.instance, CallStoreEvent.Call, (call: Call | null, forRoomId: string) => {
2121
if (forRoomId === roomId) setCall(call);
2222
});
23+
24+
// Reset the value when the roomId changes
25+
useEffect(() => {
26+
setCall(CallStore.instance.getCall(roomId));
27+
}, [roomId]);
28+
2329
return call;
2430
};
2531

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

Lines changed: 14 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,32 +5,18 @@
55
* Please see LICENSE files in the repository root for full details.
66
*/
77

8-
import { renderHook, waitFor, act } from "jest-matrix-react";
9-
import {
10-
JoinRule,
11-
type MatrixClient,
12-
MatrixEvent,
13-
type Room,
14-
type RoomMember,
15-
User,
16-
UserEvent,
17-
} from "matrix-js-sdk/src/matrix";
18-
import { mocked } from "jest-mock";
8+
import { renderHook, waitFor } from "jest-matrix-react";
9+
import { JoinRule, type MatrixClient, type Room, RoomMember, User } from "matrix-js-sdk/src/matrix";
1910

2011
import { useRoomAvatarViewModel } from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel";
2112
import { createTestClient, mkStubRoom } from "../../../../test-utils";
2213
import DMRoomMap from "../../../../../src/utils/DMRoomMap";
23-
import { getJoinedNonFunctionalMembers } from "../../../../../src/utils/room/getJoinedNonFunctionalMembers";
24-
import { isPresenceEnabled } from "../../../../../src/utils/presence";
14+
import * as PresenceIndicatorModule from "../../../../../src/components/views/avatars/WithPresenceIndicator";
2515

2616
jest.mock("../../../../../src/utils/room/getJoinedNonFunctionalMembers", () => ({
2717
getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([]),
2818
}));
2919

30-
jest.mock("../../../../../src/utils/presence", () => ({
31-
isPresenceEnabled: jest.fn().mockReturnValue(false),
32-
}));
33-
3420
describe("RoomAvatarViewModel", () => {
3521
let matrixClient: MatrixClient;
3622
let room: Room;
@@ -41,6 +27,9 @@ describe("RoomAvatarViewModel", () => {
4127

4228
DMRoomMap.makeShared(matrixClient);
4329
jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue(null);
30+
31+
jest.spyOn(PresenceIndicatorModule, "useDmMember").mockReturnValue(null);
32+
jest.spyOn(PresenceIndicatorModule, "usePresence").mockReturnValue(null);
4433
});
4534

4635
it("should has hasDecoration to false", async () => {
@@ -74,62 +63,14 @@ describe("RoomAvatarViewModel", () => {
7463
await waitFor(() => expect(vm.current.isPublic).toBe(true));
7564
});
7665

77-
describe("presence", () => {
78-
let user: User;
79-
80-
beforeEach(() => {
81-
jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue("userId");
82-
mocked(getJoinedNonFunctionalMembers).mockReturnValue([{}, {}] as RoomMember[]);
83-
mocked(isPresenceEnabled).mockReturnValue(true);
84-
85-
user = User.createUser("userId", matrixClient);
86-
jest.spyOn(matrixClient, "getUser").mockReturnValue(user);
87-
});
88-
89-
it("should has presence set to null", () => {
90-
jest.spyOn(DMRoomMap.shared(), "getUserIdForRoomId").mockReturnValue(null);
91-
92-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
93-
expect(vm.current.presence).toBe(null);
94-
});
95-
96-
it("should has online presence", async () => {
97-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
98-
expect(vm.current.presence).toBe("offline");
66+
it("should return presence", async () => {
67+
const user = User.createUser("userId", matrixClient);
68+
const roomMember = new RoomMember(room.roomId, "userId");
69+
roomMember.user = user;
70+
jest.spyOn(PresenceIndicatorModule, "useDmMember").mockReturnValue(roomMember);
71+
jest.spyOn(PresenceIndicatorModule, "usePresence").mockReturnValue(PresenceIndicatorModule.Presence.Online);
9972

100-
user.presence = "online";
101-
102-
await act(() => user.emit(UserEvent.Presence, new MatrixEvent(), user));
103-
await waitFor(() => expect(vm.current.presence).toBe("online"));
104-
105-
user.currentlyActive = true;
106-
user.presence = "offline";
107-
108-
await act(() => user.emit(UserEvent.CurrentlyActive, new MatrixEvent(), user));
109-
await waitFor(() => expect(vm.current.presence).toBe("online"));
110-
});
111-
112-
it("should has busy presence", async () => {
113-
user.presence = "busy";
114-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
115-
expect(vm.current.presence).toBe("busy");
116-
});
117-
118-
it("should has offline presence", async () => {
119-
user.presence = "offline";
120-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
121-
expect(vm.current.presence).toBe("offline");
122-
});
123-
124-
it("should has unavailable presence", async () => {
125-
user.presence = "unavailable";
126-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
127-
expect(vm.current.presence).toBe("unavailable");
128-
});
129-
130-
it("should has hasDecoration to true", async () => {
131-
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
132-
expect(vm.current.hasDecoration).toBe(true);
133-
});
73+
const { result: vm } = renderHook(() => useRoomAvatarViewModel(room));
74+
expect(vm.current.presence).toBe(PresenceIndicatorModule.Presence.Online);
13475
});
13576
});

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ import { mocked } from "jest-mock";
1212
import { RoomAvatarView } from "../../../../../src/components/views/avatars/RoomAvatarView";
1313
import { mkStubRoom, stubClient } from "../../../../test-utils";
1414
import {
15-
type Presence,
1615
type RoomAvatarViewState,
1716
useRoomAvatarViewModel,
1817
} from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel";
1918
import DMRoomMap from "../../../../../src/utils/DMRoomMap";
19+
import { Presence } from "../../../../../src/components/views/avatars/WithPresenceIndicator";
2020

2121
jest.mock("../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel", () => ({
2222
useRoomAvatarViewModel: jest.fn(),
@@ -83,10 +83,10 @@ describe("<RoomAvatarView />", () => {
8383
});
8484

8585
it.each([
86-
{ presence: "online" as Presence, label: "Online" },
87-
{ presence: "offline" as Presence, label: "Offline" },
88-
{ presence: "busy" as Presence, label: "Busy" },
89-
{ presence: "unavailable" as Presence, label: "Away" },
86+
{ presence: Presence.Online, label: "Online" },
87+
{ presence: Presence.Offline, label: "Offline" },
88+
{ presence: Presence.Busy, label: "Busy" },
89+
{ presence: Presence.Away, label: "Away" },
9090
])("should render the $presence presence", ({ presence, label }) => {
9191
mocked(useRoomAvatarViewModel).mockReturnValue({
9292
...defaultValue,

0 commit comments

Comments
 (0)