From cc7065a88a28f4bc14ca914765233092725dc7e1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 11:25:58 +0000 Subject: [PATCH 01/36] Support the mark as unread flag --- src/RoomNotifs.ts | 6 ++++-- .../NotificationBadge/StatelessNotificationBadge.tsx | 8 +++++--- src/components/views/rooms/RoomSublist.tsx | 1 - src/stores/notifications/RoomNotificationState.ts | 8 ++++++++ src/utils/notifications.ts | 3 +++ 5 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts index 66be248f4f9..81a8c3dc0a4 100644 --- a/src/RoomNotifs.ts +++ b/src/RoomNotifs.ts @@ -23,12 +23,13 @@ import { TweakName, } from "matrix-js-sdk/src/matrix"; -import type { IPushRule, Room, MatrixClient } from "matrix-js-sdk/src/matrix"; +import type { IPushRule, Room, MatrixClient, IMarkedUnreadEvent } from "matrix-js-sdk/src/matrix"; import { NotificationLevel } from "./stores/notifications/NotificationLevel"; import { getUnsentMessages } from "./components/structures/RoomStatusBar"; import { doesRoomHaveUnreadMessages, doesRoomOrThreadHaveUnreadMessages } from "./Unread"; import { EffectiveMembership, getEffectiveMembership, isKnockDenied } from "./utils/membership"; import SettingsStore from "./settings/SettingsStore"; +import { MARKED_UNREAD_TYPE } from "./utils/notifications"; export enum RoomNotifState { AllMessagesLoud = "all_messages_loud", @@ -279,7 +280,8 @@ export function determineUnreadState( return { symbol: null, count: trueCount, level: NotificationLevel.Highlight }; } - if (greyNotifs > 0) { + const markedUnreadData = room.getAccountData(MARKED_UNREAD_TYPE); + if (greyNotifs > 0 || markedUnreadData?.getContent()?.unread) { return { symbol: null, count: trueCount, level: NotificationLevel.Notification }; } diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index 69f756b3b7e..fd517dffa08 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -61,10 +61,12 @@ export const StatelessNotificationBadge = forwardRef= NotificationLevel.Highlight, - mx_NotificationBadge_dot: (isEmptyBadge && !knocked) || type === "dot", + mx_NotificationBadge_dot: type === "dot" || (!knocked && level <= NotificationLevel.Activity), mx_NotificationBadge_knocked: knocked, - mx_NotificationBadge_2char: type === "badge" && symbol && symbol.length > 0 && symbol.length < 3, - mx_NotificationBadge_3char: type === "badge" && symbol && symbol.length > 2, + mx_NotificationBadge_2char: + type === "badge" && level > NotificationLevel.Activity && (!symbol || symbol.length < 3), + mx_NotificationBadge_3char: + type === "badge" && level > NotificationLevel.Activity && symbol && symbol.length > 2, }); if (props.onClick) { diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 4e84bee0be2..0f9a394fd6b 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -657,7 +657,6 @@ export default class RoomSublist extends React.Component { const badge = ( { + if (ev.getType() === MARKED_UNREAD_TYPE) { + this.updateNotificationState(); + } + }; + private updateNotificationState(): void { const snapshot = this.snapshot(); diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 1dd2dd7788b..9cc534fde73 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -28,6 +28,9 @@ import SettingsStore from "../settings/SettingsStore"; import { NotificationLevel } from "../stores/notifications/NotificationLevel"; import { doesRoomHaveUnreadMessages } from "../Unread"; +// change when MSC2867 is in spec +export const MARKED_UNREAD_TYPE = "com.famedly.marked_unread"; + export const deviceNotificationSettingsKeys = [ "notificationsEnabled", "notificationBodyEnabled", From eb5843114585a14500492c33bef6e1472e556008 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 17:17:48 +0000 Subject: [PATCH 02/36] Add mark as unread menu option and make clering notifications also clear the unread flag --- .../context_menus/RoomGeneralContextMenu.tsx | 16 ++++++++++++---- src/i18n/strings/en_EN.json | 1 + src/utils/notifications.ts | 8 ++++++++ 3 files changed, 21 insertions(+), 4 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 4cfd2ed6040..c8859d45cc2 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -30,7 +30,7 @@ import { NotificationLevel } from "../../../stores/notifications/NotificationLev import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore"; import DMRoomMap from "../../../utils/DMRoomMap"; -import { clearRoomNotification } from "../../../utils/notifications"; +import { clearRoomNotification, markAsUnread } from "../../../utils/notifications"; import { IProps as IContextMenuProps } from "../../structures/ContextMenu"; import IconizedContextMenu, { IconizedContextMenuCheckbox, @@ -215,16 +215,24 @@ export const RoomGeneralContextMenu: React.FC = ({ const { level } = useUnreadNotifications(room); const markAsReadOption: JSX.Element | null = level > NotificationLevel.None ? ( - { clearRoomNotification(room, cli); onFinished?.(); }} - active={false} label={_t("room|context_menu|mark_read")} iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" /> - ) : null; + ) : ( + { + markAsUnread(room, cli); + onFinished?.(); + }} + label={_t("room|context_menu|mark_unread")} + iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" + /> + ); const developerModeEnabled = useSettingValue("developerMode"); const developerToolsOption = developerModeEnabled ? ( diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 99035cf9d47..5c24ca15f43 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1892,6 +1892,7 @@ "forget": "Forget Room", "low_priority": "Low Priority", "mark_read": "Mark as read", + "mark_unread": "Mark as unread", "mentions_only": "Mentions only", "notifications_default": "Match default setting", "notifications_mute": "Mute room", diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 9cc534fde73..6498d261bc6 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -77,6 +77,10 @@ export function localNotificationsAreSilenced(cli: MatrixClient): boolean { export async function clearRoomNotification(room: Room, client: MatrixClient): Promise<{} | undefined> { const lastEvent = room.getLastLiveEvent(); + if (room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread) { + await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread: false }); + } + try { if (lastEvent) { const receiptType = SettingsStore.getValue("sendReadReceipts", room.roomId) @@ -120,6 +124,10 @@ export function clearAllNotifications(client: MatrixClient): Promise { + await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread: true }); +} + /** * A helper to transform a notification color to the what the Compound Icon Button * expects From e87b938041c55da682157dd9f3d6ac266cf231fa Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 17:53:33 +0000 Subject: [PATCH 03/36] Mark as read on viewing room --- .../views/context_menus/RoomGeneralContextMenu.tsx | 4 ++-- src/stores/RoomViewStore.tsx | 3 +++ src/utils/notifications.ts | 10 +++++----- 3 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index c8859d45cc2..ffeb2d0413b 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -30,7 +30,7 @@ import { NotificationLevel } from "../../../stores/notifications/NotificationLev import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore"; import DMRoomMap from "../../../utils/DMRoomMap"; -import { clearRoomNotification, markAsUnread } from "../../../utils/notifications"; +import { clearRoomNotification, setUnreadMarker } from "../../../utils/notifications"; import { IProps as IContextMenuProps } from "../../structures/ContextMenu"; import IconizedContextMenu, { IconizedContextMenuCheckbox, @@ -226,7 +226,7 @@ export const RoomGeneralContextMenu: React.FC = ({ ) : ( { - markAsUnread(room, cli); + setUnreadMarker(room, cli, true); onFinished?.(); }} label={_t("room|context_menu|mark_unread")} diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index 83c91fdab79..f9672608906 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -62,6 +62,7 @@ import { ActionPayload } from "../dispatcher/payloads"; import { CancelAskToJoinPayload } from "../dispatcher/payloads/CancelAskToJoinPayload"; import { SubmitAskToJoinPayload } from "../dispatcher/payloads/SubmitAskToJoinPayload"; import { ModuleRunner } from "../modules/ModuleRunner"; +import { setUnreadMarker } from "../utils/notifications"; const NUM_JOIN_RETRY = 5; @@ -498,6 +499,8 @@ export class RoomViewStore extends EventEmitter { if (room) { pauseNonLiveBroadcastFromOtherRoom(room, this.stores.voiceBroadcastPlaybacksStore); this.doMaybeSetCurrentVoiceBroadcastPlayback(room); + + await setUnreadMarker(room, MatrixClientPeg.safeGet(), false); } } else if (payload.room_alias) { // Try the room alias to room ID navigation cache first to avoid diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 6498d261bc6..81c7e41cae1 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -77,9 +77,7 @@ export function localNotificationsAreSilenced(cli: MatrixClient): boolean { export async function clearRoomNotification(room: Room, client: MatrixClient): Promise<{} | undefined> { const lastEvent = room.getLastLiveEvent(); - if (room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread) { - await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread: false }); - } + await setUnreadMarker(room, client, false); try { if (lastEvent) { @@ -124,8 +122,10 @@ export function clearAllNotifications(client: MatrixClient): Promise { - await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread: true }); +export async function setUnreadMarker(room: Room, client: MatrixClient, unread: boolean): Promise { + if (room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread !== unread) { + await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread }); + } } /** From 7da410d3d8cf89aa7d981314ab82b81eb09d747a Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 19:16:54 +0000 Subject: [PATCH 04/36] Tests --- src/utils/notifications.ts | 6 ++- test/stores/RoomViewStore-test.ts | 12 ++++++ test/utils/notifications-test.ts | 70 +++++++++++++++++++++++++++++++ 3 files changed, 87 insertions(+), 1 deletion(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 81c7e41cae1..eb2c807806f 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -21,6 +21,7 @@ import { Room, LocalNotificationSettings, ReceiptType, + IMarkedUnreadEvent, } from "matrix-js-sdk/src/matrix"; import { IndicatorIcon } from "@vector-im/compound-web"; @@ -123,7 +124,10 @@ export function clearAllNotifications(client: MatrixClient): Promise { - if (room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread !== unread) { + // if there's no event, treat this as false as we don't need to send the flag to clear it if the event isn't there + const currentState = Boolean(room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread); + + if (currentState !== unread) { await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread }); } } diff --git a/test/stores/RoomViewStore-test.ts b/test/stores/RoomViewStore-test.ts index c1cf7c04781..0cd25b924e2 100644 --- a/test/stores/RoomViewStore-test.ts +++ b/test/stores/RoomViewStore-test.ts @@ -108,6 +108,7 @@ describe("RoomViewStore", function () { relations: jest.fn(), knockRoom: jest.fn(), leave: jest.fn(), + setRoomAccountData: jest.fn(), }); const room = new Room(roomId, mockClient, userId); const room2 = new Room(roomId2, mockClient, userId); @@ -334,6 +335,17 @@ describe("RoomViewStore", function () { expect(mocked(Modal).createDialog.mock.calls[0][1]).toMatchSnapshot(); }); + it("clears the unread flag when viewing a room", async () => { + room.getAccountData = jest.fn().mockReturnValue({ + getContent: jest.fn().mockReturnValue({ unread: true }), + }); + dis.dispatch({ action: Action.ViewRoom, room_id: roomId }); + await untilDispatch(Action.ActiveRoomChanged, dis); + expect(mockClient.setRoomAccountData).toHaveBeenCalledWith(roomId, "com.famedly.marked_unread", { + unread: false, + }); + }); + describe("when listening to a voice broadcast", () => { let voiceBroadcastPlayback: VoiceBroadcastPlayback; diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 30316dd5e68..820ed402630 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -16,6 +16,7 @@ limitations under the License. import { MatrixEvent, NotificationCountType, Room, MatrixClient, ReceiptType } from "matrix-js-sdk/src/matrix"; import { Mocked, mocked } from "jest-mock"; +import { set } from "lodash"; import { localNotificationsAreSilenced, @@ -26,6 +27,7 @@ import { clearRoomNotification, notificationLevelToIndicator, getThreadNotificationLevel, + setUnreadMarker, } from "../../src/utils/notifications"; import SettingsStore from "../../src/settings/SettingsStore"; import { getMockClientWithEventEmitter } from "../test-utils/client"; @@ -219,6 +221,74 @@ describe("notifications", () => { }); }); + describe("setUnreadMarker", () => { + let client: MatrixClient; + let room: Room; + + const ROOM_ID = "123"; + const USER_ID = "@bob:example.org"; + + beforeEach(() => { + stubClient(); + client = mocked(MatrixClientPeg.safeGet()); + room = new Room(ROOM_ID, client, USER_ID); + }); + + // set true, no existing event + it("sets unread flag if event doesn't exist", async () => { + await setUnreadMarker(room, client, true); + expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { + unread: true, + }); + }); + + // set false, no existing event + it("does nothing when clearing if flag is false", async () => { + await setUnreadMarker(room, client, false); + expect(client.setRoomAccountData).not.toHaveBeenCalled(); + }); + + // set true, existing event = false + it("sets unread flag to if existing event is false", async () => { + room.getAccountData = jest + .fn() + .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: false }) }); + await setUnreadMarker(room, client, true); + expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { + unread: true, + }); + }); + + // set false, existing event = false + it("does nothing if set false and existing event is false", async () => { + room.getAccountData = jest + .fn() + .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: false }) }); + await setUnreadMarker(room, client, false); + expect(client.setRoomAccountData).not.toHaveBeenCalled(); + }); + + // set true, existing event = true + it("does nothing if setting true and existing event is true", async () => { + room.getAccountData = jest + .fn() + .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: true }) }); + await setUnreadMarker(room, client, true); + expect(client.setRoomAccountData).not.toHaveBeenCalled(); + }); + + // set false, existing event = true + it("sets flag if setting false and existing event is true", async () => { + room.getAccountData = jest + .fn() + .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: true }) }); + await setUnreadMarker(room, client, false); + expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { + unread: false, + }); + }); + }); + describe("notificationLevelToIndicator", () => { it("returns undefined if notification level is None", () => { expect(notificationLevelToIndicator(NotificationLevel.None)).toBeUndefined(); From 6a98b439c4718f3ed68255fba00e0ca92b8e1636 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 19:24:45 +0000 Subject: [PATCH 05/36] Remove random import --- test/utils/notifications-test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 820ed402630..45bde02183b 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -16,7 +16,6 @@ limitations under the License. import { MatrixEvent, NotificationCountType, Room, MatrixClient, ReceiptType } from "matrix-js-sdk/src/matrix"; import { Mocked, mocked } from "jest-mock"; -import { set } from "lodash"; import { localNotificationsAreSilenced, From 54746969022fd049475c241c69ad67b12ffe4ded Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 15 Feb 2024 19:33:03 +0000 Subject: [PATCH 06/36] Don't show mark as unread for historical rooms --- .../context_menus/RoomGeneralContextMenu.tsx | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index ffeb2d0413b..036051a4336 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -213,26 +213,33 @@ export const RoomGeneralContextMenu: React.FC = ({ } const { level } = useUnreadNotifications(room); - const markAsReadOption: JSX.Element | null = - level > NotificationLevel.None ? ( - { - clearRoomNotification(room, cli); - onFinished?.(); - }} - label={_t("room|context_menu|mark_read")} - iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" - /> - ) : ( - { - setUnreadMarker(room, cli, true); - onFinished?.(); - }} - label={_t("room|context_menu|mark_unread")} - iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" - /> - ); + const markAsReadOption: JSX.Element | null = (() => { + if (level > NotificationLevel.None) { + return ( + { + clearRoomNotification(room, cli); + onFinished?.(); + }} + label={_t("room|context_menu|mark_read")} + iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" + /> + ); + } else if (!roomTags.includes(DefaultTagID.Archived)) { + return ( + { + setUnreadMarker(room, cli, true); + onFinished?.(); + }} + label={_t("room|context_menu|mark_unread")} + iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" + /> + ); + } else { + return null; + } + })(); const developerModeEnabled = useSettingValue("developerMode"); const developerToolsOption = developerModeEnabled ? ( From 2d00c14576d6e9e22c3c19c0246362d9b34fac8d Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Feb 2024 11:24:25 +0000 Subject: [PATCH 07/36] Fix tests & add test for menu option --- .../RoomGeneralContextMenu-test.tsx | 18 ++++++++++++++++++ test/utils/notifications-test.ts | 16 ++++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/test/components/views/context_menus/RoomGeneralContextMenu-test.tsx b/test/components/views/context_menus/RoomGeneralContextMenu-test.tsx index bb832612bfe..498882d83d5 100644 --- a/test/components/views/context_menus/RoomGeneralContextMenu-test.tsx +++ b/test/components/views/context_menus/RoomGeneralContextMenu-test.tsx @@ -140,10 +140,28 @@ describe("RoomGeneralContextMenu", () => { const markAsReadBtn = getByLabelText(container, "Mark as read"); fireEvent.click(markAsReadBtn); + await new Promise(setImmediate); + expect(mockClient.sendReadReceipt).toHaveBeenCalledWith(event, ReceiptType.Read, true); expect(onFinished).toHaveBeenCalled(); }); + it("marks the room as unread", async () => { + room.updateMyMembership("join"); + + const { container } = getComponent({}); + + const markAsUnreadBtn = getByLabelText(container, "Mark as unread"); + fireEvent.click(markAsUnreadBtn); + + await new Promise(setImmediate); + + expect(mockClient.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { + unread: true, + }); + expect(onFinished).toHaveBeenCalled(); + }); + it("when developer mode is disabled, it should not render the developer tools option", () => { getComponent(); expect(screen.queryByText("Developer tools")).not.toBeInTheDocument(); diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 45bde02183b..604ad158ad6 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -136,8 +136,8 @@ describe("notifications", () => { }); }); - it("sends a request even if everything has been read", () => { - clearRoomNotification(room, client); + it("sends a request even if everything has been read", async () => { + await clearRoomNotification(room, client); expect(sendReadReceiptSpy).toHaveBeenCalledWith(message, ReceiptType.Read, true); }); @@ -156,8 +156,8 @@ describe("notifications", () => { sendReceiptsSetting = false; }); - it("should send a private read receipt", () => { - clearRoomNotification(room, client); + it("should send a private read receipt", async () => { + await clearRoomNotification(room, client); expect(sendReadReceiptSpy).toHaveBeenCalledWith(message, ReceiptType.ReadPrivate, true); }); }); @@ -187,7 +187,7 @@ describe("notifications", () => { expect(sendReadReceiptSpy).not.toHaveBeenCalled(); }); - it("sends unthreaded receipt requests", () => { + it("sends unthreaded receipt requests", async () => { const message = mkMessage({ event: true, room: ROOM_ID, @@ -197,12 +197,12 @@ describe("notifications", () => { room.addLiveEvents([message]); room.setUnreadNotificationCount(NotificationCountType.Total, 1); - clearAllNotifications(client); + await clearAllNotifications(client); expect(sendReadReceiptSpy).toHaveBeenCalledWith(message, ReceiptType.Read, true); }); - it("sends private read receipts", () => { + it("sends private read receipts", async () => { const message = mkMessage({ event: true, room: ROOM_ID, @@ -214,7 +214,7 @@ describe("notifications", () => { jest.spyOn(SettingsStore, "getValue").mockReset().mockReturnValue(false); - clearAllNotifications(client); + await clearAllNotifications(client); expect(sendReadReceiptSpy).toHaveBeenCalledWith(message, ReceiptType.ReadPrivate, true); }); From ca880139c43a4baea5b520119021a19b72b41d32 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Feb 2024 11:49:26 +0000 Subject: [PATCH 08/36] Test RoomNotificationState updates on unread flag change --- .../notifications/RoomNotificationState-test.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/test/stores/notifications/RoomNotificationState-test.ts b/test/stores/notifications/RoomNotificationState-test.ts index 1e124d15272..2e94b6d6788 100644 --- a/test/stores/notifications/RoomNotificationState-test.ts +++ b/test/stores/notifications/RoomNotificationState-test.ts @@ -22,6 +22,7 @@ import { NotificationCountType, EventType, MatrixEvent, + RoomEvent, } from "matrix-js-sdk/src/matrix"; import type { MatrixClient } from "matrix-js-sdk/src/matrix"; @@ -92,6 +93,21 @@ describe("RoomNotificationState", () => { expect(listener).toHaveBeenCalled(); }); + it("updates on room account data", () => { + const roomNotifState = new RoomNotificationState(room, true); + const listener = jest.fn(); + roomNotifState.addListener(NotificationStateEvents.Update, listener); + const accountDataEvent = { + getType: () => "com.famedly.marked_unread", + getContent: () => { + return { unread: true }; + }, + } as unknown as MatrixEvent; + room.getAccountData = jest.fn().mockReturnValue(accountDataEvent); + room.emit(RoomEvent.AccountData, accountDataEvent, room); + expect(listener).toHaveBeenCalled(); + }); + it("removes listeners", () => { const roomNotifState = new RoomNotificationState(room, false); expect(() => roomNotifState.destroy()).not.toThrow(); From 0afc9be7970409ea6d8c8751d476a2327440ed05 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Feb 2024 13:49:56 +0000 Subject: [PATCH 09/36] Test it doesn't update on other room account data --- .../RoomNotificationState-test.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/test/stores/notifications/RoomNotificationState-test.ts b/test/stores/notifications/RoomNotificationState-test.ts index 2e94b6d6788..fb11665990c 100644 --- a/test/stores/notifications/RoomNotificationState-test.ts +++ b/test/stores/notifications/RoomNotificationState-test.ts @@ -81,7 +81,7 @@ describe("RoomNotificationState", () => { room.setUnreadNotificationCount(NotificationCountType.Total, greys); } - it("Updates on event decryption", () => { + it("updates on event decryption", () => { const roomNotifState = new RoomNotificationState(room, true); const listener = jest.fn(); roomNotifState.addListener(NotificationStateEvents.Update, listener); @@ -93,7 +93,7 @@ describe("RoomNotificationState", () => { expect(listener).toHaveBeenCalled(); }); - it("updates on room account data", () => { + it("updates on marked unread room account data", () => { const roomNotifState = new RoomNotificationState(room, true); const listener = jest.fn(); roomNotifState.addListener(NotificationStateEvents.Update, listener); @@ -108,6 +108,21 @@ describe("RoomNotificationState", () => { expect(listener).toHaveBeenCalled(); }); + it("does not update on other account data", () => { + const roomNotifState = new RoomNotificationState(room, true); + const listener = jest.fn(); + roomNotifState.addListener(NotificationStateEvents.Update, listener); + const accountDataEvent = { + getType: () => "else.something", + getContent: () => { + return {}; + }, + } as unknown as MatrixEvent; + room.getAccountData = jest.fn().mockReturnValue(accountDataEvent); + room.emit(RoomEvent.AccountData, accountDataEvent, room); + expect(listener).not.toHaveBeenCalled(); + }); + it("removes listeners", () => { const roomNotifState = new RoomNotificationState(room, false); expect(() => roomNotifState.destroy()).not.toThrow(); From 3be0a5d54e545c4d10a7aea9777b41a331495767 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 16 Feb 2024 17:40:41 +0000 Subject: [PATCH 10/36] New icon for mark as unread --- res/css/views/context_menus/_RoomGeneralContextMenu.pcss | 4 ++++ res/img/element-icons/roomlist/mark-as-unread.svg | 4 ++++ src/components/views/context_menus/RoomGeneralContextMenu.tsx | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 res/img/element-icons/roomlist/mark-as-unread.svg diff --git a/res/css/views/context_menus/_RoomGeneralContextMenu.pcss b/res/css/views/context_menus/_RoomGeneralContextMenu.pcss index b5162bb1bbb..4017a53f202 100644 --- a/res/css/views/context_menus/_RoomGeneralContextMenu.pcss +++ b/res/css/views/context_menus/_RoomGeneralContextMenu.pcss @@ -10,6 +10,10 @@ mask-image: url("$(res)/img/element-icons/roomlist/mark-as-read.svg"); } +.mx_RoomGeneralContextMenu_iconMarkAsUnread::before { + mask-image: url("$(res)/img/element-icons/roomlist/mark-as-unread.svg"); +} + .mx_RoomGeneralContextMenu_iconNotificationsDefault::before { mask-image: url("$(res)/img/element-icons/notifications.svg"); } diff --git a/res/img/element-icons/roomlist/mark-as-unread.svg b/res/img/element-icons/roomlist/mark-as-unread.svg new file mode 100644 index 00000000000..a3ea89e3e93 --- /dev/null +++ b/res/img/element-icons/roomlist/mark-as-unread.svg @@ -0,0 +1,4 @@ + + + + diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 036051a4336..8a989d67c15 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -233,7 +233,7 @@ export const RoomGeneralContextMenu: React.FC = ({ onFinished?.(); }} label={_t("room|context_menu|mark_unread")} - iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" + iconClassName="mx_RoomGeneralContextMenu_iconMarkAsUnread" /> ); } else { From 406fbeedaf90cc32b39995027eb5ede8d3ff548f Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 20 Feb 2024 17:59:53 +0000 Subject: [PATCH 11/36] Add analytics events for mark as (un)read --- .../views/context_menus/RoomGeneralContextMenu.tsx | 12 ++++++++---- src/components/views/rooms/RoomTile.tsx | 6 ++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 8a989d67c15..e3f5b406248 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -52,6 +52,8 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { onPostSettingsClick?: (event: ButtonEvent) => void; onPostForgetClick?: (event: ButtonEvent) => void; onPostLeaveClick?: (event: ButtonEvent) => void; + onPostMarkAsReadClick?: (event: ButtonEvent) => void; + onPostMarkAsUnreadClick?: (event: ButtonEvent) => void; } /** @@ -67,6 +69,8 @@ export const RoomGeneralContextMenu: React.FC = ({ onPostSettingsClick, onPostLeaveClick, onPostForgetClick, + onPostMarkAsReadClick, + onPostMarkAsUnreadClick, ...props }) => { const cli = useContext(MatrixClientContext); @@ -217,10 +221,10 @@ export const RoomGeneralContextMenu: React.FC = ({ if (level > NotificationLevel.None) { return ( { + onClick={wrapHandler(() => { clearRoomNotification(room, cli); onFinished?.(); - }} + }, onPostMarkAsReadClick)} label={_t("room|context_menu|mark_read")} iconClassName="mx_RoomGeneralContextMenu_iconMarkAsRead" /> @@ -228,10 +232,10 @@ export const RoomGeneralContextMenu: React.FC = ({ } else if (!roomTags.includes(DefaultTagID.Archived)) { return ( { + onClick={wrapHandler(() => { setUnreadMarker(room, cli, true); onFinished?.(); - }} + }, onPostMarkAsUnreadClick)} label={_t("room|context_menu|mark_unread")} iconClassName="mx_RoomGeneralContextMenu_iconMarkAsUnread" /> diff --git a/src/components/views/rooms/RoomTile.tsx b/src/components/views/rooms/RoomTile.tsx index ab6562a32cc..554e3efeaae 100644 --- a/src/components/views/rooms/RoomTile.tsx +++ b/src/components/views/rooms/RoomTile.tsx @@ -362,6 +362,12 @@ export class RoomTile extends React.PureComponent { onPostLeaveClick={(ev: ButtonEvent) => PosthogTrackers.trackInteraction("WebRoomListRoomTileContextMenuLeaveItem", ev) } + onPostMarkAsReadClick={(ev: ButtonEvent) => + PosthogTrackers.trackInteraction("WebRoomListRoomTileContextMenuMarkRead", ev) + } + onPostMarkAsUnreadClick={(ev: ButtonEvent) => + PosthogTrackers.trackInteraction("WebRoomListRoomTileContextMenuMarkUnread", ev) + } /> )} From dc17850ecd5da02a472f7ee3e829f5ec79db507a Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 21 Feb 2024 10:32:26 +0000 Subject: [PATCH 12/36] Bump to new analytics-events package --- package.json | 2 +- yarn.lock | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 0205783b36d..0b57e4eb2d0 100644 --- a/package.json +++ b/package.json @@ -65,7 +65,7 @@ }, "dependencies": { "@babel/runtime": "^7.12.5", - "@matrix-org/analytics-events": "^0.10.0", + "@matrix-org/analytics-events": "^0.12.0", "@matrix-org/emojibase-bindings": "^1.1.2", "@matrix-org/matrix-wysiwyg": "2.17.0", "@matrix-org/olm": "3.2.15", diff --git a/yarn.lock b/yarn.lock index 87ecdc1c390..cb72d9f4f7e 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1828,10 +1828,10 @@ resolved "https://registry.yarnpkg.com/@mapbox/whoots-js/-/whoots-js-3.1.0.tgz#497c67a1cef50d1a2459ba60f315e448d2ad87fe" integrity sha512-Es6WcD0nO5l+2BOQS4uLfNPYQaNDfbot3X1XUoloz+x0mPDS3eeORZJl06HXjwBG1fOGwCRnzK88LMdxKRrd6Q== -"@matrix-org/analytics-events@^0.10.0": - version "0.10.0" - resolved "https://registry.yarnpkg.com/@matrix-org/analytics-events/-/analytics-events-0.10.0.tgz#d4d8b7859a516e888050d616ebbb0da539a15b1e" - integrity sha512-qzi7szEWxcl3nW2LDfq+SvFH/of/B/lwhfFUelhihGfr5TBPwgqM95Euc9GeYMZkU8Xm/2f5hYfA0ZleD6RKaA== +"@matrix-org/analytics-events@^0.12.0": + version "0.12.0" + resolved "https://registry.yarnpkg.com/@matrix-org/analytics-events/-/analytics-events-0.12.0.tgz#2e48c75eb39c38cbb52f0cd479eed4c835064e9f" + integrity sha512-J/rP11P2Q9PbH7iUzHIthnAQlJL1HEorUjtdd/yCrXDSk0Gw4dNe1FM2P75E6m2lUl2yJQhzGuahMmqe9xOWaw== "@matrix-org/emojibase-bindings@^1.1.2": version "1.1.3" From 6515e493b4da113030b6747ff4262a1da05e2650 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 27 Feb 2024 17:22:44 +0000 Subject: [PATCH 13/36] Read from both stable & unstable prefixes --- src/RoomNotifs.ts | 8 ++-- .../notifications/RoomNotificationState.ts | 4 +- src/utils/notifications.ts | 27 ++++++++++-- test/utils/notifications-test.ts | 42 +++++++++++++++++++ 4 files changed, 71 insertions(+), 10 deletions(-) diff --git a/src/RoomNotifs.ts b/src/RoomNotifs.ts index 81a8c3dc0a4..1fb5e5ba4d7 100644 --- a/src/RoomNotifs.ts +++ b/src/RoomNotifs.ts @@ -23,13 +23,13 @@ import { TweakName, } from "matrix-js-sdk/src/matrix"; -import type { IPushRule, Room, MatrixClient, IMarkedUnreadEvent } from "matrix-js-sdk/src/matrix"; +import type { IPushRule, Room, MatrixClient } from "matrix-js-sdk/src/matrix"; import { NotificationLevel } from "./stores/notifications/NotificationLevel"; import { getUnsentMessages } from "./components/structures/RoomStatusBar"; import { doesRoomHaveUnreadMessages, doesRoomOrThreadHaveUnreadMessages } from "./Unread"; import { EffectiveMembership, getEffectiveMembership, isKnockDenied } from "./utils/membership"; import SettingsStore from "./settings/SettingsStore"; -import { MARKED_UNREAD_TYPE } from "./utils/notifications"; +import { getMarkedUnreadState } from "./utils/notifications"; export enum RoomNotifState { AllMessagesLoud = "all_messages_loud", @@ -280,8 +280,8 @@ export function determineUnreadState( return { symbol: null, count: trueCount, level: NotificationLevel.Highlight }; } - const markedUnreadData = room.getAccountData(MARKED_UNREAD_TYPE); - if (greyNotifs > 0 || markedUnreadData?.getContent()?.unread) { + const markedUnreadState = getMarkedUnreadState(room); + if (greyNotifs > 0 || markedUnreadState) { return { symbol: null, count: trueCount, level: NotificationLevel.Notification }; } diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 8dcb4f86b2b..93ad17b4ec7 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -23,7 +23,7 @@ import { readReceiptChangeIsFor } from "../../utils/read-receipts"; import * as RoomNotifs from "../../RoomNotifs"; import { NotificationState } from "./NotificationState"; import SettingsStore from "../../settings/SettingsStore"; -import { MARKED_UNREAD_TYPE } from "../../utils/notifications"; +import { MARKED_UNREAD_TYPE_STABLE, MARKED_UNREAD_TYPE_UNSTABLE } from "../../utils/notifications"; export class RoomNotificationState extends NotificationState implements IDestroyable { public constructor( @@ -93,7 +93,7 @@ export class RoomNotificationState extends NotificationState implements IDestroy }; private handleRoomAccountDataUpdate = (ev: MatrixEvent): void => { - if (ev.getType() === MARKED_UNREAD_TYPE) { + if ([MARKED_UNREAD_TYPE_STABLE, MARKED_UNREAD_TYPE_UNSTABLE].includes(ev.getType())) { this.updateNotificationState(); } }; diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index eb2c807806f..e1c6431ec6c 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -29,8 +29,14 @@ import SettingsStore from "../settings/SettingsStore"; import { NotificationLevel } from "../stores/notifications/NotificationLevel"; import { doesRoomHaveUnreadMessages } from "../Unread"; -// change when MSC2867 is in spec -export const MARKED_UNREAD_TYPE = "com.famedly.marked_unread"; +// MSC2867 is not yet spec at time of writing. We read from both stable +// and unstable prefixes and accept the risk that the format may change, +// since the stable prefix is not actually defined yet. +// Assuming is passes FCP with no changes, we should update to start writing +// the flag to the stable prefix (or both) and then ultimately use only the +// stable prefix. +export const MARKED_UNREAD_TYPE_UNSTABLE = "com.famedly.marked_unread"; +export const MARKED_UNREAD_TYPE_STABLE = "m.marked_unread"; export const deviceNotificationSettingsKeys = [ "notificationsEnabled", @@ -123,12 +129,25 @@ export function clearAllNotifications(client: MatrixClient): Promise()?.unread; + const currentStateUnstable = room + .getAccountData(MARKED_UNREAD_TYPE_UNSTABLE) + ?.getContent()?.unread; + return currentStateStable ?? currentStateUnstable; +} + export async function setUnreadMarker(room: Room, client: MatrixClient, unread: boolean): Promise { // if there's no event, treat this as false as we don't need to send the flag to clear it if the event isn't there - const currentState = Boolean(room.getAccountData(MARKED_UNREAD_TYPE)?.getContent()?.unread); + const currentState = getMarkedUnreadState(room); if (currentState !== unread) { - await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE, { unread }); + await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE_UNSTABLE, { unread }); } } diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 604ad158ad6..8a2cdf59bb8 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -27,6 +27,7 @@ import { notificationLevelToIndicator, getThreadNotificationLevel, setUnreadMarker, + getMarkedUnreadState, } from "../../src/utils/notifications"; import SettingsStore from "../../src/settings/SettingsStore"; import { getMockClientWithEventEmitter } from "../test-utils/client"; @@ -220,6 +221,47 @@ describe("notifications", () => { }); }); + describe("getMarkedUnreadState", () => { + let client: MatrixClient; + let room: Room; + + const ROOM_ID = "123"; + const USER_ID = "@bob:example.org"; + + beforeEach(() => { + stubClient(); + client = mocked(MatrixClientPeg.safeGet()); + room = new Room(ROOM_ID, client, USER_ID); + }); + + it("reads from stable prefix", async () => { + room.getAccountData = jest.fn().mockImplementation((eventType: string) => { + if (eventType === "m.marked_unread") { + return { getContent: jest.fn().mockReturnValue({ unread: true }) }; + } + return null; + }); + expect(getMarkedUnreadState(room)).toBe(true); + }); + + it("reads from unstable prefix", async () => { + room.getAccountData = jest.fn().mockImplementation((eventType: string) => { + if (eventType === "com.famedly.marked_unread") { + return { getContent: jest.fn().mockReturnValue({ unread: true }) }; + } + return null; + }); + expect(getMarkedUnreadState(room)).toBe(true); + }); + + it("returns undefined if neither prefix is present", async () => { + room.getAccountData = jest.fn().mockImplementation((eventType: string) => { + return null; + }); + expect(getMarkedUnreadState(room)).toBe(undefined); + }); + }); + describe("setUnreadMarker", () => { let client: MatrixClient; let room: Room; From 4bc6ea8b0c12db59d29761cc9da35d9abf964ba7 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 27 Feb 2024 17:32:43 +0000 Subject: [PATCH 14/36] Cast to boolean before checking to avoid setting state unnecessarily --- src/utils/notifications.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index e1c6431ec6c..7460dacf355 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -146,7 +146,7 @@ export async function setUnreadMarker(room: Room, client: MatrixClient, unread: // if there's no event, treat this as false as we don't need to send the flag to clear it if the event isn't there const currentState = getMarkedUnreadState(room); - if (currentState !== unread) { + if (Boolean(currentState) !== unread) { await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE_UNSTABLE, { unread }); } } From ad3dfd5a4ef3a8c6e85b3e25ddb2268472598d30 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 09:51:55 +0000 Subject: [PATCH 15/36] Typo Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/utils/notifications.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 7460dacf355..aaeb9988284 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -32,7 +32,7 @@ import { doesRoomHaveUnreadMessages } from "../Unread"; // MSC2867 is not yet spec at time of writing. We read from both stable // and unstable prefixes and accept the risk that the format may change, // since the stable prefix is not actually defined yet. -// Assuming is passes FCP with no changes, we should update to start writing +// Assuming it passes FCP with no changes, we should update to start writing // the flag to the stable prefix (or both) and then ultimately use only the // stable prefix. export const MARKED_UNREAD_TYPE_UNSTABLE = "com.famedly.marked_unread"; From 6e161e8a8b6a10756f8820959f717de5c2a44333 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 11:06:46 +0000 Subject: [PATCH 16/36] Doc external interface (and the rest at the same time) --- .../context_menus/RoomGeneralContextMenu.tsx | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index e3f5b406248..7e10f386351 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -45,14 +45,50 @@ import { useSettingValue } from "../../../hooks/useSettings"; export interface RoomGeneralContextMenuProps extends IContextMenuProps { room: Room; + /** + * Called when the 'favourite' option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostFavoriteClick?: (event: ButtonEvent) => void; + /** + * Called when the 'low priority'' option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostLowPriorityClick?: (event: ButtonEvent) => void; + /** + * Called when the invite option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostInviteClick?: (event: ButtonEvent) => void; + /** + * Called when the 'copy link' option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostCopyLinkClick?: (event: ButtonEvent) => void; + /** + * Called when the settings option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostSettingsClick?: (event: ButtonEvent) => void; + /** + * Called when the 'forget room' option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostForgetClick?: (event: ButtonEvent) => void; + /** + * Called when the leave option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostLeaveClick?: (event: ButtonEvent) => void; + /** + * Called when the mark as read option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostMarkAsReadClick?: (event: ButtonEvent) => void; + /** + * Called when the mark as unread option is clicked, after the handler is executed. + * @param event The event that caused the click + */ onPostMarkAsUnreadClick?: (event: ButtonEvent) => void; } From f057de8163b72662b8bcbe5f2d5cea17aa282073 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 11:25:18 +0000 Subject: [PATCH 17/36] Doc & rename unread market set function --- .../views/context_menus/RoomGeneralContextMenu.tsx | 4 ++-- src/stores/RoomViewStore.tsx | 4 ++-- src/utils/notifications.ts | 10 ++++++++-- test/utils/notifications-test.ts | 14 +++++++------- 4 files changed, 19 insertions(+), 13 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 7e10f386351..ccc850967fc 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -30,7 +30,7 @@ import { NotificationLevel } from "../../../stores/notifications/NotificationLev import { DefaultTagID, TagID } from "../../../stores/room-list/models"; import RoomListStore, { LISTS_UPDATE_EVENT } from "../../../stores/room-list/RoomListStore"; import DMRoomMap from "../../../utils/DMRoomMap"; -import { clearRoomNotification, setUnreadMarker } from "../../../utils/notifications"; +import { clearRoomNotification, setMarkedUnreadState } from "../../../utils/notifications"; import { IProps as IContextMenuProps } from "../../structures/ContextMenu"; import IconizedContextMenu, { IconizedContextMenuCheckbox, @@ -269,7 +269,7 @@ export const RoomGeneralContextMenu: React.FC = ({ return ( { - setUnreadMarker(room, cli, true); + setMarkedUnreadState(room, cli, true); onFinished?.(); }, onPostMarkAsUnreadClick)} label={_t("room|context_menu|mark_unread")} diff --git a/src/stores/RoomViewStore.tsx b/src/stores/RoomViewStore.tsx index d350dea53b6..007ae7b5b0f 100644 --- a/src/stores/RoomViewStore.tsx +++ b/src/stores/RoomViewStore.tsx @@ -62,7 +62,7 @@ import { ActionPayload } from "../dispatcher/payloads"; import { CancelAskToJoinPayload } from "../dispatcher/payloads/CancelAskToJoinPayload"; import { SubmitAskToJoinPayload } from "../dispatcher/payloads/SubmitAskToJoinPayload"; import { ModuleRunner } from "../modules/ModuleRunner"; -import { setUnreadMarker } from "../utils/notifications"; +import { setMarkedUnreadState } from "../utils/notifications"; const NUM_JOIN_RETRY = 5; @@ -499,7 +499,7 @@ export class RoomViewStore extends EventEmitter { pauseNonLiveBroadcastFromOtherRoom(room, this.stores.voiceBroadcastPlaybacksStore); this.doMaybeSetCurrentVoiceBroadcastPlayback(room); - await setUnreadMarker(room, MatrixClientPeg.safeGet(), false); + await setMarkedUnreadState(room, MatrixClientPeg.safeGet(), false); } } else if (payload.room_alias) { // Try the room alias to room ID navigation cache first to avoid diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index aaeb9988284..6359a26c655 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -84,7 +84,7 @@ export function localNotificationsAreSilenced(cli: MatrixClient): boolean { export async function clearRoomNotification(room: Room, client: MatrixClient): Promise<{} | undefined> { const lastEvent = room.getLastLiveEvent(); - await setUnreadMarker(room, client, false); + await setMarkedUnreadState(room, client, false); try { if (lastEvent) { @@ -142,7 +142,13 @@ export function getMarkedUnreadState(room: Room): boolean | undefined { return currentStateStable ?? currentStateUnstable; } -export async function setUnreadMarker(room: Room, client: MatrixClient, unread: boolean): Promise { +/** + * Sets the marked_unread state of the given room + * @param room The room to set + * @param client MatrixClient object to use + * @param unread The new marked_unread state of the room + */ +export async function setMarkedUnreadState(room: Room, client: MatrixClient, unread: boolean): Promise { // if there's no event, treat this as false as we don't need to send the flag to clear it if the event isn't there const currentState = getMarkedUnreadState(room); diff --git a/test/utils/notifications-test.ts b/test/utils/notifications-test.ts index 8a2cdf59bb8..6e67ca9b029 100644 --- a/test/utils/notifications-test.ts +++ b/test/utils/notifications-test.ts @@ -26,8 +26,8 @@ import { clearRoomNotification, notificationLevelToIndicator, getThreadNotificationLevel, - setUnreadMarker, getMarkedUnreadState, + setMarkedUnreadState, } from "../../src/utils/notifications"; import SettingsStore from "../../src/settings/SettingsStore"; import { getMockClientWithEventEmitter } from "../test-utils/client"; @@ -277,7 +277,7 @@ describe("notifications", () => { // set true, no existing event it("sets unread flag if event doesn't exist", async () => { - await setUnreadMarker(room, client, true); + await setMarkedUnreadState(room, client, true); expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { unread: true, }); @@ -285,7 +285,7 @@ describe("notifications", () => { // set false, no existing event it("does nothing when clearing if flag is false", async () => { - await setUnreadMarker(room, client, false); + await setMarkedUnreadState(room, client, false); expect(client.setRoomAccountData).not.toHaveBeenCalled(); }); @@ -294,7 +294,7 @@ describe("notifications", () => { room.getAccountData = jest .fn() .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: false }) }); - await setUnreadMarker(room, client, true); + await setMarkedUnreadState(room, client, true); expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { unread: true, }); @@ -305,7 +305,7 @@ describe("notifications", () => { room.getAccountData = jest .fn() .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: false }) }); - await setUnreadMarker(room, client, false); + await setMarkedUnreadState(room, client, false); expect(client.setRoomAccountData).not.toHaveBeenCalled(); }); @@ -314,7 +314,7 @@ describe("notifications", () => { room.getAccountData = jest .fn() .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: true }) }); - await setUnreadMarker(room, client, true); + await setMarkedUnreadState(room, client, true); expect(client.setRoomAccountData).not.toHaveBeenCalled(); }); @@ -323,7 +323,7 @@ describe("notifications", () => { room.getAccountData = jest .fn() .mockReturnValue({ getContent: jest.fn().mockReturnValue({ unread: true }) }); - await setUnreadMarker(room, client, false); + await setMarkedUnreadState(room, client, false); expect(client.setRoomAccountData).toHaveBeenCalledWith(ROOM_ID, "com.famedly.marked_unread", { unread: false, }); From 2ced1d66220d40faa1063c1bb12633b153663adf Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 11:27:41 +0000 Subject: [PATCH 18/36] Doc const exports --- src/utils/notifications.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 6359a26c655..6f1073f0c41 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -35,7 +35,14 @@ import { doesRoomHaveUnreadMessages } from "../Unread"; // Assuming it passes FCP with no changes, we should update to start writing // the flag to the stable prefix (or both) and then ultimately use only the // stable prefix. + +/** + * Unstable identifier for the marked_unread event + */ export const MARKED_UNREAD_TYPE_UNSTABLE = "com.famedly.marked_unread"; +/** + * Stable identifier for the marked_unread event + */ export const MARKED_UNREAD_TYPE_STABLE = "m.marked_unread"; export const deviceNotificationSettingsKeys = [ From 86870d23cc1ecf6261975bff0d683c8774402a41 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 11:28:49 +0000 Subject: [PATCH 19/36] Remove listener on destroy --- src/stores/notifications/RoomNotificationState.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/stores/notifications/RoomNotificationState.ts b/src/stores/notifications/RoomNotificationState.ts index 93ad17b4ec7..449a6968fd9 100644 --- a/src/stores/notifications/RoomNotificationState.ts +++ b/src/stores/notifications/RoomNotificationState.ts @@ -53,6 +53,7 @@ export class RoomNotificationState extends NotificationState implements IDestroy this.room.removeListener(RoomEvent.LocalEchoUpdated, this.handleLocalEchoUpdated); this.room.removeListener(RoomEvent.Timeline, this.handleRoomEventUpdate); this.room.removeListener(RoomEvent.Redaction, this.handleRoomEventUpdate); + this.room.removeListener(RoomEvent.AccountData, this.handleRoomAccountDataUpdate); cli.removeListener(MatrixEventEvent.Decrypted, this.onEventDecrypted); cli.removeListener(ClientEvent.AccountData, this.handleAccountDataUpdate); } From 716a2e4977b30b5efc2d48bd9518a4547d1aea3e Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 29 Feb 2024 14:56:40 +0000 Subject: [PATCH 20/36] Add playwright test --- .../e2e/room_options/marked_unread.spec.ts | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 playwright/e2e/room_options/marked_unread.spec.ts diff --git a/playwright/e2e/room_options/marked_unread.spec.ts b/playwright/e2e/room_options/marked_unread.spec.ts new file mode 100644 index 00000000000..85ebdf0c9ad --- /dev/null +++ b/playwright/e2e/room_options/marked_unread.spec.ts @@ -0,0 +1,79 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +/* +Copyright 2023 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { test, expect } from "../../element-web-test"; + +const TEST_ROOM_NAME = "The mark unread test room"; + +test.describe("Threads", () => { + test.use({ + displayName: "Tom", + botCreateOpts: { + displayName: "BotBob", + autoAcceptInvites: true, + }, + }); + + test.beforeEach(async ({ page }) => {}); + + test("should mark a room as unread", async ({ page, app, bot }) => { + const roomId = await app.client.createRoom({ + name: TEST_ROOM_NAME, + }); + const dummyRoomId = await app.client.createRoom({ + name: "Room of no consequence", + }); + await app.client.inviteUser(roomId, bot.credentials.userId); + await bot.joinRoom(roomId); + await bot.sendMessage(roomId, "I am a robot. Beep."); + + // Regular notification on new message + await expect(page.getByLabel(TEST_ROOM_NAME + " 1 unread message.")).toBeVisible(); + await expect(page).toHaveTitle("Element [1]"); + + await page.goto("/#/room/" + roomId); + + // should now be read, since we viewed the room (we have to assert the page title: + // the room badge isn't visible since we're viewing the room) + await expect(page).toHaveTitle("Element | " + TEST_ROOM_NAME); + + // navigate away from the room again + await page.goto("/#/room/" + dummyRoomId); + + const roomTile = page.getByLabel(TEST_ROOM_NAME); + await roomTile.focus(); + await roomTile.getByRole("button", { name: "Room options" }).click(); + await page.getByRole("menuitem", { name: "Mark as unread" }).click(); + + expect(page.getByLabel(TEST_ROOM_NAME + " Unread messages.")).toBeVisible(); + }); +}); From 523b1421cf8022e9d579395941f715b74f72b1d5 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 16:12:33 +0000 Subject: [PATCH 21/36] Clearer language, hopefully --- .../context_menus/RoomGeneralContextMenu.tsx | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index ccc850967fc..f2577484674 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -46,47 +46,56 @@ import { useSettingValue } from "../../../hooks/useSettings"; export interface RoomGeneralContextMenuProps extends IContextMenuProps { room: Room; /** - * Called when the 'favourite' option is clicked, after the handler is executed. + * Called when the 'favourite' option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostFavoriteClick?: (event: ButtonEvent) => void; /** - * Called when the 'low priority'' option is clicked, after the handler is executed. + * Called when the 'low priority'' option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostLowPriorityClick?: (event: ButtonEvent) => void; /** - * Called when the invite option is clicked, after the handler is executed. + * Called when the invite option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostInviteClick?: (event: ButtonEvent) => void; /** - * Called when the 'copy link' option is clicked, after the handler is executed. + * Called when the 'copy link' option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostCopyLinkClick?: (event: ButtonEvent) => void; /** - * Called when the settings option is clicked, after the handler is executed. + * Called when the settings option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostSettingsClick?: (event: ButtonEvent) => void; /** - * Called when the 'forget room' option is clicked, after the handler is executed. + * Called when the 'forget room' option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostForgetClick?: (event: ButtonEvent) => void; /** - * Called when the leave option is clicked, after the handler is executed. + * Called when the leave option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostLeaveClick?: (event: ButtonEvent) => void; /** - * Called when the mark as read option is clicked, after the handler is executed. + * Called when the mark as read option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostMarkAsReadClick?: (event: ButtonEvent) => void; /** - * Called when the mark as unread option is clicked, after the handler is executed. + * Called when the mark as unread option is clicked, after the menu has processed + * whatever the click in the normal way. * @param event The event that caused the click */ onPostMarkAsUnreadClick?: (event: ButtonEvent) => void; From d8aca51e47022f01b1f11a3b65bfd8b77188df39 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 16:15:10 +0000 Subject: [PATCH 22/36] Move comment --- src/utils/notifications.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 6f1073f0c41..7161ef562af 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -32,9 +32,6 @@ import { doesRoomHaveUnreadMessages } from "../Unread"; // MSC2867 is not yet spec at time of writing. We read from both stable // and unstable prefixes and accept the risk that the format may change, // since the stable prefix is not actually defined yet. -// Assuming it passes FCP with no changes, we should update to start writing -// the flag to the stable prefix (or both) and then ultimately use only the -// stable prefix. /** * Unstable identifier for the marked_unread event @@ -160,6 +157,9 @@ export async function setMarkedUnreadState(room: Room, client: MatrixClient, unr const currentState = getMarkedUnreadState(room); if (Boolean(currentState) !== unread) { + // Assuming MSC2867 passes FCP with no changes, we should update to start writing + // the flag to the stable prefix (or both) and then ultimately use only the + // stable prefix. await client.setRoomAccountData(room.roomId, MARKED_UNREAD_TYPE_UNSTABLE, { unread }); } } From b556a920dcbb88ac393821ab0243964280a4611f Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 16:16:15 +0000 Subject: [PATCH 23/36] Add reference to the MSC Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/utils/notifications.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 7161ef562af..8e9b53dde90 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -34,7 +34,7 @@ import { doesRoomHaveUnreadMessages } from "../Unread"; // since the stable prefix is not actually defined yet. /** - * Unstable identifier for the marked_unread event + * Unstable identifier for the marked_unread event, per MSC2867 */ export const MARKED_UNREAD_TYPE_UNSTABLE = "com.famedly.marked_unread"; /** From e0bd722afa4bb4ee8c7603993c50e3983e102137 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 16:20:02 +0000 Subject: [PATCH 24/36] Expand on function doc --- src/utils/notifications.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/utils/notifications.ts b/src/utils/notifications.ts index 8e9b53dde90..46e61fc9841 100644 --- a/src/utils/notifications.ts +++ b/src/utils/notifications.ts @@ -147,7 +147,9 @@ export function getMarkedUnreadState(room: Room): boolean | undefined { } /** - * Sets the marked_unread state of the given room + * Sets the marked_unread state of the given room. This sets some room account data that indicates to + * clients that the user considers this room to be 'unread', but without any actual notifications. + * * @param room The room to set * @param client MatrixClient object to use * @param unread The new marked_unread state of the room From 17eb159b3196a2931492869e5a8168625d197f47 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 16:22:50 +0000 Subject: [PATCH 25/36] Remove empty beforeEach --- playwright/e2e/room_options/marked_unread.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/playwright/e2e/room_options/marked_unread.spec.ts b/playwright/e2e/room_options/marked_unread.spec.ts index 85ebdf0c9ad..8aa773c1bae 100644 --- a/playwright/e2e/room_options/marked_unread.spec.ts +++ b/playwright/e2e/room_options/marked_unread.spec.ts @@ -43,8 +43,6 @@ test.describe("Threads", () => { }, }); - test.beforeEach(async ({ page }) => {}); - test("should mark a room as unread", async ({ page, app, bot }) => { const roomId = await app.client.createRoom({ name: TEST_ROOM_NAME, From ae809dc7ab99b7ba4c01a96544568dd8032e63f0 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 5 Mar 2024 18:00:21 +0000 Subject: [PATCH 26/36] Rejig badge logic a little and add tests --- src/components/views/rooms/EventTile.tsx | 2 +- .../StatelessNotificationBadge.tsx | 20 +++++++++----- .../UnreadNotificationBadge.tsx | 9 ++++--- .../ThreadsActivityCentre.tsx | 2 +- .../StatelessNotificationBadge-test.tsx | 26 +++++++++++++++++++ 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index cb414173387..b36fb972555 100644 --- a/src/components/views/rooms/EventTile.tsx +++ b/src/components/views/rooms/EventTile.tsx @@ -1311,7 +1311,7 @@ export class UnwrappedEventTile extends React.Component {isRenderingNotification && room ? ( diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index fd517dffa08..6a66e909103 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -28,7 +28,10 @@ interface Props { count: number; level: NotificationLevel; knocked?: boolean; - type?: "badge" | "dot"; + /** + * If true, the badge will always be displayed as a dot. Count will be ignored. + */ + forceDot?: boolean; } interface ClickableProps extends Props { @@ -40,7 +43,7 @@ interface ClickableProps extends Props { } export const StatelessNotificationBadge = forwardRef>( - ({ symbol, count, level, knocked, type = "badge", ...props }, ref) => { + ({ symbol, count, level, knocked, forceDot = false, ...props }, ref) => { const hideBold = useSettingValue("feature_hidebold"); // Don't show a badge if we don't need to @@ -56,17 +59,20 @@ export const StatelessNotificationBadge = forwardRef= NotificationLevel.Highlight, - mx_NotificationBadge_dot: type === "dot" || (!knocked && level <= NotificationLevel.Activity), + mx_NotificationBadge_dot: badgeType === "dot", mx_NotificationBadge_knocked: knocked, - mx_NotificationBadge_2char: - type === "badge" && level > NotificationLevel.Activity && (!symbol || symbol.length < 3), - mx_NotificationBadge_3char: - type === "badge" && level > NotificationLevel.Activity && symbol && symbol.length > 2, + mx_NotificationBadge_2char: badgeType === "badge" && (!symbol || symbol.length < 3), + mx_NotificationBadge_3char: badgeType === "badge" && symbol && symbol.length > 2, }); if (props.onClick) { diff --git a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx index 5864a63be01..ed213c9aa17 100644 --- a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx @@ -23,11 +23,14 @@ import { StatelessNotificationBadge } from "./StatelessNotificationBadge"; interface Props { room?: Room; threadId?: string; - type?: "badge" | "dot"; + /** + * If true, the badge will always be displayed as a dot. Count will be ignored. + */ + forceDot?: boolean; } -export function UnreadNotificationBadge({ room, threadId, type }: Props): JSX.Element { +export function UnreadNotificationBadge({ room, threadId, forceDot }: Props): JSX.Element { const { symbol, count, level } = useUnreadNotifications(room, threadId); - return ; + return ; } diff --git a/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx b/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx index f6374ef32a9..6c42d2458a5 100644 --- a/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx +++ b/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx @@ -130,7 +130,7 @@ function ThreadsActivityRow({ room, onClick, notificationLevel }: ThreadsActivit label={room.name} Icon={} > - + ); } diff --git a/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx b/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx index 66ae273e247..612eec286b5 100644 --- a/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx +++ b/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx @@ -35,4 +35,30 @@ describe("StatelessNotificationBadge", () => { expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument(); expect(container.querySelector(".mx_NotificationBadge_knocked")).toBeInTheDocument(); }); + + it("has dot style for activity", () => { + const { container } = render( + , + ); + expect(container.querySelector(".mx_NotificationBadge_dot")).toBeInTheDocument(); + }); + + it("has badge style for notification", () => { + const { container } = render( + , + ); + expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument(); + }); + + it("has dot style for notification when forced", () => { + const { container } = render( + , + ); + expect(container.querySelector(".mx_NotificationBadge_dot")).toBeInTheDocument(); + }); }); From ff7d281f54a568a162f20515aaeb5fc5fe781e00 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 6 Mar 2024 11:03:19 +0000 Subject: [PATCH 27/36] Fix basdges to not display dots in room sublists again and hopefully rename the forceDot option to something that better indicates what it does, and add tests. --- .../views/avatars/DecoratedRoomAvatar.tsx | 10 ++++-- src/components/views/rooms/ExtraTile.tsx | 2 +- .../views/rooms/NotificationBadge.tsx | 13 ++++---- .../views/rooms/RoomBreadcrumbs.tsx | 2 +- src/components/views/rooms/RoomSublist.tsx | 1 + src/components/views/rooms/RoomTile.tsx | 6 +--- .../views/spaces/SpaceTreeLevel.tsx | 1 - .../NotificationBadge-test.tsx | 33 +++++++++++++++++++ 8 files changed, 51 insertions(+), 17 deletions(-) diff --git a/src/components/views/avatars/DecoratedRoomAvatar.tsx b/src/components/views/avatars/DecoratedRoomAvatar.tsx index 5bdc90a1f7c..88e3c239bfa 100644 --- a/src/components/views/avatars/DecoratedRoomAvatar.tsx +++ b/src/components/views/avatars/DecoratedRoomAvatar.tsx @@ -34,7 +34,11 @@ interface IProps { room: Room; size: string; displayBadge?: boolean; - forceCount?: boolean; + /** + * If true, show nothing if the notification would only cause a dot to be shown rather than + * a badge. That is: only display badges and not dots. Default: false. + */ + hideIfDot?: boolean; oobData?: IOOBData; viewAvatarOnClick?: boolean; tooltipProps?: { @@ -177,14 +181,14 @@ export default class DecoratedRoomAvatar extends React.PureComponent ); diff --git a/src/components/views/rooms/ExtraTile.tsx b/src/components/views/rooms/ExtraTile.tsx index 157bfc4d562..3bb3a21525a 100644 --- a/src/components/views/rooms/ExtraTile.tsx +++ b/src/components/views/rooms/ExtraTile.tsx @@ -52,7 +52,7 @@ export default function ExtraTile({ let badge: JSX.Element | null = null; if (notificationState) { - badge = ; + badge = ; } let name = displayName; diff --git a/src/components/views/rooms/NotificationBadge.tsx b/src/components/views/rooms/NotificationBadge.tsx index d152ab6a626..c257f02f141 100644 --- a/src/components/views/rooms/NotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge.tsx @@ -28,10 +28,10 @@ interface IProps { notification: NotificationState; /** - * If true, the badge will show a count if at all possible. This is typically - * used to override the user's preference for things like room sublists. + * If true, show nothing if the notification would only cause a dot to be shown rather than + * a badge. That is: only display badges and not dots. Default: false. */ - forceCount?: boolean; + hideIfDot?: boolean; /** * The room ID, if any, the badge represents. @@ -97,11 +97,12 @@ export default class NotificationBadge extends React.PureComponent = { diff --git a/src/components/views/rooms/RoomBreadcrumbs.tsx b/src/components/views/rooms/RoomBreadcrumbs.tsx index eca8da46240..cd31dbd8e79 100644 --- a/src/components/views/rooms/RoomBreadcrumbs.tsx +++ b/src/components/views/rooms/RoomBreadcrumbs.tsx @@ -61,7 +61,7 @@ const RoomBreadcrumbTile: React.FC<{ room: Room; onClick: (ev: ButtonEvent) => v room={room} size="32px" displayBadge={true} - forceCount={true} + hideIfDot={true} tooltipProps={{ tabIndex: isActive ? 0 : -1 }} /> diff --git a/src/components/views/rooms/RoomSublist.tsx b/src/components/views/rooms/RoomSublist.tsx index 0f9a394fd6b..c8ad9d4acab 100644 --- a/src/components/views/rooms/RoomSublist.tsx +++ b/src/components/views/rooms/RoomSublist.tsx @@ -657,6 +657,7 @@ export default class RoomSublist extends React.Component { const badge = ( { // aria-hidden because we summarise the unread count/highlight status in a manual aria-label below badge = ( ); } diff --git a/src/components/views/spaces/SpaceTreeLevel.tsx b/src/components/views/spaces/SpaceTreeLevel.tsx index c0b71709238..315cba3c1cc 100644 --- a/src/components/views/spaces/SpaceTreeLevel.tsx +++ b/src/components/views/spaces/SpaceTreeLevel.tsx @@ -113,7 +113,6 @@ export const SpaceButton = ({
{ + it("shows a dot if the level is activity", () => { + const notif = new DummyNotificationState(NotificationLevel.Activity); + + const { container } = render(); + expect(container.querySelector(".mx_NotificationBadge_dot")).toBeInTheDocument(); + expect(container.querySelector(".mx_NotificationBadge")).toBeInTheDocument(); + }); + + it("does not show a dot if the level is activity and hideIfDot is true", () => { + const notif = new DummyNotificationState(NotificationLevel.Activity); + + const { container } = render(); + expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument(); + expect(container.querySelector(".mx_NotificationBadge")).not.toBeInTheDocument(); + }); + + it("still shows an empty badge if hideIfDot us true", () => { + const notif = new DummyNotificationState(NotificationLevel.Notification); + + const { container } = render(); + expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument(); + expect(container.querySelector(".mx_NotificationBadge")).toBeInTheDocument(); + }); + describe("StatelessNotificationBadge", () => { it("lets you click it", () => { const cb = jest.fn(); From 50ba376947d60c523f1dd3f032e6009f4eb72b59 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 6 Mar 2024 11:41:51 +0000 Subject: [PATCH 28/36] Remove duplicate license header (?) --- .../e2e/room_options/marked_unread.spec.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/playwright/e2e/room_options/marked_unread.spec.ts b/playwright/e2e/room_options/marked_unread.spec.ts index 8aa773c1bae..196e990b279 100644 --- a/playwright/e2e/room_options/marked_unread.spec.ts +++ b/playwright/e2e/room_options/marked_unread.spec.ts @@ -14,22 +14,6 @@ See the License for the specific language governing permissions and limitations under the License. */ -/* -Copyright 2023 The Matrix.org Foundation C.I.C. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - import { test, expect } from "../../element-web-test"; const TEST_ROOM_NAME = "The mark unread test room"; From 60f8fc7581174b9148706b2af574c1fe1640f915 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Mar 2024 19:07:33 +0000 Subject: [PATCH 29/36] Missing word (several times...) --- .../context_menus/RoomGeneralContextMenu.tsx | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index f2577484674..074b4ef99fb 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -47,55 +47,55 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { room: Room; /** * Called when the 'favourite' option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostFavoriteClick?: (event: ButtonEvent) => void; /** * Called when the 'low priority'' option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostLowPriorityClick?: (event: ButtonEvent) => void; /** * Called when the invite option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostInviteClick?: (event: ButtonEvent) => void; /** * Called when the 'copy link' option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostCopyLinkClick?: (event: ButtonEvent) => void; /** * Called when the settings option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostSettingsClick?: (event: ButtonEvent) => void; /** * Called when the 'forget room' option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostForgetClick?: (event: ButtonEvent) => void; /** * Called when the leave option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostLeaveClick?: (event: ButtonEvent) => void; /** * Called when the mark as read option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostMarkAsReadClick?: (event: ButtonEvent) => void; /** * Called when the mark as unread option is clicked, after the menu has processed - * whatever the click in the normal way. + * whatever caused the click in the normal way. * @param event The event that caused the click */ onPostMarkAsUnreadClick?: (event: ButtonEvent) => void; From b418dc88ab8460fd2f2529e1f5b880dc7360a51f Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 15 Mar 2024 12:10:26 +0000 Subject: [PATCH 30/36] Incorporate PR suggestion on badge type switch --- .../NotificationBadge/StatelessNotificationBadge.tsx | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index 3cd3245ff10..f9051f3aa14 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -73,7 +73,12 @@ export const StatelessNotificationBadge = forwardRef 2, + mx_NotificationBadge_2char: badgeType === "badge_2char", + mx_NotificationBadge_3char: badgeType === "badge_3char", }); if (props.onClick) { From 2454abaa13e6f8fad00826a7eba6df833e00517d Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 09:40:47 +0000 Subject: [PATCH 31/36] Better description in doc comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- .../views/context_menus/RoomGeneralContextMenu.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 074b4ef99fb..9c6c19ea238 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -46,9 +46,9 @@ import { useSettingValue } from "../../../hooks/useSettings"; export interface RoomGeneralContextMenuProps extends IContextMenuProps { room: Room; /** - * Called when the 'favourite' option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the 'favourite' option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostFavoriteClick?: (event: ButtonEvent) => void; /** From 4ee353ecf939fc7d397689ec3fdd065433a53c0e Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 09:43:54 +0000 Subject: [PATCH 32/36] Update other doc comments in the same way --- .../context_menus/RoomGeneralContextMenu.tsx | 48 +++++++++---------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 9c6c19ea238..b099118f782 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -52,51 +52,51 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { */ onPostFavoriteClick?: (event: ButtonEvent) => void; /** - * Called when the 'low priority'' option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the 'low priority'' option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostLowPriorityClick?: (event: ButtonEvent) => void; /** - * Called when the invite option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the invite option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostInviteClick?: (event: ButtonEvent) => void; /** - * Called when the 'copy link' option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the 'copy link' option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostCopyLinkClick?: (event: ButtonEvent) => void; /** - * Called when the settings option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the settings option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostSettingsClick?: (event: ButtonEvent) => void; /** - * Called when the 'forget room' option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the 'forget room' option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostForgetClick?: (event: ButtonEvent) => void; /** - * Called when the leave option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the leave option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostLeaveClick?: (event: ButtonEvent) => void; /** - * Called when the mark as read option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the mark as read option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostMarkAsReadClick?: (event: ButtonEvent) => void; /** - * Called when the mark as unread option is clicked, after the menu has processed - * whatever caused the click in the normal way. - * @param event The event that caused the click + * Called when the mark as unread option is selected, after the menu has processed + * the mouse or keyboard event. + * @param event The event that caused the option to be selected. */ onPostMarkAsUnreadClick?: (event: ButtonEvent) => void; } From c4a96ad74b14fe19bc8e47f78aa90502039afc27 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 09:44:45 +0000 Subject: [PATCH 33/36] Remove duplicate quote --- src/components/views/context_menus/RoomGeneralContextMenu.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index b099118f782..0db3490a73f 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -52,7 +52,7 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { */ onPostFavoriteClick?: (event: ButtonEvent) => void; /** - * Called when the 'low priority'' option is selected, after the menu has processed + * Called when the 'low priority' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ From 1e50d22675c5855085a5938f23094dabff842c40 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 09:45:51 +0000 Subject: [PATCH 34/36] Use quotes consistently --- .../views/context_menus/RoomGeneralContextMenu.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/components/views/context_menus/RoomGeneralContextMenu.tsx b/src/components/views/context_menus/RoomGeneralContextMenu.tsx index 0db3490a73f..4465c219024 100644 --- a/src/components/views/context_menus/RoomGeneralContextMenu.tsx +++ b/src/components/views/context_menus/RoomGeneralContextMenu.tsx @@ -58,7 +58,7 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { */ onPostLowPriorityClick?: (event: ButtonEvent) => void; /** - * Called when the invite option is selected, after the menu has processed + * Called when the 'invite' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ @@ -70,7 +70,7 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { */ onPostCopyLinkClick?: (event: ButtonEvent) => void; /** - * Called when the settings option is selected, after the menu has processed + * Called when the 'settings' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ @@ -82,19 +82,19 @@ export interface RoomGeneralContextMenuProps extends IContextMenuProps { */ onPostForgetClick?: (event: ButtonEvent) => void; /** - * Called when the leave option is selected, after the menu has processed + * Called when the 'leave' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ onPostLeaveClick?: (event: ButtonEvent) => void; /** - * Called when the mark as read option is selected, after the menu has processed + * Called when the 'mark as read' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ onPostMarkAsReadClick?: (event: ButtonEvent) => void; /** - * Called when the mark as unread option is selected, after the menu has processed + * Called when the 'mark as unread' option is selected, after the menu has processed * the mouse or keyboard event. * @param event The event that caused the option to be selected. */ From 0eb14a37ad6de54c3fe01115f95af08666534c14 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 09:50:06 +0000 Subject: [PATCH 35/36] Better test name --- test/stores/notifications/RoomNotificationState-test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/stores/notifications/RoomNotificationState-test.ts b/test/stores/notifications/RoomNotificationState-test.ts index fb11665990c..f41f13ff13c 100644 --- a/test/stores/notifications/RoomNotificationState-test.ts +++ b/test/stores/notifications/RoomNotificationState-test.ts @@ -93,7 +93,7 @@ describe("RoomNotificationState", () => { expect(listener).toHaveBeenCalled(); }); - it("updates on marked unread room account data", () => { + it("emits an Update event on marked unread room account data", () => { const roomNotifState = new RoomNotificationState(room, true); const listener = jest.fn(); roomNotifState.addListener(NotificationStateEvents.Update, listener); From d18579c961cd8ce28d084cb177466f051f36de8b Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 18 Mar 2024 10:07:37 +0000 Subject: [PATCH 36/36] c+p fail --- playwright/e2e/room_options/marked_unread.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/playwright/e2e/room_options/marked_unread.spec.ts b/playwright/e2e/room_options/marked_unread.spec.ts index 196e990b279..799acf22500 100644 --- a/playwright/e2e/room_options/marked_unread.spec.ts +++ b/playwright/e2e/room_options/marked_unread.spec.ts @@ -18,7 +18,7 @@ import { test, expect } from "../../element-web-test"; const TEST_ROOM_NAME = "The mark unread test room"; -test.describe("Threads", () => { +test.describe("Mark as Unread", () => { test.use({ displayName: "Tom", botCreateOpts: {