first-run-ux: welcome modal + first-run gate (PR-B of #1101)#1108
first-run-ux: welcome modal + first-run gate (PR-B of #1101)#1108RebelTechPro wants to merge 2 commits into
Conversation
Three additive pieces that PR-B (welcome modal) will sit on top of, and that already improve UX in their own right by replacing blank list panels with explanatory empty states. 1. UserEntity gains `hasOnboarded?: boolean` (BooleanField, nullable). Per-user, cross-device — falsy on existing rows so the welcome modal (PR-B) defaults to "show." entity_schemas.json regenerated. 2. New shared widget `widgets/shared/ModalWidget.ts` — generic Lit dialog. Reactive `open` / `modalTitle` / `closable` properties; focus trap, Escape + backdrop dismiss, focus restoration on close; role=dialog + aria-modal=true + aria-labelledby out of the box. Slots for default body + footer. Reusable for any future modal need beyond onboarding (settings dialogs, confirms, etc.). 3. New shared widget `widgets/shared/EmptyStateWidget.ts` — `<empty-state>` custom element with icon / title / subtitle / optional action button. Fires `empty-state-action` event when the action is clicked. Drops into any list or content area that can be legitimately empty. Wiring (3 surfaces, all behind a load-completed gate so the empty state never flashes during initial scroller hydration): ReactiveEntityScrollerWidget — new `protected get isEmpty()` returns true only after the scroller's first load has completed AND the entity count is zero. Subclasses use this to decide whether to render an empty UI. ReactiveListWidget — new virtual `renderEmptyState()` returning `nothing` by default; main render hides the container and shows the empty state when `isEmpty` is true. RoomListWidget — overrides `renderEmptyState()`. Copy depends on the active filter (DM filter → "No direct messages yet" / "Open a DM..."; rooms filter → "No rooms yet" / "Rooms are shared spaces..."). No "create your first room" CTA wired yet; left for a follow-up once room-creation UX lands. UserListWidget — overrides `renderEmptyState()`. Copy depends on whether any type/status filter is active. The widget overrides `render()` directly (bypasses base render) so we mirror the same ?hidden + empty-state conditional locally. ChatWidget — uses string-based templates (not Lit), so wired differently. `<empty-state>` element added to renderTemplate with `hidden` set; `updateEntityCount()` is overridden to toggle the attribute based on `getEntityCount() === 0` after every CRUD event + the initial post-load count update. Initial `hidden` prevents flash during room-switch loading. Out of scope (PR-B): welcome modal, first-run gate in MainWidget, write-back of `hasOnboarded=true` on modal completion, tutorial- persona seeding verification. `npm run build:ts` is green. Not visually validated locally — deploy + screenshot is the gate before un-drafting. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stacked on top of PR-A (feat/empty-states-foundation), which adds the
ModalWidget primitive and the `hasOnboarded` field. PR-B is the
user-facing flow that consumes both.
What ships:
- `widgets/onboarding/WelcomeModalWidget.ts` (new). Two short panels
wrapped in `<modal-widget>`:
1. Intro — what Continuum is, in one paragraph
2. Hand-off — "Helper AI is in General. Send a message there to
see the system in motion." Skip-keys note: optional cloud
providers in Settings.
Step indicator + Back/Next/Got-it buttons. Backdrop / Escape /
X dismissal is treated as "completed" so the modal doesn't nag
the user the next session.
- MainWidget gate. In `onFirstRender`, checks
`this.currentUser.hasOnboarded` (loaded by ReactiveWidget's
connectedCallback before onFirstRender runs). Falsy → open the
modal. On `welcome-complete`, persists `hasOnboarded: true` via
`data/update` and reflects the value on the in-memory entity so a
re-render doesn't re-open the modal. Persist failure is logged
but not surfaced — the worst-case is "modal shows again next
session."
- Seed: `SYSTEM_ROOM_UNIQUE_IDS` extended with `'general'`. Previous
set was `['settings', 'help', 'theme', 'canvas']`, so a fresh
install put Helper AI into support rooms only and left General
with no AI for users running with zero API keys. The welcome
modal's hand-off copy now matches what's actually in the room.
The constant is referenced from both the fresh-seed and
existing-rooms paths in seed-continuum.ts, so the change applies
in both flows.
Copy choices (per CambrianTech#1101 discussion):
- Skip the API-key step — local inference is enough out-of-box
after CambrianTech#336's model evaluation work; Settings is a follow-up, not a
blocker.
- Feature Helper AI specifically (not GeneralAI, which requires
ANTHROPIC_API_KEY and won't be seeded for no-key users).
- Tone: warm, brief, system-confident — three short paragraphs total
across both panels. Easy to edit at the strings in WelcomeModalWidget.
`npm run build:ts` is green. Not visually validated locally — flow gate
is in the test plan.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Substantive review (claude tab #2 / claude-tab-2, picked up via continuum#1126 intake card). Reviewed #1107 first; this picks up the PR-B incremental delta. What this PR adds (over #1107)
What I verified
Issues worth fixing before un-drafting
Things to consider (not blocking)
Test plan additionsYour
RecommendationLGTM to land after fixing nit #1 (role=presentation + aria-label conflict). The architecture is right (per-user flag, race-free gate, dismissal=completed), the seed change is correctly placed in both code paths, and the modal-widget reuse is clean. The PR-A/PR-B split paid off here — this review was 30 minutes instead of 2 hours because the foundation was already validated. Thanks for the careful gate ordering and the in-memory mutation safety net. New-contributor work that gets the lifecycle details right is the rare kind. |
joelteply
left a comment
There was a problem hiding this comment.
Review (claude-tab-1) — REQUEST CHANGES (stale-branch regression of #1100)
This branch is stale relative to canary. The diff against current canary shows it reverts the XSS hardening shipped in #1100 (chat XSS hardening — DOM-returning adapter render path):
- this.renderAdapterContentInto(contentDiv, adapter, message);
+ contentDiv.innerHTML = contentHtml; // ← re-introduces innerHTML XSS surfacePlus deletes the renderAdapterContentInto private method that was the seam for that hardening.
This is the same root cause I documented in detail on #1107 — see #1107 (review) latest review for the full breakdown including the a11y phase-2 regression in #1108 specifically.
Fix
Rebase this branch onto current origin/canary and resolve the conflict in ChatWidget.ts so the new work in this PR sits ON TOP of the kept renderAdapterContentInto path, not in place of it.
After rebase, this PR's actual changes will likely be much smaller and easier to evaluate on their own merits. As-is the diff is dominated by accidental reverts.
The PR's intentional changes look fine; this is purely a substrate hygiene issue. Sorry for the late catch — these PRs sat unreviewed too long. The substrate's stale-PR signal (airc#608 / #609) would have flagged it earlier.
Summary
widgets/onboarding/WelcomeModalWidget.ts— two-panel welcome flow using PR-A's<modal-widget>.currentUser.hasOnboardedis falsy, persiststrueon completion.Decisions (from the #1101 planning thread)
UserEntity(PR-A)WelcomeModalWidget.renderStep()What changed
WelcomeModalWidget.ts(new, 178 lines)<modal-widget>:welcome-completeevent for the parent to react to.MainWidgetgate@reactive() private _showWelcomestate.onFirstRender: after URL routing and other setup, checksthis.currentUser?.hasOnboarded. Falsy → opens the modal.currentUseris loaded byReactiveWidget.connectedCallback()beforeonFirstRenderruns, so this is race-free.onWelcomeComplete: closes the modal, callsdata/updateonuserscollection with{ hasOnboarded: true }, mutates the in-memory entity so a hot re-render doesn't reopen it. Persist failure is logged, not surfaced — worst case the modal shows again next session.Seed extension
scripts/seed-continuum.ts:335—SYSTEM_ROOM_UNIQUE_IDSextended from['settings', 'help', 'theme', 'canvas']to['general', 'settings', 'help', 'theme', 'canvas'].Test plan
npm run build:ts→ green (verified locally on top of PR-A)npm run data:reseed): Helper AI appears in members list of General.data/updatefires,usersrow hashasOnboarded: true.hasOnboardedpersists (dismissal = completed).Copy policy
The text in
WelcomeModalWidget.renderStep()is intentionally short and meant to be revised. Edit the strings directly; no separate i18n table yet. Tone target was warm-brief-system-confident-not-salesy.Closes
Closes #1101 when both this and #1107 merge. (Tracker stays open until both are in.)