Public Access
docs: record Sprint 9 post-deploy dismiss-bug fix across all 6 running docs
Sprint 9 (commit6e386ba) shipped a working OnboardingTour but a broken dismiss path: clicking X / Skip / Esc / "Got it" did nothing. Root cause: useOnboarding().reset() was wired to the dismiss handler at App.tsx, but reset() does the inverse of dismiss — it clears the localStorage key and flips isComplete to FALSE, so the tour re-rendered, the early-return did not fire, and the dialog stayed visible. Fix: commit1562929split the dismiss and reset paths into two distinct callbacks (onComplete and onReset). User confirmed browser smoke passes. This commit updates the 6 running docs that track Sprint 9: - .agent/plan.md — S9.4.1 sub-task (post-deploy fix) added. - .agent/context.md — D9 (root cause + fix) + Q4 (Vitest?) added. - Review/sprint9-verification.md — full post-deploy fix section appended (root cause, fix, post-fix verification, lessons). - Review/handoff-ui-audit.md — Sprint 9 status banner + Last updated footer updated to reference the fix commit. - fix-ui-audit.md — T3.4.1 sub-task added under the T3.4 verification gate. - docs/HANDOFF.md — post-deploy fix paragraph added to the Sprint 9 section. All 6 docs now reflect the post-deploy reality. No code changes.
This commit is contained in:
@@ -194,12 +194,14 @@ User direction 2026-06-05: "Proceed with the next phase in the redesign." §Futu
|
|||||||
- **D6. rAF polling for the anchor's `getBoundingClientRect`.** Runs only while the tour is open. Cancellable. One DOM read per frame; well under 1% CPU on a 60Hz display.
|
- **D6. rAF polling for the anchor's `getBoundingClientRect`.** Runs only while the tour is open. Cancellable. One DOM read per frame; well under 1% CPU on a 60Hz display.
|
||||||
- **D7. Focus captured on open (primary action), restored on close.** Uses `previouslyFocused.current = document.activeElement` on mount; restores on unmount. Standard focus-trap pattern, minus the trap (the dialog is intentionally non-modal — the user can interact with the page below).
|
- **D7. Focus captured on open (primary action), restored on close.** Uses `previouslyFocused.current = document.activeElement` on mount; restores on unmount. Standard focus-trap pattern, minus the trap (the dialog is intentionally non-modal — the user can interact with the page below).
|
||||||
- **D8. The 4 anchor points are stable elements that already exist in the DOM.** The Dashboard's `<Card>` wrapping the Weekly Overview, the Pantry's page header, the Recipes Filters button, the Shopping List page header. Each gets `data-tour="<id>"`. The anchor also has an off-route fallback (centered card + "Open <page>" CTA) so a first-time user who lands on `/pantry` can still see the Dashboard step (with a one-click nav).
|
- **D8. The 4 anchor points are stable elements that already exist in the DOM.** The Dashboard's `<Card>` wrapping the Weekly Overview, the Pantry's page header, the Recipes Filters button, the Shopping List page header. Each gets `data-tour="<id>"`. The anchor also has an off-route fallback (centered card + "Open <page>" CTA) so a first-time user who lands on `/pantry` can still see the Dashboard step (with a one-click nav).
|
||||||
|
- **D9. Sprint 9 post-deploy bug fix (2026-06-05).** The dismiss path (X / Skip / Esc / "Got it") was wired to `useOnboarding().reset()` via `onComplete`, but `reset()` does the *inverse* of dismiss — clears the localStorage key AND flips `isComplete` to `false`. So clicking X wrote the key, but the App-level flag flipped in the wrong direction, the tour's `if (isComplete || !currentStep) return null` early-return never fired, and the dialog stayed visible. **Fix (`1562929`)**: split the dismiss and reset paths into two distinct callbacks. `useOnboarding` now exposes `markComplete()` (state flip to `true`) in addition to `reset()` (state flip to `false`). `OnboardingTour` takes two props: `onComplete` (dismiss) and `onReset` (re-show). `App.tsx` wires `onComplete → onboarding.markComplete()` and `onReset → onboarding.reset()`. The tour's `finish()` still calls `writeComplete()` + `onComplete()`; `markComplete` is the matching App-side state setter. Cleaned up: `markComplete` no longer double-writes localStorage. The bug was missed in initial verification because `npm run build` was green and no browser smoke was run before deploy.
|
||||||
|
|
||||||
## Open questions to surface to the user, not to assume
|
## Open questions to surface to the user, not to assume
|
||||||
|
|
||||||
- **Q1. Should the tour show on every page or only `/`?** Default: `/` only. Other pages need `?reset-tour=1`. If the user lands on a non-root page first, the tour does NOT auto-show. Documented in `Review/sprint9-verification.md` smoke step 2.
|
- **Q1. Should the tour show on every page or only `/`?** Default: `/` only. Other pages need `?reset-tour=1`. If the user lands on a non-root page first, the tour does NOT auto-show. Documented in `Review/sprint9-verification.md` smoke step 2.
|
||||||
- **Q2. Should the tour re-show on logout / new device?** Default: no. The localStorage key is per-browser, not per-family-profile. If the user has multiple devices or shares a device, the tour shows once per browser. A future migration could move the key to the family profile, but that's a Sprint 11+.
|
- **Q2. Should the tour re-show on logout / new device?** Default: no. The localStorage key is per-browser, not per-family-profile. If the user has multiple devices or shares a device, the tour shows once per browser. A future migration could move the key to the family profile, but that's a Sprint 11+.
|
||||||
- **Q3. Should the tour re-show on a recipe update / catalog change?** Default: no. The tour is a one-shot. New users see it; existing users don't.
|
- **Q3. Should the tour re-show on a recipe update / catalog change?** Default: no. The tour is a one-shot. New users see it; existing users don't.
|
||||||
|
- **Q4. Should we add a Vitest unit test for `useOnboarding` to lock the dismiss/reset/show state transitions?** Default: not now (would require adding `vitest` + `happy-dom` to frontend dev-deps; violates "no new npm deps"). Trade-off: relying on browser smoke for the dismiss path means the same class of bug can re-appear if a future change mis-wires the callbacks. Worth lifting the "no new npm deps" rule *for testing only* in a future sprint.
|
||||||
|
|
||||||
## Sprint 9 verification gate
|
## Sprint 9 verification gate
|
||||||
|
|
||||||
|
|||||||
+16
-2
@@ -160,8 +160,22 @@ Goal: bring implementation back into alignment with `Review/reviewconcensus.md`.
|
|||||||
### S9.4 — Verify
|
### S9.4 — Verify
|
||||||
|
|
||||||
- [x] `npm run build` green (tsc 0 errors, vite 0 errors).
|
- [x] `npm run build` green (tsc 0 errors, vite 0 errors).
|
||||||
- [ ] Browser smoke (8 steps) on `http://100.108.208.56:8082/` per `Review/sprint9-verification.md`.
|
- [x] Browser smoke (8 steps) on `http://100.108.208.56:8082/` per `Review/sprint9-verification.md`.
|
||||||
- [ ] No regression in Sprints 1–8 (keyboard shortcuts, error toast, 3-button vote row, WeekRangeNav, bulk pantry add).
|
- [x] No regression in Sprints 1–8 (keyboard shortcuts, error toast, 3-button vote row, WeekRangeNav, bulk pantry add).
|
||||||
|
|
||||||
|
#### S9.4.1 — Post-deploy fix (2026-06-05)
|
||||||
|
|
||||||
|
User reported post-deploy: "The tour window looks great, but Clicking the X nor skip tour do anything. I cannot exit the tour." Build was green but the dismiss path was broken.
|
||||||
|
|
||||||
|
- [x] **Root cause identified** (systematic-debugging Phase 4): `useOnboarding().reset()` was wired to the dismiss handler at `App.tsx:104-109`. `reset()` does the *inverse* of dismiss — it clears the localStorage key AND flips `isComplete` to `false`. So clicking X wrote the key, but the App-level flag flipped in the wrong direction, the tour's `if (isComplete || !currentStep) return null` early-return never fired, and the dialog stayed visible.
|
||||||
|
- [x] **Fix committed** (`1562929`): split the dismiss and reset paths into two distinct callbacks.
|
||||||
|
- `useOnboarding` now exposes `markComplete()` (state flip to `true`) in addition to `reset()` (state flip to `false`).
|
||||||
|
- `OnboardingTour` takes two props: `onComplete` (dismiss) and `onReset` (re-show).
|
||||||
|
- `App.tsx` wires `onComplete → onboarding.markComplete()` and `onReset → onboarding.reset()`.
|
||||||
|
- Cleaned up: `markComplete` no longer double-writes localStorage (the tour's `finish()` already does that).
|
||||||
|
- [x] `npm run build` green on `docker-willester` after the fix (495.64 kB, no size change).
|
||||||
|
- [x] User confirmed post-deploy smoke test passes (2026-06-05).
|
||||||
|
- [x] Anchors + URL effect re-verified: 5/5 `data-tour` anchors present at `Dashboard.tsx:602`, `Pantry.tsx:185, 208`, `Recipes.tsx:131`, `ShoppingList.tsx:231`; `?reset-tour=1` effect calls `onReset()` correctly.
|
||||||
|
|
||||||
### S9.5 — Docs (all 6 running docs updated)
|
### S9.5 — Docs (all 6 running docs updated)
|
||||||
|
|
||||||
|
|||||||
@@ -56,7 +56,7 @@ If you are a new agent continuing this work, do this **in order**:
|
|||||||
|
|
||||||
### Sprint 9 — F1 Onboarding Tour (H10)
|
### Sprint 9 — F1 Onboarding Tour (H10)
|
||||||
|
|
||||||
**Status: COMMITTED on 2026-06-05. Build green. Frontend-only.** Awaiting user to `git pull` + `docker compose up -d --build frontend` (no migration, no backend rebuild).
|
**Status: COMMITTED + POST-DEPLOY FIX on 2026-06-05. Build green. Frontend-only.** Awaiting user to `git pull` + `docker compose up -d --build frontend` (no migration, no backend rebuild). The original commit (`6e386ba`) had a broken dismiss path (X / Skip / Esc / "Got it" did nothing — `onComplete` was wired to `useOnboarding().reset()` which is the inverse operation). Post-deploy fix is `1562929`: split into two callbacks `onComplete` (dismiss → `markComplete()`) and `onReset` (re-show → `reset()`). User confirmed browser smoke passes after the fix.
|
||||||
|
|
||||||
**Root cause (one-liner):** new users land on the Dashboard with no orientation. The audit's F1 §Future item ("Onboarding hints / tour") was the natural next phase.
|
**Root cause (one-liner):** new users land on the Dashboard with no orientation. The audit's F1 §Future item ("Onboarding hints / tour") was the natural next phase.
|
||||||
|
|
||||||
@@ -362,4 +362,4 @@ cd frontend && npm run build
|
|||||||
|
|
||||||
Trust the build output. Trust the smoke checklist. Don't trust the deployment host's UI until the user confirms. The verification model is "I shipped, you verified, you reported, I fixed" — the agent in this role never sees the live UI directly.
|
Trust the build output. Trust the smoke checklist. Don't trust the deployment host's UI until the user confirms. The verification model is "I shipped, you verified, you reported, I fixed" — the agent in this role never sees the live UI directly.
|
||||||
|
|
||||||
**Last updated: 2026-06-05** — Sprint 1 deployed; Sprints 2-6 awaiting user deploy; **Sprint 7 (`09c7525`), Sprint 8 (`efd1fc6`), Sprint 9 (F1 Onboarding Tour), and Sprint 10 (Deny Forever on Recipes) committed on 2026-06-05, awaiting user deploy**. See the "How to take over" and "Pending user deploy" sections at the top of this file.
|
**Last updated: 2026-06-05** — Sprint 1 deployed; Sprints 2-6 awaiting user deploy; **Sprint 7 (`09c7525`), Sprint 8 (`efd1fc6`), Sprint 9 (F1 Onboarding Tour) + post-deploy fix (`1562929`), and Sprint 10 (Deny Forever on Recipes) committed on 2026-06-05, awaiting user deploy**. See the "How to take over" and "Pending user deploy" sections at the top of this file.
|
||||||
|
|||||||
@@ -117,3 +117,58 @@ One commit: `feat(ui): Sprint 9 — F1 onboarding tour (4-step welcome)`. Files:
|
|||||||
- `frontend/src/pages/Pantry.tsx` (2 anchors)
|
- `frontend/src/pages/Pantry.tsx` (2 anchors)
|
||||||
- `frontend/src/pages/Recipes.tsx` (anchor on Filters button)
|
- `frontend/src/pages/Recipes.tsx` (anchor on Filters button)
|
||||||
- `frontend/src/pages/ShoppingList.tsx` (anchor on header)
|
- `frontend/src/pages/ShoppingList.tsx` (anchor on header)
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Post-deploy fix (2026-06-05) — tour dismiss did nothing
|
||||||
|
|
||||||
|
User reported after first deploy: clicking the X button, "Skip tour" text, "Got it" final-step button, OR pressing Esc on the dialog did nothing — the tour stayed visible and could not be exited.
|
||||||
|
|
||||||
|
### Root cause
|
||||||
|
|
||||||
|
The OnboardingTour dialog's early-return is gated on `isComplete === true`:
|
||||||
|
|
||||||
|
```ts
|
||||||
|
if (isComplete || !currentStep) return null
|
||||||
|
```
|
||||||
|
|
||||||
|
But `App.tsx` was wiring the dismiss handler (`onComplete`) to `useOnboarding().reset()`. `reset()` does the **inverse** of dismiss: it clears the localStorage key AND flips `isComplete` to `false`. So when the user clicked X:
|
||||||
|
|
||||||
|
1. `finish()` ran — `writeComplete()` wrote `"1"` to `localStorage.mealplanner:onboarding-complete` ✓
|
||||||
|
2. `onComplete()` ran — `onboarding.reset()` cleared the key AND set `isComplete = false` ✗
|
||||||
|
3. Tour re-rendered — `isComplete` was now `false`, so the early-return did NOT fire
|
||||||
|
4. The dialog stayed visible. Forever (until `?reset-tour=1` was visited).
|
||||||
|
|
||||||
|
All four dismiss paths (X button, "Skip tour" text, Esc keyboard, "Got it" final-step button) shared the same broken path through `finish()` → `onComplete()`. None of them worked.
|
||||||
|
|
||||||
|
The `?reset-tour=1` effect was working "accidentally" — it called `clearComplete()` (which was the right thing for the reset path) and `onComplete()` (which was the wrong thing for that path too, but the `clearComplete()` had already done the work, and the dialog re-appearing was the correct user-visible behavior).
|
||||||
|
|
||||||
|
### Fix (commit `1562929`)
|
||||||
|
|
||||||
|
Split the dismiss and reset paths into two distinct callbacks:
|
||||||
|
|
||||||
|
1. Added `markComplete()` to `useOnboarding`: flips `isComplete` to `true` (matches the early-return's expected state). `reset()` and `show()` are unchanged.
|
||||||
|
2. `OnboardingTour` now takes two props: `onComplete` (dismiss) and `onReset` (re-show).
|
||||||
|
3. `App.tsx` wires:
|
||||||
|
- `onComplete → onboarding.markComplete()` (X / Skip / Esc / "Got it" all hide the dialog)
|
||||||
|
- `onReset → onboarding.reset()` (`?reset-tour=1` re-shows)
|
||||||
|
4. The tour's `finish()` still calls `writeComplete()` + `onComplete()`. Cleaned up: `markComplete` no longer double-writes localStorage (the tour's `finish()` already did that).
|
||||||
|
|
||||||
|
### Post-fix verification
|
||||||
|
|
||||||
|
- `npm run build` green on `docker-willester` (495.64 kB, no size change from Sprint 9 build).
|
||||||
|
- User confirmed browser smoke on `http://100.108.208.56:8082/`:
|
||||||
|
- X button → dialog disappears, `localStorage.mealplanner:onboarding-complete === "1"`
|
||||||
|
- "Skip tour" → dialog disappears, key set
|
||||||
|
- Esc → dialog disappears, key set
|
||||||
|
- "Got it" on last step → dialog disappears, key set
|
||||||
|
- Refresh → dialog does NOT re-show (key persists)
|
||||||
|
- `?reset-tour=1` → dialog re-appears, then URL strips the param on its own
|
||||||
|
- Re-verified all 5 `data-tour` anchors still resolve: `Dashboard.tsx:602`, `Pantry.tsx:185, 208`, `Recipes.tsx:131`, `ShoppingList.tsx:231`.
|
||||||
|
- Re-verified the URL effect calls `onReset()` (not the old `onComplete()`) for the `?reset-tour=1` path.
|
||||||
|
|
||||||
|
### Lessons learned
|
||||||
|
|
||||||
|
- **Lesson 1: A green build is not verification.** The bug was missed in initial verification because `tsc 0 errors + vite 0 errors` does not exercise the dismiss path. A 4-step browser smoke (open `/`, click X, check `localStorage`, refresh) would have caught it. Future sprints: when the deliverable is user-visible interaction (not just data rendering), browser smoke is part of the verification gate, not optional.
|
||||||
|
- **Lesson 2: Inverse paths deserve inverse APIs.** `reset()` and `markComplete()` are inverses, and they were collapsed onto a single `onComplete` callback. The collapse worked for `reset` and broke for `markComplete`. A two-callback API (`onComplete` + `onReset`) would have caught this at code-review time.
|
||||||
|
- **Lesson 3: Unit tests for state hooks are cheap insurance.** A 10-line Vitest test for `useOnboarding` would have caught this in CI without browser smoke. Worth lifting the "no new npm deps" rule for testing-only deps in a future sprint (see `.agent/context.md` Q4).
|
||||||
|
|||||||
@@ -338,6 +338,8 @@ Trust the tests. Trust the live runs. Don't trust prose claims that something is
|
|||||||
|
|
||||||
**No regression expected:** Sprint 9 does not touch Sprints 1-8. The anchor `data-tour` attributes are additive; the page components still render the same. The KeyboardShortcuts hook (Sprint 5) is mounted in `App.tsx` and unaffected. The react-query error handler (Sprint 4) is unaffected.
|
**No regression expected:** Sprint 9 does not touch Sprints 1-8. The anchor `data-tour` attributes are additive; the page components still render the same. The KeyboardShortcuts hook (Sprint 5) is mounted in `App.tsx` and unaffected. The react-query error handler (Sprint 4) is unaffected.
|
||||||
|
|
||||||
|
**Post-deploy fix (2026-06-05, commit `1562929`):** user reported the X / Skip / Esc / "Got it" buttons did not dismiss the tour. Root cause: `App.tsx` wired the dismiss handler to `useOnboarding().reset()`, which is the *inverse* of dismiss (clears the localStorage key AND flips `isComplete` to `false`). Fix: split into two distinct callbacks — `onComplete` (dismiss) and `onReset` (re-show). User confirmed browser smoke passes after the fix. Full root-cause + lessons in `Review/sprint9-verification.md` (Post-deploy fix section).
|
||||||
|
|
||||||
### Sprint 10 — "Deny Forever" on Recipes (user-driven) — COMMITTED 2026-06-05
|
### Sprint 10 — "Deny Forever" on Recipes (user-driven) — COMMITTED 2026-06-05
|
||||||
|
|
||||||
**User direction (2026-06-05):** "Proceed with the next phase in the redesign. Also add a phase to include a 'Deny Forever' button in the Recipes endpoint."
|
**User direction (2026-06-05):** "Proceed with the next phase in the redesign. Also add a phase to include a 'Deny Forever' button in the Recipes endpoint."
|
||||||
|
|||||||
+6
-2
@@ -538,10 +538,14 @@ User direction 2026-06-05: "Proceed with the next phase in the redesign." F1 was
|
|||||||
### T3.4 · Sprint 9 verification gate
|
### T3.4 · Sprint 9 verification gate
|
||||||
|
|
||||||
- [x] `npm run build` green for Sprint 9 (tsc 0 errors, vite 0 errors).
|
- [x] `npm run build` green for Sprint 9 (tsc 0 errors, vite 0 errors).
|
||||||
- [ ] Browser smoke (8 steps) on `http://100.108.208.56:8082/` per `Review/sprint9-verification.md`.
|
- [x] Browser smoke (8 steps) on `http://100.108.208.56:8082/` per `Review/sprint9-verification.md`.
|
||||||
- [ ] No regression in Sprints 1-8.
|
- [x] No regression in Sprints 1-8.
|
||||||
- [x] `Review/sprint9-verification.md` written.
|
- [x] `Review/sprint9-verification.md` written.
|
||||||
|
|
||||||
|
#### T3.4.1 · Post-deploy fix (2026-06-05, commit `1562929`)
|
||||||
|
|
||||||
|
User reported post-deploy that X / Skip / Esc / "Got it" did not dismiss the tour. Root cause: `App.tsx` wired the dismiss handler to `useOnboarding().reset()`, which is the inverse of dismiss (clears the localStorage key AND flips `isComplete` to `false`). Fix: split the dismiss and reset paths into two distinct callbacks (`onComplete` → `markComplete()` and `onReset` → `reset()`). The tour's `finish()` still calls `writeComplete()` + `onComplete()`. Full root-cause + fix + lessons in `Review/sprint9-verification.md` (Post-deploy fix section).
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## Sprint 10 — "Deny Forever" on Recipes — ✅ COMPLETE, awaiting deploy
|
## Sprint 10 — "Deny Forever" on Recipes — ✅ COMPLETE, awaiting deploy
|
||||||
|
|||||||
Reference in New Issue
Block a user