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

Commit

Permalink
Rework how the onboarding notifications task works (#12839)
Browse files Browse the repository at this point in the history
* Rework how the onboarding notifications task works

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Add test

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
  • Loading branch information
t3chguy authored Jul 30, 2024
1 parent dd20741 commit 0204724
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 17 deletions.
12 changes: 11 additions & 1 deletion src/Notifier.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
IRoomTimelineData,
M_LOCATION,
EventType,
TypedEventEmitter,
} from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { PermissionChanged as PermissionChangedEvent } from "@matrix-org/analytics-events/types/typescript/PermissionChanged";
Expand Down Expand Up @@ -103,7 +104,15 @@ const msgTypeHandlers: Record<string, (event: MatrixEvent) => string | null> = {
},
};

class NotifierClass {
export const enum NotifierEvent {
NotificationHiddenChange = "notification_hidden_change",
}

interface EmittedEvents {
[NotifierEvent.NotificationHiddenChange]: (hidden: boolean) => void;
}

class NotifierClass extends TypedEventEmitter<keyof EmittedEvents, EmittedEvents> {
private notifsByRoom: Record<string, Notification[]> = {};

// A list of event IDs that we've received but need to wait until
Expand Down Expand Up @@ -357,6 +366,7 @@ class NotifierClass {
if (persistent && global.localStorage) {
global.localStorage.setItem("notifications_hidden", String(hidden));
}
this.emit(NotifierEvent.NotificationHiddenChange, hidden);
}

public shouldShowPrompt(): boolean {
Expand Down
26 changes: 19 additions & 7 deletions src/hooks/useUserOnboardingContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,15 +18,17 @@ import { logger } from "matrix-js-sdk/src/logger";
import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/matrix";
import { useCallback, useEffect, useMemo, useRef, useState } from "react";

import { Notifier } from "../Notifier";
import { Notifier, NotifierEvent } from "../Notifier";
import DMRoomMap from "../utils/DMRoomMap";
import { useMatrixClientContext } from "../contexts/MatrixClientContext";
import { useSettingValue } from "./useSettings";
import { useEventEmitter } from "./useEventEmitter";

export interface UserOnboardingContext {
hasAvatar: boolean;
hasDevices: boolean;
hasDmRooms: boolean;
hasNotificationsEnabled: boolean;
showNotificationsPrompt: boolean;
}

const USER_ONBOARDING_CONTEXT_INTERVAL = 5000;
Expand Down Expand Up @@ -82,6 +84,18 @@ function useUserOnboardingContextValue<T>(defaultValue: T, callback: (cli: Matri
return value;
}

function useShowNotificationsPrompt(): boolean {
const [value, setValue] = useState<boolean>(Notifier.shouldShowPrompt());
useEventEmitter(Notifier, NotifierEvent.NotificationHiddenChange, () => {
setValue(Notifier.shouldShowPrompt());
});
const setting = useSettingValue("notificationsEnabled");
useEffect(() => {
setValue(Notifier.shouldShowPrompt());
}, [setting]);
return value;
}

export function useUserOnboardingContext(): UserOnboardingContext {
const hasAvatar = useUserOnboardingContextValue(false, async (cli) => {
const profile = await cli.getProfileInfo(cli.getUserId()!);
Expand All @@ -96,12 +110,10 @@ export function useUserOnboardingContext(): UserOnboardingContext {
const dmRooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals() ?? {};
return Boolean(Object.keys(dmRooms).length);
});
const hasNotificationsEnabled = useUserOnboardingContextValue(false, async () => {
return Notifier.isPossible();
});
const showNotificationsPrompt = useShowNotificationsPrompt();

return useMemo(
() => ({ hasAvatar, hasDevices, hasDmRooms, hasNotificationsEnabled }),
[hasAvatar, hasDevices, hasDmRooms, hasNotificationsEnabled],
() => ({ hasAvatar, hasDevices, hasDmRooms, showNotificationsPrompt }),
[hasAvatar, hasDevices, hasDmRooms, showNotificationsPrompt],
);
}
10 changes: 7 additions & 3 deletions src/hooks/useUserOnboardingTasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,18 @@ const tasks: UserOnboardingTask[] = [
id: "permission-notifications",
title: _t("onboarding|enable_notifications"),
description: _t("onboarding|enable_notifications_description"),
completed: (ctx: UserOnboardingContext) => ctx.hasNotificationsEnabled,
completed: (ctx: UserOnboardingContext) => !ctx.showNotificationsPrompt,
action: {
label: _t("onboarding|enable_notifications_action"),
onClick: (ev: ButtonEvent) => {
PosthogTrackers.trackInteraction("WebUserOnboardingTaskEnableNotifications", ev);
Notifier.setEnabled(true);
defaultDispatcher.dispatch({
action: Action.ViewUserSettings,
initialTabId: UserTab.Notifications,
});
Notifier.setPromptHidden(true);
},
hideOnComplete: true,
hideOnComplete: !Notifier.isPossible(),
},
},
];
Expand Down
4 changes: 2 additions & 2 deletions src/i18n/strings/en_EN.json
Original file line number Diff line number Diff line change
Expand Up @@ -1652,8 +1652,8 @@
"download_brand_desktop": "Download %(brand)s Desktop",
"download_f_droid": "Get it on F-Droid",
"download_google_play": "Get it on Google Play",
"enable_notifications": "Turn on notifications",
"enable_notifications_action": "Enable notifications",
"enable_notifications": "Turn on desktop notifications",
"enable_notifications_action": "Open settings",
"enable_notifications_description": "Don’t miss a reply or important message",
"explore_rooms": "Explore Public Rooms",
"find_community_members": "Find and invite your community members",
Expand Down
6 changes: 4 additions & 2 deletions src/toasts/DesktopNotificationsToast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import GenericToast from "../components/views/toasts/GenericToast";
import ToastStore from "../stores/ToastStore";
import { MatrixClientPeg } from "../MatrixClientPeg";
import { getLocalNotificationAccountDataEventType } from "../utils/notifications";
import SettingsStore from "../settings/SettingsStore";
import { SettingLevel } from "../settings/SettingLevel";

const onAccept = (): void => {
Notifier.setEnabled(true);
const onAccept = async (): Promise<void> => {
await SettingsStore.setValue("notificationsEnabled", null, SettingLevel.DEVICE, true);
const cli = MatrixClientPeg.safeGet();
const eventType = getLocalNotificationAccountDataEventType(cli.deviceId!);
cli.setAccountData(eventType, {
Expand Down
44 changes: 42 additions & 2 deletions test/hooks/useUserOnboardingTasks-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,16 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React from "react";
import { renderHook } from "@testing-library/react-hooks";
import { waitFor } from "@testing-library/react";

import { useUserOnboardingTasks } from "../../src/hooks/useUserOnboardingTasks";
import { useUserOnboardingContext } from "../../src/hooks/useUserOnboardingContext";
import { stubClient } from "../test-utils";
import MatrixClientContext from "../../src/contexts/MatrixClientContext";
import DMRoomMap from "../../src/utils/DMRoomMap";
import PlatformPeg from "../../src/PlatformPeg";

describe("useUserOnboardingTasks", () => {
it.each([
Expand All @@ -25,15 +32,15 @@ describe("useUserOnboardingTasks", () => {
hasAvatar: false,
hasDevices: false,
hasDmRooms: false,
hasNotificationsEnabled: false,
showNotificationsPrompt: false,
},
},
{
context: {
hasAvatar: true,
hasDevices: false,
hasDmRooms: false,
hasNotificationsEnabled: true,
showNotificationsPrompt: true,
},
},
])("sequence should stay static", async ({ context }) => {
Expand All @@ -46,4 +53,37 @@ describe("useUserOnboardingTasks", () => {
expect(result.current[3].id).toBe("setup-profile");
expect(result.current[4].id).toBe("permission-notifications");
});

it("should mark desktop notifications task completed on click", async () => {
jest.spyOn(PlatformPeg, "get").mockReturnValue({
supportsNotifications: jest.fn().mockReturnValue(true),
maySendNotifications: jest.fn().mockReturnValue(false),
} as any);

const cli = stubClient();
cli.pushRules = {
global: {
override: [
{
rule_id: ".m.rule.master",
enabled: false,
actions: [],
default: true,
},
],
},
};
DMRoomMap.makeShared(cli);
const context = renderHook(() => useUserOnboardingContext(), {
wrapper: (props) => {
return <MatrixClientContext.Provider value={cli}>{props.children}</MatrixClientContext.Provider>;
},
});
const { result, rerender } = renderHook(() => useUserOnboardingTasks(context.result.current));
expect(result.current[4].id).toBe("permission-notifications");
await waitFor(() => expect(result.current[4].completed).toBe(false));
result.current[4].action!.onClick!({ type: "click" } as any);
rerender();
await waitFor(() => expect(result.current[4].completed).toBe(true));
});
});

0 comments on commit 0204724

Please sign in to comment.