From 6103f7e3b40887fa783aa80e3b32bcfbc58bb34e Mon Sep 17 00:00:00 2001 From: R Midhun Suresh Date: Tue, 10 Jun 2025 13:45:38 +0530 Subject: [PATCH] Add low priority avatar decoration to room tile (#30065) * Add avatar decoration for low priority rooms * Write tests * Remove unnecesasry step in test * Make the vm expose which decoration to render * Fix jest test * Fix broken e2e test --- .../room-list-panel/room-list.spec.ts | 22 ++++ .../room-list-item-low-priority-linux.png | Bin 0 -> 2674 bytes .../avatars/RoomAvatarViewModel.tsx | 40 ++++--- .../views/avatars/RoomAvatarView.tsx | 83 ++++++++------ src/i18n/strings/en_EN.json | 1 + .../avatars/RoomAvatarViewModel-test.tsx | 54 +++++++-- .../views/avatars/RoomAvatarView-test.tsx | 43 ++++--- .../RoomAvatarView-test.tsx.snap | 106 ++++++++---------- 8 files changed, 207 insertions(+), 142 deletions(-) create mode 100644 playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png diff --git a/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts b/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts index b0b32a765a..73eb98512b 100644 --- a/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts +++ b/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts @@ -255,6 +255,28 @@ test.describe("Room list", () => { await expect(publicRoom).toMatchScreenshot("room-list-item-public.png"); }); + test("should be a low priority room", { tag: "@screenshot" }, async ({ page, app, user }) => { + // @ts-ignore Visibility enum is not accessible + await app.client.createRoom({ name: "low priority room", visibility: "public" }); + const roomListView = getRoomList(page); + const publicRoom = roomListView.getByRole("gridcell", { name: "low priority room" }); + + // Make room low priority + await publicRoom.hover(); + const roomItemMenu = publicRoom.getByRole("button", { name: "More Options" }); + await roomItemMenu.click(); + await page.getByRole("menuitemcheckbox", { name: "Low priority" }).click(); + + // Should have low priority decoration + await expect(publicRoom.locator(".mx_RoomAvatarView_icon")).toHaveAccessibleName( + "This is a low priority room", + ); + + // focus the user menu to avoid to have hover decoration + await page.getByRole("button", { name: "User menu" }).focus(); + await expect(publicRoom).toMatchScreenshot("room-list-item-low-priority.png"); + }); + test("should be a video room", { tag: "@screenshot" }, async ({ page, app, user }) => { await page.getByTestId("room-list-panel").getByRole("button", { name: "Add" }).click(); await page.getByRole("menuitem", { name: "New video room" }).click(); diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png new file mode 100644 index 0000000000000000000000000000000000000000..cc2adcc5989ba6ab2b8c1b9ca41f5969d5f222a3 GIT binary patch literal 2674 zcmV-&3XS!NP)Px?ZTBns;SLP*pA znat8y_4;mn>7%Ujny2+ukLy4V=(PcTl=>T@yh-al*sn zL6@4U--zi(8=*3Ho2!xYND4*jPza&kBYj;czx({(e&=h-8%Xw7*QWh=grjYuDP1^( zP_L1uE)WE9VdKrjsB9_X%?M{rS!F}Pl>+(VE)ea+ZtlsFl*iqSI)zvk0rLH2>s?C=ljXd1+m2os zg;3ujRTl_?I2CfGC&Hf&jsI3Qj8alm96#RFz`#K2QO}-L{9RlBZedZ6<%p1Mkh%_< zU)9{w{GlhpzkA)nuWDA+7~B(+%iOMr?Y%Y%p*}=vy-=F=M7B`}D9AHBjC_Be5uW_x z;30#gnBl>Lhd+gcSy|iJ+Oa}^URzf77y#n0-O$k+)!f`H=(Eg!g~7;iEiLZ_hqF9q zuU)r6WS;Q#8<#Fye(>O7dk6LebF0-sA*Bxv$nAZ0Jo_#>?S(`z>1YYo*eJblVLl%Zun)hNQO>q z*PRiObLaVv9Bq6f;U)kiCMGRj;%{VP?lyJCmaW?x8~MV{T{SteU3c%<8@49g*wkY7 z9G}9%qL_A|si}!+ZznG=m%>W}fVQ@_wDb%*T|GB94*(h( z8uIcBoSax4XG_n>Oy==+M@F1HakRF!&d0~^&6|(6d!+Ev3=Iu<$q9bG3&-TTIyc_;KiAheJiKD{F$J`ox6o4r4PI~vpQyY>hzh# zix*~Pq%bVZZ{_6r`YqAb)rs7-W8SJMEFKY}Nwq$9* zm8U1*9eYln=H|fs_@8$8@CJ{W?&0gg1cz3ZjLA%=`}a%6jvaIO z@X@ZC0KkdOvay*203kmGS=-o0?AWTSqXPhbzVkQ#`kR!$3G|&JwO)8$QZ18U)54hD zFA4yVUvS&P+)VJ81ONpE1%`!rUVZ@pI6FJ>Qquq+Jw1bAX<=o>NYBUwfVA`sN0x(d z$mdLqjRZvj0F;%LCyX~OxP3=(NT;hyng9SODJgxmC;$M3Lg8>c&P87UfDa! WM< z&E<63lTN1#|DBVYXJut6Jk-|HoAAc4_MMVG@IPhcggQ!&DN(Ui< z-rJYOm~H45GFc{55dyZX`TTxKDUX+C$*{0wSQHf%*VWY}C8sc%T}1CN zRcmXj)Cow0Od_wJqh^l4umA||w~vOBNzajJHgmHHdHDsxNALUh?{jkV%*`eMfV%q7iB^^g2{-TEFL7W_9y)X=gJBUH zce$pfw$qaf08m_9%;$f6!fbACF1US1Utdq^1SF!VsmW$L#$UUBHU65btBY{iB8DZy zB0Kw*@UX1xaYaR?g}IsdCWItHsxAyS?0h**-b61Gm0_K~8ChH3SW{Oo(QI?Pxr3Aj zty~qHk&&5^kr}uuSeZ)op6w-Qb#h`IKXKB^k|9{kvmBU*{y1V|V@*?4?Q}L8jpiS) zGA%75Jw4MuAdn!4nKRwHlR#J0)Mx)qR#jCwAAM@LhDLW1=*r;1gV(MJ4GIpO;xbLR)?hg>`Hg%vHQFTWiDZ8E z(W6EkJ#z5Eg;+1{+(k>4Sy?i+Z~g77O$bSZ)P2^CuU>~4MTqaSDKntcWkvGAoU`k$ zZ!HQ^9sajY2S&%;pisbb`V_J$%T)OKjTM!Z$BrE8ljwqz0ycIIbLY%n9T>Kok-)9=pKZ`obpK)g-Fx%7?pm6kR_qne zUmZUiXJuyWXm3p__itQnZSBliTwPtAsC~QD)ra*8j`<{DVO7>Pu&5`O$k9u|gl4^udPwVFA))M#rDmxY! z`_<2_owsx9I7fZFy$_*pLbvLI5Wl+l&ef7f+2z$w>#LvCK|2i78=|hGW@xQ#>N;Ar zi#w(eLj99$>H)&+!+C{p{f^aqVi g;x3&+2+7d@0Mqm}!5Bh-Pyhe`07*qoM6N<$f*#r|=l}o! literal 0 HcmV?d00001 diff --git a/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx b/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx index 8879f5ae69..3832616a9c 100644 --- a/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx +++ b/src/components/viewmodels/avatars/RoomAvatarViewModel.tsx @@ -10,26 +10,26 @@ import { useEffect, useState } from "react"; import { useTypedEventEmitter } from "../../../hooks/useEventEmitter"; import { useDmMember, usePresence, type Presence } from "../../views/avatars/WithPresenceIndicator"; +import { DefaultTagID } from "../../../stores/room-list/models"; + +export enum AvatarBadgeDecoration { + LowPriority = "LowPriority", + VideoRoom = "VideoRoom", + PublicRoom = "PublicRoom", + Presence = "Presence", +} export interface RoomAvatarViewState { - /** - * Whether the room avatar has a decoration. - * A decoration can be a public or a video call icon or an indicator of presence. - */ - hasDecoration: boolean; - /** - * Whether the room is public. - */ - isPublic: boolean; - /** - * Whether the room is a video room. - */ - isVideoRoom: boolean; /** * 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 | null; + + /** + * The decoration that should be rendered. + */ + badgeDecoration?: AvatarBadgeDecoration; } /** @@ -41,10 +41,20 @@ export function useRoomAvatarViewModel(room: Room): RoomAvatarViewState { const roomMember = useDmMember(room); const presence = usePresence(room, roomMember); const isPublic = useIsPublic(room); + const isLowPriority = !!room.tags[DefaultTagID.LowPriority]; - const hasDecoration = isPublic || isVideoRoom || presence !== null; + let badgeDecoration: AvatarBadgeDecoration | undefined; + if (isLowPriority) { + badgeDecoration = AvatarBadgeDecoration.LowPriority; + } else if (isVideoRoom) { + badgeDecoration = AvatarBadgeDecoration.VideoRoom; + } else if (isPublic) { + badgeDecoration = AvatarBadgeDecoration.PublicRoom; + } else if (presence) { + badgeDecoration = AvatarBadgeDecoration.Presence; + } - return { hasDecoration, isPublic, isVideoRoom, presence }; + return { badgeDecoration, presence }; } /** diff --git a/src/components/views/avatars/RoomAvatarView.tsx b/src/components/views/avatars/RoomAvatarView.tsx index 8810d073c5..5ddf355d6f 100644 --- a/src/components/views/avatars/RoomAvatarView.tsx +++ b/src/components/views/avatars/RoomAvatarView.tsx @@ -9,13 +9,14 @@ import React, { type JSX } from "react"; import { type Room } from "matrix-js-sdk/src/matrix"; import PublicIcon from "@vector-im/compound-design-tokens/assets/web/icons/public"; import VideoIcon from "@vector-im/compound-design-tokens/assets/web/icons/video-call-solid"; +import ArrowDownIcon from "@vector-im/compound-design-tokens/assets/web/icons/arrow-down"; import OnlineOrUnavailableIcon from "@vector-im/compound-design-tokens/assets/web/icons/presence-solid-8x8"; import OfflineIcon from "@vector-im/compound-design-tokens/assets/web/icons/presence-outline-8x8"; import BusyIcon from "@vector-im/compound-design-tokens/assets/web/icons/presence-strikethrough-8x8"; import classNames from "classnames"; import RoomAvatar from "./RoomAvatar"; -import { useRoomAvatarViewModel } from "../../viewmodels/avatars/RoomAvatarViewModel"; +import { AvatarBadgeDecoration, useRoomAvatarViewModel } from "../../viewmodels/avatars/RoomAvatarViewModel"; import { _t } from "../../../languageHandler"; import { Presence } from "./WithPresenceIndicator"; @@ -33,41 +34,21 @@ interface RoomAvatarViewProps { export function RoomAvatarView({ room }: RoomAvatarViewProps): JSX.Element { const vm = useRoomAvatarViewModel(room); // No decoration, we just show the avatar - if (!vm.hasDecoration) return ; + if (!vm.badgeDecoration) return ; + + const icon = getAvatarDecoration(vm.badgeDecoration, vm.presence); + + // Presence indicator and video/public icons don't have the same size + // We use different masks + const maskClass = + vm.badgeDecoration === AvatarBadgeDecoration.Presence + ? "mx_RoomAvatarView_RoomAvatar_presence" + : "mx_RoomAvatarView_RoomAvatar_icon"; return (
- - - {/* If the room is a public video room, we prefer to display only the video icon */} - {vm.isPublic && !vm.isVideoRoom && ( - - )} - {vm.isVideoRoom && ( - - )} - {vm.presence && } + + {icon}
); } @@ -126,3 +107,39 @@ function PresenceDecoration({ presence }: PresenceDecorationProps): JSX.Element ); } } + +function getAvatarDecoration(decoration: AvatarBadgeDecoration, presence: Presence | null): React.ReactNode { + if (decoration === AvatarBadgeDecoration.LowPriority) { + return ( + + ); + } else if (decoration === AvatarBadgeDecoration.VideoRoom) { + return ( + + ); + } else if (decoration === AvatarBadgeDecoration.PublicRoom) { + return ( + + ); + } else if (decoration === AvatarBadgeDecoration.Presence) { + return ; + } +} diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 0316615c72..3a55fdb75f 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -2047,6 +2047,7 @@ "read_topic": "Click to read topic", "rejecting": "Rejecting invite…", "rejoin_button": "Re-join", + "room_is_low_priority": "This is a low priority room", "search": { "all_rooms_button": "Search all rooms", "placeholder": "Search messages…", diff --git a/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx b/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx index ffaa152a53..5f29f3ff33 100644 --- a/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx +++ b/test/unit-tests/components/viewmodels/avatars/RoomAvatarViewModel-test.tsx @@ -8,10 +8,14 @@ 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 { + AvatarBadgeDecoration, + useRoomAvatarViewModel, +} from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel"; import { createTestClient, mkStubRoom } from "../../../../test-utils"; import DMRoomMap from "../../../../../src/utils/DMRoomMap"; import * as PresenceIndicatorModule from "../../../../../src/components/views/avatars/WithPresenceIndicator"; +import { DefaultTagID } from "../../../../../src/stores/room-list/models"; jest.mock("../../../../../src/utils/room/getJoinedNonFunctionalMembers", () => ({ getJoinedNonFunctionalMembers: jest.fn().mockReturnValue([]), @@ -32,35 +36,61 @@ describe("RoomAvatarViewModel", () => { jest.spyOn(PresenceIndicatorModule, "usePresence").mockReturnValue(null); }); - it("should has hasDecoration to false", async () => { + it("should have badgeDecoration set to LowPriority", () => { + room.tags[DefaultTagID.LowPriority] = {}; const { result: vm } = renderHook(() => useRoomAvatarViewModel(room)); - expect(vm.current.hasDecoration).toBe(false); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.LowPriority); }); - it("should has isVideoRoom set to true", () => { + it("should have badgeDecoration set to VideoRoom", () => { jest.spyOn(room, "isCallRoom").mockReturnValue(true); const { result: vm } = renderHook(() => useRoomAvatarViewModel(room)); - expect(vm.current.isVideoRoom).toBe(true); - expect(vm.current.hasDecoration).toBe(true); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.VideoRoom); }); - it("should has isPublic set to true", () => { + it("should have badgeDecoration set to PublicRoom", () => { jest.spyOn(room, "getJoinRule").mockReturnValue(JoinRule.Public); - const { result: vm } = renderHook(() => useRoomAvatarViewModel(room)); - expect(vm.current.isPublic).toBe(true); - expect(vm.current.hasDecoration).toBe(true); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.PublicRoom); + }); + + it("should set badgeDecoration based on priority", () => { + // 1. Presence has the least priority + 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); + + const { result: vm1 } = renderHook(() => useRoomAvatarViewModel(room)); + expect(vm1.current.badgeDecoration).toBe(AvatarBadgeDecoration.Presence); + + // 2. With presence and public room, presence takes precedence + jest.spyOn(room, "getJoinRule").mockReturnValue(JoinRule.Public); + // Render again, it's easier than mocking the event emitter. + const { result: vm, rerender } = renderHook(() => useRoomAvatarViewModel(room)); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.PublicRoom); + + // 3. With presence, public-room and video room, video room takes precedence + jest.spyOn(room, "isCallRoom").mockReturnValue(true); + rerender(room); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.VideoRoom); + + // 4. With presence, public room, video room and low priority, low priority takes precedence + room.tags[DefaultTagID.LowPriority] = {}; + rerender(room); + expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.LowPriority); }); it("should recompute isPublic when room changed", async () => { const { result: vm, rerender } = renderHook((props) => useRoomAvatarViewModel(props), { initialProps: room }); - expect(vm.current.isPublic).toBe(false); + expect(vm.current.badgeDecoration).not.toBe(AvatarBadgeDecoration.PublicRoom); const publicRoom = mkStubRoom("roomId2", "roomName2", matrixClient); jest.spyOn(publicRoom, "getJoinRule").mockReturnValue(JoinRule.Public); rerender(publicRoom); - await waitFor(() => expect(vm.current.isPublic).toBe(true)); + await waitFor(() => expect(vm.current.badgeDecoration).toBe(AvatarBadgeDecoration.PublicRoom)); }); it("should return presence", async () => { diff --git a/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx b/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx index 020b92227d..00ab886b4f 100644 --- a/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx +++ b/test/unit-tests/components/views/avatars/RoomAvatarView-test.tsx @@ -12,6 +12,7 @@ import { mocked } from "jest-mock"; import { RoomAvatarView } from "../../../../../src/components/views/avatars/RoomAvatarView"; import { mkStubRoom, stubClient } from "../../../../test-utils"; import { + AvatarBadgeDecoration, type RoomAvatarViewState, useRoomAvatarViewModel, } from "../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel"; @@ -19,6 +20,7 @@ import DMRoomMap from "../../../../../src/utils/DMRoomMap"; import { Presence } from "../../../../../src/components/views/avatars/WithPresenceIndicator"; jest.mock("../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel", () => ({ + ...jest.requireActual("../../../../../src/components/viewmodels/avatars/RoomAvatarViewModel"), useRoomAvatarViewModel: jest.fn(), })); @@ -33,9 +35,7 @@ describe("", () => { beforeEach(() => { defaultValue = { - hasDecoration: true, - isPublic: true, - isVideoRoom: true, + badgeDecoration: undefined, presence: null, }; @@ -43,13 +43,27 @@ describe("", () => { }); it("should not render a decoration", () => { - mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue, hasDecoration: false }); + mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue }); const { asFragment } = render(); expect(asFragment()).toMatchSnapshot(); }); + it("should render a low priority room decoration", () => { + mocked(useRoomAvatarViewModel).mockReturnValue({ + ...defaultValue, + badgeDecoration: AvatarBadgeDecoration.LowPriority, + }); + const { asFragment } = render(); + + expect(screen.getByLabelText("This is a low priority room")).toBeInTheDocument(); + expect(asFragment()).toMatchSnapshot(); + }); + it("should render a video room decoration", () => { - mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue, hasDecoration: true, isVideoRoom: true }); + mocked(useRoomAvatarViewModel).mockReturnValue({ + ...defaultValue, + badgeDecoration: AvatarBadgeDecoration.VideoRoom, + }); const { asFragment } = render(); expect(screen.getByLabelText("This room is a video room")).toBeInTheDocument(); @@ -59,9 +73,7 @@ describe("", () => { it("should render a public room decoration", () => { mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue, - hasDecoration: true, - isPublic: true, - isVideoRoom: false, + badgeDecoration: AvatarBadgeDecoration.PublicRoom, }); const { asFragment } = render(); @@ -69,19 +81,6 @@ describe("", () => { expect(asFragment()).toMatchSnapshot(); }); - it("should not render a public room decoration if the room is a video room", () => { - mocked(useRoomAvatarViewModel).mockReturnValue({ - ...defaultValue, - hasDecoration: true, - isPublic: true, - isVideoRoom: true, - }); - render(); - - expect(screen.getByLabelText("This room is a video room")).toBeInTheDocument(); - expect(screen.queryByLabelText("This room is public")).toBeNull(); - }); - it.each([ { presence: Presence.Online, label: "Online" }, { presence: Presence.Offline, label: "Offline" }, @@ -90,7 +89,7 @@ describe("", () => { ])("should render the $presence presence", ({ presence, label }) => { mocked(useRoomAvatarViewModel).mockReturnValue({ ...defaultValue, - hasDecoration: true, + badgeDecoration: AvatarBadgeDecoration.Presence, presence, }); const { asFragment } = render(); 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 1a4263a0c9..9aeb54ab0f 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 @@ -24,6 +24,48 @@ exports[` should not render a decoration 1`] = ` `; +exports[` should render a low priority room decoration 1`] = ` + +
+ + + + + + +
+ +`; + exports[` should render a public room decoration 1`] = `
should render the AWAY presence 1`] = ` > should render the AWAY presence 1`] = ` width="32px" /> - - - should render the BUSY presence 1`] = ` > should render the BUSY presence 1`] = ` width="32px" /> - - - should render the OFFLINE presence 1`] = ` > should render the OFFLINE presence 1`] = ` width="32px" /> - - - should render the ONLINE presence 1`] = ` > should render the ONLINE presence 1`] = ` width="32px" /> - - -