fix(ui): Sprint 9 — dismiss X / Skip tour did not hide the dialog

Root cause: the OnboardingTour early-return is gated on
isComplete=true, but App.tsx was calling onboarding.reset() on
onComplete. reset() does the inverse: clears the localStorage key
and flips isComplete to FALSE. The user clicked X, the localStorage
key got written, but the App-level flag flipped to false, so the
tour re-rendered and the early-return did not fire — the dialog
stayed visible.

Fix: split the dismiss and reset paths into two distinct callbacks
onComplete (dismiss) and onReset (re-show). Added markComplete to
useOnboarding: flips isComplete to true. App wires:
  onComplete -> onboarding.markComplete()
  onReset    -> onboarding.reset()
The tour itself still calls writeComplete() before invoking
onComplete, so the localStorage key is written once on dismiss.

Also cleaned markComplete: it now only flips state (the tour already
wrote the key), removing a redundant double-write.

Verified npm run build green on docker-willester. No regression
expected; all other Sprint 9 code paths untouched.
This commit is contained in:
2026-06-05 13:40:51 -07:00
parent 0b6c5dcfb7
commit 1562929f6b
2 changed files with 36 additions and 7 deletions
+10 -3
View File
@@ -102,9 +102,16 @@ function App() {
<OnboardingTour
isComplete={onboarding.isComplete}
onComplete={() => {
// The tour already wrote the localStorage key via
// writeComplete(); flip the App-level flag so it stays
// hidden after the next render.
// Dismiss path (X / Skip / Esc / "Got it"). The tour
// already wrote the localStorage key; flip the
// App-level flag to true so the early-return fires and
// the dialog disappears.
onboarding.markComplete()
}}
onReset={() => {
// Re-show path (?reset-tour=1). The tour already
// cleared the localStorage key; flip the App-level
// flag to false so the tour re-appears.
onboarding.reset()
}}
/>
+26 -4
View File
@@ -100,9 +100,16 @@ function clearComplete(): void {
}
}
export function useOnboarding(): { reset: () => void; show: () => void; isComplete: boolean } {
export function useOnboarding(): {
reset: () => void
show: () => void
markComplete: () => void
isComplete: boolean
} {
const [isComplete, setIsComplete] = useState<boolean>(readComplete)
// Used by ?reset-tour=1 and any other "re-show" path. Clears the
// localStorage key and flips the App-level flag so the tour re-appears.
const reset = useCallback(() => {
clearComplete()
setIsComplete(false)
@@ -113,15 +120,28 @@ export function useOnboarding(): { reset: () => void; show: () => void; isComple
setIsComplete(false)
}, [])
return { reset, show, isComplete }
// Used by the tour's dismiss path (X / Skip / Esc / "Got it" on the
// last step). Called from App when onComplete fires. Just flips
// the App-level flag to true so the tour's early-return fires on
// the next render. The localStorage key is already written by the
// tour's finish() before it invokes onComplete.
const markComplete = useCallback(() => {
setIsComplete(true)
}, [])
return { reset, show, markComplete, isComplete }
}
export function OnboardingTour({
isComplete,
onComplete,
onReset,
}: {
isComplete: boolean
/** Called when the user dismisses the tour (X / Skip / Esc / "Got it"). */
onComplete: () => void
/** Called when the tour re-shows (e.g. ?reset-tour=1). Inverse of onComplete. */
onReset: () => void
}) {
const location = useLocation()
const navigate = useNavigate()
@@ -135,18 +155,20 @@ export function OnboardingTour({
const isLast = step === STEPS.length - 1
// Handle ?reset-tour=1 (clears the key; tour shows on next render).
// Calls onReset() to flip the App-level flag to false (the inverse
// of onComplete, which flips it to true on dismiss).
useEffect(() => {
const sp = new URLSearchParams(location.search)
if (sp.get(RESET_PARAM) === '1') {
clearComplete()
onComplete()
onReset()
// Strip the param so a refresh doesn't re-trigger the reset.
sp.delete(RESET_PARAM)
const next = sp.toString()
navigate(`${location.pathname}${next ? `?${next}` : ''}`, { replace: true })
setStep(0)
}
// We intentionally don't depend on `onComplete` (changes per render)
// We intentionally don't depend on `onReset` (changes per render)
// — the only effect we want is when the URL search changes.
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [location.search])