Skip to content

Commit 7148725

Browse files
committed
refactor(onboarding): streamline CmdPaletteGuide and onboarding step configurations
- Removed deprecated visibility rules and unnecessary tests from CmdPaletteGuide to simplify the onboarding logic. - Updated onboarding step configurations to include route prefixes for better route detection. - Enhanced the CmdPaletteGuide component to conditionally render based on the onboarding state and user progress. - Adjusted tests to reflect changes in the onboarding flow and ensure accurate rendering of instructions based on the current view.
1 parent 38139f8 commit 7148725

File tree

5 files changed

+18
-105
lines changed

5 files changed

+18
-105
lines changed

packages/web/src/views/Onboarding/components/CmdPaletteGuide.test.tsx

Lines changed: 3 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -286,12 +286,11 @@ describe("CmdPaletteGuide", () => {
286286
// Should show step 1 instructions instead since step 1 wasn't completed
287287
expect(screen.getByText("Welcome to the Day View")).toBeInTheDocument();
288288
expect(
289-
screen.getByText(
289+
screen.getAllByText(
290290
(_, element) =>
291-
element?.textContent === "Press 2 to go to the Day view" ?? false,
292-
),
291+
element?.textContent === "You're already on the Day view." ?? false,
292+
)[0],
293293
).toBeInTheDocument();
294-
expect(screen.getByText("2")).toBeInTheDocument();
295294
expect(screen.getByText("Step 1 of 6")).toBeInTheDocument();
296295
});
297296

@@ -348,30 +347,6 @@ describe("CmdPaletteGuide", () => {
348347
expect(screen.getByText("Step 5 of 6")).toBeInTheDocument();
349348
});
350349

