Uh oh!
There was an error while loading. Please reload this page.
fix(plugin-form): tick EmbeddableForm's thank-you countdown - #5311
Merged
Conversation
"Redirecting in {{seconds}} seconds…" is documented as a remaining-seconds
countdown in all ten locale packs, but the number was computed once at
render time and never touched again — a static value for the whole wait
(objectui#5083).
Add useRedirectCountdownSeconds, a per-second setInterval owned by an
effect keyed on the accepted redirect destination, mirroring the
ownership model PR #5070 established for the navigation wait itself:
cancelled on unmount and on handleReset, the exact regression surface
objectui#5049 fixed. The interval self-clears on reaching 0.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkEPredicted before running, in the file's own docblock; this records what was actually measured after reverting EmbeddableForm.tsx to de4e29a with the tests unchanged — matches the prediction exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RV6yuVCxymHYE16PL9vQkE
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
os-support-ai
marked this pull request as ready for review
August 19, 2026 12:11
Uh oh!
There was an error while loading. Please reload this page.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixes#5083
What was broken
EmbeddableForm's public thank-you panel rendersRedirecting in {{seconds}} seconds…. All ten locale packs (packages/i18n/src/locales/*.ts, keypublicForm.redirecting) document{{seconds}}as "the remaining seconds" — but the number was computed exactly once, at the render that first shows the panel (Math.ceil(pendingRedirect.delayMs / 1000)), and never touched again. On the 3 second default delay, a submitter saw a fully staticRedirecting in 3 seconds…for the whole wait.Route taken — route 1 (implement the countdown), not route 2 (reword the copy)
The locale strings already promise decrementing semantics in all ten languages; changing the copy would be retreating from a promise the product already makes. Confirmed by reading
packages/i18n/src/locales/en.ts:3082andzh.ts:2898before choosing — both explicitly document "remaining seconds". No locale pack is touched by this PR.The fix
New file
packages/plugin-form/src/thankYouRedirectCountdown.ts—useRedirectCountdownSeconds(pending), a per-secondsetIntervalon the same ownership model PR #5070 established for the redirect wait itself (thankYouRedirectNavigation.ts):PendingThankYouRedirect), not the authored one — a destination the guard refused never reaches this hook with a non-null value (same as plugin-form: EmbeddableForm still promises "Redirecting in 3 seconds…" after its own safety guard refused the redirect — and theredirectBlockedstring it sets can never render #5073's fix);pendingstands; its cleanup runs on unmount and on every change ofpending's identity — a fresh accept, orhandleReset'ssetPendingRedirect(null)— which is the exact regression surface plugin-form: EmbeddableForm's thank-you redirect timer is unowned too — and its default 3s window fires into "Submit Another Response" #5049 fixed for the navigation timer. Nothing here reaches for a module-level or ref-only handle the way the pre-plugin-form: EmbeddableForm's thank-you redirect timer is unowned too — and its default 3s window fires into "Submit Another Response" #5049 code did.EmbeddableForm.tsxnow readsremainingRedirectSecondsfrom this hook instead of re-deriving the seconds frompendingRedirect.delayMson every render.Tests
New file
EmbeddableForm.redirectCountdownTick.test.tsx, three cases:4 → 3 → 2 → 1 → 0, and that the interval self-clears at 0. A test asserting only the initial render would pass against the pre-fix behaviour too (see reverse-verification below) — this is why it isn't enough on its own, and why the suite also asserts an interval was genuinely armed (setIntervalspy), not just that the text happened to read right once.handleReset(Submit Another Response) mid-countdown — the plugin-form: EmbeddableForm's thank-you redirect timer is unowned too — and its default 3s window fires into "Submit Another Response" #5049 regression surface — paired with a counter-probe (getByLabelText(/email/i)) proving the "no countdown" assertion is earned by the reset, not by the panel failing to render at all.Fake timers, deliberately different from the sibling
EmbeddableForm.redirectTimerLifetime.test.tsx(#5049), which keeps the real clock because faking it around the awaited submit chain would race the resolution. That reasoning applies to@testing-library/react'swaitFor/findBy*, which poll with a plain, timer-blindsetTimeoutand would hang under fake timers.vi.waitFor(vitest's own) doesn't have that problem — its polling loop is backed by real, unmocked timers and it advances the fake clock itself on every poll, so the awaiteddataSource.create(...)still resolves on its own microtask while the countdown'ssetIntervalstays fully controllable viavi.advanceTimersByTime. Every wait in the new file usesvi.waitFor, never testing-library's. (One dead end recorded in the file: wrapping those waits in an extraact(async () => …)stallsObjectForm's Suspense-loaded field widgets — measured, not theoretical — so the waits are intentionally left unwrapped, at the cost of some benign "not wrapped in act(...)" console noise.)Reverse-verification
Predicted before running, in the test file's own docblock: reverting
EmbeddableForm.tsxonly (tode4e29a81, one commit before this fix) with every test kept would fail all three tests at theexpect(armed.length).toBe(1)check (the reverted code never callssetInterval), while theRedirecting in 4 secondsinitial-render assertion immediately above it would stay green (it was already correct pre-fix).Measured: exactly that. All three tests failed with the identical
AssertionError: expected +0 to be 1at that line; nothing failed on the initial-render text. Restoring the committedEmbeddableForm.tsxreturns all three to green with no other change. (Recorded in the test file's docblock.)Commands run (HEAD
6029bdfa8)pnpm --filter '@object-ui/plugin-form^...' build— dependency closure, clean.pnpm --filter @object-ui/plugin-form build— clean, no TS errors.pnpm --filter @object-ui/plugin-form type-check— clean (tsc --noEmit && tsc -p tsconfig.test.json).pnpm --filter @object-ui/plugin-form test— 574/574 passed (56 test files), including the 3 new cases.eslint packages/plugin-form/src/EmbeddableForm.tsx packages/plugin-form/src/thankYouRedirectCountdown.ts packages/plugin-form/src/EmbeddableForm.redirectCountdownTick.test.tsx— 0 errors (17 pre-existing-styleany/fast-refresh warnings, same pattern as sibling files).node scripts/check-changeset-presence.mjs/check-changeset-no-major.mjs/check-changeset-fixed.mjs— ✅ all three.node scripts/check-control-bytes.mjs— ✅ (plus a manualgrep -naPself-scan of every changed/new file — clean).node scripts/check-lint-coverage.mjs/check-type-check-coverage.mjs— ✅ both.Scope
packages/plugin-form/src/**only, per the dispatch's file surface. No locale packs touched (route 1). No public type surface changed — the new hook is an internal module, not exported fromindex.tsx, matching the existingthankYouRedirectNavigation.tsprecedent. Confirmed no other package imports it.Changeset
patchon@object-ui/plugin-form(.changeset/embeddable-form-thankyou-countdown-tick-5083.md).Generated by Claude Code