Back asks before it discards unsaved work - #701
Merged
Merged
Conversation
`use-unsaved-changes-guard` had two layers — `beforeunload` and a capture-phase anchor-click interceptor — and neither of them sees a `popstate`. So the browser's Back, and a phone's back gesture, threw away an afternoon of typing without a word. The waypoints e2e never caught it because its guard test only ever clicked a link. The gap mattered more than it used to: `lib/use-back-dismiss.ts` made Back first-class navigation on the waypoints page, walking out of one sheet per press. Once the overlays were gone, the next press dropped the work. The guard now joins that same layer stack rather than adding a second, competing popstate listener. It holds a history entry for as long as the form is dirty; a Back on that entry is answered with the same dialog, and the same wording, a link click gets. "Keep editing" puts the entry straight back, so it is a no-op by construction. "Discard changes" unwinds out of the entries the prompt stacked up and then takes the step the reader asked for. Every page using the guard is covered: comp and task settings, the profile form and the waypoints editor. Four things `useBackDismiss` needed, and one of them was not on the plan: - `enabled`, so the layer arms and disarms with `dirty`. The StrictMode pending-release cancel is skipped on the disabled path on purpose — there the pending release IS the point. - `ground`, a layer that belongs to the PAGE rather than to something drawn over it. "The guard arms first, so the ordering falls out for free" turned out not to hold: on the waypoints page it is editing an altitude INSIDE the review sheet that turns the form dirty, two layers deep, and an entry pushed there takes the Back that was aimed at the sheet. History is linear and an entry cannot be inserted underneath one, so a ground-floor layer waits for the stack to empty and takes the ground floor when it is free. - `rearm()` and `unwind(then)` on the returned layer. `unwind` counts nothing: it reads the marker off each entry as it goes, so it is right whether or not the confirm dialog has released its own entry yet. - Strict serialisation of walks. `history.back()` does not take effect until its popstate lands, so two walks started in the same tick — which is what a sheet's Done does, since it both releases its entry and settles the form — each pop for the same entry and throw the reader two pages back. Related: a release that finds somebody else's entry on top rightly refuses to pop, leaving a dead entry wearing the page's own URL; walks pop straight through those rather than landing on one and appearing to do nothing. A confirmed LINK click unwinds too, before it pushes. Without that the guard's entry is left buried between the page and the destination, and Back from the destination lands on it and has to be pressed again. No user-visible wording changed: the Back path reuses each page's existing dialog copy verbatim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Preview Deployment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch preview:
https://claude-charming-pike-f1817b.glidecomp.pages.dev
use-unsaved-changes-guardhad two layers —beforeunloadand a capture-phaseanchor-click interceptor — and neither of them sees a
popstate. So thebrowser's Back, and a phone's back gesture, threw away an afternoon of typing
without a word. The waypoints e2e never caught it because its guard test only
ever clicked a link.
The gap mattered more than it used to.
lib/use-back-dismiss.ts(#700) madeBack first-class navigation on
/comp/:id/waypoints: every full-screen sheetand every dialog owns a history entry, so Back walks out one overlay at a time.
Once the overlays were gone, the next press dropped the work. Every page using
the guard is affected — comp and task settings, the profile form and the
waypoints editor — so this is an app-wide navigation change with its own e2e
coverage rather than a rider on the waypoints page.
What Back does now
The guard joins the same layer stack rather than adding a second, competing
popstate listener. It holds a history entry for as long as the form is dirty,
and a Back on that entry is answered with the same dialog, and the same
wording, a link click gets.
the reader never actually moves and declining is a no-op by construction
rather than by a compensating push. The stack is left exactly as it was
found, so the next press asks again instead of appearing to do nothing.
then takes the step the reader asked for.
closing a sheet does.
No user-visible wording changed: the Back path reuses each page's existing
dialog copy verbatim.
Four things
useBackDismissneededThe first three were the plan. The fourth was not, and neither was the shape of
the second.
enabled, so the layer arms and disarms withdirty. The StrictModepending-release cancel is skipped on the disabled path on purpose — there
the pending release is the point.
ground: a layer that belongs to the PAGE, not to something drawn overit. "The guard arms when the form goes dirty and sheets open later, so the
ordering falls out for free" turned out not to hold. On the waypoints page
it is editing an altitude inside the altitude-review sheet that turns
the form dirty — two layers deep — and an entry pushed there takes the Back
that was aimed at the sheet. The existing e2e caught it: Back stopped
closing the review and asked "Discard changes?" over the top of it instead.
History is linear and an entry cannot be inserted underneath one, so a
ground-floor layer waits for the stack to empty and takes the ground floor
when it is free.
rearm()andunwind(then)on the returned layer.unwindcountsnothing — it reads the marker off each entry as it goes, so it is right
whether or not the confirm dialog has released its own entry yet.
Strict serialisation of walks.
history.back()does not take effectuntil its popstate lands, so two walks started in the same tick — which is
exactly what a sheet's Done does, since it both releases its entry and
settles the form — each pop for the same entry and throw the reader two
pages back. Walks now run one at a time and re-read the stack when their
turn comes. Related: a release that finds somebody else's entry on top
rightly refuses to pop, leaving a dead entry wearing the page's own URL
that nothing will ever return to; walks pop straight through those rather
than landing on one and appearing to do nothing.
The one-shot
ignoringPopflag and its microtask-clearing dance are gone,replaced by a single
unwindingflag held for the whole of a walk.The traps from the brief
All three were real.
rac/dialog'sModalregisters a back-dismiss layer whenever the dialog is keyboard-dismissable). Nothing hard-codes a count for it; the walk reads entries.
Keeping that layer also buys a free correctness win: Back while the prompt
is open cancels the prompt, exactly as Escape does.
a release asked for mid-walk waits its turn, and a release that arrives after
the walk re-reads the stack, finds an entry no layer marked, and stops
without popping.
pressing Back a second time and expecting the question again.
A confirmed link click unwinds too, before it pushes. Without that the
guard's entry is left buried between the page and the destination, and Back
from the destination lands on it and has to be pressed again.
Coverage
Both specs were already in
e2e/fixtures/mobile.ts, so both Playwrightprojects run them with no change to the list.
e2e/comp-waypoints.spec.ts— "the back gesture on unsaved waypoints isguarded" (a sheet takes the first press, the guard takes the next; Keep
editing leaves the stack untouched; Discard lands where the press was
headed), and "a reverted edit disarms the back guard and leaves no stray
entry". The save round-trip now asserts one press leaves after saving.
e2e/comp-settings-pages.spec.ts— the same journey on/settings/general.Both tests reach their page through an in-app link rather than a
goto, soBack has a real route underneath it instead of unloading the document and
raising a
beforeunloadprompt the guard's own dialog would hide behind.Verification
bun run test:e2e --project=chromium— 147 passed, 6 skipped.bun run test:e2e --project=mobile— 56 passed, 1 skipped.bun run test:e2e:ssr— 42/42.bun run test:all— 836 frontend, 773 competition-api, 108 auth-api, engineand scripts, every typecheck. Green.
bun run check:scoring-note— not required, no scoring source changed.Run in this worktree on its own ports (
DEV_FRONTEND_PORT=3100,DEV_API_PORT=8890,DEV_INSPECTOR_PORT=9330), perdocs/local-dev.md.Docs
docs/2026-07-18-rac-adoption-guide.mdgains gotcha #27: where a layerbelongs in the stack, why two traversals must never overlap, and why the walk
reads entries instead of counting them. The hook's own note carries the rest.
Note the branch was one commit behind
masterand was fast-forwarded onto#700, which is where
use-back-dismiss.tscomes from.🤖 Generated with Claude Code