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
Merged

Move notifications bell back in labs #11508

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
a5e186b
Move notifications bell back in labs
Aug 31, 2023
84922fc
Update tests
Aug 31, 2023
6b57ab5
Remove ReloadOnChangeController for feature_notifications
Sep 1, 2023
de88a7c
Apply labs flag to legacy room header too
Sep 1, 2023
f8ed681
Snapshot updates
Sep 1, 2023
91e23de
Update wording for the notifications labs flag
Sep 1, 2023
44d4de1
Merge branch 'develop' into germain-gg/notifications-labs
Johennes Sep 4, 2023
9ceac6e
Merge branch 'develop' into germain-gg/notifications-labs
MidhunSureshR Sep 6, 2023
601f219
Merge branch 'develop' into germain-gg/notifications-labs
MidhunSureshR Sep 7, 2023
1ba419f
Merge branch 'develop' into germain-gg/notifications-labs
MidhunSureshR Sep 14, 2023
610f939
Fix broken tests
MidhunSureshR Sep 14, 2023
2f69013
Merge branch 'develop' into germain-gg/notifications-labs
MidhunSureshR Sep 15, 2023
d5d4b52
Remove unused import
MidhunSureshR Sep 15, 2023
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
1 change: 1 addition & 0 deletions cypress/e2e/right-panel/notification-panel.spec.ts
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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“.

Copy link

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
7 changes: 0 additions & 7 deletions test/components/views/right_panel/__snapshots__/LegacyRoomHeaderButtons-test.tsx.snap
View file
Open in desktop
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
View file
Open in desktop
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
8 changes: 0 additions & 8 deletions test/components/views/rooms/__snapshots__/RoomHeader-test.tsx.snap
View file
Open in desktop
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