Skip to content

fix(onboarding): resume onboarding after the import-to-Astra detour - #338

Merged
thomasluizon merged 1 commit into
mainfrom
fix/onboarding-import-resume
Jun 27, 2026
Merged

fix(onboarding): resume onboarding after the import-to-Astra detour#338
thomasluizon merged 1 commit into
mainfrom
fix/onboarding-import-resume

Conversation

@thomasluizon

Copy link
Copy Markdown
Owner

Onboarding resumes after the import detour (fixes the #333 regression)

Re-fixes the bug from #333: clicking "Import from another app" during onboarding routes to Astra, but leaving Astra left the user stranded on an empty Today — onboarding neither completed nor resumed.

Root cause

#333 added a sticky in-memory onboardingHandedOff flag (shared ui-store) to hide the onboarding overlay during the Astra detour — but it never reset, so the overlay stayed permanently suppressed after returning.

Fix — route-scoped suppression

Hide the onboarding overlay only while on the chat/Astra route; show it again (while !hasCompletedOnboarding) everywhere else.

  • Web: OnboardingFlow lives in (app) layout and /chat is a separate (chat) route group, so the overlay already unmounts on chat + remounts on return — removing the flag is the whole fix.
  • Mobile: OnboardingFlow is a <Modal> in the always-mounted root layout, so it needs a usePathname() guard (/chat → null).

Import still does NOT complete onboarding, so the first-run coach tour stays gated and never fires mid-detour (#333's tour-gating intact).

Tests

Pin the regression: import routes to /chat without completing onboarding; returning re-shows the onboarding overlay (web remount; mobile route-scoped).

Validation

type-check 3/3; lint 0 errors; tests shared 1235, mobile 718, web 1745.

Note: the server-fetch 401-retry tests are flaky under concurrent suite runs (timing) — they pass in isolation / full-suite-alone; unrelated to this change.

🤖 Generated with Claude Code

#333 hid the onboarding overlay during the "Import from another app"
handoff with a sticky in-memory `onboardingHandedOff` UI flag that never
reset, stranding users on an empty Today (onboarding neither completed
nor resumed) once they left Astra.

Replace the one-way flag with route-scoped suppression so the overlay
hides only while on the chat/Astra screen and reappears (onboarding
still incomplete) on every other route:

- shared: drop the `onboardingHandedOff` flag + setter from ui-store.
- web: the overlay lives in the (app) layout and already unmounts when
  navigating into the (chat) route group, so removing the flag is enough
  -- returning to (app) remounts it while onboarding is incomplete.
- mobile: the overlay is a Modal in the always-mounted root layout, so
  gate it on the active route (usePathname) -- hidden on /chat, shown
  elsewhere.

Import still does NOT complete onboarding, so the first-run coach tour
stays gated on genuine completion and never fires mid-detour.

Tests: web + mobile import handoff routes to chat without completing
onboarding; returning from chat re-shows the overlay (the stranded-user
regression); shared/web/mobile suites green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 27, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
orbit-ui-mobile-web Ignored Ignored Jun 27, 2026 9:32pm

Request Review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR #338 — fix(onboarding): resume onboarding after the import-to-Astra detour

Recommendation: APPROVE

Summary

Fixes the regression where onboardingHandedOff never reset, permanently suppressing the overlay after the Astra import detour. The flag and its setter are fully removed from the shared store; web relies on its natural unmount/remount lifecycle, and mobile gets a usePathname() guard that returns null only while on /chat. Both platforms gain regression-pinning tests.

Findings

Critical: None
High: None
Medium: None
Low / Info: None

Subagents

Agent Verdict
parity-checker PAIRED — both platforms have equivalent changes; web unmount vs mobile pathname guard is correct platform-specific difference
i18n-syncer IN SYNC — no keys added or removed
contract-aligner N/A — onboardingHandedOff was never in PersistedUIState or any API DTO; ephemeral runtime state only
security-reviewer N/A — no orbit-api changes

Validation

Check Result
Lint PASS (per PR: 0 errors)
Type check PASS (per PR: 3/3)
Tests PASS (per PR: shared 1235, mobile 718, web 1745)
Build (api) N/A

Backward-compat guard

onboardingHandedOff was never part of PersistedUIState — not serialized to AsyncStorage or localStorage, not in any DTO. Safe to remove with no migration step. No old mobile client breaks.

Files verdicted

  • apps/mobile/__tests__/components/onboarding/onboarding-flow.test.tsx — correct mobile regression tests; PASS
  • apps/mobile/components/onboarding/onboarding-flow.tsx — correct pathname guard; PASS
  • apps/web/__tests__/components/onboarding/onboarding-flow.test.tsx — correct web regression tests; PASS
  • apps/web/components/onboarding/onboarding-flow.tsx — correct flag removal; PASS
  • packages/shared/src/stores/ui-store.ts — clean dead-state removal; PASS

Deferred: Backend hard rules, security (API), contract drift, DESIGN.md — all N/A (no orbit-api or UI rendering changes).

What is good

Root-cause fix — removes the broken flag rather than patching around it. Platform-appropriate implementations: web unmounts naturally on route change, mobile gets the usePathname() guard. startsWith('/chat') is correct (only one chat route exists: app/chat.tsx). No storage migration needed (field was never persisted). Regression tests are well-scoped at the behavior layer. Zero references to the removed flag remain in the codebase.

@sonarqubecloud

Copy link
Copy Markdown

@thomasluizon
thomasluizon merged commit 510d8c7 into main Jun 27, 2026
10 checks passed
@thomasluizon
thomasluizon deleted the fix/onboarding-import-resume branch June 27, 2026 21:56
thomasluizon added a commit that referenced this pull request Aug 19, 2026
…he five missing surfaces (#36)

The canvas is the design of record now, and nothing said so where a worker
would read it. /orchestrate reads the ticket, not the project.

Twenty one existing tickets gained a comment naming the document that defines
their surface. Their bodies were already corrected against D69 on 2026-08-16,
so the comment adds the drawing rather than restating the job, and says plainly
that the document is the later artefact where the two differ.

Five surfaces gained a document in this run and had no ticket at all:

  #335  the notification bell and its list          Orbit Avisos
  #336  search results, including which matched     Orbit Busca
  #337  the step up code screen                     Orbit Verificacao
  #338  the error and static surfaces               Orbit Estados
  #339  offline, and the dropped change             Orbit Offline

#335 is ordered behind #334, because rewriting the client while the server
still writes /streak leaves the two disagreeing. #336 needs no API work at all:
the server already returns searchMatches and computeHabitMatchBadges has zero
consumers, so the app has been throwing away a better result than it draws.

D4, the fourth system round, landed five of nine. The canvas read the whole
brief, judged it lacked the headroom to build nine components plus their cards
without leaving the system half edited, and wrote nothing that round rather
than risk it. Sheet's mount contract, CapacityNotice's body, the Skeleton grid
variant, the ListRow read only variant and EventRow are done. DayCell and
MonthGrid, OtpInput, Pager and Columns stay on its todo list.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant