From 35411bd76e0fbf27ae5ef0184b034e9a2b6abe9f Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Tue, 26 May 2026 01:49:39 -0700 Subject: [PATCH] Confirm onboarding dismissal --- .../onboarding/OnboardingFlow.test.tsx | 10 ++++ .../components/onboarding/OnboardingFlow.tsx | 52 ++++++++++++++++++- .../OnboardingSkipConfirmationDialog.tsx | 45 ++++++++++++++++ .../use-onboarding-flow-persistence.test.ts | 44 ++++++++++++++++ .../use-onboarding-flow-persistence.ts | 35 +++++++++---- .../onboarding/use-onboarding-flow.ts | 19 +++++++ src/shared/telemetry-events.ts | 7 ++- 7 files changed, 196 insertions(+), 16 deletions(-) create mode 100644 src/renderer/src/components/onboarding/OnboardingSkipConfirmationDialog.tsx create mode 100644 src/renderer/src/components/onboarding/use-onboarding-flow-persistence.test.ts diff --git a/src/renderer/src/components/onboarding/OnboardingFlow.test.tsx b/src/renderer/src/components/onboarding/OnboardingFlow.test.tsx index 1b560c451..df74d4760 100644 --- a/src/renderer/src/components/onboarding/OnboardingFlow.test.tsx +++ b/src/renderer/src/components/onboarding/OnboardingFlow.test.tsx @@ -3,6 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { getDefaultOnboardingState, getDefaultSettings } from '../../../../shared/constants' import { useAppStore } from '@/store' import OnboardingFlow from './OnboardingFlow' +import { ONBOARDING_SKIP_CONFIRMATION_COPY } from './OnboardingSkipConfirmationDialog' describe('OnboardingFlow', () => { beforeEach(() => { @@ -77,4 +78,13 @@ describe('OnboardingFlow', () => { expect(html).not.toContain('min-h-screen') expect(html).not.toContain('background-color:#12181e') }) + + it('renders concise skip confirmation copy', () => { + expect(ONBOARDING_SKIP_CONFIRMATION_COPY).toEqual({ + title: 'Skip onboarding?', + description: "It won't take long!", + skipLabel: 'Skip', + keepGoingLabel: 'No, keep going' + }) + }) }) diff --git a/src/renderer/src/components/onboarding/OnboardingFlow.tsx b/src/renderer/src/components/onboarding/OnboardingFlow.tsx index 7ee847501..b6a87412f 100644 --- a/src/renderer/src/components/onboarding/OnboardingFlow.tsx +++ b/src/renderer/src/components/onboarding/OnboardingFlow.tsx @@ -1,4 +1,4 @@ -import { useEffect } from 'react' +import { useCallback, useEffect, useRef, useState } from 'react' import { ChevronLeft, CornerDownLeft, Loader2 } from 'lucide-react' import { cn } from '@/lib/utils' import { isEditableTarget } from '@/lib/editable-target' @@ -13,6 +13,7 @@ import { IntegrationsStep } from './IntegrationsStep' import { RepoStep } from './RepoStep' import { OnboardingTourStep } from './OnboardingTourStep' import { STEPS, useOnboardingFlow } from './use-onboarding-flow' +import { OnboardingSkipConfirmationDialog } from './OnboardingSkipConfirmationDialog' import logo from '../../../../../resources/logo.svg' const stepCopy = { @@ -81,13 +82,33 @@ export default function OnboardingFlow({ const shouldShowFooterBusy = Boolean(busyLabel) && currentStep.id !== 'agentSetup' const footerPrimaryLabel = currentStep.id === 'agentSetup' ? 'Continue' : (busyLabel ?? 'Continue') + const [skipConfirmOpen, setSkipConfirmOpen] = useState(false) + const skipConfirmAdvancedViaRef = useRef<'button' | 'keyboard'>('button') const { next: flowNext, openFolder: flowOpenFolder, continueWithExistingProject: flowContinueWithExistingProject, - skipTourToRepo: flowSkipTourToRepo + skipTourToRepo: flowSkipTourToRepo, + dismissOnboarding: flowDismissOnboarding } = flow + const requestSkipConfirmation = useCallback( + (advancedVia: 'button' | 'keyboard') => { + if (busyLabel || skipConfirmOpen) { + return + } + skipConfirmAdvancedViaRef.current = advancedVia + setSkipConfirmOpen(true) + }, + [busyLabel, skipConfirmOpen] + ) + + const confirmSkipOnboarding = useCallback(() => { + const advancedVia = skipConfirmAdvancedViaRef.current + setSkipConfirmOpen(false) + void flowDismissOnboarding(advancedVia) + }, [flowDismissOnboarding]) + // Why: depend on stable callbacks + step id only so the listener doesn't // re-bind on every render of the parent (flow object identity changes). useEffect(() => { @@ -134,10 +155,32 @@ export default function OnboardingFlow({ tourStarted ]) + useEffect(() => { + const onKeyDown = (event: KeyboardEvent): void => { + if (event.key !== 'Escape' || skipConfirmOpen) { + return + } + event.preventDefault() + requestSkipConfirmation('keyboard') + } + window.addEventListener('keydown', onKeyDown, { capture: true }) + return () => window.removeEventListener('keydown', onKeyDown, { capture: true }) + }, [requestSkipConfirmation, skipConfirmOpen]) + return (
{ + if (event.button !== 0) { + return + } + const target = event.target + if (!(target instanceof Element) || target.closest('[data-onboarding-modal]')) { + return + } + requestSkipConfirmation('button') + }} >
+
) } diff --git a/src/renderer/src/components/onboarding/OnboardingSkipConfirmationDialog.tsx b/src/renderer/src/components/onboarding/OnboardingSkipConfirmationDialog.tsx new file mode 100644 index 000000000..19c9cdcc5 --- /dev/null +++ b/src/renderer/src/components/onboarding/OnboardingSkipConfirmationDialog.tsx @@ -0,0 +1,45 @@ +import { Button } from '@/components/ui/button' +import { + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle +} from '@/components/ui/dialog' + +export const ONBOARDING_SKIP_CONFIRMATION_COPY = { + title: 'Skip onboarding?', + description: "It won't take long!", + skipLabel: 'Skip', + keepGoingLabel: 'No, keep going' +} as const + +export function OnboardingSkipConfirmationDialog(props: { + open: boolean + onOpenChange: (open: boolean) => void + onSkip: () => void +}): React.JSX.Element { + return ( + + + + {ONBOARDING_SKIP_CONFIRMATION_COPY.title} + {ONBOARDING_SKIP_CONFIRMATION_COPY.description} + + + + + + + + ) +} diff --git a/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.test.ts b/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.test.ts new file mode 100644 index 000000000..b37b9a817 --- /dev/null +++ b/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.test.ts @@ -0,0 +1,44 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' + +const trackMock = vi.hoisted(() => vi.fn()) + +vi.mock('@/lib/telemetry', () => ({ + track: trackMock +})) + +import { + buildOnboardingDismissedPayload, + trackOnboardingDismissed +} from './use-onboarding-flow-persistence' + +describe('onboarding flow persistence', () => { + beforeEach(() => { + trackMock.mockClear() + }) + + it('builds dismissed telemetry with the triggering advance path', () => { + expect( + buildOnboardingDismissedPayload(3, { + durationMs: 250, + advancedVia: 'keyboard' + }) + ).toEqual({ + last_step: 3, + duration_ms: 250, + advanced_via: 'keyboard' + }) + }) + + it('tracks dismissed onboarding telemetry with the triggering advance path', () => { + trackOnboardingDismissed(3, { + durationMs: 250, + advancedVia: 'keyboard' + }) + + expect(trackMock).toHaveBeenCalledWith('onboarding_dismissed', { + last_step: 3, + duration_ms: 250, + advanced_via: 'keyboard' + }) + }) +}) diff --git a/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts b/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts index e5ee4e975..65f46f644 100644 --- a/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts +++ b/src/renderer/src/components/onboarding/use-onboarding-flow-persistence.ts @@ -2,6 +2,7 @@ import { useCallback } from 'react' import { toast } from 'sonner' import { track } from '@/lib/telemetry' import { ONBOARDING_FINAL_STEP } from '../../../../shared/constants' +import type { EventProps } from '../../../../shared/telemetry-events' import type { GlobalSettings, OnboardingState, TuiAgent } from '../../../../shared/types' import { hasSelectedOnboardingFeatureSetup, @@ -34,10 +35,32 @@ type CloseWithDeps = { } export type DismissedExtras = { - advancedVia: 'button' | 'keyboard' + advancedVia: NonNullable['advanced_via']> durationMs: number } +export function buildOnboardingDismissedPayload( + lastStepReached: StepNumber, + dismissedExtras?: DismissedExtras +): EventProps<'onboarding_dismissed'> { + return { + last_step: lastStepReached, + ...(dismissedExtras + ? { + duration_ms: dismissedExtras.durationMs, + advanced_via: dismissedExtras.advancedVia + } + : {}) + } +} + +export function trackOnboardingDismissed( + lastStepReached: StepNumber, + dismissedExtras?: DismissedExtras +): void { + track('onboarding_dismissed', buildOnboardingDismissedPayload(lastStepReached, dismissedExtras)) +} + export function useCloseWith({ onOnboardingChange, onboardingChecklist, @@ -95,15 +118,7 @@ export function useCloseWith({ }) } } else if (outcome === 'dismissed') { - track('onboarding_dismissed', { - last_step: lastStepReached, - ...(dismissedExtras - ? { - duration_ms: dismissedExtras.durationMs, - advanced_via: dismissedExtras.advancedVia - } - : {}) - }) + trackOnboardingDismissed(lastStepReached, dismissedExtras) } return true }, diff --git a/src/renderer/src/components/onboarding/use-onboarding-flow.ts b/src/renderer/src/components/onboarding/use-onboarding-flow.ts index 07b28efd8..65c1d2e3d 100644 --- a/src/renderer/src/components/onboarding/use-onboarding-flow.ts +++ b/src/renderer/src/components/onboarding/use-onboarding-flow.ts @@ -770,6 +770,24 @@ export function useOnboardingFlow( updateSettings ]) + const dismissOnboarding = useCallback( + async (advancedVia: 'button' | 'keyboard' = 'button'): Promise => { + if (busyLabel) { + return false + } + setError(null) + const closed = await closeWith('dismissed', {}, currentStep.stepNumber, undefined, { + durationMs: consumeStepDurationMs(), + advancedVia + }) + if (closed) { + emitPendingTourOutcome() + } + return closed + }, + [busyLabel, closeWith, consumeStepDurationMs, currentStep.stepNumber, emitPendingTourOutcome] + ) + const startTour = useCallback(() => { if (busyLabel) { return @@ -983,6 +1001,7 @@ export function useOnboardingFlow( startFeatureSetup, skipAgentSetup, skipToRepo, + dismissOnboarding, startTour, completeTour, skipTourToRepo, diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 8378de8a8..3f802752c 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -581,10 +581,9 @@ void _onboardingChecklistItemSyncCheck const cohortSchema = z.enum(['fresh_install', 'upgrade_backfill']).optional() // `'button' | 'keyboard'` records whether the user advanced via a footer -// button click or via Cmd/Ctrl+Enter. Skip and dismiss don't have a keyboard -// path today (the field will only ever be `'button'` for those events) but -// the uniform shape lets a future keyboard skip arrive without a schema -// migration. +// button click, Cmd/Ctrl+Enter, or an equivalent keyboard exit like Escape. +// The uniform shape lets keyboard skip/dismiss paths arrive without a +// schema migration. const advancedViaSchema = z.enum(['button', 'keyboard']).optional() const onboardingStartedSchema = z