From 5933f5093033edf4c2b8f675f941572e1e3d1ec5 Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Tue, 22 Apr 2025 15:21:50 +0200 Subject: [PATCH] New room list: fix missing/incorrect notification decoration (#29796) * fix: recompute notification when room change in room list item vm * test: add use case when room list change * test(e2e): add screenshot to unread filter test --- .../room-list-filter-sort.spec.ts | 42 ++++++++++-------- .../unread-dm-linux.png | Bin 0 -> 3114 bytes .../roomlist/RoomListItemViewModel.tsx | 5 +++ .../roomlist/RoomListItemViewModel-test.tsx | 18 ++++++++ 4 files changed, 47 insertions(+), 18 deletions(-) create mode 100644 playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png diff --git a/playwright/e2e/left-panel/room-list-panel/room-list-filter-sort.spec.ts b/playwright/e2e/left-panel/room-list-panel/room-list-filter-sort.spec.ts index 21d22d8ccd..0ce13173b8 100644 --- a/playwright/e2e/left-panel/room-list-panel/room-list-filter-sort.spec.ts +++ b/playwright/e2e/left-panel/room-list-panel/room-list-filter-sort.spec.ts @@ -140,29 +140,35 @@ test.describe("Room list filters and sort", () => { expect(await roomList.locator("role=gridcell").count()).toBe(3); }); - test("unread filter should only match unread rooms that have a count", async ({ page, app, bot }) => { - const roomListView = getRoomList(page); + test( + "unread filter should only match unread rooms that have a count", + { tag: "@screenshot" }, + async ({ page, app, bot }) => { + const roomListView = getRoomList(page); - // Let's configure unread dm room so that we only get notification for mentions and keywords - await app.viewRoomById(unReadDmId); - await app.settings.openRoomSettings("Notifications"); - await page.getByText("@mentions & keywords").click(); - await app.settings.closeDialog(); + // Let's configure unread dm room so that we only get notification for mentions and keywords + await app.viewRoomById(unReadDmId); + await app.settings.openRoomSettings("Notifications"); + await page.getByText("@mentions & keywords").click(); + await app.settings.closeDialog(); - // Let's open a room other than unread room or unread dm - await roomListView.getByRole("gridcell", { name: "Open room favourite room" }).click(); + // Let's open a room other than unread room or unread dm + await roomListView.getByRole("gridcell", { name: "Open room favourite room" }).click(); - // Let's make the bot send a new message in both rooms - await bot.sendMessage(unReadDmId, "Hello!"); - await bot.sendMessage(unReadRoomId, "Hello!"); + // Let's make the bot send a new message in both rooms + await bot.sendMessage(unReadDmId, "Hello!"); + await bot.sendMessage(unReadRoomId, "Hello!"); - // Let's activate the unread filter now - await page.getByRole("option", { name: "Unread" }).click(); + // Let's activate the unread filter now + await page.getByRole("option", { name: "Unread" }).click(); - // Unread filter should only show unread room and not unread dm! - await expect(roomListView.getByRole("gridcell", { name: "Open room unread room" })).toBeVisible(); - await expect(roomListView.getByRole("gridcell", { name: "Open room unread dm" })).not.toBeVisible(); - }); + // Unread filter should only show unread room and not unread dm! + const unreadDm = roomListView.getByRole("gridcell", { name: "Open room unread room" }); + await expect(unreadDm).toBeVisible(); + await expect(unreadDm).toMatchScreenshot("unread-dm.png"); + await expect(roomListView.getByRole("gridcell", { name: "Open room unread dm" })).not.toBeVisible(); + }, + ); }); test.describe("Empty room list", () => { diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png new file mode 100644 index 0000000000000000000000000000000000000000..8f33802a879095c7c43e73b9c5ef330e9f6149d7 GIT binary patch literal 3114 zcmaKu`#;l<>tR0ldDvSn2{Uahox5~N+u${T1)TI)VxFfy5(L*6 zaXwzrF0-wW;Q@OHrAoyt*foIX$7O8Kc#&B08cBH=F-$UBB%WhJ-1Uy?9%E0`d4-2I zKTr0+i zVfjmE2p9FzF!4h7dQeMK4Wa5;1QpOHZ=1#H!Niwr}9*)nRGXC)0@aYksIPkl|LWx&b zeG{J>j`{gP6GHTsTG{pt-QtMTtY-vT(NYTxjZLyaA?l3i{ws-J>N4~Ncajj zz$y$!p?qhOQQ*P}=&cE8uN>MjMS^!yd=IDUmMbEhRjvW&lYGUuABr<_8O)nm>O& zsuI2k)H*9zrb}j<3xb$M+jLv!4>#ZUS&_caFJh`6eGpC`rQDH9;yf{@mb>l%!1vU# zb~{{3ZqQMKA&UgPirXpycY(1n$M`Xm8<8rR$TJzD$)|C4gMAD;cV`J(Uw;gzOr?~H zsG;C<fQqk+J#l_T>H{V45g?zms zeV18DNeOD#>Q|pNDgtl39pfINez07_RmZUO(5}woFVmFaUM6yF`Uu@TZE1n$%6Y?_)=-dtT;K|A7pzs?KC zFU_n)Iz9YR@sp7;;$ZjQZPj;ivBu2P4c&cXeojM}QW{MWxv57YX|0`v8CXrXp}W5q zJcFb)l$E_O9U!(IG6WKvkaH?Ly|;aAfLOPe$A=NCPHHb-zLYfCDXBuJ?)6r&3%ly; zA9;EY9MUOY5smB1Fjr4cD(78&tgwj6Kb6z<_9R387ilLxXIpdJtPvv)S_- ztSg4p4nh20pZ6yGu}ORF^r7p=Tdm6M9>Z?rb=`=PcL+77w_fz0}-T1kOLb@PYYPC_V5Q!wXf*THOZWMr3v8j*qF zP@c6Vc%Sw!4#x$B@hf(XBQn?D&JZgqD(;(@loIS^(;r)GwQi9v;I)0A+nTX%Lg(Es%5(|nhZ@u4fLpcC(&c|);8&sVzhCDc(0;t*le z(Vbppzgn|gcPmFMob0;Q#kS(kbeQKDN{ry}Zy@U(7 zEx19q#)p?mZXP4u^|kw))tYg3j^zGIE^?*cE9v<)C|Ls=0QC*Km(M@M#-VdlD$Z0T z!N87D$4P&92;a51c%PMM_xJF$tT65Br~S9N6s03e7Sg|n25aW@F2s;7@y8iSCbQoY z0x=)@6@@-=ceg>c)LW0(^`l8kjns)aC%Jo9!r#4nCnzK|&qf^S#F(mRGb0I;TU(%_ zjLSc}&J*PnGXBtLI+DrTTcDoa-UTpAgibg|gpRzjvKR+5zbl(UM!BLZb82a<)B8&7 z#c16m1M!U__M!Y1uy1_NwMW!&vkW>4KD9e7>)&pL&vg=6aty$6m$UOl(*M4ogwCpV zB2U}Xp@cuQd)RtI@1}+Z>s>YfHO)X;`#a97!NILs8@p}6`$i6EbZF#oS!8wTn=gu_ zB%wKxomzstI^8$!HRUs=*ksL$#dnX_kF@~|sWpMt%6T&xC0S)GEyyVV$!oQLk!mu&A6W}=k%fF;N02f zkU%|&&dt+3^&LF2NlW`^DtsjS95-EG!WTAFlPw@{`RTJ~8X-FcwOfoj#ih+s(ko#j zZkI1#&Ui1U>b5{21gc_Vb@Nbr3MA*v;s+2eF0Md5Zct&PBj~lMOH?K~Sf@kIab-A4 z3l=W=XL9u5dHzhYjF+F1>j~~9f@-*SjKSjl(QcOUda77{m~u4ZB^iFwWCs3js6%p| zu{#0B`Hkk~40j03w-Qs~x9Vf$kLz;Hl#uucape3nvu0!d(CWbhe91%$O5z50kDu1` z*ci@fI>N(sve2UB1_iOPFM~P}nt6gSE2O!*xrwNSj0_Kp%5OS{goK5KY3h)>+*4xf zEG>L{GKc5`vZjH7``}b)imb7tBjUJ~fNPuh=S9W-d^NH{qgh&1s@U%y(o45ye~r+o!J<+E6eh}KUdyt|Kv!#whQOMBe-dsZyHW#yj7-J0;F zgvWG$2b-1Y2gy}*;lg2dRw)CCpBfZ}BH0Dp-g1-O%};UK}tuZ$&x8HlloIcS|hs zt*9f@I`*wPQek(tP3U3R&S;53A~oF6QA9b zSIhB9T_5x(DGR3UFDUnW^Fr0g8tomv9#YxCGFq#FJBLjNOF2g8ss>^8jSWjhX}W{) zmV%(Cx(vNXfYyFcluEKh6-FhQ z4iQ%;Pnp1NjTWC>+WeY08sAQ~-Z3bmy~h$M;j-|HC_kTprOZ@nkdGv<~kfWxUF&GLI zaS5uVm&@#p6-w3bT$i3(x^o@afSJ=jE6C70i}#U*dj|^gI|gQpBg8PfAfHAkupR|# zjk<{F9vxI~tzLNrGl`8ciRAMfn4F(eo^IQ|3<5e@swnFf;7y5dT9eIz5X=8i&pz|q z+OOPPUOK}J{9avoDeLFSW&8q>=IRh&WBqqgWA86+Z?bL~tLXLon%$QJ7a+ilF3$@9 yp)S>iOaK{U5B%Q<`Y7V{e3s>yCJJnr?SwY*usFF(u=Jdsf51@BRJT&+3Fd!@fDiTn literal 0 HcmV?d00001 diff --git a/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx b/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx index 0f10f600fb..e543dd46b2 100644 --- a/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx +++ b/src/components/viewmodels/roomlist/RoomListItemViewModel.tsx @@ -92,6 +92,11 @@ export function useRoomListItemViewModel(room: Room): RoomListItemViewState { setNotificationValues(getNotificationValues(notificationState)); }); + // If the notification reference change due to room change, update the values + useEffect(() => { + setNotificationValues(getNotificationValues(notificationState)); + }, [notificationState]); + // We don't want to show the hover menu if // - there is an invitation for this room // - the user doesn't have access to both notification and more options menus diff --git a/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx b/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx index 68ba08c596..c00c38227d 100644 --- a/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx +++ b/test/unit-tests/components/viewmodels/roomlist/RoomListItemViewModel-test.tsx @@ -128,6 +128,24 @@ describe("RoomListItemViewModel", () => { ); expect(vm.current.isBold).toBe(true); }); + + it("should recompute notification state when room changes", () => { + const newRoom = mkStubRoom("room2", "Room 2", room.client); + const newNotificationState = new RoomNotificationState(newRoom, false); + + const { result, rerender } = renderHook((room) => useRoomListItemViewModel(room), { + ...withClientContextRenderOptions(room.client), + initialProps: room, + }); + + expect(result.current.showNotificationDecoration).toBe(false); + + jest.spyOn(newNotificationState, "hasAnyNotificationOrActivity", "get").mockReturnValue(true); + jest.spyOn(RoomNotificationStateStore.instance, "getRoomState").mockReturnValue(newNotificationState); + rerender(newRoom); + + expect(result.current.showNotificationDecoration).toBe(true); + }); }); describe("a11yLabel", () => {