From bd6ebc9630e35637b53215bdf5af45ef8f958e23 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 14 Mar 2024 14:30:47 +0000 Subject: [PATCH] Change 'type' prop on badges to 'forceDot' (#12327) * 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 * Missed a file * More comments * Oops, there is no count here. * 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. * Fix doc comment Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Clarify doc on displaying the count * Update doc for the forceDot param here too. --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/components/views/rooms/EventTile.tsx | 2 +- .../StatelessNotificationBadge.tsx | 26 +++++++++++++++---- .../UnreadNotificationBadge.tsx | 10 ++++--- .../ThreadsActivityCentre.tsx | 2 +- .../StatelessNotificationBadge-test.tsx | 19 ++++++++++++++ 5 files changed, 49 insertions(+), 10 deletions(-) diff --git a/src/components/views/rooms/EventTile.tsx b/src/components/views/rooms/EventTile.tsx index cb41417338..b36fb97255 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 69f756b3b7..1d26083b6a 100644 --- a/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/StatelessNotificationBadge.tsx @@ -28,7 +28,12 @@ interface Props { count: number; level: NotificationLevel; knocked?: boolean; - type?: "badge" | "dot"; + /** + * 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; } interface ClickableProps extends Props { @@ -39,8 +44,17 @@ 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 (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. + */ 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 @@ -61,10 +75,12 @@ 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, + + // 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, }); if (props.onClick) { diff --git a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx index 5864a63be0..c3c8cf7df8 100644 --- a/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx +++ b/src/components/views/rooms/NotificationBadge/UnreadNotificationBadge.tsx @@ -23,11 +23,15 @@ import { StatelessNotificationBadge } from "./StatelessNotificationBadge"; interface Props { room?: Room; threadId?: string; - type?: "badge" | "dot"; + /** + * If true, where we would normally show a badge, we instead show a dot. No numeric count will + * be displayed. + */ + 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 098f2d828c..ddb1dd98d3 100644 --- a/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx +++ b/src/components/views/spaces/threads-activity-centre/ThreadsActivityCentre.tsx @@ -147,7 +147,7 @@ function ThreadsActivityCentreRow({ room, onClick, notificationLevel }: ThreadsA 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 66ae273e24..6ee93d82db 100644 --- a/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx +++ b/test/components/views/rooms/NotificationBadge/StatelessNotificationBadge-test.tsx @@ -35,4 +35,23 @@ describe("StatelessNotificationBadge", () => { expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument(); expect(container.querySelector(".mx_NotificationBadge_knocked")).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(); + }); });