From 6fc3dd462810e868a0610ef7e82d27b7d9f92c67 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Tue, 15 Apr 2025 10:23:26 +0100 Subject: [PATCH] Refactor RoomAvatar into a functional component. (#29743) * Refactor RoomAvatar into a functional component * Add useRoomAvatar hook * Remove useRoomAvatar hook and fix RoomAvatarEvents not using thumbnails. * lint * Ensure stable version of roomIdName * Use new hook * lint * remove unused param * Fixup tests * remove console * Update test --- src/Avatar.ts | 7 +- src/components/views/avatars/RoomAvatar.tsx | 175 ++++++------------ .../views/avatars/WithPresenceIndicator.tsx | 10 +- .../views/messages/RoomAvatarEvent.tsx | 2 +- .../room_settings/RoomProfileSettings.tsx | 18 +- src/hooks/room/useRoomIdName.ts | 32 ++++ .../avatars/DecoratedRoomAvatar-test.tsx | 3 +- .../views/avatars/RoomAvatar-test.tsx | 14 +- 8 files changed, 128 insertions(+), 133 deletions(-) create mode 100644 src/hooks/room/useRoomIdName.ts diff --git a/src/Avatar.ts b/src/Avatar.ts index abddfd87f1..921332c250 100644 --- a/src/Avatar.ts +++ b/src/Avatar.ts @@ -147,11 +147,12 @@ export function avatarUrlForRoom( width?: number, height?: number, resizeMethod?: ResizeMethod, + avatarMxcOverride?: string, ): string | null { if (!room) return null; // null-guard - - if (room.getMxcAvatarUrl()) { - const media = mediaFromMxc(room.getMxcAvatarUrl() ?? undefined); + const mxc = avatarMxcOverride ?? room.getMxcAvatarUrl(); + if (mxc) { + const media = mediaFromMxc(mxc); if (width !== undefined && height !== undefined) { return media.getThumbnailOfSourceHttp(width, height, resizeMethod); } diff --git a/src/components/views/avatars/RoomAvatar.tsx b/src/components/views/avatars/RoomAvatar.tsx index 2c0e079c83..ae4d6b3605 100644 --- a/src/components/views/avatars/RoomAvatar.tsx +++ b/src/components/views/avatars/RoomAvatar.tsx @@ -6,156 +6,91 @@ 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 React, { type ComponentProps } from "react"; -import { - type Room, - RoomStateEvent, - type MatrixEvent, - EventType, - RoomType, - KnownMembership, -} from "matrix-js-sdk/src/matrix"; +import React, { useCallback, useMemo, type ComponentProps } from "react"; +import { type Room, RoomType, KnownMembership, EventType } from "matrix-js-sdk/src/matrix"; +import { type RoomAvatarEventContent } from "matrix-js-sdk/src/types"; import BaseAvatar from "./BaseAvatar"; import ImageView from "../elements/ImageView"; -import { MatrixClientPeg } from "../../../MatrixClientPeg"; import Modal from "../../../Modal"; import * as Avatar from "../../../Avatar"; -import DMRoomMap from "../../../utils/DMRoomMap"; import { mediaFromMxc } from "../../../customisations/Media"; import { type IOOBData } from "../../../stores/ThreepidInviteStore"; -import { LocalRoom } from "../../../models/LocalRoom"; import { filterBoolean } from "../../../utils/arrays"; -import SettingsStore from "../../../settings/SettingsStore"; +import { useSettingValue } from "../../../hooks/useSettings"; +import { useRoomState } from "../../../hooks/useRoomState"; +import { useRoomIdName } from "../../../hooks/room/useRoomIdName"; -interface IProps extends Omit, "name" | "idName" | "url" | "onClick"> { +interface IProps extends Omit, "name" | "idName" | "url" | "onClick" | "size"> { // Room may be left unset here, but if it is, // oobData.avatarUrl should be set (else there // would be nowhere to get the avatar from) room?: Room; - oobData: IOOBData & { + // Optional here. + size?: ComponentProps["size"]; + oobData?: IOOBData & { roomId?: string; }; viewAvatarOnClick?: boolean; onClick?(): void; } -interface IState { - urls: string[]; -} +const RoomAvatar: React.FC = ({ room, viewAvatarOnClick, onClick, oobData, size = "36px", ...otherProps }) => { + const roomName = room?.name ?? oobData?.name ?? "?"; + const avatarEvent = useRoomState(room, (state) => state.getStateEvents(EventType.RoomAvatar, "")); + const roomIdName = useRoomIdName(room, oobData); -export function idNameForRoom(room: Room): string { - const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId); - // If the room is a DM, we use the other user's ID for the color hash - // in order to match the room avatar with their avatar - if (dmMapUserId) return dmMapUserId; + const showAvatarsOnInvites = useSettingValue("showAvatarsOnInvites", room?.roomId); - if (room instanceof LocalRoom && room.targets.length === 1) { - return room.targets[0].userId; - } - - return room.roomId; -} - -export default class RoomAvatar extends React.Component { - public static defaultProps = { - size: "36px", - oobData: {}, - }; - - public constructor(props: IProps) { - super(props); - - this.state = { - urls: RoomAvatar.getImageUrls(this.props), + const onRoomAvatarClick = useCallback(() => { + const avatarUrl = Avatar.avatarUrlForRoom(room ?? null); + if (!avatarUrl) return; + const params = { + src: avatarUrl, + name: room?.name, }; - } - public componentDidMount(): void { - MatrixClientPeg.safeGet().on(RoomStateEvent.Events, this.onRoomStateEvents); - } + Modal.createDialog(ImageView, params, "mx_Dialog_lightbox", undefined, true); + }, [room]); - public componentWillUnmount(): void { - MatrixClientPeg.get()?.removeListener(RoomStateEvent.Events, this.onRoomStateEvents); - } - - public static getDerivedStateFromProps(nextProps: IProps): IState { - return { - urls: RoomAvatar.getImageUrls(nextProps), - }; - } - - private onRoomStateEvents = (ev: MatrixEvent): void => { - if (ev.getRoomId() !== this.props.room?.roomId || ev.getType() !== EventType.RoomAvatar) return; - - this.setState({ - urls: RoomAvatar.getImageUrls(this.props), - }); - }; - - private static getImageUrls(props: IProps): string[] { - const myMembership = props.room?.getMyMembership(); - if (myMembership === KnownMembership.Invite || !myMembership) { - if (SettingsStore.getValue("showAvatarsOnInvites") === false) { - // The user has opted out of showing avatars, so return no urls here. - return []; - } + const urls = useMemo(() => { + const myMembership = room?.getMyMembership(); + if (!showAvatarsOnInvites && (myMembership === KnownMembership.Invite || !myMembership)) { + // The user has opted out of showing avatars, so return no urls here. + return []; } + + // parseInt ignores suffixes. + const sizeInt = parseInt(size, 10); let oobAvatar: string | null = null; - if (props.oobData.avatarUrl) { - oobAvatar = mediaFromMxc(props.oobData.avatarUrl).getThumbnailOfSourceHttp( - parseInt(props.size, 10), - parseInt(props.size, 10), - "crop", - ); + + if (oobData?.avatarUrl) { + oobAvatar = mediaFromMxc(oobData?.avatarUrl).getThumbnailOfSourceHttp(sizeInt, sizeInt, "crop"); } return filterBoolean([ oobAvatar, // highest priority - RoomAvatar.getRoomAvatarUrl(props), + Avatar.avatarUrlForRoom( + room ?? null, + sizeInt, + sizeInt, + "crop", + avatarEvent?.getContent().url, + ), ]); - } + }, [showAvatarsOnInvites, room, size, avatarEvent, oobData]); - private static getRoomAvatarUrl(props: IProps): string | null { - if (!props.room) return null; + return ( + + ); +}; - return Avatar.avatarUrlForRoom(props.room, parseInt(props.size, 10), parseInt(props.size, 10), "crop"); - } - - private onRoomAvatarClick = (): void => { - const avatarUrl = Avatar.avatarUrlForRoom(this.props.room ?? null, undefined, undefined, undefined); - if (!avatarUrl) return; - const params = { - src: avatarUrl, - name: this.props.room?.name, - }; - - Modal.createDialog(ImageView, params, "mx_Dialog_lightbox", undefined, true); - }; - - private get roomIdName(): string | undefined { - const room = this.props.room; - - if (room) { - return idNameForRoom(room); - } else { - return this.props.oobData?.roomId; - } - } - - public render(): React.ReactNode { - const { room, oobData, viewAvatarOnClick, onClick, ...otherProps } = this.props; - const roomName = room?.name ?? oobData.name ?? "?"; - - return ( - - ); - } -} +export default RoomAvatar; diff --git a/src/components/views/avatars/WithPresenceIndicator.tsx b/src/components/views/avatars/WithPresenceIndicator.tsx index a08250a17c..7059900767 100644 --- a/src/components/views/avatars/WithPresenceIndicator.tsx +++ b/src/components/views/avatars/WithPresenceIndicator.tsx @@ -52,14 +52,14 @@ function getDmMember(room: Room): RoomMember | null { return otherUserId ? room.getMember(otherUserId) : null; } -export const useDmMember = (room: Room): RoomMember | null => { - const [dmMember, setDmMember] = useState(getDmMember(room)); +export const useDmMember = (room?: Room): RoomMember | null => { + const [dmMember, setDmMember] = useState(room ? getDmMember(room) : null); const updateDmMember = (): void => { - setDmMember(getDmMember(room)); + setDmMember(room ? getDmMember(room) : null); }; - useEventEmitter(room.currentState, RoomStateEvent.Members, updateDmMember); - useEventEmitter(room.client, ClientEvent.AccountData, updateDmMember); + useEventEmitter(room?.currentState, RoomStateEvent.Members, updateDmMember); + useEventEmitter(room?.client, ClientEvent.AccountData, updateDmMember); useEffect(updateDmMember, [room]); return dmMember; diff --git a/src/components/views/messages/RoomAvatarEvent.tsx b/src/components/views/messages/RoomAvatarEvent.tsx index 7c155e59d6..c80c80c836 100644 --- a/src/components/views/messages/RoomAvatarEvent.tsx +++ b/src/components/views/messages/RoomAvatarEvent.tsx @@ -70,7 +70,7 @@ export default class RoomAvatarEvent extends React.Component { className="mx_RoomAvatarEvent_avatar" onClick={this.onAvatarClick} > - + ), }, diff --git a/src/components/views/room_settings/RoomProfileSettings.tsx b/src/components/views/room_settings/RoomProfileSettings.tsx index 26cd16e873..e7674a109a 100644 --- a/src/components/views/room_settings/RoomProfileSettings.tsx +++ b/src/components/views/room_settings/RoomProfileSettings.tsx @@ -7,7 +7,7 @@ Please see LICENSE files in the repository root for full details. import React, { createRef } from "react"; import classNames from "classnames"; -import { ContentHelpers, EventType } from "matrix-js-sdk/src/matrix"; +import { ContentHelpers, EventType, type Room } from "matrix-js-sdk/src/matrix"; import { _t } from "../../../languageHandler"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; @@ -15,7 +15,8 @@ import Field from "../elements/Field"; import AccessibleButton, { type ButtonEvent } from "../elements/AccessibleButton"; import AvatarSetting from "../settings/AvatarSetting"; import { htmlSerializeFromMdIfNeeded } from "../../../editor/serialize"; -import { idNameForRoom } from "../avatars/RoomAvatar"; +import DMRoomMap from "../../../utils/DMRoomMap"; +import { LocalRoom } from "../../../models/LocalRoom"; interface IProps { roomId: string; @@ -36,6 +37,19 @@ interface IState { canSetAvatar: boolean; } +function idNameForRoom(room: Room): string { + const dmMapUserId = DMRoomMap.shared().getUserIdForRoomId(room.roomId); + // If the room is a DM, we use the other user's ID for the color hash + // in order to match the room avatar with their avatar + if (dmMapUserId) return dmMapUserId; + + if (room instanceof LocalRoom && room.targets.length === 1) { + return room.targets[0].userId; + } + + return room.roomId; +} + // TODO: Merge with ProfileSettings? export default class RoomProfileSettings extends React.Component { private avatarUpload = createRef(); diff --git a/src/hooks/room/useRoomIdName.ts b/src/hooks/room/useRoomIdName.ts new file mode 100644 index 0000000000..c7930029e2 --- /dev/null +++ b/src/hooks/room/useRoomIdName.ts @@ -0,0 +1,32 @@ +/* +Copyright 2025 New Vector Ltd. + +SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial +Please see LICENSE files in the repository root for full details. +*/ + +import { type Room } from "matrix-js-sdk/src/matrix"; + +import { useDmMember } from "../../components/views/avatars/WithPresenceIndicator.tsx"; +import { LocalRoom } from "../../models/LocalRoom.ts"; + +/** + * Determine a stable ID for generating hash colours. If the room + * is a DM (or local room), then the other user's ID will be used. + * @param oobData - out-of-band information about the room + * @returns An ID string, or undefined if the room and oobData are undefined. + */ +export function useRoomIdName(room?: Room, oobData?: { roomId?: string }): string | undefined { + const dmMember = useDmMember(room); + if (dmMember) { + // If the room is a DM, we use the other user's ID for the color hash + // in order to match the room avatar with their avatar + return dmMember.userId; + } else if (room instanceof LocalRoom && room.targets.length === 1) { + return room.targets[0].userId; + } else if (room) { + return room.roomId; + } else { + return oobData?.roomId; + } +} diff --git a/test/unit-tests/components/views/avatars/DecoratedRoomAvatar-test.tsx b/test/unit-tests/components/views/avatars/DecoratedRoomAvatar-test.tsx index f4a386f9cb..e41f068d21 100644 --- a/test/unit-tests/components/views/avatars/DecoratedRoomAvatar-test.tsx +++ b/test/unit-tests/components/views/avatars/DecoratedRoomAvatar-test.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import { render, waitFor } from "jest-matrix-react"; import { mocked } from "jest-mock"; -import { JoinRule, type MatrixClient, PendingEventOrdering, Room } from "matrix-js-sdk/src/matrix"; +import { JoinRule, type MatrixClient, PendingEventOrdering, Room, RoomMember } from "matrix-js-sdk/src/matrix"; import React from "react"; import userEvent from "@testing-library/user-event"; @@ -79,6 +79,7 @@ describe("DecoratedRoomAvatar", () => { } as unknown as DMRoomMap; jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap); jest.spyOn(DecoratedRoomAvatar.prototype as any, "getPresenceIcon").mockImplementation(() => "ONLINE"); + jest.spyOn(room, "getMember").mockReturnValue(new RoomMember(room.roomId, DM_USER_ID)); const { container, asFragment } = renderComponent(); diff --git a/test/unit-tests/components/views/avatars/RoomAvatar-test.tsx b/test/unit-tests/components/views/avatars/RoomAvatar-test.tsx index 615e24fa69..26c215bf19 100644 --- a/test/unit-tests/components/views/avatars/RoomAvatar-test.tsx +++ b/test/unit-tests/components/views/avatars/RoomAvatar-test.tsx @@ -8,7 +8,7 @@ Please see LICENSE files in the repository root for full details. import React from "react"; import { render } from "jest-matrix-react"; -import { type MatrixClient, Room } from "matrix-js-sdk/src/matrix"; +import { EventType, type MatrixClient, MatrixEvent, Room, RoomMember } from "matrix-js-sdk/src/matrix"; import { mocked } from "jest-mock"; import RoomAvatar from "../../../../../src/components/views/avatars/RoomAvatar"; @@ -60,6 +60,7 @@ describe("RoomAvatar", () => { it("should render as expected for a DM room", () => { const userId = "@dm_user@example.com"; const room = new Room("!room:example.com", client, client.getSafeUserId()); + room.getMember = jest.fn().mockImplementation(() => new RoomMember(room.roomId, userId)); room.name = "DM room"; mocked(DMRoomMap.shared().getUserIdForRoomId).mockReturnValue(userId); expect(render().container).toMatchSnapshot(); @@ -78,6 +79,17 @@ describe("RoomAvatar", () => { jest.spyOn(room, "getMxcAvatarUrl").mockImplementation(() => "mxc://example.com/foobar"); room.name = "test room"; room.updateMyMembership("invite"); + room.currentState.setStateEvents([ + new MatrixEvent({ + sender: "@sender:server", + room_id: room.roomId, + type: EventType.RoomAvatar, + state_key: "", + content: { + url: "mxc://example.com/foobar", + }, + }), + ]); expect(render().container).toMatchSnapshot(); }); it("should not render an invite avatar if the user has disabled it", () => {