351-
it("should not render step 3 on Day view", () => {
352-
mockUseLocation.mockReturnValue({ pathname: "/day" } as any);
353-
markStepCompleted(ONBOARDING_STEPS.NAVIGATE_TO_DAY);
354-
markStepCompleted(ONBOARDING_STEPS.CREATE_TASK);
355-
markStepCompleted(ONBOARDING_STEPS.NAVIGATE_TO_NOW);
356-
mockUseCmdPaletteGuide.mockReturnValue({
357-
currentStep: ONBOARDING_STEPS.EDIT_DESCRIPTION,
358-
isGuideActive: true,
359-
completeStep: jest.fn(),
360-
skipGuide: jest.fn(),
361-
completeGuide: jest.fn(),
362-
});
363-
364-
render(<CmdPaletteGuide />);
365-
366-
expect(screen.queryByText("Welcome to Compass")).not.toBeInTheDocument();
367-
expect(
368-
screen.queryByText("Welcome to the Day View"),
369-
).not.toBeInTheDocument();
370-
expect(
371-
screen.queryByText("Welcome to the Now View"),
372-
).not.toBeInTheDocument();
373-
});
374-
375350
it("should render on Day view when authenticated", () => {
376351
mockUseLocation.mockReturnValue({ pathname: "/day" } as any);
377352
mockUseSession.mockReturnValue({ authenticated: true });
@@ -497,31 +472,6 @@ describe("CmdPaletteGuide", () => {
497472
expect(progressDots).toHaveLength(6);
498473
});
499474

500-
it("should not render step 4 on Day view", () => {
501-
mockUseLocation.mockReturnValue({ pathname: "/day" } as any);
502-
markStepCompleted(ONBOARDING_STEPS.NAVIGATE_TO_DAY);
503-
markStepCompleted(ONBOARDING_STEPS.CREATE_TASK);
504-
markStepCompleted(ONBOARDING_STEPS.NAVIGATE_TO_NOW);
505-
markStepCompleted(ONBOARDING_STEPS.EDIT_DESCRIPTION);
506-
mockUseCmdPaletteGuide.mockReturnValue({
507-
currentStep: ONBOARDING_STEPS.EDIT_REMINDER,
508-
isGuideActive: true,
509-
completeStep: jest.fn(),
510-
skipGuide: jest.fn(),
511-
completeGuide: jest.fn(),
512-
});
513-
514-
render(<CmdPaletteGuide />);
515-
516-
expect(screen.queryByText("Welcome to Compass")).not.toBeInTheDocument();
517-
expect(
518-
screen.queryByText("Welcome to the Day View"),
519-
).not.toBeInTheDocument();
520-
expect(
521-
screen.queryByText("Welcome to the Now View"),
522-
).not.toBeInTheDocument();
523-
});
524-
525475
it("should call unified step detection hook", () => {
526476
mockUseLocation.mockReturnValue({ pathname: "/day" } as any);
527477
mockUseCmdPaletteGuide.mockReturnValue({

packages/web/src/views/Onboarding/components/CmdPaletteGuide.tsx

Lines changed: 5 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import React, { FC, useEffect, useMemo, useRef, useState } from "react";
22
import { useLocation } from "react-router-dom";
3-
import { useSession } from "@web/common/hooks/useSession";
43
import { getModifierKey } from "@web/common/utils/shortcut/shortcut.util";
54
import {
65
ONBOARDING_GUIDE_VIEWS,
@@ -18,8 +17,7 @@ import {
1817

1918
export const CmdPaletteGuide: FC = () => {
2019
const location = useLocation();
21-
const { authenticated } = useSession();
22-
const { currentStep, isGuideActive, completeStep, skipGuide } =
20+
const { currentStep, completeStep, skipGuide, isGuideActive } =
2321
useCmdPaletteGuide();
2422
const step2CompletionTimeoutRef = useRef<NodeJS.Timeout | null>(null);
2523
const [isSuccessMessageDismissed, setIsSuccessMessageDismissed] =
@@ -79,23 +77,13 @@ export const CmdPaletteGuide: FC = () => {
7977
isStepCompleted(ONBOARDING_STEPS.NAVIGATE_TO_WEEK) &&
8078
!isSuccessMessageDismissed;
8179

82-
// Determine if we should show the overlay
83-
// Show success message if final step is completed, OR show guide if active with a step
84-
// But respect view restrictions: step 1 on any view, step 2 on day/now, steps 3/4 only on now, step 5 on any
80+
if (!isGuideActive && !showSuccessMessage) {
81+
return null;
82+
}
83+
8584
const actualStepConfig = actualStep
8685
? ONBOARDING_STEP_CONFIGS.find((config) => config.id === actualStep)
8786
: null;
88-
const visibilityByAuth = authenticated
89-
? actualStepConfig?.guide.visibilityByAuth.authenticated
90-
: actualStepConfig?.guide.visibilityByAuth.unauthenticated;
91-
const canRenderStep = visibilityByAuth?.includes(currentView) ?? false;
92-
const shouldShowOverlay =
93-
showSuccessMessage ||
94-
(isGuideActive && actualStep !== null && canRenderStep);
95-
96-
if (!shouldShowOverlay) {
97-
return null;
98-
}
9987

10088
const modifierKey = getModifierKey();
10189
const modifierKeyDisplay = modifierKey === "Meta" ? "⌘" : "Ctrl";

packages/web/src/views/Onboarding/constants/onboarding.constants.ts

Lines changed: 3 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -27,13 +27,10 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
2727
id: ONBOARDING_STEPS.NAVIGATE_TO_DAY,
2828
order: 0,
2929
detectionType: "route",
30-
detectionConfig: { route: "/day" },
30+
detectionConfig: { route: "/day", routePrefixes: ["/day/"] },
3131
guide: {
32-
visibilityByAuth: {
33-
authenticated: ["day", "now", "week"],
34-
unauthenticated: ["day", "now", "week"],
35-
},
3632
instructionsByView: {
33+
day: [{ type: "text", value: "You're already on the Day view." }],
3734
default: [
3835
{ type: "text", value: "Press " },
3936
{ type: "kbd", value: "2" },
@@ -47,10 +44,6 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
4744
order: 1,
4845
detectionType: "task-count",
4946
guide: {
50-
visibilityByAuth: {
51-
authenticated: ["day"],
52-
unauthenticated: ["day"],
53-
},
5447
instructionsByView: {
5548
day: [
5649
{ type: "text", value: "Type " },
@@ -64,12 +57,8 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
6457
id: ONBOARDING_STEPS.NAVIGATE_TO_NOW,
6558
order: 2,
6659
detectionType: "route",
67-
detectionConfig: { route: "/now" },
60+
detectionConfig: { route: "/now", routePrefixes: ["/now/"] },
6861
guide: {
69-
visibilityByAuth: {
70-
authenticated: ["now", "week"],
71-
unauthenticated: ["day", "now", "week"],
72-
},
7362
instructionsByView: {
7463
default: [
7564
{ type: "text", value: "Press " },
@@ -84,10 +73,6 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
8473
order: 3,
8574
detectionType: "task-description",
8675
guide: {
87-
visibilityByAuth: {
88-
authenticated: ["now"],
89-
unauthenticated: ["now"],
90-
},
9176
instructionsByView: {
9277
default: [
9378
{ type: "text", value: "Press " },
@@ -102,10 +87,6 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
10287
order: 4,
10388
detectionType: "reminder-poll",
10489
guide: {
105-
visibilityByAuth: {
106-
authenticated: ["now"],
107-
unauthenticated: ["now"],
108-
},
10990
instructionsByView: {
11091
default: [
11192
{ type: "text", value: "Press " },
@@ -121,10 +102,6 @@ export const ONBOARDING_STEP_CONFIGS: readonly OnboardingStepConfig[] = [
121102
detectionType: "route",
122103
detectionConfig: { route: "/" },
123104
guide: {
124-
visibilityByAuth: {
125-
authenticated: ["day", "now", "week"],
126-
unauthenticated: ["day", "now", "week"],
127-
},
128105
instructionsByView: {
129106
default: [
130107
{ type: "text", value: "Type " },

packages/web/src/views/Onboarding/hooks/useStepDetection.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,21 +204,23 @@ export function useStepDetection({
204204

205205
case "route": {
206206
const routeConfig = stepConfig.detectionConfig as
207-
| { route: string }
207+
| { route: string; routePrefixes?: string[] }
208208
| undefined;
209209
if (!routeConfig) return;
210210

211211
// Normalize route paths for comparison
212212
const targetRoute =
213213
routeConfig.route === "/" ? ROOT_ROUTES.ROOT : routeConfig.route;
214214
const currentPath = location.pathname;
215+
const matchesPrefix =
216+
routeConfig.routePrefixes?.some((prefix) =>
217+
currentPath.startsWith(prefix),
218+
) ?? false;
215219

216220
// Check if we're on the target route
217221
if (
218222
!hasCompletedRef.current &&
219-
(currentPath === targetRoute ||
220-
(targetRoute === ROOT_ROUTES.NOW &&
221-
currentPath.startsWith(`${ROOT_ROUTES.NOW}/`)))
223+
(currentPath === targetRoute || matchesPrefix)
222224
) {
223225
hasCompletedRef.current = true;
224226
onStepComplete(currentStep);

packages/web/src/views/Onboarding/types/onboarding.guide.types.ts

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,8 @@ export interface OnboardingStepConfig {
4848
id: OnboardingStepName;
4949
order: number;
5050
detectionType: StepDetectionType;
51-
detectionConfig?: { route: string };
51+
detectionConfig?: { route: string; routePrefixes?: string[] };
5252
guide: {
53-
visibilityByAuth: {
54-
authenticated: OnboardingGuideView[];
55-
unauthenticated: OnboardingGuideView[];
56-
};
5753
instructionsByView: Partial<
5854
Record<OnboardingInstructionVariant, OnboardingInstructionPart[]>
5955
>;

0 commit comments

Comments
 (0)