From acdc9c58db53be3535f505f4c257245f179f105c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 30 Jul 2024 08:50:51 +0100 Subject: [PATCH 1/3] Rework how the onboarding notifications task works Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/hooks/useUserOnboardingContext.ts | 27 +++++++++++++++++++------ src/hooks/useUserOnboardingTasks.ts | 10 ++++++--- src/i18n/strings/en_EN.json | 4 ++-- src/toasts/DesktopNotificationsToast.ts | 6 ++++-- 4 files changed, 34 insertions(+), 13 deletions(-) diff --git a/src/hooks/useUserOnboardingContext.ts b/src/hooks/useUserOnboardingContext.ts index 1d622173f65..ad4aa880d43 100644 --- a/src/hooks/useUserOnboardingContext.ts +++ b/src/hooks/useUserOnboardingContext.ts @@ -21,12 +21,13 @@ import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { Notifier } from "../Notifier"; import DMRoomMap from "../utils/DMRoomMap"; import { useMatrixClientContext } from "../contexts/MatrixClientContext"; +import { useSettingValue } from "./useSettings"; export interface UserOnboardingContext { hasAvatar: boolean; hasDevices: boolean; hasDmRooms: boolean; - hasNotificationsEnabled: boolean; + showNotificationsPrompt: boolean; } const USER_ONBOARDING_CONTEXT_INTERVAL = 5000; @@ -82,6 +83,22 @@ function useUserOnboardingContextValue<T>(defaultValue: T, callback: (cli: Matri return value; } +function useShowNotificationsPrompt(): boolean { + const [value, setValue] = useState<boolean>(Notifier.shouldShowPrompt()); + useEffect(() => { + window.addEventListener("storage", (event) => { + if (event.storageArea === window.localStorage && event.key === "notifications_hidden") { + 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()!); @@ -96,12 +113,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], ); } diff --git a/src/hooks/useUserOnboardingTasks.ts b/src/hooks/useUserOnboardingTasks.ts index 8dc06efa5b8..176242b1243 100644 --- a/src/hooks/useUserOnboardingTasks.ts +++ b/src/hooks/useUserOnboardingTasks.ts @@ -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(), }, }, ]; diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 37e69b7a77c..d6475ad149e 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -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", diff --git a/src/toasts/DesktopNotificationsToast.ts b/src/toasts/DesktopNotificationsToast.ts index ba8340ca3a0..b0fca719638 100644 --- a/src/toasts/DesktopNotificationsToast.ts +++ b/src/toasts/DesktopNotificationsToast.ts @@ -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, { From a3d93744ca423d63528481c01c60b758b1b3655c Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 30 Jul 2024 09:34:21 +0100 Subject: [PATCH 2/3] Add test Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- test/hooks/useUserOnboardingTasks-test.tsx | 42 ++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/test/hooks/useUserOnboardingTasks-test.tsx b/test/hooks/useUserOnboardingTasks-test.tsx index f2d65382a4d..2ce265fa869 100644 --- a/test/hooks/useUserOnboardingTasks-test.tsx +++ b/test/hooks/useUserOnboardingTasks-test.tsx @@ -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([ @@ -25,7 +32,7 @@ describe("useUserOnboardingTasks", () => { hasAvatar: false, hasDevices: false, hasDmRooms: false, - hasNotificationsEnabled: false, + showNotificationsPrompt: false, }, }, { @@ -33,7 +40,7 @@ describe("useUserOnboardingTasks", () => { hasAvatar: true, hasDevices: false, hasDmRooms: false, - hasNotificationsEnabled: true, + showNotificationsPrompt: true, }, }, ])("sequence should stay static", async ({ context }) => { @@ -46,4 +53,35 @@ 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(true), + } 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 } = renderHook(() => useUserOnboardingTasks(context.result.current)); + expect(result.current[4].id).toBe("permission-notifications"); + result.current[4].action!.onClick!({ type: "click" } as any); + await waitFor(() => expect(result.current[4].completed).toBe(true)); + }); }); From e3c78d1df297351b6409ca73b07be7ccb8c68d63 Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Tue, 30 Jul 2024 15:13:48 +0100 Subject: [PATCH 3/3] Iterate Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/Notifier.ts | 12 +++++++++++- src/hooks/useUserOnboardingContext.ts | 13 +++++-------- test/hooks/useUserOnboardingTasks-test.tsx | 6 ++++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/src/Notifier.ts b/src/Notifier.ts index dbef9bccf47..d482091c3a4 100644 --- a/src/Notifier.ts +++ b/src/Notifier.ts @@ -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"; @@ -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 @@ -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 { diff --git a/src/hooks/useUserOnboardingContext.ts b/src/hooks/useUserOnboardingContext.ts index ad4aa880d43..db1b8081f84 100644 --- a/src/hooks/useUserOnboardingContext.ts +++ b/src/hooks/useUserOnboardingContext.ts @@ -18,10 +18,11 @@ 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; @@ -85,13 +86,9 @@ function useUserOnboardingContextValue<T>(defaultValue: T, callback: (cli: Matri function useShowNotificationsPrompt(): boolean { const [value, setValue] = useState<boolean>(Notifier.shouldShowPrompt()); - useEffect(() => { - window.addEventListener("storage", (event) => { - if (event.storageArea === window.localStorage && event.key === "notifications_hidden") { - setValue(Notifier.shouldShowPrompt()); - } - }); - }, []); + useEventEmitter(Notifier, NotifierEvent.NotificationHiddenChange, () => { + setValue(Notifier.shouldShowPrompt()); + }); const setting = useSettingValue("notificationsEnabled"); useEffect(() => { setValue(Notifier.shouldShowPrompt()); diff --git a/test/hooks/useUserOnboardingTasks-test.tsx b/test/hooks/useUserOnboardingTasks-test.tsx index 2ce265fa869..26a2ba4b756 100644 --- a/test/hooks/useUserOnboardingTasks-test.tsx +++ b/test/hooks/useUserOnboardingTasks-test.tsx @@ -57,7 +57,7 @@ describe("useUserOnboardingTasks", () => { it("should mark desktop notifications task completed on click", async () => { jest.spyOn(PlatformPeg, "get").mockReturnValue({ supportsNotifications: jest.fn().mockReturnValue(true), - maySendNotifications: jest.fn().mockReturnValue(true), + maySendNotifications: jest.fn().mockReturnValue(false), } as any); const cli = stubClient(); @@ -79,9 +79,11 @@ describe("useUserOnboardingTasks", () => { return <MatrixClientContext.Provider value={cli}>{props.children}</MatrixClientContext.Provider>; }, }); - const { result } = renderHook(() => useUserOnboardingTasks(context.result.current)); + 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)); }); });