Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Change 'type' prop on badges to 'forceDot' #12327

Merged
merged 8 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/views/rooms/EventTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,7 @@ export class UnwrappedEventTile extends React.Component<EventTileProps, IState>
<UnreadNotificationBadge
room={room || undefined}
threadId={this.props.mxEvent.getId()}
type="dot"
forceDot={true}
/>
</div>
{isRenderingNotification && room ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. Count will
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we mean count the property here, or "Notification count" as a concept? Given the text about it affecting whether the dot is displayed, I think we mean count? In which case, backticks and lower case.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mmm... I would say more the notification count concept: I'm not clear what exactly it would mean to display or not display a prop. I've referenced the count in the class doc and updated this to hopefully be clearer in 940e9d3.

* not 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 {
Expand All @@ -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, 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<HTMLDivElement, XOR<Props, ClickableProps>>(
({ 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
Expand All @@ -61,10 +75,12 @@ export const StatelessNotificationBadge = forwardRef<HTMLDivElement, XOR<Props,
mx_NotificationBadge_visible: isEmptyBadge || knocked ? true : hasUnreadCount,
mx_NotificationBadge_level_notification: level == NotificationLevel.Notification,
mx_NotificationBadge_level_highlight: level >= 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
dbkr marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this needs updating in much the same way as StatelessNotificationBadge's version (though with differences to account for the fact that count comes from a different place)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 37e972e

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 <StatelessNotificationBadge symbol={symbol} count={count} level={level} type={type} />;
return <StatelessNotificationBadge symbol={symbol} count={count} level={level} forceDot={forceDot} />;
}
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ function ThreadsActivityRow({ room, onClick, notificationLevel }: ThreadsActivit
label={room.name}
Icon={<DecoratedRoomAvatar room={room} size="32px" />}
>
<StatelessNotificationBadge level={notificationLevel} count={0} symbol={null} type="dot" />
<StatelessNotificationBadge level={notificationLevel} count={0} symbol={null} forceDot={true} />
</MenuItem>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<StatelessNotificationBadge symbol={null} count={3} level={NotificationLevel.Notification} />,
);
expect(container.querySelector(".mx_NotificationBadge_dot")).not.toBeInTheDocument();
});

it("has dot style for notification when forced", () => {
const { container } = render(
<StatelessNotificationBadge
symbol={null}
count={3}
level={NotificationLevel.Notification}
forceDot={true}
/>,
);
expect(container.querySelector(".mx_NotificationBadge_dot")).toBeInTheDocument();
});
});
Loading