From 511fbd76ec1ac1a71b0f92f0671aeaecbdb015e2 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 12 Dec 2022 09:34:05 -0500 Subject: [PATCH 1/9] Display rooms as unread (bold) if threads have unread messages. This re-applies the changes from 9de5654353a9209ff85cdaea5832043822af5e4c (#9723) which were reverted in 9668a24ca705a3d38c240174e04e0d66abe8953d (#9745). --- src/Unread.ts | 45 +++--- test/Unread-test.ts | 354 +++++++++++++++++++++++++++++++++++--------- 2 files changed, 302 insertions(+), 97 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 17fe76f03f9..4f38c8e76ab 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -15,6 +15,7 @@ limitations under the License. */ import { Room } from "matrix-js-sdk/src/models/room"; +import { Thread } from "matrix-js-sdk/src/models/thread"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { EventType } from "matrix-js-sdk/src/@types/event"; import { M_BEACON } from "matrix-js-sdk/src/@types/beacon"; @@ -59,36 +60,34 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { return false; } + for (const timeline of [room, ...room.getThreads()]) { + // If the current timeline has unread messages, we're done. + if (doesRoomOrThreadHaveUnreadMessages(timeline)) { + return true; + } + } + // If we got here then no timelines were found with unread messages. + return false; +} + +function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { const myUserId = MatrixClientPeg.get().getUserId(); + // as we don't send RRs for our own messages, make sure we special case that + // if *we* sent the last message into the room, we consider it not unread! + // Should fix: https://github.com/vector-im/element-web/issues/3263 + // https://github.com/vector-im/element-web/issues/2427 + // ...and possibly some of the others at + // https://github.com/vector-im/element-web/issues/3363 + if (room.timeline.at(-1)?.getSender() === myUserId) { + return false; + } + // get the most recent read receipt sent by our account. // N.B. this is NOT a read marker (RM, aka "read up to marker"), // despite the name of the method :(( const readUpToId = room.getEventReadUpTo(myUserId!); - if (!SettingsStore.getValue("feature_threadstable")) { - // as we don't send RRs for our own messages, make sure we special case that - // if *we* sent the last message into the room, we consider it not unread! - // Should fix: https://github.com/vector-im/element-web/issues/3263 - // https://github.com/vector-im/element-web/issues/2427 - // ...and possibly some of the others at - // https://github.com/vector-im/element-web/issues/3363 - if (room.timeline.length && room.timeline[room.timeline.length - 1].getSender() === myUserId) { - return false; - } - } - - // if the read receipt relates to an event is that part of a thread - // we consider that there are no unread messages - // This might be a false negative, but probably the best we can do until - // the read receipts have evolved to cater for threads - if (readUpToId) { - const event = room.findEventById(readUpToId); - if (event?.getThread()) { - return false; - } - } - // this just looks at whatever history we have, which if we've only just started // up probably won't be very much, so if the last couple of events are ones that // don't count, we don't know if there are any events that do count between where diff --git a/test/Unread-test.ts b/test/Unread-test.ts index 7a271354de1..8ff759b142b 100644 --- a/test/Unread-test.ts +++ b/test/Unread-test.ts @@ -15,100 +15,306 @@ limitations under the License. */ import { mocked } from "jest-mock"; -import { MatrixEvent, EventType, MsgType } from "matrix-js-sdk/src/matrix"; +import { MatrixEvent, EventType, MsgType, Room } from "matrix-js-sdk/src/matrix"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import { haveRendererForEvent } from "../src/events/EventTileFactory"; -import { getMockClientWithEventEmitter, makeBeaconEvent, mockClientMethodsUser } from "./test-utils"; -import { eventTriggersUnreadCount } from "../src/Unread"; +import { makeBeaconEvent, mkEvent, stubClient } from "./test-utils"; +import { mkThread } from "./test-utils/threads"; +import { doesRoomHaveUnreadMessages, eventTriggersUnreadCount } from "../src/Unread"; +import { MatrixClientPeg } from "../src/MatrixClientPeg"; jest.mock("../src/events/EventTileFactory", () => ({ haveRendererForEvent: jest.fn(), })); -describe("eventTriggersUnreadCount()", () => { +describe("Unread", () => { + // A different user. const aliceId = "@alice:server.org"; - const bobId = "@bob:server.org"; + stubClient(); + const client = MatrixClientPeg.get(); - // mock user credentials - getMockClientWithEventEmitter({ - ...mockClientMethodsUser(bobId), - }); + describe("eventTriggersUnreadCount()", () => { + // setup events + const alicesMessage = new MatrixEvent({ + type: EventType.RoomMessage, + sender: aliceId, + content: { + msgtype: MsgType.Text, + body: "Hello from Alice", + }, + }); - // setup events - const alicesMessage = new MatrixEvent({ - type: EventType.RoomMessage, - sender: aliceId, - content: { - msgtype: MsgType.Text, - body: "Hello from Alice", - }, - }); + const ourMessage = new MatrixEvent({ + type: EventType.RoomMessage, + sender: client.getUserId()!, + content: { + msgtype: MsgType.Text, + body: "Hello from Bob", + }, + }); - const bobsMessage = new MatrixEvent({ - type: EventType.RoomMessage, - sender: bobId, - content: { - msgtype: MsgType.Text, - body: "Hello from Bob", - }, - }); + const redactedEvent = new MatrixEvent({ + type: EventType.RoomMessage, + sender: aliceId, + }); + redactedEvent.makeRedacted(redactedEvent); - const redactedEvent = new MatrixEvent({ - type: EventType.RoomMessage, - sender: aliceId, - }); - redactedEvent.makeRedacted(redactedEvent); + beforeEach(() => { + jest.clearAllMocks(); + mocked(haveRendererForEvent).mockClear().mockReturnValue(false); + }); - beforeEach(() => { - jest.clearAllMocks(); - mocked(haveRendererForEvent).mockClear().mockReturnValue(false); - }); + it("returns false when the event was sent by the current user", () => { + expect(eventTriggersUnreadCount(ourMessage)).toBe(false); + // returned early before checking renderer + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); - it("returns false when the event was sent by the current user", () => { - expect(eventTriggersUnreadCount(bobsMessage)).toBe(false); - // returned early before checking renderer - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }); + it("returns false for a redacted event", () => { + expect(eventTriggersUnreadCount(redactedEvent)).toBe(false); + // returned early before checking renderer + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); - it("returns false for a redacted event", () => { - expect(eventTriggersUnreadCount(redactedEvent)).toBe(false); - // returned early before checking renderer - expect(haveRendererForEvent).not.toHaveBeenCalled(); - }); + it("returns false for an event without a renderer", () => { + mocked(haveRendererForEvent).mockReturnValue(false); + expect(eventTriggersUnreadCount(alicesMessage)).toBe(false); + expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); + }); - it("returns false for an event without a renderer", () => { - mocked(haveRendererForEvent).mockReturnValue(false); - expect(eventTriggersUnreadCount(alicesMessage)).toBe(false); - expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); - }); + it("returns true for an event with a renderer", () => { + mocked(haveRendererForEvent).mockReturnValue(true); + expect(eventTriggersUnreadCount(alicesMessage)).toBe(true); + expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); + }); - it("returns true for an event with a renderer", () => { - mocked(haveRendererForEvent).mockReturnValue(true); - expect(eventTriggersUnreadCount(alicesMessage)).toBe(true); - expect(haveRendererForEvent).toHaveBeenCalledWith(alicesMessage, false); - }); + it("returns false for beacon locations", () => { + const beaconLocationEvent = makeBeaconEvent(aliceId); + expect(eventTriggersUnreadCount(beaconLocationEvent)).toBe(false); + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }); + + const noUnreadEventTypes = [ + EventType.RoomMember, + EventType.RoomThirdPartyInvite, + EventType.CallAnswer, + EventType.CallHangup, + EventType.RoomCanonicalAlias, + EventType.RoomServerAcl, + ]; - it("returns false for beacon locations", () => { - const beaconLocationEvent = makeBeaconEvent(aliceId); - expect(eventTriggersUnreadCount(beaconLocationEvent)).toBe(false); - expect(haveRendererForEvent).not.toHaveBeenCalled(); + it.each(noUnreadEventTypes)( + "returns false without checking for renderer for events with type %s", + (eventType) => { + const event = new MatrixEvent({ + type: eventType, + sender: aliceId, + }); + expect(eventTriggersUnreadCount(event)).toBe(false); + expect(haveRendererForEvent).not.toHaveBeenCalled(); + }, + ); }); - const noUnreadEventTypes = [ - EventType.RoomMember, - EventType.RoomThirdPartyInvite, - EventType.CallAnswer, - EventType.CallHangup, - EventType.RoomCanonicalAlias, - EventType.RoomServerAcl, - ]; - - it.each(noUnreadEventTypes)("returns false without checking for renderer for events with type %s", (eventType) => { - const event = new MatrixEvent({ - type: eventType, - sender: aliceId, + describe("doesRoomHaveUnreadMessages()", () => { + let room: Room; + let event: MatrixEvent; + const roomId = "!abc:server.org"; + const myId = client.getUserId()!; + + beforeAll(() => { + client.supportsExperimentalThreads = () => true; + }); + + beforeEach(() => { + // Create a room and initial event in it. + room = new Room(roomId, client, myId); + event = mkEvent({ + event: true, + type: "m.room.message", + user: aliceId, + room: roomId, + content: {}, + }); + room.addLiveEvents([event]); + + // Don't care about the code path of hidden events. + mocked(haveRendererForEvent).mockClear().mockReturnValue(true); + }); + + it("returns true for a room with no receipts", () => { + expect(doesRoomHaveUnreadMessages(room)).toBe(true); + }); + + it("returns false for a room when the latest event was sent by the current user", () => { + event = mkEvent({ + event: true, + type: "m.room.message", + user: myId, + room: roomId, + content: {}, + }); + // Only for timeline events. + room.addLiveEvents([event]); + + expect(doesRoomHaveUnreadMessages(room)).toBe(false); + }); + + it("returns false for a room when the read receipt is at the latest event", () => { + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + expect(doesRoomHaveUnreadMessages(room)).toBe(false); + }); + + it("returns true for a room when the read receipt is earlier than the latest event", () => { + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + const event2 = mkEvent({ + event: true, + type: "m.room.message", + user: aliceId, + room: roomId, + content: {}, + }); + // Only for timeline events. + room.addLiveEvents([event2]); + + expect(doesRoomHaveUnreadMessages(room)).toBe(true); + }); + + it("returns true for a room with an unread message in a thread", () => { + // Mark the main timeline as read. + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + // Create a thread as a different user. + mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); + + expect(doesRoomHaveUnreadMessages(room)).toBe(true); + }); + + it("returns false for a room when the latest thread event was sent by the current user", () => { + // Mark the main timeline as read. + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + // Create a thread as the current user. + mkThread({ room, client, authorId: myId, participantUserIds: [myId] }); + + expect(doesRoomHaveUnreadMessages(room)).toBe(false); + }); + + it("returns false for a room with read thread messages", () => { + // Mark the main timeline as read. + let receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + // Create threads. + const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); + + // Mark the thread as read. + receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [events[events.length - 1].getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1, thread_id: rootEvent.getId()! }, + }, + }, + }, + }); + room.addReceipt(receipt); + + expect(doesRoomHaveUnreadMessages(room)).toBe(false); + }); + + it("returns true for a room when read receipt is not on the latest thread messages", () => { + // Mark the main timeline as read. + let receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + + // Create threads. + const { rootEvent, events } = mkThread({ room, client, authorId: myId, participantUserIds: [aliceId] }); + + // Mark the thread as read. + receipt = new MatrixEvent({ + type: "m.receipt", + room_id: "!foo:bar", + content: { + [events[0].getId()!]: { + [ReceiptType.Read]: { + [myId]: { ts: 1, threadId: rootEvent.getId()! }, + }, + }, + }, + }); + room.addReceipt(receipt); + + expect(doesRoomHaveUnreadMessages(room)).toBe(true); }); - expect(eventTriggersUnreadCount(event)).toBe(false); - expect(haveRendererForEvent).not.toHaveBeenCalled(); }); }); From ad4feefefbd08feb1d00165d95bde1ece8ddb964 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 13 Dec 2022 14:30:58 -0500 Subject: [PATCH 2/9] Display threads in the threads list as unread (bold). --- src/Unread.ts | 2 +- src/hooks/useUnreadNotifications.ts | 13 +++-- .../components/views/rooms/EventTile-test.tsx | 3 +- .../UnreadNotificationBadge-test.tsx | 58 ++++++++++++++++--- 4 files changed, 60 insertions(+), 16 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 4f38c8e76ab..268b2a4a7fa 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -70,7 +70,7 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { return false; } -function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { +export function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { const myUserId = MatrixClientPeg.get().getUserId(); // as we don't send RRs for our own messages, make sure we special case that diff --git a/src/hooks/useUnreadNotifications.ts b/src/hooks/useUnreadNotifications.ts index bca2b0c2d42..22236d832f9 100644 --- a/src/hooks/useUnreadNotifications.ts +++ b/src/hooks/useUnreadNotifications.ts @@ -15,12 +15,13 @@ limitations under the License. */ import { NotificationCount, NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room"; +import { Thread } from "matrix-js-sdk/src/models/thread"; import { useCallback, useEffect, useState } from "react"; import { getUnsentMessages } from "../components/structures/RoomStatusBar"; import { getRoomNotifsState, getUnreadNotificationCount, RoomNotifState } from "../RoomNotifs"; import { NotificationColor } from "../stores/notifications/NotificationColor"; -import { doesRoomHaveUnreadMessages } from "../Unread"; +import { doesRoomOrThreadHaveUnreadMessages } from "../Unread"; import { EffectiveMembership, getEffectiveMembership } from "../utils/membership"; import { useEventEmitter } from "./useEventEmitter"; @@ -75,12 +76,14 @@ export const useUnreadNotifications = ( setColor(NotificationColor.Red); } else if (greyNotifs > 0) { setColor(NotificationColor.Grey); - } else if (!threadId) { - // TODO: No support for `Bold` on threads at the moment - + } else { // We don't have any notified messages, but we might have unread messages. Let's // find out. - const hasUnread = doesRoomHaveUnreadMessages(room); + let roomOrThread: Room | Thread = room; + if (threadId) { + roomOrThread = room.getThread(threadId)!; + } + const hasUnread = doesRoomOrThreadHaveUnreadMessages(roomOrThread); setColor(hasUnread ? NotificationColor.Bold : NotificationColor.None); } } diff --git a/test/components/views/rooms/EventTile-test.tsx b/test/components/views/rooms/EventTile-test.tsx index bebec2efc84..26be4a360e7 100644 --- a/test/components/views/rooms/EventTile-test.tsx +++ b/test/components/views/rooms/EventTile-test.tsx @@ -137,7 +137,8 @@ describe("EventTile", () => { it("shows an unread notification bage", () => { const { container } = getComponent({}, TimelineRenderingType.ThreadsList); - expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(0); + // By default, the thread will assume there's unread activity in it. + expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(1); act(() => { room.setThreadUnreadNotificationCount(mxEvent.getId(), NotificationCountType.Total, 3); diff --git a/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx b/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx index acffe31ff3e..58f552f283c 100644 --- a/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx +++ b/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx @@ -17,13 +17,14 @@ limitations under the License. import React from "react"; import "jest-mock"; import { screen, act, render } from "@testing-library/react"; -import { MatrixClient, PendingEventOrdering } from "matrix-js-sdk/src/client"; +import { MsgType, RelationType } from "matrix-js-sdk/src/matrix"; +import { PendingEventOrdering } from "matrix-js-sdk/src/client"; import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room"; -import { mocked } from "jest-mock"; import { EventStatus } from "matrix-js-sdk/src/models/event-status"; +import { mkThread } from "../../../../test-utils/threads"; import { UnreadNotificationBadge } from "../../../../../src/components/views/rooms/NotificationBadge/UnreadNotificationBadge"; -import { mkMessage, stubClient } from "../../../../test-utils/test-utils"; +import { mkEvent, mkMessage, stubClient } from "../../../../test-utils/test-utils"; import { MatrixClientPeg } from "../../../../../src/MatrixClientPeg"; import * as RoomNotifs from "../../../../../src/RoomNotifs"; @@ -34,28 +35,38 @@ jest.mock("../../../../../src/RoomNotifs", () => ({ })); const ROOM_ID = "!roomId:example.org"; -let THREAD_ID; +let THREAD_ID: string; describe("UnreadNotificationBadge", () => { - let mockClient: MatrixClient; + stubClient(); + const client = MatrixClientPeg.get(); let room: Room; function getComponent(threadId?: string) { return ; } + beforeAll(() => { + client.supportsExperimentalThreads = () => true; + }); + beforeEach(() => { jest.clearAllMocks(); - stubClient(); - mockClient = mocked(MatrixClientPeg.get()); - - room = new Room(ROOM_ID, mockClient, mockClient.getUserId() ?? "", { + room = new Room(ROOM_ID, client, client.getUserId()!, { pendingEventOrdering: PendingEventOrdering.Detached, }); room.setUnreadNotificationCount(NotificationCountType.Total, 1); room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); + const { rootEvent } = mkThread({ + room, + client, + authorId: client.getUserId()!, + participantUserIds: [client.getUserId()!], + }); + THREAD_ID = rootEvent.getId()!; + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 1); room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0); @@ -125,4 +136,33 @@ describe("UnreadNotificationBadge", () => { const { container } = render(getComponent()); expect(container.querySelector(".mx_NotificationBadge")).toBeNull(); }); + + it("activity renders unread notification badge", () => { + act(() => { + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Total, 0); + room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 0); + + // Add another event on the thread which is not sent by us. + const event = mkEvent({ + event: true, + type: "m.room.message", + user: "@alice:server.org", + room: room.roomId, + content: { + "msgtype": MsgType.Text, + "body": "Hello from Bob", + "m.relates_to": { + event_id: THREAD_ID, + rel_type: RelationType.Thread, + }, + }, + }); + room.addLiveEvents([event]); + }); + + const { container } = render(getComponent(THREAD_ID)); + expect(container.querySelector(".mx_NotificationBadge_dot")).toBeTruthy(); + expect(container.querySelector(".mx_NotificationBadge_visible")).toBeTruthy(); + expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeFalsy(); + }); }); From 82fdbacba9aa47d2d596cc751520845ca86639a1 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 13 Dec 2022 14:35:53 -0500 Subject: [PATCH 3/9] Add a badge to the threads icon if any threads are unread. --- .../views/right_panel/RoomHeaderButtons.tsx | 31 ++++++- .../right_panel/RoomHeaderButtons-test.tsx | 89 ++++++++++++++++++- 2 files changed, 115 insertions(+), 5 deletions(-) diff --git a/src/components/views/right_panel/RoomHeaderButtons.tsx b/src/components/views/right_panel/RoomHeaderButtons.tsx index d7f0af13b6e..558f64c8e6d 100644 --- a/src/components/views/right_panel/RoomHeaderButtons.tsx +++ b/src/components/views/right_panel/RoomHeaderButtons.tsx @@ -21,6 +21,7 @@ limitations under the License. import React from "react"; import classNames from "classnames"; import { NotificationCountType, Room, RoomEvent } from "matrix-js-sdk/src/models/room"; +import { ThreadEvent } from "matrix-js-sdk/src/models/thread"; import { Feature, ServerSupport } from "matrix-js-sdk/src/feature"; import { _t } from "../../../languageHandler"; @@ -44,6 +45,7 @@ import { NotificationStateEvents } from "../../../stores/notifications/Notificat import PosthogTrackers from "../../../PosthogTrackers"; import { ButtonEvent } from "../elements/AccessibleButton"; import { MatrixClientPeg } from "../../../MatrixClientPeg"; +import { doesRoomOrThreadHaveUnreadMessages } from "../../../Unread"; const ROOM_INFO_PHASES = [ RightPanelPhases.RoomSummary, @@ -154,7 +156,17 @@ export default class RoomHeaderButtons extends HeaderButtons { if (!this.supportsThreadNotifications) { this.threadNotificationState?.on(NotificationStateEvents.Update, this.onNotificationUpdate); } else { + // Notification badge may change if the notification counts from the + // server change, if a new thread is created or updated, or if a + // receipt is sent in the thread. this.props.room?.on(RoomEvent.UnreadNotifications, this.onNotificationUpdate); + this.props.room?.on(RoomEvent.Receipt, this.onNotificationUpdate); + this.props.room?.on(RoomEvent.Timeline, this.onNotificationUpdate); + this.props.room?.on(RoomEvent.Redaction, this.onNotificationUpdate); + this.props.room?.on(RoomEvent.LocalEchoUpdated, this.onNotificationUpdate); + this.props.room?.on(RoomEvent.MyMembership, this.onNotificationUpdate); + this.props.room?.on(ThreadEvent.New, this.onNotificationUpdate); + this.props.room?.on(ThreadEvent.Update, this.onNotificationUpdate); } this.onNotificationUpdate(); RoomNotificationStateStore.instance.on(UPDATE_STATUS_INDICATOR, this.onUpdateStatus); @@ -166,6 +178,13 @@ export default class RoomHeaderButtons extends HeaderButtons { this.threadNotificationState?.off(NotificationStateEvents.Update, this.onNotificationUpdate); } else { this.props.room?.off(RoomEvent.UnreadNotifications, this.onNotificationUpdate); + this.props.room?.off(RoomEvent.Receipt, this.onNotificationUpdate); + this.props.room?.off(RoomEvent.Timeline, this.onNotificationUpdate); + this.props.room?.off(RoomEvent.Redaction, this.onNotificationUpdate); + this.props.room?.off(RoomEvent.LocalEchoUpdated, this.onNotificationUpdate); + this.props.room?.off(RoomEvent.MyMembership, this.onNotificationUpdate); + this.props.room?.off(ThreadEvent.New, this.onNotificationUpdate); + this.props.room?.off(ThreadEvent.Update, this.onNotificationUpdate); } RoomNotificationStateStore.instance.off(UPDATE_STATUS_INDICATOR, this.onUpdateStatus); } @@ -191,9 +210,17 @@ export default class RoomHeaderButtons extends HeaderButtons { return NotificationColor.Red; case NotificationCountType.Total: return NotificationColor.Grey; - default: - return NotificationColor.None; } + // We don't have any notified messages, but we might have unread messages. Let's + // find out. + for (const thread of this.props.room!.getThreads()) { + // If the current thread has unread messages, we're done. + if (doesRoomOrThreadHaveUnreadMessages(thread)) { + return NotificationColor.Bold; + } + } + // Otherwise, no notification color. + return NotificationColor.None; } private onUpdateStatus = (notificationState: SummarizedNotificationState): void => { diff --git a/test/components/views/right_panel/RoomHeaderButtons-test.tsx b/test/components/views/right_panel/RoomHeaderButtons-test.tsx index f9a3572aa82..5c860acbd2a 100644 --- a/test/components/views/right_panel/RoomHeaderButtons-test.tsx +++ b/test/components/views/right_panel/RoomHeaderButtons-test.tsx @@ -15,15 +15,18 @@ limitations under the License. */ import { render } from "@testing-library/react"; +import { MatrixEvent, MsgType, RelationType } from "matrix-js-sdk/src/matrix"; import { MatrixClient, PendingEventOrdering } from "matrix-js-sdk/src/client"; import { Feature, ServerSupport } from "matrix-js-sdk/src/feature"; import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import React from "react"; import RoomHeaderButtons from "../../../../src/components/views/right_panel/RoomHeaderButtons"; import { MatrixClientPeg } from "../../../../src/MatrixClientPeg"; import SettingsStore from "../../../../src/settings/SettingsStore"; -import { stubClient } from "../../../test-utils"; +import { mkEvent, stubClient } from "../../../test-utils"; +import { mkThread } from "../../../test-utils/threads"; describe("RoomHeaderButtons-test.tsx", function () { const ROOM_ID = "!roomId:example.org"; @@ -35,6 +38,7 @@ describe("RoomHeaderButtons-test.tsx", function () { stubClient(); client = MatrixClientPeg.get(); + client.supportsExperimentalThreads = () => true; room = new Room(ROOM_ID, client, client.getUserId() ?? "", { pendingEventOrdering: PendingEventOrdering.Detached, }); @@ -52,7 +56,7 @@ describe("RoomHeaderButtons-test.tsx", function () { return container.querySelector(".mx_RightPanel_threadsButton"); } - function isIndicatorOfType(container, type: "red" | "gray") { + function isIndicatorOfType(container, type: "red" | "gray" | "bold") { return container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator").className.includes(type); } @@ -76,7 +80,7 @@ describe("RoomHeaderButtons-test.tsx", function () { expect(container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator")).toBeNull(); }); - it("room wide notification does not change the thread button", () => { + it("thread notification does change the thread button", () => { const { container } = getComponent(room); room.setThreadUnreadNotificationCount("$123", NotificationCountType.Total, 1); @@ -91,6 +95,85 @@ describe("RoomHeaderButtons-test.tsx", function () { expect(container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator")).toBeNull(); }); + it("thread activity does change the thread button", async () => { + const { container } = getComponent(room); + + // Thread activity should appear on the icon. + const { rootEvent, events } = mkThread({ + room, + client, + authorId: client.getUserId()!, + participantUserIds: ["@alice:example.org"], + }); + expect(isIndicatorOfType(container, "bold")).toBe(true); + + // Sending the last event should clear the notification. + let event = mkEvent({ + event: true, + type: "m.room.message", + user: client.getUserId()!, + room: room.roomId, + content: { + "msgtype": MsgType.Text, + "body": "Test", + "m.relates_to": { + event_id: rootEvent.getId(), + rel_type: RelationType.Thread, + }, + }, + }); + room.addLiveEvents([event]); + await expect(container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator")).toBeNull(); + + // Mark it as unread again. + event = mkEvent({ + event: true, + type: "m.room.message", + user: "@alice:example.org", + room: room.roomId, + content: { + "msgtype": MsgType.Text, + "body": "Test", + "m.relates_to": { + event_id: rootEvent.getId(), + rel_type: RelationType.Thread, + }, + }, + }); + room.addLiveEvents([event]); + expect(isIndicatorOfType(container, "bold")).toBe(true); + + // Sending a read receipt on an earlier event shouldn't do anything. + let receipt = new MatrixEvent({ + type: "m.receipt", + room_id: room.roomId, + content: { + [events.at(-1).getId()!]: { + [ReceiptType.Read]: { + [client.getUserId()!]: { ts: 1, thread_id: rootEvent.getId() }, + }, + }, + }, + }); + room.addReceipt(receipt); + expect(isIndicatorOfType(container, "bold")).toBe(true); + + // Sending a receipt on the latest event should clear the notification. + receipt = new MatrixEvent({ + type: "m.receipt", + room_id: room.roomId, + content: { + [event.getId()!]: { + [ReceiptType.Read]: { + [client.getUserId()!]: { ts: 1, thread_id: rootEvent.getId() }, + }, + }, + }, + }); + room.addReceipt(receipt); + expect(container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator")).toBeNull(); + }); + it("does not explode without a room", () => { client.canSupport.set(Feature.ThreadUnreadNotifications, ServerSupport.Unsupported); expect(() => getComponent()).not.toThrow(); From b8a42cabe2e4ca06ebdd4b871bd755aa63f8ab5e Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 Jan 2023 14:05:30 -0500 Subject: [PATCH 4/9] Add an extra check for uninitialized timelines. --- src/Unread.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/Unread.ts b/src/Unread.ts index 268b2a4a7fa..6c917e790a8 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -71,6 +71,12 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { } export function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { + // If there are no messages yet in the timeline then it isn't fully initialised + // and cannot be unread. + if (room.timeline.length === 0) { + return false; + } + const myUserId = MatrixClientPeg.get().getUserId(); // as we don't send RRs for our own messages, make sure we special case that From 2daf604b824b6d68261496e95544839b38ce0701 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 9 Jan 2023 14:26:29 -0500 Subject: [PATCH 5/9] Attempt to appease linter. --- test/components/views/right_panel/RoomHeaderButtons-test.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/components/views/right_panel/RoomHeaderButtons-test.tsx b/test/components/views/right_panel/RoomHeaderButtons-test.tsx index 5c860acbd2a..d57c361d071 100644 --- a/test/components/views/right_panel/RoomHeaderButtons-test.tsx +++ b/test/components/views/right_panel/RoomHeaderButtons-test.tsx @@ -56,7 +56,7 @@ describe("RoomHeaderButtons-test.tsx", function () { return container.querySelector(".mx_RightPanel_threadsButton"); } - function isIndicatorOfType(container, type: "red" | "gray" | "bold") { + function isIndicatorOfType(container: React.ReactElement, type: "red" | "gray" | "bold") { return container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator").className.includes(type); } @@ -148,7 +148,7 @@ describe("RoomHeaderButtons-test.tsx", function () { type: "m.receipt", room_id: room.roomId, content: { - [events.at(-1).getId()!]: { + [events.at(-1)!.getId()!]: { [ReceiptType.Read]: { [client.getUserId()!]: { ts: 1, thread_id: rootEvent.getId() }, }, From 8e7dc94a2622f007115fb52bd8fd9ad13303d782 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 10 Jan 2023 08:17:48 -0500 Subject: [PATCH 6/9] More linting fixes. --- .../components/views/right_panel/RoomHeaderButtons-test.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/components/views/right_panel/RoomHeaderButtons-test.tsx b/test/components/views/right_panel/RoomHeaderButtons-test.tsx index d57c361d071..7d898904d8c 100644 --- a/test/components/views/right_panel/RoomHeaderButtons-test.tsx +++ b/test/components/views/right_panel/RoomHeaderButtons-test.tsx @@ -52,12 +52,12 @@ describe("RoomHeaderButtons-test.tsx", function () { return render(); } - function getThreadButton(container) { + function getThreadButton(container: HTMLElement) { return container.querySelector(".mx_RightPanel_threadsButton"); } - function isIndicatorOfType(container: React.ReactElement, type: "red" | "gray" | "bold") { - return container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator").className.includes(type); + function isIndicatorOfType(container: HTMLElement, type: "red" | "gray" | "bold") { + return container.querySelector(".mx_RightPanel_threadsButton .mx_Indicator")!.className.includes(type); } it("shows the thread button", () => { From 22afd725ad2933e1da8f6f3abfee64ef23380677 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 10 Jan 2023 08:22:55 -0500 Subject: [PATCH 7/9] Rename variable. --- src/Unread.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 6c917e790a8..5c1df27aaeb 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -70,10 +70,10 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { return false; } -export function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean { +export function doesRoomOrThreadHaveUnreadMessages(roomOrThread: Room | Thread): boolean { // If there are no messages yet in the timeline then it isn't fully initialised // and cannot be unread. - if (room.timeline.length === 0) { + if (roomOrThread.timeline.length === 0) { return false; } @@ -85,14 +85,14 @@ export function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean // https://github.com/vector-im/element-web/issues/2427 // ...and possibly some of the others at // https://github.com/vector-im/element-web/issues/3363 - if (room.timeline.at(-1)?.getSender() === myUserId) { + if (roomOrThread.timeline.at(-1)?.getSender() === myUserId) { return false; } // get the most recent read receipt sent by our account. // N.B. this is NOT a read marker (RM, aka "read up to marker"), // despite the name of the method :(( - const readUpToId = room.getEventReadUpTo(myUserId!); + const readUpToId = roomOrThread.getEventReadUpTo(myUserId!); // this just looks at whatever history we have, which if we've only just started // up probably won't be very much, so if the last couple of events are ones that @@ -101,8 +101,8 @@ export function doesRoomOrThreadHaveUnreadMessages(room: Room | Thread): boolean // but currently we just guess. // Loop through messages, starting with the most recent... - for (let i = room.timeline.length - 1; i >= 0; --i) { - const ev = room.timeline[i]; + for (let i = roomOrThread.timeline.length - 1; i >= 0; --i) { + const ev = roomOrThread.timeline[i]; if (ev.getId() == readUpToId) { // If we've read up to this event, there's nothing more recent From 5ca3fe15fbd54667d4dcf997a5e07645e9eff941 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 10 Jan 2023 08:53:08 -0500 Subject: [PATCH 8/9] Fix tests. --- src/Unread.ts | 2 +- test/components/views/rooms/EventTile-test.tsx | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Unread.ts b/src/Unread.ts index 5c1df27aaeb..cbd30b2bb8e 100644 --- a/src/Unread.ts +++ b/src/Unread.ts @@ -73,7 +73,7 @@ export function doesRoomHaveUnreadMessages(room: Room): boolean { export function doesRoomOrThreadHaveUnreadMessages(roomOrThread: Room | Thread): boolean { // If there are no messages yet in the timeline then it isn't fully initialised // and cannot be unread. - if (roomOrThread.timeline.length === 0) { + if (!roomOrThread || roomOrThread.timeline.length === 0) { return false; } diff --git a/test/components/views/rooms/EventTile-test.tsx b/test/components/views/rooms/EventTile-test.tsx index 2893aeac644..7c46f6aa3a5 100644 --- a/test/components/views/rooms/EventTile-test.tsx +++ b/test/components/views/rooms/EventTile-test.tsx @@ -141,11 +141,11 @@ describe("EventTile", () => { mxEvent = rootEvent; }); - it("shows an unread notification bage", () => { + it("shows an unread notification badge", () => { const { container } = getComponent({}, TimelineRenderingType.ThreadsList); - // By default, the thread will assume there's unread activity in it. - expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(1); + // By default, the thread will assume it is read. + expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(0); act(() => { room.setThreadUnreadNotificationCount(mxEvent.getId()!, NotificationCountType.Total, 3); From 771d6daaa9115f70ec47d708926e4aff18156419 Mon Sep 17 00:00:00 2001 From: Germain Date: Tue, 10 Jan 2023 17:47:27 +0000 Subject: [PATCH 9/9] Update tests --- .../RoomGeneralContextMenu-test.tsx.snap | 44 +------------------ .../UnreadNotificationBadge-test.tsx | 23 +++++++++- .../__snapshots__/RoomTile-test.tsx.snap | 14 ++---- .../views/settings/Notifications-test.tsx | 3 +- 4 files changed, 29 insertions(+), 55 deletions(-) diff --git a/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap b/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap index 8594b2f2056..c1c49e5666c 100644 --- a/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap +++ b/test/components/views/context_menus/__snapshots__/RoomGeneralContextMenu-test.tsx.snap @@ -21,27 +21,7 @@ exports[`RoomGeneralContextMenu renders an empty context menu for archived rooms >
-
- - - Mark as read - - -
-
+ />
@@ -88,27 +68,7 @@ exports[`RoomGeneralContextMenu renders the default context menu 1`] = ` >
-
- - - Mark as read - - -
-
+ />
diff --git a/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx b/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx index 58f552f283c..cfa44165765 100644 --- a/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx +++ b/test/components/views/rooms/NotificationBadge/UnreadNotificationBadge-test.tsx @@ -17,10 +17,11 @@ limitations under the License. import React from "react"; import "jest-mock"; import { screen, act, render } from "@testing-library/react"; -import { MsgType, RelationType } from "matrix-js-sdk/src/matrix"; +import { MatrixEvent, MsgType, RelationType } from "matrix-js-sdk/src/matrix"; import { PendingEventOrdering } from "matrix-js-sdk/src/client"; import { NotificationCountType, Room } from "matrix-js-sdk/src/models/room"; import { EventStatus } from "matrix-js-sdk/src/models/event-status"; +import { ReceiptType } from "matrix-js-sdk/src/@types/read_receipts"; import { mkThread } from "../../../../test-utils/threads"; import { UnreadNotificationBadge } from "../../../../../src/components/views/rooms/NotificationBadge/UnreadNotificationBadge"; @@ -56,6 +57,25 @@ describe("UnreadNotificationBadge", () => { room = new Room(ROOM_ID, client, client.getUserId()!, { pendingEventOrdering: PendingEventOrdering.Detached, }); + + const receipt = new MatrixEvent({ + type: "m.receipt", + room_id: room.roomId, + content: { + "$event0:localhost": { + [ReceiptType.Read]: { + [client.getUserId()!]: { ts: 1, thread_id: "$otherthread:localhost" }, + }, + }, + "$event1:localhost": { + [ReceiptType.Read]: { + [client.getUserId()!]: { ts: 1 }, + }, + }, + }, + }); + room.addReceipt(receipt); + room.setUnreadNotificationCount(NotificationCountType.Total, 1); room.setUnreadNotificationCount(NotificationCountType.Highlight, 0); @@ -156,6 +176,7 @@ describe("UnreadNotificationBadge", () => { rel_type: RelationType.Thread, }, }, + ts: 5, }); room.addLiveEvents([event]); }); diff --git a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap index b4114bcb537..bcbb7932c69 100644 --- a/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap +++ b/test/components/views/rooms/__snapshots__/RoomTile-test.tsx.snap @@ -3,7 +3,7 @@ exports[`RoomTile should render the room 1`] = `
@@ -51,15 +51,7 @@ exports[`RoomTile should render the room 1`] = ` + />