rbrooks/Quest-Board

Watch

Fork 0

feat(frontend): stop three editors losing unsaved work (#410) #463

Merged

claude-bot merged 1 commit from fix/410-unsaved-changes-guard into main

2026-08-31 05:25:19 +00:00

claude-bot commented

2026-08-31 05:14:56 +00:00

Contributor

Partial for #410 (MEDIUM) — three of four acceptance criteria. The in-app navigation half is deliberately excluded; reasoning below, agreed with the issue's author.

What ships

The Shelf's tonight-notes saved only on onBlur. Ctrl+W with focus still in the field never fires onBlur, so everything typed since the last accidental blur was lost — during live play, on the app's primary at-the-table writing surface. Now debounced at 1.5s, and still committed on blur so leaving the field doesn't wait out the timer.

Beat Planner's own session dropdown overwrote the draft unconditionally. Not an external navigation a GM might expect to cost them something — the tool's own control, silently discarding what they were mid-typing. It asks now.

useUnsavedChangesWarning covers tab close and reload on the summary/transcript editors, the Beat Planner and the Shelf. It arms only while dirty and detaches on unmount: a leaked listener would nag on pages the user never typed into, which is exactly how a warning becomes noise people click through.

useBlocker calls useDataRouterContext, which invariant-throws without a data router. This app mounts <BrowserRouter>, not createBrowserRouter/RouterProvider. Getting router-level blocking means migrating every route — a large, risky refactor that has nothing to do with unsaved-changes handling and shouldn't ride along inside it.

The hook is therefore named for what it does rather than what one might wish it did, and says so in its own docstring. Two further limits documented there:

Browsers ignore the returned string; the user sees a generic "Leave site?" dialog. Criterion 1 was always going to land softer than it reads — no app can supply that text.
The dialog only appears after user interaction with the page, which is fine: someone who hasn't typed has nothing to lose.

Acceptance criteria

Beat Planner's session-switch does not silently discard an unsaved draft
Shelf tonight-notes auto-saves on a debounce
A shared reusable dirty-state utility exists
Summary/transcript editors warn before navigating away — they warn on tab close/reload, but in-app navigation needs the router migration

#410 stays open for that last one.

Verification

Mutation-checked: reverting the debounce fails the two tests that type without ever blurring — the Ctrl+W-during-play case that motivated the change. A blur-only implementation passes every other shelf test, so those two are the ones carrying the fix.

The hook's own tests cover disarming when clean and on unmount, so it cannot pass by simply warning always.

465 frontend tests pass (was 457). Eslint clean on every touched file.

🤖 Generated with Claude Code

**Partial for #410** (MEDIUM) — three of four acceptance criteria. The in-app navigation half is deliberately excluded; reasoning below, agreed with the issue's author. ## What ships **The Shelf's tonight-notes** saved only on `onBlur`. `Ctrl+W` with focus still in the field never fires `onBlur`, so everything typed since the last accidental blur was lost — **during live play, on the app's primary at-the-table writing surface**. Now debounced at 1.5s, and still committed on blur so leaving the field doesn't wait out the timer. **Beat Planner's own session dropdown** overwrote the draft unconditionally. Not an external navigation a GM might expect to cost them something — the tool's own control, silently discarding what they were mid-typing. It asks now. **`useUnsavedChangesWarning`** covers tab close and reload on the summary/transcript editors, the Beat Planner and the Shelf. It arms only while dirty and detaches on unmount: a leaked listener would nag on pages the user never typed into, which is exactly how a warning becomes noise people click through. ## Why in-app navigation blocking is not here `useBlocker` calls `useDataRouterContext`, which `invariant`-throws without a data router. This app mounts `<BrowserRouter>`, not `createBrowserRouter`/`RouterProvider`. Getting router-level blocking means migrating **every route** — a large, risky refactor that has nothing to do with unsaved-changes handling and shouldn't ride along inside it. The hook is therefore named for what it does rather than what one might wish it did, and says so in its own docstring. Two further limits documented there: - Browsers ignore the returned string; the user sees a generic "Leave site?" dialog. **Criterion 1 was always going to land softer than it reads** — no app can supply that text. - The dialog only appears after user interaction with the page, which is fine: someone who hasn't typed has nothing to lose. ## Acceptance criteria - [x] Beat Planner's session-switch does not silently discard an unsaved draft - [x] Shelf tonight-notes auto-saves on a debounce - [x] A shared reusable dirty-state utility exists - [ ] Summary/transcript editors warn before **navigating away** — they warn on tab close/reload, but in-app navigation needs the router migration **#410 stays open** for that last one. ## Verification Mutation-checked: reverting the debounce fails the two tests that type **without ever blurring** — the `Ctrl+W`-during-play case that motivated the change. A blur-only implementation passes every other shelf test, so those two are the ones carrying the fix. The hook's own tests cover disarming when clean and on unmount, so it cannot pass by simply warning always. **465 frontend tests pass** (was 457). Eslint clean on every touched file. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-08-31 05:14:56 +00:00

feat(frontend): stop three editors losing unsaved work (#410 )

CI / Backend lint (ruff) (pull_request) Successful in 55s

Details

CI / Bot/backend version sync (pull_request) Successful in 55s

Details

CI / Summarisation accuracy eval harness (stub provider) (pull_request) Successful in 1m33s

Details

CI / Docker image build (pull_request) Successful in 36s

Details

CI / Frontend tests, audit, and build (pull_request) Successful in 2m8s

Details

CI / Bot tests and audit (pull_request) Successful in 2m18s

Details

CI / Backend migration, tests, and audit (pull_request) Successful in 9m8s

Details

9264615d75

Partial — the in-app navigation half is deliberately not here; see below.

The Shelf's tonight-notes saved only on onBlur. Ctrl+W with focus still in
the field never fires onBlur, so everything typed since the last accidental
blur was lost — during live play, on the app's primary at-the-table writing
surface. Now debounced at 1.5s, and still committed on blur so leaving the
field does not wait out the timer.

Beat Planner's own session dropdown overwrote the draft unconditionally.
Not an external navigation a GM might expect to cost them something: the
tool's own control, silently discarding what they were mid-typing. It now
asks.

useUnsavedChangesWarning covers tab close and reload on the summary and
transcript editors, the Beat Planner and the Shelf. It arms only while
dirty and detaches on unmount — a leaked listener would nag on pages the
user never typed into, which is how a warning becomes noise.

Named for what it does, not what one might wish. It does NOT block in-app
navigation: useBlocker invariant-throws without a data router, and this app
mounts <BrowserRouter>, so having it would mean migrating every route to
createBrowserRouter/RouterProvider. That is its own change and is tracked
separately rather than smuggled in behind a hook name implying more than it
delivers. Browsers also ignore the returned string, so the prompt is the
generic "Leave site?" either way.

Mutation-checked: reverting the debounce fails the two tests that type
without ever blurring — the Ctrl+W-during-play case. The hook's tests cover
disarming when clean and on unmount, so it cannot pass by warning always.

465 frontend tests pass, up from 457.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot referenced this pull request

2026-08-31 05:15:16 +00:00

[Frontend] Warn before an in-app navigation discards an unsaved edit (needs a data router) #464

claude-bot referenced this pull request

2026-08-31 05:15:54 +00:00

[Frontend] Add a dirty-state guard to session, beat-planner, and shelf editors #410

claude-bot merged commit de2b1aa3fa into main

2026-08-31 05:25:19 +00:00

claude-bot deleted branch fix/410-unsaved-changes-guard

2026-08-31 05:25:20 +00:00

claude-bot referenced this pull request from a commit

2026-08-31 05:25:21 +00:00