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

Commit

Permalink
Add notification dots to thread summary icons (#12146)
Browse files Browse the repository at this point in the history
* Add notification dots to thread summary icons

Adopts new IndicatorIcon from compound to have threads icons with
indicator dot (that aren't also buttons). Adds green & red dots on
the threads icon in the thread summary to indicate notifications.
Changes the notification level dots colours in the threads panel to
be green to match.

* Update test for new CSS class

* Update snapshots with new class name

* Another snapshot update for new class name

* Replace more uses of old class name in tests

* More snapshot updates for new class name

* Unsure how this ever worked in chronological mode

* More snapshot updates

* Fix dot colours

* Upgrade to compound-web 3

* Fix computed notification levels

* Add test for notificationLevelToIndicator
  • Loading branch information
dbkr committed Jan 25, 2024
1 parent f684ad5 commit 95430ce
Show file tree
Hide file tree
Showing 19 changed files with 130 additions and 54 deletions.
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@
"@sentry/browser": "^7.0.0",
"@testing-library/react-hooks": "^8.0.1",
"@vector-im/compound-design-tokens": "^0.1.0",
"@vector-im/compound-web": "2.0.0",
"@vector-im/compound-web": "3.0.0",
"@zxcvbn-ts/core": "^3.0.4",
"@zxcvbn-ts/language-common": "^3.0.4",
"@zxcvbn-ts/language-en": "^3.0.2",
Expand Down
23 changes: 18 additions & 5 deletions res/css/views/rooms/_NotificationBadge.pcss
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,34 @@ limitations under the License.
align-items: center;
justify-content: center;

&.mx_NotificationBadge_highlighted {
/* TODO: Use a more specific variable */
background-color: $alert;
}

/* These are the 3 background types */

&.mx_NotificationBadge_dot {
width: 8px;
height: 8px;
border-radius: 8px;
background-color: var(--cpd-color-text-primary);

.mx_NotificationBadge_count {
display: none;
}

/* Redundant sounding name, but a notification badge that indicates there is a regular,
* non-highlight notification
* The green colour only applies for notification dot: badges indicating the same notification
* level are the standard grey.
*/
&.mx_NotificationBadge_level_notification {
background-color: var(--cpd-color-icon-success-primary);
}
}

/* Badges for highlight notifications. Style for notification level
* badges is in _EventTile.scss because it applies only to notification
* dots, not badges.
*/
&.mx_NotificationBadge_level_highlight {
background-color: var(--cpd-color-icon-critical-primary);
}

&.mx_NotificationBadge_knocked {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,8 @@ export const StatelessNotificationBadge = forwardRef<HTMLDivElement, XOR<Props,
const classes = classNames({
mx_NotificationBadge: true,
mx_NotificationBadge_visible: isEmptyBadge || knocked ? true : hasUnreadCount,
mx_NotificationBadge_highlighted: level >= NotificationLevel.Highlight,
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,
Expand Down
16 changes: 1 addition & 15 deletions src/components/views/rooms/RoomHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ import { Flex } from "../../utils/Flex";
import { Box } from "../../utils/Box";
import { useRoomCall } from "../../../hooks/room/useRoomCall";
import { useRoomThreadNotifications } from "../../../hooks/room/useRoomThreadNotifications";
import { NotificationLevel } from "../../../stores/notifications/NotificationLevel";
import { useGlobalNotificationState } from "../../../hooks/useGlobalNotificationState";
import SdkConfig from "../../../SdkConfig";
import { useFeatureEnabled } from "../../../hooks/useSettings";
Expand All @@ -52,20 +51,7 @@ import { Linkify, topicToHtml } from "../../../HtmlUtils";
import PosthogTrackers from "../../../PosthogTrackers";
import { VideoRoomChatButton } from "./RoomHeader/VideoRoomChatButton";
import { RoomKnocksBar } from "./RoomKnocksBar";

/**
* A helper to transform a notification color to the what the Compound Icon Button
* expects
*/
function notificationLevelToIndicator(color: NotificationLevel): React.ComponentProps<typeof IconButton>["indicator"] {
if (color <= NotificationLevel.None) {
return undefined;
} else if (color <= NotificationLevel.Notification) {
return "default";
} else {
return "highlight";
}
}
import { notificationLevelToIndicator } from "../../../utils/notifications";

export default function RoomHeader({
room,
Expand Down
9 changes: 9 additions & 0 deletions src/components/views/rooms/ThreadSummary.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ limitations under the License.

import React, { useContext, useState } from "react";
import { Thread, ThreadEvent, IContent, MatrixEvent, MatrixEventEvent } from "matrix-js-sdk/src/matrix";
import { IndicatorIcon } from "@vector-im/compound-web";
import { Icon as ThreadIconSolid } from "@vector-im/compound-design-tokens/icons/threads-solid.svg";

import { _t } from "../../../languageHandler";
import { CardContext } from "../right_panel/context";
Expand All @@ -30,6 +32,8 @@ import MatrixClientContext from "../../../contexts/MatrixClientContext";
import { Action } from "../../../dispatcher/actions";
import { ShowThreadPayload } from "../../../dispatcher/payloads/ShowThreadPayload";
import defaultDispatcher from "../../../dispatcher/dispatcher";
import { useUnreadNotifications } from "../../../hooks/useUnreadNotifications";
import { notificationLevelToIndicator } from "../../../utils/notifications";

interface IProps {
mxEvent: MatrixEvent;
Expand All @@ -40,6 +44,8 @@ const ThreadSummary: React.FC<IProps> = ({ mxEvent, thread, ...props }) => {
const roomContext = useContext(RoomContext);
const cardContext = useContext(CardContext);
const count = useTypedEventEmitterState(thread, ThreadEvent.Update, () => thread.length);
const { level } = useUnreadNotifications(thread.room, thread.id);

if (!count) return null; // We don't want to show a thread summary if the thread doesn't have replies yet

let countSection: string | number = count;
Expand All @@ -61,6 +67,9 @@ const ThreadSummary: React.FC<IProps> = ({ mxEvent, thread, ...props }) => {
}}
aria-label={_t("threads|open_thread")}
>
<IndicatorIcon size="24px" indicator={notificationLevelToIndicator(level)}>
<ThreadIconSolid />
</IndicatorIcon>
<span className="mx_ThreadSummary_replies_amount">{countSection}</span>
<ThreadMessagePreview thread={thread} showDisplayname={!roomContext.narrow} />
<div className="mx_ThreadSummary_chevron" />
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/room/useRoomThreadNotifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ export const useRoomThreadNotifications = (room: Room): NotificationLevel => {
switch (room?.threadsAggregateNotificationType) {
case NotificationCountType.Highlight:
setNotificationLevel(NotificationLevel.Highlight);
break;
return;
case NotificationCountType.Total:
setNotificationLevel(NotificationLevel.Notification);
break;
return;
}
// We don't have any notified messages, but we might have unread messages. Let's
// find out.
Expand Down
20 changes: 20 additions & 0 deletions src/utils/notifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ import {
LocalNotificationSettings,
ReceiptType,
} from "matrix-js-sdk/src/matrix";
import { IndicatorIcon } from "@vector-im/compound-web";

import SettingsStore from "../settings/SettingsStore";
import { NotificationLevel } from "../stores/notifications/NotificationLevel";

export const deviceNotificationSettingsKeys = [
"notificationsEnabled",
Expand Down Expand Up @@ -113,3 +115,21 @@ export function clearAllNotifications(client: MatrixClient): Promise<Array<{} |

return Promise.all(receiptPromises);
}

/**
* A helper to transform a notification color to the what the Compound Icon Button
* expects
*/
export function notificationLevelToIndicator(
level: NotificationLevel,
): React.ComponentPropsWithRef<typeof IndicatorIcon>["indicator"] {
if (level <= NotificationLevel.None) {
return undefined;
} else if (level <= NotificationLevel.Activity) {
return "default";
} else if (level <= NotificationLevel.Notification) {
return "success";
} else {
return "critical";
}
}
2 changes: 1 addition & 1 deletion test/components/structures/TimelinePanel-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ describe("TimelinePanel", () => {
client = MatrixClientPeg.safeGet();

Thread.hasServerSideSupport = FeatureSupport.Stable;
room = new Room("roomId", client, "userId");
room = new Room("roomId", client, "userId", { pendingEventOrdering: PendingEventOrdering.Detached });
allThreads = new EventTimelineSet(
room,
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ exports[`RoomStatusBar <RoomStatusBar /> unsent messages should render warning w
class="mx_RoomStatusBar_unsentBadge"
>
<div
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_highlighted mx_NotificationBadge_2char"
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_highlight mx_NotificationBadge_2char"
>
<span
class="mx_NotificationBadge_count"
Expand Down Expand Up @@ -81,7 +81,7 @@ exports[`RoomStatusBar <RoomStatusBar /> unsent messages should render warning w
class="mx_RoomStatusBar_unsentBadge"
>
<div
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_highlighted mx_NotificationBadge_2char"
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_highlight mx_NotificationBadge_2char"
>
<span
class="mx_NotificationBadge_count"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ exports[`RoomView for a local room in state ERROR should match the snapshot 1`]
class="mx_RoomStatusBar_unsentBadge"
>
<div
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_highlighted mx_NotificationBadge_2char"
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_highlight mx_NotificationBadge_2char"
>
<span
class="mx_NotificationBadge_count"
Expand Down
4 changes: 2 additions & 2 deletions test/components/views/rooms/EventTile-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ describe("EventTile", () => {
});

expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(1);
expect(container.getElementsByClassName("mx_NotificationBadge_highlighted")).toHaveLength(0);
expect(container.getElementsByClassName("mx_NotificationBadge_level_highlight")).toHaveLength(0);

act(() => {
room.setThreadUnreadNotificationCount(mxEvent.getId()!, NotificationCountType.Highlight, 1);
});

expect(container.getElementsByClassName("mx_NotificationBadge")).toHaveLength(1);
expect(container.getElementsByClassName("mx_NotificationBadge_highlighted")).toHaveLength(1);
expect(container.getElementsByClassName("mx_NotificationBadge_level_highlight")).toHaveLength(1);
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ describe("StatelessNotificationBadge", () => {
const { container } = render(
<StatelessNotificationBadge symbol="!" count={0} level={NotificationLevel.Unsent} />,
);
expect(container.querySelector(".mx_NotificationBadge_highlighted")).not.toBe(null);
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).not.toBe(null);
});

it("has knock style", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,26 +92,26 @@ describe("UnreadNotificationBadge", () => {
const { container } = render(getComponent());

expect(container.querySelector(".mx_NotificationBadge_visible")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeFalsy();
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).toBeFalsy();

act(() => {
room.setUnreadNotificationCount(NotificationCountType.Highlight, 1);
});

expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).toBeTruthy();
});

it("renders unread thread notification badge", () => {
const { container } = render(getComponent(THREAD_ID));

expect(container.querySelector(".mx_NotificationBadge_visible")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeFalsy();
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).toBeFalsy();

act(() => {
room.setThreadUnreadNotificationCount(THREAD_ID, NotificationCountType.Highlight, 1);
});

expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).toBeTruthy();
});

it("hides unread notification badge", () => {
Expand Down Expand Up @@ -177,6 +177,6 @@ describe("UnreadNotificationBadge", () => {
const { container } = render(getComponent(THREAD_ID));
expect(container.querySelector(".mx_NotificationBadge_dot")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_visible")).toBeTruthy();
expect(container.querySelector(".mx_NotificationBadge_highlighted")).toBeFalsy();
expect(container.querySelector(".mx_NotificationBadge_level_highlight")).toBeFalsy();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -3,26 +3,37 @@
exports[`<VideoRoomChatButton /> renders button when room is a video room 1`] = `
<button
aria-label="Chat"
class="_icon-button_ur2sw_17"
class="_icon-button_16nk7_17"
data-state="closed"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div />
<div
class="_indicator-icon_jtb4d_26"
style="--cpd-icon-button-size: 100%;"
>
<div />
</div>
</button>
`;

exports[`<VideoRoomChatButton /> renders button with an unread marker when room is unread 1`] = `
<button
aria-label="Chat"
class="_icon-button_ur2sw_17"
class="_icon-button_16nk7_17"
data-indicator="default"
data-state="closed"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div />
<div
class="_indicator-icon_jtb4d_26"
data-indicator="default"
style="--cpd-icon-button-size: 100%;"
>
<div />
</div>
</button>
`;
27 changes: 21 additions & 6 deletions test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -46,34 +46,49 @@ exports[`RoomHeader does not show the face pile for DMs 1`] = `
<button
aria-disabled="true"
aria-label="There's no one here to call"
class="_icon-button_ur2sw_17"
class="_icon-button_16nk7_17"
data-state="closed"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div />
<div
class="_indicator-icon_jtb4d_26"
style="--cpd-icon-button-size: 100%; --cpd-color-icon-tertiary: var(--cpd-color-icon-disabled);"
>
<div />
</div>
</button>
<button
aria-disabled="true"
aria-label="There's no one here to call"
class="_icon-button_ur2sw_17"
class="_icon-button_16nk7_17"
data-state="closed"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div />
<div
class="_indicator-icon_jtb4d_26"
style="--cpd-icon-button-size: 100%; --cpd-color-icon-tertiary: var(--cpd-color-icon-disabled);"
>
<div />
</div>
</button>
<button
aria-label="Threads"
class="_icon-button_ur2sw_17"
class="_icon-button_16nk7_17"
data-state="closed"
role="button"
style="--cpd-icon-button-size: 32px;"
tabindex="0"
>
<div />
<div
class="_indicator-icon_jtb4d_26"
style="--cpd-icon-button-size: 100%;"
>
<div />
</div>
</button>
</div>
</header>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -480,7 +480,7 @@ exports[`<Notifications /> correctly handles the loading/disabled state 1`] = `
<span>
Show a badge
<div
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_2char"
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_notification mx_NotificationBadge_2char"
>
<span
class="mx_NotificationBadge_count"
Expand Down Expand Up @@ -1190,7 +1190,7 @@ exports[`<Notifications /> matches the snapshot 1`] = `
<span>
Show a badge
<div
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_2char"
class="mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_notification mx_NotificationBadge_2char"
>
<span
class="mx_NotificationBadge_count"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ exports[`SpaceButton metaspace should render notificationState if one is provide
class="mx_SpacePanel_badgeContainer"
>
<div
class="mx_AccessibleButton mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_2char"
class="mx_AccessibleButton mx_NotificationBadge mx_NotificationBadge_visible mx_NotificationBadge_level_notification mx_NotificationBadge_2char"
role="button"
tabindex="-1"
>
Expand Down
Loading

0 comments on commit 95430ce

Please sign in to comment.