Skip to content

The owl welcome leads straight into Start debate - #239

Merged
Programcode merged 3 commits into
mainfrom
design/onboarding-merge
Oct 27, 2026
Merged

Programcode merged 3 commits into
mainfrom
design/onboarding-merge

Conversation

@Programcode

@Programcode Programcode commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Closing Ollie's one-time intro now continues straight into the tour's Start debate step, so the first visit is one moment instead of two pop-ups.

What changed

  • The welcome still opens once per browser (debateowl_intro_seen_v1). ?intro=show still forces it, including under Playwright and the local verify stack. ?intro=skip still keeps it closed.
  • Got it, Esc, and the backdrop mark it seen and open the tour's first step (the Current Debate card, with Start debate when there is no debate yet). Skip animation only skips the owl's lines.
  • A finished tour stays finished, including when ?intro=show replays the welcome. With no welcome, the tour still starts on its own.
  • The tour chunk loads while the welcome is open, and the Start debate step is started before the next paint, so the page does not sit between the two.
  • If that chunk fails twice, the app logs once, remembers the welcome, and continues without the tour. Focus goes to the Current Debate card.
  • Closing the welcome moves focus to the tour's Next button. Skipping or finishing the tour moves it back to the button in the Current Debate card. The welcome does not restore focus to the page body.
  • English, Hungarian, and Spanish copy is unchanged.

Checks

Check Result
npm run docs:check passed
npm run lint (eslint, knip, repo rules) passed
npm run typecheck passed
npm test 674 passed. src/test/scripts/backend-status.test.ts timed out in beforeEach (10s, temp git repo). A rerun failed a different case in that file the same way. Unrelated to onboarding. Intro and chunk tests: 20 passed.
npm run test:collaboration 30 passed (previous commit; this follow-up does not touch it)
npm run build passed (previous commit; this follow-up does not change the build)
e2e/first-run.spec.ts desktop Chromium (en and hu) 4 passed. The welcome stays closed under VITE_E2E, so this is the tour on its own.
npm run test:edge not run. Deno is not installed here.
npm run test:sw and the rest of npm run verify (full @smoke set) not run

Browser check, stub session, prefers-reduced-motion: reduce, no horizontal overflow and no inner dialog scroll:

  • 390×844, 820×1180, 1440×900 in English. The welcome fits (dialog bottom 785 / 833 / 655). The Start debate tooltip fits (bottom 691 / 650 / 600).
  • Hungarian and Spanish at the same three widths. The longer welcome still fits at 390 (dialog bottom 798 and 797 of 844). The tour step followed in each language.

Screens

The two surfaces are the same before and after. What changed is the handoff: closing the welcome opens the Start debate step, with the tour chunk already loaded.

After, 390

Phone welcome after
Phone Start debate step after

After, 820

Tablet welcome after
Tablet Start debate step after

After, 1440

Desktop welcome after
Desktop Start debate step after

Before, on main

Phone welcome before
Phone Start debate step before
Tablet welcome before
Tablet Start debate step before
Desktop welcome before
Desktop Start debate step before

Likely conflicts

These branches were not on origin when this PR was opened, except design/library-name. Overlap is based on the files this PR touches and what those efforts are expected to edit.

  • design/debateowl-polish — likely OnboardingTour.tsx (last-step styling) and possibly App.tsx if it also edits AppWelcome. Locale copy is unchanged, so a copy-only polish commit should not collide.
  • design/debateowl-animations — likely DebateOwlIntro.tsx motion. This PR adds onCloseAutoFocus on that dialog so closing the welcome does not return focus to the page body. A motion edit on the same DialogContent can conflict there.
  • feat/case-structure-v2 — low risk. This PR does not touch case generation. A conflict needs an edit to App.tsx routing or the onboarding files.
  • Phone bottom tab bar — expected in AppLayout.tsx and the data-tour="nav-*" targets. This PR does not change the nav or those targets. The Start debate step still spotlights [data-tour="current-debate"] on the home card. Re-check the handoff if that card moves.
  • design/library-name (on origin) — locales only (en, es, hu). This PR does not edit locales, so the two diffs do not overlap. The tour's Library step still uses the existing onboarding.library strings.
  • DebateOwl rename / move-notice reword — intro copy and the header wordmark. This PR keeps the current EN/HU/ES welcome strings, including "DebateLab is slowly becoming DebateOwl". No locale conflict. A conflict needs an edit to src/lib/debateowl-intro.ts or App.tsx.

To show artifacts inline, enable in settings.

Open in Web Open in Cursor 

Closing Ollie's one-time intro continues into the tour's first step, instead of leaving a second popup to start on its own. The welcome is still shown once, ?intro=show still replays it, and a finished tour stays finished.

Co-authored-by: Gáborjáni-Szabó Ádám <Programcode@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 27, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Essentials by visiting https://app.coderabbit.ai/settings/billing.

Comment @coderabbitai help to get the list of available commands.

@Programcode

Copy link
Copy Markdown
Owner Author

Codex review (gpt-6-luna)

Independent read-only review of git diff origin/main...HEAD at dd18519, run via codex-cli 0.164.0 on the VPS. Comment only, not an approval.

Blocker

None

Should-fix

None

Nit

None

Verdict: Adam, approve; I found no verified issues in the requested diff.

