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

Move notifications bell back in labs #11508

Merged
merged 13 commits into from
Sep 15, 2023
1 change: 1 addition & 0 deletions cypress/e2e/right-panel/notification-panel.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe("NotificationPanel", () => {
});

it("should render empty state", () => {
cy.enableLabsFeature("feature_notifications");
cy.viewRoomByName(ROOM_NAME);
cy.findByRole("button", { name: "Notifications" }).click();

Expand Down
4 changes: 4 additions & 0 deletions cypress/e2e/room/room-header.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe("Room Header", () => {
});

it("should render default buttons properly", () => {
cy.enableLabsFeature("feature_notifications");
cy.createRoom({ name: "Test Room" }).viewRoomByName("Test Room");

cy.get(".mx_LegacyRoomHeader").within(() => {
Expand Down Expand Up @@ -79,6 +80,7 @@ describe("Room Header", () => {
});

it("should render a very long room name without collapsing the buttons", () => {
cy.enableLabsFeature("feature_notifications");
const LONG_ROOM_NAME =
"Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod tempor incididunt ut labore " +
"et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut " +
Expand Down Expand Up @@ -109,6 +111,7 @@ describe("Room Header", () => {
});

it("should have buttons highlighted by being clicked", () => {
cy.enableLabsFeature("feature_notifications");
cy.createRoom({ name: "Test Room" }).viewRoomByName("Test Room");

cy.get(".mx_LegacyRoomHeader").within(() => {
Expand Down Expand Up @@ -142,6 +145,7 @@ describe("Room Header", () => {
};

it("should render buttons for room options, beta pill, invite, chat, and room info", () => {
cy.enableLabsFeature("feature_notifications");
createVideoRoom();

cy.get(".mx_LegacyRoomHeader").within(() => {
Expand Down
8 changes: 8 additions & 0 deletions src/components/views/right_panel/HeaderButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import RightPanelStore from "../../../stores/right-panel/RightPanelStore";
import { RightPanelPhases } from "../../../stores/right-panel/RightPanelStorePhases";
import { UPDATE_EVENT } from "../../../stores/AsyncStore";
import { NotificationColor } from "../../../stores/notifications/NotificationColor";
import SettingsStore from "../../../settings/SettingsStore";

export enum HeaderKind {
Room = "room",
Expand All @@ -35,13 +36,15 @@ interface IState {
phase: RightPanelPhases | null;
threadNotificationColor: NotificationColor;
globalNotificationColor: NotificationColor;
notificationsEnabled?: boolean;
}

interface IProps {}

export default abstract class HeaderButtons<P = {}> extends React.Component<IProps & P, IState> {
private unmounted = false;
private dispatcherRef?: string = undefined;
private readonly watcherRef: string;

public constructor(props: IProps & P, kind: HeaderKind) {
super(props);
Expand All @@ -52,7 +55,11 @@ export default abstract class HeaderButtons<P = {}> extends React.Component<IPro
phase: rps.currentCard.phase,
threadNotificationColor: NotificationColor.None,
globalNotificationColor: NotificationColor.None,
notificationsEnabled: SettingsStore.getValue("feature_notifications"),
};
this.watcherRef = SettingsStore.watchSetting("feature_notifications", null, (...[, , , value]) =>
this.setState({ notificationsEnabled: value }),
);
}

public componentDidMount(): void {
Expand All @@ -63,6 +70,7 @@ export default abstract class HeaderButtons<P = {}> extends React.Component<IPro
this.unmounted = true;
RightPanelStore.instance.off(UPDATE_EVENT, this.onRightPanelStoreUpdate);
if (this.dispatcherRef) dis.unregister(this.dispatcherRef);
if (this.watcherRef) SettingsStore.unwatchSetting(this.watcherRef);
}

public isPhase(phases: string | string[]): boolean {
Expand Down
32 changes: 17 additions & 15 deletions src/components/views/right_panel/LegacyRoomHeaderButtons.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -282,21 +282,23 @@ export default class LegacyRoomHeaderButtons extends HeaderButtons<IProps> {
<UnreadIndicator color={this.state.threadNotificationColor} />
</HeaderButton>,
);
rightPanelPhaseButtons.set(
RightPanelPhases.NotificationPanel,
<HeaderButton
key="notifsButton"
name="notifsButton"
title={_t("Notifications")}
isHighlighted={this.isPhase(RightPanelPhases.NotificationPanel)}
onClick={this.onNotificationsClicked}
isUnread={this.globalNotificationState.color === NotificationColor.Red}
>
{this.globalNotificationState.color === NotificationColor.Red ? (
<UnreadIndicator color={this.globalNotificationState.color} />
) : null}
</HeaderButton>,
);
if (this.state.notificationsEnabled) {
rightPanelPhaseButtons.set(
RightPanelPhases.NotificationPanel,
<HeaderButton
key="notifsButton"
name="notifsButton"
title={_t("Notifications")}
isHighlighted={this.isPhase(RightPanelPhases.NotificationPanel)}
onClick={this.onNotificationsClicked}
isUnread={this.globalNotificationState.color === NotificationColor.Red}
>
{this.globalNotificationState.color === NotificationColor.Red ? (
<UnreadIndicator color={this.globalNotificationState.color} />
) : null}
</HeaderButton>,
);
}
rightPanelPhaseButtons.set(
RightPanelPhases.RoomSummary,
<HeaderButton
Expand Down
28 changes: 16 additions & 12 deletions src/components/views/rooms/RoomHeader.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ export default function RoomHeader({ room }: { room: Room }): JSX.Element {
}, [room, directRoomsList]);
const e2eStatus = useEncryptionStatus(client, room);

const notificationsEnabled = useFeatureEnabled("feature_notifications");
germain-gg marked this conversation as resolved.
Show resolved Hide resolved

return (
<Flex
as="header"
Expand Down Expand Up @@ -202,18 +204,20 @@ export default function RoomHeader({ room }: { room: Room }): JSX.Element {
<ThreadsIcon />
</IconButton>
</Tooltip>
<Tooltip label={_t("Notifications")}>
<IconButton
indicator={notificationColorToIndicator(globalNotificationState.color)}
onClick={(evt) => {
evt.stopPropagation();
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.NotificationPanel);
}}
title={_t("Notifications")}
>
<NotificationsIcon />
</IconButton>
</Tooltip>
{notificationsEnabled && (
<Tooltip label={_t("Notifications")}>
<IconButton
indicator={notificationColorToIndicator(globalNotificationState.color)}
onClick={(evt) => {
evt.stopPropagation();
RightPanelStore.instance.showOrHidePanel(RightPanelPhases.NotificationPanel);
}}
title={_t("Notifications")}
>
<NotificationsIcon />
</IconButton>
</Tooltip>
)}
</Flex>
{!isDirectMessage && (
<BodyText
Expand Down
4 changes: 3 additions & 1 deletion src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,9 @@
"rust_crypto": "Rust cryptography implementation",
"hidebold": "Hide notification dot (only display counters badges)",
"ask_to_join": "Enable ask to join",
"new_room_decoration_ui": "New room header & details interface",
"new_room_decoration_ui": "Under active development, new room header & details interface",
"notifications": "Enable the notifications panel in the room header",
"unrealiable_e2e": "Unreliable in encrypted rooms",
"beta_feature": "This is a beta feature",
"click_for_info": "Click for more info",
"leave_beta_reload": "Leaving the beta will reload %(brand)s.",
Expand Down
8 changes: 8 additions & 0 deletions src/settings/Settings.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -547,6 +547,14 @@ export const SETTINGS: { [setting: string]: ISetting } = {
default: false,
controller: new ReloadOnChangeController(),
},
"feature_notifications": {
isFeature: true,
labsGroup: LabGroup.Messaging,
displayName: _td("labs|notifications"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I think notifications is a bit too generic here. What about notifications_panel?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's the current terminology we refer this piece of UI with. I'll defer to @daniellekirkwood for the final wording

Copy link
Contributor

Choose a reason for hiding this comment

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

In translation the name is „Notifications panel“. If I see notifications as a translator I would naively translate it with „Notifications“ / „Benachrichtigungen“ instead of „Notifications panel“ / „Benachrichtigungsbereich“.

Choose a reason for hiding this comment

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

Labs flags in product appear to have the structure: Header, subhead so my suggestion would be this:

Enable the notifications panel in the room header
Unreliable in encrypted rooms

element-hq/element-web#25924

description: _td("labs|unrealiable_e2e"),
supportedLevels: LEVELS_FEATURE,
default: false,
},
"useCompactLayout": {
supportedLevels: LEVELS_DEVICE_ONLY_SETTINGS,
displayName: _td("Use a more compact 'Modern' layout"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,6 @@ exports[`LegacyRoomHeaderButtons-test.tsx should render 1`] = `
role="button"
tabindex="0"
/>
<div
aria-current="false"
aria-label="Notifications"
class="mx_AccessibleButton mx_LegacyRoomHeader_button mx_RightPanel_notifsButton"
role="button"
tabindex="0"
/>
<div
aria-current="false"
aria-label="Room info"
Expand Down
4 changes: 4 additions & 0 deletions test/components/views/rooms/RoomHeader-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,10 @@ describe("RoomHeader", () => {
});

it("opens the notifications panel", async () => {
jest.spyOn(SettingsStore, "getValue").mockImplementation((name: string) => {
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
if (name === "feature_notifications") return true;
});

const { container } = render(
<RoomHeader room={room} />,
withClientContextRenderOptions(MatrixClientPeg.get()!),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,6 @@ exports[`RoomHeader does not show the face pile for DMs 1`] = `
>
<div />
</button>
<button
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
class="_icon-button_1segd_17"
data-state="closed"
style="--cpd-icon-button-size: 32px;"
title="Notifications"
>
<div />
</button>
</nav>
</header>
</DocumentFragment>
Expand Down
Loading