fix(frontend): warn before an in-app navigation discards an unsaved edit (#464) #537

Merged

claude-bot merged 2 commits from fix/464-data-router into main

2026-09-06 20:26:44 +00:00

claude-bot commented

2026-09-06 20:03:38 +00:00

Contributor

Closes #464. First engineering lane of v4.4.0 (#535 phase 0).

What changes

App.jsx mounts the route table on createBrowserRouter + RouterProvider (from react-router/dom, the build that supplies flushSync). The JSX route tree is kept via createRoutesFromElements and exported as routes so tests mount it under createMemoryRouter. Every route, the legacy redirects, the single AuthGuard + AppLayout, and the catch-all are unchanged. AuthProvider and CampaignProvider move into a pathless root layout route because CampaignProvider calls router hooks and a data router has no "between the router and the routes" any more; ThemeProvider stays outside as before.
A sibling hook useUnsavedChangesPrompt adds the in-app half on top of the existing beforeunload guard: a confirm before a pathname change while an edit is dirty (?tab= and the #recording anchor never prompt). It registers the blocker through the router's public getBlocker/deleteBlocker and no-ops without a data router, so the ~30 page tests that mount MemoryRouter keep working and the hook cannot crash a page rendered on its own. React Router's useBlocker throws outside a data router and there is no public probe, which is why the hook reads the router off UNSAFE_DataRouterContext; a future react-router bump that renames it fails the hook's tests, not users.
SessionDetail's summary and transcript editors use the new hook. The Shelf tonight-notes and the Beat Planner are deliberately untouched (#536 for the former, which has a real flush bug the prompt would not fix).

One thing beyond the brief, on purpose. A data router wraps the root match in its own error boundary whether or not you ask, and its default element renders "Unexpected Application Error!" over a raw stack trace, in the production build too. The migration alone would therefore have bypassed the existing <ErrorBoundary> in main.jsx and shown users a JS stack. The root route now carries an errorElement that renders the same ErrorFallback the class boundary uses (extracted, not duplicated), with a test that locks it in.

Verification (Docker, node 22): 64 files, 809 tests pass; eslint 0 errors (one pre-existing warning in a file this PR does not touch); vite build ok. The blocking tests were seen to fail with the blocker registration removed (2 failed, 9 passed) and pass with it restored.

🤖 Generated with Claude Code

Closes #464. First engineering lane of v4.4.0 (#535 phase 0). **What changes** - `App.jsx` mounts the route table on `createBrowserRouter` + `RouterProvider` (from `react-router/dom`, the build that supplies `flushSync`). The JSX route tree is kept via `createRoutesFromElements` and exported as `routes` so tests mount it under `createMemoryRouter`. Every route, the legacy redirects, the single `AuthGuard` + `AppLayout`, and the catch-all are unchanged. `AuthProvider` and `CampaignProvider` move into a pathless root layout route because `CampaignProvider` calls router hooks and a data router has no "between the router and the routes" any more; `ThemeProvider` stays outside as before. - A sibling hook `useUnsavedChangesPrompt` adds the in-app half on top of the existing `beforeunload` guard: a confirm before a *pathname* change while an edit is dirty (`?tab=` and the `#recording` anchor never prompt). It registers the blocker through the router's public `getBlocker`/`deleteBlocker` and no-ops without a data router, so the ~30 page tests that mount `MemoryRouter` keep working and the hook cannot crash a page rendered on its own. React Router's `useBlocker` throws outside a data router and there is no public probe, which is why the hook reads the router off `UNSAFE_DataRouterContext`; a future react-router bump that renames it fails the hook's tests, not users. - `SessionDetail`'s summary and transcript editors use the new hook. The Shelf tonight-notes and the Beat Planner are deliberately untouched (#536 for the former, which has a real flush bug the prompt would not fix). **One thing beyond the brief, on purpose.** A data router wraps the root match in its own error boundary whether or not you ask, and its default element renders "Unexpected Application Error!" over a raw stack trace, in the production build too. The migration alone would therefore have bypassed the existing `<ErrorBoundary>` in `main.jsx` and shown users a JS stack. The root route now carries an `errorElement` that renders the same `ErrorFallback` the class boundary uses (extracted, not duplicated), with a test that locks it in. **Verification** (Docker, node 22): 64 files, 809 tests pass; eslint 0 errors (one pre-existing warning in a file this PR does not touch); vite build ok. The blocking tests were seen to fail with the blocker registration removed (2 failed, 9 passed) and pass with it restored. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 2 commits

2026-09-06 20:03:38 +00:00

refactor(frontend): mount the route table on a data router (#464 ) 300a29a7a0

`useBlocker` — the only way to stop an in-app navigation from discarding an
unsaved edit — asserts it is inside a data router, and the app mounted
`<BrowserRouter>`. So the router has to move first.

`createRoutesFromElements` keeps the existing JSX route tree verbatim; every
path, redirect and the `*` catch-all resolve exactly as before. Two things did
have to change shape:

- `AuthProvider` and `CampaignProvider` wrapped `<Routes>`; a data router has no
  such place, and `CampaignProvider` reads `useLocation`/`useMatch`, so they now
  live in a pathless root layout route. Same nesting, inside the router instead
  of around it.
- A data router wraps the root match in its own error boundary whether you ask
  for one or not, so a page that throws no longer reaches the `<ErrorBoundary>`
  around `<App />`, and its untouched fallback is an unstyled "Unexpected
  Application Error!" over a raw stack trace — in production too. The root route
  now carries an `errorElement` that renders the app's own error screen, so this
  is a no-op for anyone who hits a broken page.

The router is created once at module scope, not in React state: it starts
listening to history the moment it exists, so one per render would leave orphans
subscribed to the same history. That is also why `App.test.jsx` now mounts the
exported route table under `createMemoryRouter` instead of pushing onto
`window.history` — a data router reads the URL when it is created, long before
a test could push.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

fix(frontend): warn before an in-app navigation discards an unsaved edit (#464 )

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

Details

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

Details

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

Details

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

Details

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

Details

CI / Docker image build (pull_request) Successful in 5m22s

Details

CI / Backend migration, tests, and audit (pull_request) Successful in 11m28s

Details

CI / Synthetic session harness (no GPU, no LLM) (pull_request) Successful in 16m5s

Details

a4eff9a44a

Correcting a session summary or transcript and then clicking the nav bar, a
campaign link or Back threw the draft away in silence. `beforeunload` (#410)
only ever covered leaving the *site*; an in-app navigation never leaves the
page as far as the browser is concerned, so nothing was there to ask.

`useUnsavedChangesPrompt` is a sibling of `useUnsavedChangesWarning` that adds
the in-app half on top of the unload half: it registers a router blocker, and
confirms with `window.confirm` before letting the navigation through — the
codebase's convention for a destructive confirmation until the milestone's
design-system dialog exists. Only a change of pathname counts, so `?tab=` and
the `#recording` anchor are not treated as leaving.

It is a sibling rather than a change to the existing hook so the other two call
sites — the shelf's tonight-notes, which auto-saves on a debounce, and the beat
notes editor — keep their current behaviour until each is looked at on its own
terms.

The hook reads the data router off its context and treats "no data router" as
"no in-app blocking", rather than calling `useBlocker` directly: that hook
throws outside a data router, which would mean every one of the ~30 tests that
mounts a page under a plain `<MemoryRouter>` crashing instead of degrading.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

claude-bot referenced this pull request

2026-09-06 20:04:27 +00:00

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

claude-bot merged commit cce8601e72 into main

2026-09-06 20:26:44 +00:00

claude-bot deleted branch fix/464-data-router

2026-09-06 20:26:44 +00:00

claude-bot referenced this pull request from a commit

2026-09-06 20:26:45 +00:00

Merge PR #537: fix(frontend): warn before an in-app navigation discards an unsaved edit (#464)

claude-bot referenced this pull request

2026-09-06 21:03:12 +00:00