diff --git a/playwright/e2e/spaces/threads-activity-centre/index.ts b/playwright/e2e/spaces/threads-activity-centre/index.ts index 224028d06f..4360ddb981 100644 --- a/playwright/e2e/spaces/threads-activity-centre/index.ts +++ b/playwright/e2e/spaces/threads-activity-centre/index.ts @@ -30,30 +30,30 @@ import { ElementAppPage } from "../../../pages/ElementAppPage"; * - Invite the bot to both rooms and ensure that it has joined */ export const test = base.extend<{ - roomAlphaName?: string; - roomAlpha: { name: string; roomId: string }; - roomBetaName?: string; - roomBeta: { name: string; roomId: string }; + room1Name?: string; + room1: { name: string; roomId: string }; + room2Name?: string; + room2: { name: string; roomId: string }; msg: MessageBuilder; util: Helpers; }>({ displayName: "Mae", botCreateOpts: { displayName: "Other User" }, - roomAlphaName: "Room Alpha", - roomAlpha: async ({ roomAlphaName: name, app, user, bot }, use) => { + room1Name: "Room 1", + room1: async ({ room1Name: name, app, user, bot }, use) => { const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] }); await use({ name, roomId }); }, - roomBetaName: "Room Beta", - roomBeta: async ({ roomBetaName: name, app, user, bot }, use) => { + room2Name: "Room 2", + room2: async ({ room2Name: name, app, user, bot }, use) => { const roomId = await app.client.createRoom({ name, invite: [bot.credentials.userId] }); await use({ name, roomId }); }, msg: async ({ page, app, util }, use) => { await use(new MessageBuilder(page, app, util)); }, - util: async ({ roomAlpha, roomBeta, page, app, bot }, use) => { + util: async ({ room1, room2, page, app, bot }, use) => { await use(new Helpers(page, app, bot)); }, }); @@ -337,23 +337,27 @@ export class Helpers { * @param room1 * @param room2 * @param msg - MessageBuilder + * @param hasMention - whether to include a mention in the first message */ async populateThreads( room1: { name: string; roomId: string }, room2: { name: string; roomId: string }, msg: MessageBuilder, + hasMention = true, ) { - await this.receiveMessages(room2, [ - "Msg1", - msg.threadedOff("Msg1", { - "body": "User", - "format": "org.matrix.custom.html", - "formatted_body": "User", - "m.mentions": { - user_ids: ["@user:localhost"], - }, - }), - ]); + if (hasMention) { + await this.receiveMessages(room2, [ + "Msg1", + msg.threadedOff("Msg1", { + "body": "User", + "format": "org.matrix.custom.html", + "formatted_body": "User", + "m.mentions": { + user_ids: ["@user:localhost"], + }, + }), + ]); + } await this.receiveMessages(room2, ["Msg2", msg.threadedOff("Msg2", "Resp2")]); await this.receiveMessages(room1, ["Msg3", msg.threadedOff("Msg3", "Resp3")]); } diff --git a/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts b/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts index 065d7d82ec..93094073b3 100644 --- a/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts +++ b/playwright/e2e/spaces/threads-activity-centre/threadsActivityCentre.spec.ts @@ -35,7 +35,7 @@ test.describe("Threads Activity Centre", () => { await expect(util.getSpacePanel()).toMatchScreenshot("tac-button-expanded.png"); }); - test("should not show indicator when there is no thread", async ({ roomAlpha: room1, util }) => { + test("should not show indicator when there is no thread", async ({ room1, util }) => { // No indicator should be shown await util.assertNoTacIndicator(); @@ -46,11 +46,7 @@ test.describe("Threads Activity Centre", () => { await util.assertNoTacIndicator(); }); - test("should show a notification indicator when there is a message in a thread", async ({ - roomAlpha: room1, - util, - msg, - }) => { + test("should show a notification indicator when there is a message in a thread", async ({ room1, util, msg }) => { await util.goTo(room1); await util.receiveMessages(room1, ["Msg1", msg.threadedOff("Msg1", "Resp1")]); @@ -58,11 +54,7 @@ test.describe("Threads Activity Centre", () => { await util.assertNotificationTac(); }); - test("should show a highlight indicator when there is a mention in a thread", async ({ - roomAlpha: room1, - util, - msg, - }) => { + test("should show a highlight indicator when there is a mention in a thread", async ({ room1, util, msg }) => { await util.goTo(room1); await util.receiveMessages(room1, [ "Msg1", @@ -80,7 +72,7 @@ test.describe("Threads Activity Centre", () => { await util.assertHighlightIndicator(); }); - test("should show the rooms with unread threads", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => { + test("should show the rooms with unread threads", async ({ room1, room2, util, msg }) => { await util.goTo(room2); await util.populateThreads(room1, room2, msg); // The indicator should be shown @@ -97,7 +89,7 @@ test.describe("Threads Activity Centre", () => { await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-mix-unread.png"); }); - test("should update with a thread is read", async ({ roomAlpha: room1, roomBeta: room2, util, msg }) => { + test("should update with a thread is read", async ({ room1, room2, util, msg }) => { await util.goTo(room2); await util.populateThreads(room1, room2, msg); @@ -120,6 +112,17 @@ test.describe("Threads Activity Centre", () => { await expect(util.getTacPanel()).toMatchScreenshot("tac-panel-notification-unread.png"); }); + test("should order by recency after notification level", async ({ room1, room2, util, msg }) => { + await util.goTo(room2); + await util.populateThreads(room1, room2, msg, false); + + await util.openTac(); + await util.assertRoomsInTac([ + { room: room1.name, notificationLevel: "notification" }, + { room: room2.name, notificationLevel: "notification" }, + ]); + }); + test("should block the Spotlight to open when the TAC is opened", async ({ util, page }) => { const toggleSpotlight = () => page.keyboard.press(`${CommandOrControl}+k`); diff --git a/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-mix-unread-linux.png b/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-mix-unread-linux.png index f02a943d6b..ec5a8193d2 100644 Binary files a/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-mix-unread-linux.png and b/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-mix-unread-linux.png differ diff --git a/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-notification-unread-linux.png b/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-notification-unread-linux.png index 1a94524d68..f0f6cee3e6 100644 Binary files a/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-notification-unread-linux.png and b/playwright/snapshots/spaces/threads-activity-centre/threadsActivityCentre.spec.ts/tac-panel-notification-unread-linux.png differ diff --git a/src/components/views/spaces/threads-activity-centre/useUnreadThreadRooms.ts b/src/components/views/spaces/threads-activity-centre/useUnreadThreadRooms.ts index 115b208560..72b5380fbd 100644 --- a/src/components/views/spaces/threads-activity-centre/useUnreadThreadRooms.ts +++ b/src/components/views/spaces/threads-activity-centre/useUnreadThreadRooms.ts @@ -89,12 +89,12 @@ function computeUnreadThreadRooms(mxClient: MatrixClient, msc3946ProcessDynamicP const visibleRooms = mxClient.getVisibleRooms(msc3946ProcessDynamicPredecessor); let greatestNotificationLevel = NotificationLevel.None; - const rooms = []; + const rooms: Result["rooms"] = []; for (const room of visibleRooms) { // We only care about rooms with unread threads if (VisibilityProvider.instance.isRoomVisible(room) && doesRoomHaveUnreadThreads(room)) { - // Get the greatest notification level of all rooms + // Get the greatest notification level of all threads const notificationLevel = getThreadNotificationLevel(room); // If the room has an activity notification or less, we ignore it @@ -110,20 +110,35 @@ function computeUnreadThreadRooms(mxClient: MatrixClient, msc3946ProcessDynamicP } } - const sortedRooms = rooms.sort((a, b) => sortRoom(a.notificationLevel, b.notificationLevel)); + const sortedRooms = rooms.sort((a, b) => sortRoom(a, b)); return { greatestNotificationLevel, rooms: sortedRooms }; } +/** + * Store the room and its thread notification level + */ +type RoomData = Result["rooms"][0]; + /** * Sort notification level by the most important notification level to the least important * Highlight > Notification > Activity - * @param notificationLevelA - notification level of room A - * @param notificationLevelB - notification level of room B + * If the notification level is the same, we sort by the most recent thread + * @param roomDataA - room and notification level of room A + * @param roomDataB - room and notification level of room B * @returns {number} */ -function sortRoom(notificationLevelA: NotificationLevel, notificationLevelB: NotificationLevel): number { +function sortRoom(roomDataA: RoomData, roomDataB: RoomData): number { + const { notificationLevel: notificationLevelA, room: roomA } = roomDataA; + const { notificationLevel: notificationLevelB, room: roomB } = roomDataB; + + const timestampA = roomA.getLastThread()?.events.at(-1)?.getTs(); + const timestampB = roomB.getLastThread()?.events.at(-1)?.getTs(); + // NotificationLevel is a numeric enum, so we can compare them directly if (notificationLevelA > notificationLevelB) return -1; else if (notificationLevelB > notificationLevelA) return 1; - else return 0; + // Display most recent first + else if (!timestampA) return 1; + else if (!timestampB) return -1; + else return timestampB - timestampA; } diff --git a/test/components/views/spaces/ThreadsActivityCentre-test.tsx b/test/components/views/spaces/ThreadsActivityCentre-test.tsx index f5f183e7c6..8deb27ec7e 100644 --- a/test/components/views/spaces/ThreadsActivityCentre-test.tsx +++ b/test/components/views/spaces/ThreadsActivityCentre-test.tsx @@ -59,16 +59,23 @@ describe("ThreadsActivityCentre", () => { }); roomWithActivity.name = "Just activity"; - const roomWithNotif = new Room("!room:server", cli, cli.getSafeUserId(), { + const roomWithNotif = new Room("!room2:server", cli, cli.getSafeUserId(), { pendingEventOrdering: PendingEventOrdering.Detached, }); roomWithNotif.name = "A notification"; - const roomWithHighlight = new Room("!room:server", cli, cli.getSafeUserId(), { + const roomWithHighlight = new Room("!room3:server", cli, cli.getSafeUserId(), { pendingEventOrdering: PendingEventOrdering.Detached, }); roomWithHighlight.name = "This is a real highlight"; + const getDefaultThreadArgs = (room: Room) => ({ + room: room, + client: cli, + authorId: "@foo:bar", + participantUserIds: ["@fee:bar"], + }); + beforeAll(async () => { jest.spyOn(MatrixClientPeg, "get").mockReturnValue(cli); jest.spyOn(MatrixClientPeg, "safeGet").mockReturnValue(cli); @@ -77,26 +84,15 @@ describe("ThreadsActivityCentre", () => { jest.spyOn(dmRoomMap, "getUserIdForRoomId"); jest.spyOn(DMRoomMap, "shared").mockReturnValue(dmRoomMap); - await populateThread({ - room: roomWithActivity, - client: cli, - authorId: "@foo:bar", - participantUserIds: ["@fee:bar"], - }); + await populateThread(getDefaultThreadArgs(roomWithActivity)); - const notifThreadInfo = await populateThread({ - room: roomWithNotif, - client: cli, - authorId: "@foo:bar", - participantUserIds: ["@fee:bar"], - }); + const notifThreadInfo = await populateThread(getDefaultThreadArgs(roomWithNotif)); roomWithNotif.setThreadUnreadNotificationCount(notifThreadInfo.thread.id, NotificationCountType.Total, 1); const highlightThreadInfo = await populateThread({ - room: roomWithHighlight, - client: cli, - authorId: "@foo:bar", - participantUserIds: ["@fee:bar"], + ...getDefaultThreadArgs(roomWithHighlight), + // timestamp + ts: 5, }); roomWithHighlight.setThreadUnreadNotificationCount( highlightThreadInfo.thread.id, @@ -181,6 +177,52 @@ describe("ThreadsActivityCentre", () => { expect(screen.getByRole("menu")).toMatchSnapshot(); }); + it("should order the room with the same notification level by most recent", async () => { + // Generate two new rooms with threads + const secondRoomWithHighlight = new Room("!room4:server", cli, cli.getSafeUserId(), { + pendingEventOrdering: PendingEventOrdering.Detached, + }); + secondRoomWithHighlight.name = "This is a second real highlight"; + + const secondHighlightThreadInfo = await populateThread({ + ...getDefaultThreadArgs(secondRoomWithHighlight), + // timestamp + ts: 1, + }); + secondRoomWithHighlight.setThreadUnreadNotificationCount( + secondHighlightThreadInfo.thread.id, + NotificationCountType.Highlight, + 1, + ); + + const thirdRoomWithHighlight = new Room("!room5:server", cli, cli.getSafeUserId(), { + pendingEventOrdering: PendingEventOrdering.Detached, + }); + thirdRoomWithHighlight.name = "This is a third real highlight"; + + const thirdHighlightThreadInfo = await populateThread({ + ...getDefaultThreadArgs(thirdRoomWithHighlight), + // timestamp + ts: 7, + }); + thirdRoomWithHighlight.setThreadUnreadNotificationCount( + thirdHighlightThreadInfo.thread.id, + NotificationCountType.Highlight, + 1, + ); + + cli.getVisibleRooms = jest + .fn() + .mockReturnValue([roomWithHighlight, secondRoomWithHighlight, thirdRoomWithHighlight]); + + renderTAC(); + await userEvent.click(getTACButton()); + + // The room should be ordered by the most recent thread + // thirdHighlightThreadInfo (timestamp 7) > highlightThreadInfo (timestamp 5) > secondHighlightThreadInfo (timestamp 1) + expect(screen.getByRole("menu")).toMatchSnapshot(); + }); + it("should block Ctrl/CMD + k shortcut", async () => { cli.getVisibleRooms = jest.fn().mockReturnValue([roomWithHighlight]); diff --git a/test/components/views/spaces/__snapshots__/ThreadsActivityCentre-test.tsx.snap b/test/components/views/spaces/__snapshots__/ThreadsActivityCentre-test.tsx.snap index 5b9e2091b7..0d2841c614 100644 --- a/test/components/views/spaces/__snapshots__/ThreadsActivityCentre-test.tsx.snap +++ b/test/components/views/spaces/__snapshots__/ThreadsActivityCentre-test.tsx.snap @@ -38,7 +38,7 @@ exports[`ThreadsActivityCentre renders notifications matching the snapshot 1`] = > `; + +exports[`ThreadsActivityCentre should order the room with the same notification level by most recent 1`] = ` + +`;