feat: Onboarding Checklist#2421
Conversation
c455af7 to
fddfd22
Compare
🎩 PreviewA preview build has been created at: |
247aa01 to
a27cdb0
Compare
a27cdb0 to
72eadf3
Compare
65047a8 to
1823626
Compare
ebc0b79 to
3b09b65
Compare
1823626 to
f6fe1e5
Compare
3b09b65 to
30b5b30
Compare
5d180fd to
4e68a1c
Compare
f6fe1e5 to
ab71553
Compare
4e68a1c to
c818dc8
Compare
camielvs
left a comment
There was a problem hiding this comment.
Reviewed the foundation PR. Solid structure — backend-backed progress, derived vs. self-reported steps, and good test coverage (parseProgress, hooks offline/online, docs-visit tracking). A few non-blocking observations left inline.
8bdff0e to
2741cce
Compare
ab71553 to
c1e70ec
Compare
2741cce to
7e5fb3b
Compare
c1e70ec to
f8a3078
Compare
3691b8e to
12d1304
Compare
f8a3078 to
01cfb6d
Compare
| const filterQuery = filtersToFilterQuery(parseFilterParam("created_by:me")); | ||
|
|
||
| const { data } = useQuery({ | ||
| queryKey: ["onboarding", "myRunCount", backendUrl], |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
The new run-completion query uses its own key (["onboarding", "myRunCount", backendUrl]) with a 5-minute stale window and refetchOnWindowFocus: false. Existing pipeline submission invalidates queryKey: ["pipelineRuns"], so a brand-new user who starts with zero runs can submit a run and return to Learn while this query remains cached at 0. That leaves the new execute_run checklist item incomplete until the cache eventually refetches, which breaks the core onboarding flow introduced by thi…
Suggestion: Wire this query into the same invalidation path as run creation: either use/share the existing pipeline-runs query key, invalidate ["onboarding", "myRunCount"] from the submit success path, or emit a run-created event and refetch this query immediately. Add a test that starts with no runs, simulates run creation/invalidation, and verifies `execut…
Rule: Tangle UI review / General quality: TanStack Query server state must stay fresh through the relevant mutation invalidation path; review only changed code.
| queryFn: async () => { | ||
| const url = new URL(PIPELINE_RUNS_QUERY_URL, backendUrl); | ||
| if (filterQuery) url.searchParams.set("filter_query", filterQuery); | ||
| const payload = (await fetchWithErrorHandling( |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
The onboarding provider casts the raw fetchWithErrorHandling result to ListPipelineJobsResponse and then reads pipeline_runs. Project TypeScript standards avoid as Type assertions for external data because they bypass runtime validation and can hide malformed API responses.
Suggestion: Use a small parser/type guard for the response, or type the query function around a validated shape before reading pipeline_runs. For example, check that the payload is a record and pipeline_runs is an array before returning its length.
Rule: .claude/skills/typescript-standards: avoid unsafe type casting; prefer type guards/runtime validation.
| return (data ?? 0) > 0; | ||
| } | ||
|
|
||
| export function OnboardingProvider({ children }: { children: ReactNode }) { |
There was a problem hiding this comment.
🤖 This is an AI-generated code review comment.
FTA reports 53.6 with cyclomatic complexity 13 for this new provider. The file is still under the large-file threshold, but it already combines settings persistence, docs click tracking, pipeline-write events, tour-derived state, run-derived state, analytics, and context shaping. That makes future onboarding-step changes more likely to regress.
Suggestion: Split the derived step sources and actions into focused hooks/helpers, especially the run-completion query and persisted-step actions. The run-completion refetch fix above is a natural place to extract a useHasMyRun hook with an exported query key.
Rule: .tangent/skills/static-analysis: FTA 50-60 and cyclo 11-20 are medium flags that should map to an extraction/readability suggestion.
12d1304 to
db9a9fd
Compare
- Render OnboardingHero once, drive placement via CSS order toggle - Document why derived onboarding steps don't emit step.completed - Reconcile onboarding progress cache on PATCH failure via onError Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0667d58 to
b5fde41
Compare
db9a9fd to
a9f00df
Compare

Description
First PR in a stacked series that builds an onboarding experience for new users (see #622). This one lays the foundation and surfaces the first piece of UI — an onboarding checklist on the Learning Hub.
Foundation (
OnboardingProvider)TourProvidercompletions.userPipelineWriteEvents) emitted from the pipeline-write chokepoints (componentStoreandPipelineFile), so v1 edits, v2 edits, imports, and the new-pipeline button all count./api/users/me/settings), not localStorage. Writes usekeepaliveso a fire-and-forget save survives navigation/reload.UI
OnboardingChecklist— a reusable progress + step-list component (extracted up front so the next PR doesn't have to refactor it).OnboardingHeroon the Learning Hub renders the checklist with progress, per-step CTAs, and a dismiss control.Provider and Onboarding components are enabled in the React Compiler.
Related Issue and Pull requests
Progresses https://github.com/Shopify/oasis-frontend/issues/622
Stacked: #2440 → #2435
Type of Change
Checklist
Screenshots (if applicable)
Test Instructions
Additional Comments
The provider exposes
isReady(queries settled, for routing) vsisResolved(backend resolved + state loaded, for gating visible UI). The downstream pill/welcome PRs rely on this distinction to avoid flicker.