@Programcode

Copy link
Copy Markdown
Owner Author

Sonnet 5.5 review (claude-sonnet-5-5)

Independent read-only review of git diff origin/main...HEAD at dd18519, run via Claude Code 2.1.309 on the VPS. Comment only, not an approval.

Blocker

None

Should-fix

None

Nit

  • src/App.tsx:371 — A failed tour chunk load is now silent. The old React.lazy threw to the error boundary. Now loadOnboardingTour().then(…, () => undefined) swallows the rejection, and the effect depends only on [Tour], so it never retries. A retry does happen in practice: the AppWelcome preload fails and resets the promise, then TimedOnboardingTour mounts and tries again. A second failure leaves the first-run tour missing for the rest of the page with no signal. This is acceptable for a one-time onboarding aid, but a comment saying the failure is deliberate would help.
  • src/components/features/onboarding/OnboardingTour.tsx:133 — continueFromIntro stays true for the whole page life, and the layout effect re-fires on every prepareTour identity change. That happens on a language switch or a Current Debate change. The existing auto-start effect has the same dependencies and also re-calls startTour, so this isn't a regression. I checked that startTour doesn't reset stepIndex and that run is gated on debateDialog === null, so the Start debate dialog still pauses the tour correctly. A comment saying the one-shot flag is deliberately level-triggered would stop a later reader from "fixing" it.
  • src/App.tsx:321 — Boolean(userId) && hasCompletedOnboarding(…) re-implements the "no user" guard that hasCompletedOnboarding inverts. AppWelcome sits under RequireAuth, so userId is always set here. The guard is harmless but dead. Passing tourCompleted={false} while there is no user would be simpler to read.
  • src/test/onboarding/debateowl-intro.test.tsx — Coverage is good. The FirstRun and firstRunStep tests plus the real OnboardingTour check are the right kind of test. There is no test for the AppWelcome wiring: the tourCompleted selector, the preload, and the module-level chunk cache. The firstRunStep unit test is largely a truth table that FirstRun already covers. It is cheap, but you could drop it. The Skip animation, Esc and Got it paths are covered. The backdrop path goes through the same onOpenChange as Esc, so that's fine.
  • Not verified by running: focus return after the dialog unmounts, with Joyride taking focus right after. I didn't launch the app or measure layouts at 390/820/1440. The PR changes no copy and no layout, so EN/HU/ES and responsive behaviour are unaffected beyond what the tour already did.

Checked and fine

  • debateowl_intro_seen_v1 persistence: markIntroSeen runs only in the close handler. A reload mid-intro shows the welcome again, which is right. A second account in the same browser doesn't see the welcome, and its unfinished tour opens on its own via the showIntro=false path.
  • ?intro=show with a finished tour: the step is intro, then idle. The tour owner stays mounted, so claimLegacyCompletion still runs.
  • Chunk race: the tour chunk is preloaded while the welcome is open. TimedOnboardingTour seeds its state from the module cache, so the Start debate step normally opens on the same commit as the close. If the chunk isn't ready, the tour mounts when it arrives. continueFromIntro is still true then, so the layout effect fires on that mount.
  • Tour behind the dialog: the tour isn't mounted while the welcome is open, so it can't start behind it.
  • Closing the welcome off /app: the tour waits for /app and then starts immediately.
  • SSR/prerender: window access in AppWelcome was already there and sits under RequireAuth. The new module-level state is guarded.
  • Docs and verify-skill edits: they match the new behaviour. No recipe selectors changed.

Verdict: Approve: the change is small and correct, and the edge cases I traced (reload, replay, finished tour, chunk race) all hold. Only nits remain, none blocking.

Co-authored-by: Gáborjáni-Szabó Ádám <Programcode@users.noreply.github.com>
@cursor

cursor Bot commented Oct 27, 2026

Copy link
Copy Markdown

Addressed the review notes in ccd0112. Still a draft.

  1. The tour chunk is fetched twice: once while the welcome is open, then again when the tour mounts. The second failure logs once ([Onboarding] Could not load the tour. Continuing without it.), marks the welcome seen, and continues without the tour. Focus goes to the Current Debate card.
  2. continueFromIntro stays true for the whole Start debate step. The layout effect consumes it once the spotlight has opened, so a later language or debate change does not run the handoff again. startTour still does not reset stepIndex.
  3. Removed the Boolean(userId) guard in AppWelcome. It only renders under RequireAuth, after user is set.
  4. AppWelcome now has a test: Got it closes the welcome into the Start debate step. Focus lands on the tour's Next button, and Skip tour returns it to the button inside [data-tour="current-debate"].

Lint, typecheck, and the docs check passed. Unit tests: 674 passed. backend-status.test.ts timed out in beforeEach (10s) on a different case each run, the same environmental flake as before. Intro and chunk tests: 20 passed. e2e/first-run.spec.ts on desktop Chromium: 4 passed.

# Conflicts:
#	.claude/skills/verify-debatelab/features/auth-demo-login.md
#	src/lib/debateowl-intro.ts
@Programcode
Programcode marked this pull request as ready for review October 27, 2026 11:03
Copilot AI balanced review requested due to automatic review settings October 27, 2026 11:03
@Programcode
Programcode merged commit 27ce0fe into main Oct 27, 2026

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

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.

3 participants