From 94d292a6ec293513783e43bb9e4cbcc8aa868fbc Mon Sep 17 00:00:00 2001 From: Janne Mareike Koschinski Date: Mon, 22 Aug 2022 13:48:54 +0200 Subject: [PATCH] Reduce amount of requests done by the onboarding task list (#9194) * Significantly reduce work of useUserOnboardingContext * Wrap UserOnboardingButton to avoid unnecessary work when it's not shown * Remove progress from user onboarding button --- .../user-onboarding/user-onboarding-new.ts | 13 +- .../user-onboarding/UserOnboardingButton.tsx | 45 +++---- src/hooks/useUserOnboardingContext.ts | 124 ++++++++++++------ src/hooks/useUserOnboardingTasks.ts | 16 +-- 4 files changed, 117 insertions(+), 81 deletions(-) diff --git a/cypress/e2e/user-onboarding/user-onboarding-new.ts b/cypress/e2e/user-onboarding/user-onboarding-new.ts index cef27803ed..c6eac8ce27 100644 --- a/cypress/e2e/user-onboarding/user-onboarding-new.ts +++ b/cypress/e2e/user-onboarding/user-onboarding-new.ts @@ -80,7 +80,18 @@ describe("User Onboarding (new user)", () => { cy.get(".mx_InviteDialog_editor input").type(bot1.getUserId()); cy.get(".mx_InviteDialog_buttonAndSpinner").click(); cy.get(".mx_InviteDialog_buttonAndSpinner").should("not.exist"); - cy.get(".mx_SendMessageComposer").type("Hi!{enter}"); + const message = "Hi!"; + cy.get(".mx_SendMessageComposer").type(`${message}!{enter}`); + cy.contains(".mx_MTextBody.mx_EventTile_content", message); + cy.visit("/#/home"); + cy.get('.mx_UserOnboardingPage').should('exist'); + cy.get('.mx_UserOnboardingButton').should('exist'); + cy.get('.mx_UserOnboardingList') + .should('exist') + .should(($list) => { + const list = $list.get(0); + expect(getComputedStyle(list).opacity).to.be.eq("1"); + }); cy.get(".mx_ProgressBar").invoke("val").should("be.greaterThan", oldProgress); }); }); diff --git a/src/components/views/user-onboarding/UserOnboardingButton.tsx b/src/components/views/user-onboarding/UserOnboardingButton.tsx index 26904829e8..ba2189370a 100644 --- a/src/components/views/user-onboarding/UserOnboardingButton.tsx +++ b/src/components/views/user-onboarding/UserOnboardingButton.tsx @@ -20,62 +20,55 @@ import React, { useCallback } from "react"; import { Action } from "../../../dispatcher/actions"; import defaultDispatcher from "../../../dispatcher/dispatcher"; import { useSettingValue } from "../../../hooks/useSettings"; -import { useUserOnboardingContext } from "../../../hooks/useUserOnboardingContext"; -import { useUserOnboardingTasks } from "../../../hooks/useUserOnboardingTasks"; import { _t } from "../../../languageHandler"; import PosthogTrackers from "../../../PosthogTrackers"; import { UseCase } from "../../../settings/enums/UseCase"; import { SettingLevel } from "../../../settings/SettingLevel"; import SettingsStore from "../../../settings/SettingsStore"; import AccessibleButton, { ButtonEvent } from "../../views/elements/AccessibleButton"; -import ProgressBar from "../../views/elements/ProgressBar"; import Heading from "../../views/typography/Heading"; import { showUserOnboardingPage } from "./UserOnboardingPage"; -function toPercentage(progress: number): string { - return (progress * 100).toFixed(0); -} - interface Props { selected: boolean; minimized: boolean; } export function UserOnboardingButton({ selected, minimized }: Props) { - const context = useUserOnboardingContext(); - const [completedTasks, waitingTasks] = useUserOnboardingTasks(context); + const useCase = useSettingValue("FTUE.useCaseSelection"); + const visible = useSettingValue("FTUE.userOnboardingButton"); - const completed = completedTasks.length; - const waiting = waitingTasks.length; - const total = completed + waiting; - - let progress = 1; - if (context && waiting) { - progress = completed / total; + if (!visible || minimized || !showUserOnboardingPage(useCase)) { + return null; } + return ( + + ); +} + +function UserOnboardingButtonInternal({ selected, minimized }: Props) { const onDismiss = useCallback((ev: ButtonEvent) => { + ev.preventDefault(); + ev.stopPropagation(); + PosthogTrackers.trackInteraction("WebRoomListUserOnboardingIgnoreButton", ev); SettingsStore.setValue("FTUE.userOnboardingButton", null, SettingLevel.ACCOUNT, false); }, []); const onClick = useCallback((ev: ButtonEvent) => { + ev.preventDefault(); + ev.stopPropagation(); + PosthogTrackers.trackInteraction("WebRoomListUserOnboardingButton", ev); defaultDispatcher.fire(Action.ViewHomePage); }, []); - const useCase = useSettingValue("FTUE.useCaseSelection"); - const visible = useSettingValue("FTUE.userOnboardingButton"); - if (!visible || minimized || !showUserOnboardingPage(useCase)) { - return null; - } - return ( { !minimized && ( @@ -84,17 +77,11 @@ export function UserOnboardingButton({ selected, minimized }: Props) { { _t("Welcome") } - { context && !completed && ( -
- { toPercentage(progress) }% -
- ) } - ) } diff --git a/src/hooks/useUserOnboardingContext.ts b/src/hooks/useUserOnboardingContext.ts index d1f45217f8..90d1eb09c6 100644 --- a/src/hooks/useUserOnboardingContext.ts +++ b/src/hooks/useUserOnboardingContext.ts @@ -15,54 +15,96 @@ limitations under the License. */ import { logger } from "matrix-js-sdk/src/logger"; -import { ClientEvent, IMyDevice, Room } from "matrix-js-sdk/src/matrix"; -import { useCallback, useEffect, useState } from "react"; +import { ClientEvent, MatrixClient } from "matrix-js-sdk/src/matrix"; +import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { MatrixClientPeg } from "../MatrixClientPeg"; +import { Notifier } from "../Notifier"; import DMRoomMap from "../utils/DMRoomMap"; -import { useEventEmitter } from "./useEventEmitter"; export interface UserOnboardingContext { - avatar: string | null; - myDevice: string; - devices: IMyDevice[]; - dmRooms: {[userId: string]: Room}; + hasAvatar: boolean; + hasDevices: boolean; + hasDmRooms: boolean; + hasNotificationsEnabled: boolean; +} + +const USER_ONBOARDING_CONTEXT_INTERVAL = 5000; + +/** + * Returns a persistent, non-changing reference to a function + * This function proxies all its calls to the current value of the given input callback + * + * This allows you to use the current value of e.g., a state in a callback that’s used by e.g., a useEventEmitter or + * similar hook without re-registering the hook when the state changes + * @param value changing callback + */ +function useRefOf(value: (...values: T) => R): (...values: T) => R { + const ref = useRef(value); + ref.current = value; + return useCallback( + (...values: T) => ref.current(...values), + [], + ); +} + +function useUserOnboardingContextValue(defaultValue: T, callback: (cli: MatrixClient) => Promise): T { + const [value, setValue] = useState(defaultValue); + const cli = MatrixClientPeg.get(); + + const handler = useRefOf(callback); + + useEffect(() => { + if (value) { + return; + } + + let handle: number | null = null; + let enabled = true; + const repeater = async () => { + if (handle !== null) { + clearTimeout(handle); + handle = null; + } + setValue(await handler(cli)); + if (enabled) { + handle = setTimeout(repeater, USER_ONBOARDING_CONTEXT_INTERVAL); + } + }; + repeater().catch(err => logger.warn("could not update user onboarding context", err)); + cli.on(ClientEvent.AccountData, repeater); + return () => { + enabled = false; + cli.off(ClientEvent.AccountData, repeater); + if (handle !== null) { + clearTimeout(handle); + handle = null; + } + }; + }, [cli, handler, value]); + return value; } export function useUserOnboardingContext(): UserOnboardingContext | null { - const [context, setContext] = useState(null); + const hasAvatar = useUserOnboardingContextValue(false, async (cli) => { + const profile = await cli.getProfileInfo(cli.getUserId()); + return Boolean(profile?.avatar_url); + }); + const hasDevices = useUserOnboardingContextValue(false, async (cli) => { + const myDevice = cli.getDeviceId(); + const devices = await cli.getDevices(); + return Boolean(devices.devices.find(device => device.device_id !== myDevice)); + }); + const hasDmRooms = useUserOnboardingContextValue(false, async () => { + const dmRooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals() ?? {}; + return Boolean(Object.keys(dmRooms).length); + }); + const hasNotificationsEnabled = useUserOnboardingContextValue(false, async () => { + return Notifier.isPossible(); + }); - const cli = MatrixClientPeg.get(); - const handler = useCallback(async () => { - try { - const profile = await cli.getProfileInfo(cli.getUserId()); - - const myDevice = cli.getDeviceId(); - const devices = await cli.getDevices(); - - const dmRooms = DMRoomMap.shared().getUniqueRoomsWithIndividuals() ?? {}; - setContext({ - avatar: profile?.avatar_url ?? null, - myDevice, - devices: devices.devices, - dmRooms: dmRooms, - }); - } catch (e) { - logger.warn("Could not load context for user onboarding task list: ", e); - setContext(null); - } - }, [cli]); - - useEventEmitter(cli, ClientEvent.AccountData, handler); - useEffect(() => { - const handle = setInterval(handler, 2000); - handler(); - return () => { - if (handle) { - clearInterval(handle); - } - }; - }, [handler]); - - return context; + return useMemo( + () => ({ hasAvatar, hasDevices, hasDmRooms, hasNotificationsEnabled }), + [hasAvatar, hasDevices, hasDmRooms, hasNotificationsEnabled], + ); } diff --git a/src/hooks/useUserOnboardingTasks.ts b/src/hooks/useUserOnboardingTasks.ts index e4499cdb32..65fb032d22 100644 --- a/src/hooks/useUserOnboardingTasks.ts +++ b/src/hooks/useUserOnboardingTasks.ts @@ -47,8 +47,6 @@ interface InternalUserOnboardingTask extends UserOnboardingTask { completed: (ctx: UserOnboardingContext) => boolean; } -const hasOpenDMs = (ctx: UserOnboardingContext) => Boolean(Object.entries(ctx.dmRooms).length); - const onClickStartDm = (ev: ButtonEvent) => { PosthogTrackers.trackInteraction("WebUserOnboardingTaskSendDm", ev); defaultDispatcher.dispatch({ action: 'view_create_chat' }); @@ -65,7 +63,7 @@ const tasks: InternalUserOnboardingTask[] = [ id: "find-friends", title: _t("Find and invite your friends"), description: _t("It’s what you’re here for, so lets get to it"), - completed: hasOpenDMs, + completed: (ctx: UserOnboardingContext) => ctx.hasDmRooms, relevant: [UseCase.PersonalMessaging, UseCase.Skip], action: { label: _t("Find friends"), @@ -76,7 +74,7 @@ const tasks: InternalUserOnboardingTask[] = [ id: "find-coworkers", title: _t("Find and invite your co-workers"), description: _t("Get stuff done by finding your teammates"), - completed: hasOpenDMs, + completed: (ctx: UserOnboardingContext) => ctx.hasDmRooms, relevant: [UseCase.WorkMessaging], action: { label: _t("Find people"), @@ -87,7 +85,7 @@ const tasks: InternalUserOnboardingTask[] = [ id: "find-community-members", title: _t("Find and invite your community members"), description: _t("Get stuff done by finding your teammates"), - completed: hasOpenDMs, + completed: (ctx: UserOnboardingContext) => ctx.hasDmRooms, relevant: [UseCase.CommunityMessaging], action: { label: _t("Find people"), @@ -102,9 +100,7 @@ const tasks: InternalUserOnboardingTask[] = [ description: () => _t("Don’t miss a thing by taking %(brand)s with you", { brand: SdkConfig.get("brand"), }), - completed: (ctx: UserOnboardingContext) => { - return Boolean(ctx.devices.filter(it => it.device_id !== ctx.myDevice).length); - }, + completed: (ctx: UserOnboardingContext) => ctx.hasDevices, action: { label: _t("Download apps"), onClick: (ev: ButtonEvent) => { @@ -117,7 +113,7 @@ const tasks: InternalUserOnboardingTask[] = [ id: "setup-profile", title: _t("Set up your profile"), description: _t("Make sure people know it’s really you"), - completed: (info: UserOnboardingContext) => Boolean(info.avatar), + completed: (ctx: UserOnboardingContext) => ctx.hasAvatar, action: { label: _t("Your profile"), onClick: (ev: ButtonEvent) => { @@ -133,7 +129,7 @@ const tasks: InternalUserOnboardingTask[] = [ id: "permission-notifications", title: _t("Turn on notifications"), description: _t("Don’t miss a reply or important message"), - completed: () => Notifier.isPossible(), + completed: (ctx: UserOnboardingContext) => ctx.hasNotificationsEnabled, action: { label: _t("Enable notifications"), onClick: (ev: ButtonEvent) => {