From 9e732ae8ef3741ae9aa30868d17651fd0b56daaf Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 7 Mar 2024 19:35:51 +0000 Subject: [PATCH 1/8] Change 'type' prop on badges tio 'forceDot' Which, hopefully, better represents what it actually does. Tidies up some of the logic. Split out from https://github.com/matrix-org/matrix-react-sdk/pull/12254 --- .../StatelessNotificationBadge.tsx | 25 ++++++++++++++---- .../UnreadNotificationBadge.tsx | 9 ++++--- .../ThreadsActivityCentre.tsx | 2 +- .../StatelessNotificationBadge-test.tsx | 26 +++++++++++++++++++ 4 files changed, 53 insertions(+), 9 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index 69f756b3b7e..053bf1c0572 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,15 +59,27 @@ export const StatelessNotificationBadge = forwardRef= NotificationLevel.Highlight, - mx_NotificationBadge_dot: (isEmptyBadge && !knocked) || type === "dot", 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, + + // Exactly one of mx_NotificationBadge_dot, mx_NotificationBadge_2char, mx_NotificationBadge_3char + mx_NotificationBadge_dot: badgeType === "dot", + mx_NotificationBadge_2char: badgeType === "badge_2char", + mx_NotificationBadge_3char: badgeType === "badge_3char", }); 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 bf27839e1cfd7a3deccddeb9ec2714905eed13c4 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 7 Mar 2024 19:43:32 +0000 Subject: [PATCH 2/8] Missed a file --- src/components/views/rooms/EventTile.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 ? ( From 3d649865a965a487b7290fd1fa1a35cd12816c74 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2024 15:38:00 +0000 Subject: [PATCH 3/8] More comments --- .../StatelessNotificationBadge.tsx | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index 053bf1c0572..803b05511f5 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -29,7 +29,9 @@ interface Props { level: NotificationLevel; knocked?: boolean; /** - * If true, the badge will always be displayed as a dot. Count will be ignored. + * If true, where we would normally show a badge, we instead show a dot. Count will + * not be displayed (but may affect whether the the dot is displayed). See class doc + * for the difference between the two. */ forceDot?: boolean; } @@ -42,6 +44,15 @@ interface ClickableProps extends Props { tabIndex?: number; } +/** + * A notification indicator that conveys what activity / notifications the user has in whatever + * context it is being used. + * + * Can either be a 'badge': a small circle with a number in it, or a 'dot': a smaller, empty circle. + * The two can be used to convey the same meaning but in different contexts, for example: for unread + * notifications in the room list, it may have a green badge with the number of unread notifications, + * but somewhere else it may just have a green dot as a more compact representation of the same information. + */ export const StatelessNotificationBadge = forwardRef>( ({ symbol, count, level, knocked, forceDot = false, ...props }, ref) => { const hideBold = useSettingValue("feature_hidebold"); From 92cd4ebd94b368a7dd4b9a0aa34674a08683ea46 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2024 15:43:42 +0000 Subject: [PATCH 4/8] Oops, there is no count here. --- .../views/rooms/NotificationBadge/UnreadNotificationBadge.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx index ed213c9aa17..6d44cf2e370 100644 --- a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx @@ -24,7 +24,7 @@ interface Props { room?: Room; threadId?: string; /** - * If true, the badge will always be displayed as a dot. Count will be ignored. + * If true, the badge will always be displayed as a dot. */ forceDot?: boolean; } From 42d63ef80257ab152851d3c806d1521e9a9e88ce Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 8 Mar 2024 17:49:27 +0000 Subject: [PATCH 5/8] Back out the logic refactor of StatelessNotificationBadge because it was also updating the logic for mark as unread badges and rewriting the ternary to the previous logic would be quite complex. --- .../StatelessNotificationBadge.tsx | 16 +++------------- .../StatelessNotificationBadge-test.tsx | 7 ------- 2 files changed, 3 insertions(+), 20 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index 803b05511f5..ce70ab76a44 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -70,16 +70,6 @@ export const StatelessNotificationBadge = forwardRef 0 && symbol.length < 3, + mx_NotificationBadge_3char: !forceDot && symbol && symbol.length > 2, }); if (props.onClick) { diff --git a/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx b/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx index 612eec286b5..6ee93d82db4 100644 --- a/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx +++ b/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx @@ -36,13 +36,6 @@ describe("StatelessNotificationBadge", () => { 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( , From bdd14b5e166562f9ca8f3dd529d89f1b5f452f87 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 11 Mar 2024 10:11:01 +0000 Subject: [PATCH 6/8] Fix doc comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- .../rooms/NotificationBadge/StatelessNotificationBadge.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index ce70ab76a44..f03ceb73238 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -77,7 +77,7 @@ export const StatelessNotificationBadge = forwardRef= NotificationLevel.Highlight, mx_NotificationBadge_knocked: knocked, - // Exactly one of mx_NotificationBadge_dot, mx_NotificationBadge_2char, mx_NotificationBadge_3char + // At most one of mx_NotificationBadge_dot, mx_NotificationBadge_2char, mx_NotificationBadge_3char mx_NotificationBadge_dot: (isEmptyBadge && !knocked) || forceDot, mx_NotificationBadge_2char: !forceDot && symbol && symbol.length > 0 && symbol.length < 3, mx_NotificationBadge_3char: !forceDot && symbol && symbol.length > 2, From 940e9d34ac131a1c39c50ec840c3a33a811a359c Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 11 Mar 2024 11:36:31 +0000 Subject: [PATCH 7/8] Clarify doc on displaying the count --- .../rooms/NotificationBadge/StatelessNotificationBadge.tsx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx index f03ceb73238..1d26083b6a0 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -29,8 +29,8 @@ interface Props { level: NotificationLevel; knocked?: boolean; /** - * If true, where we would normally show a badge, we instead show a dot. Count will - * not be displayed (but may affect whether the the dot is displayed). See class doc + * If true, where we would normally show a badge, we instead show a dot. No numeric count will + * be displayed (but may affect whether the the dot is displayed). See class doc * for the difference between the two. */ forceDot?: boolean; @@ -48,7 +48,7 @@ interface ClickableProps extends Props { * A notification indicator that conveys what activity / notifications the user has in whatever * context it is being used. * - * Can either be a 'badge': a small circle with a number in it, or a 'dot': a smaller, empty circle. + * Can either be a 'badge': a small circle with a number in it (the 'count'), or a 'dot': a smaller, empty circle. * The two can be used to convey the same meaning but in different contexts, for example: for unread * notifications in the room list, it may have a green badge with the number of unread notifications, * but somewhere else it may just have a green dot as a more compact representation of the same information. From 37e972ea20cf11eafda9ce8996fd328a51133f87 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 11 Mar 2024 11:49:09 +0000 Subject: [PATCH 8/8] Update doc for the forceDot param here too. --- .../views/rooms/NotificationBadge/UnreadNotificationBadge.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx index 6d44cf2e370..c3c8cf7df89 100644 --- a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx @@ -24,7 +24,8 @@ interface Props { room?: Room; threadId?: string; /** - * If true, the badge will always be displayed as a dot. + * If true, where we would normally show a badge, we instead show a dot. No numeric count will + * be displayed. */ forceDot?: boolean; }