Repository navigation
fix(tui): preserve form drafts across tabs - #48952
Conversation
|
Thanks for your contribution! This PR doesn't have a linked issue. All PRs must reference an existing issue. Please:
See CONTRIBUTING.md for details. |
holny
left a comment
There was a problem hiding this comment.
Ran the suite on top of e73442c locally — 29/29 in form.test.tsx including the new remount test, and the restore path reads clean. Two observations:
The one gap I see: drafts are only evicted on reply/cancel success or on the next mount. If the form gets settled somewhere else (another tab or client answers it), the form list drops it, FormPrompt unmounts through the keyed Show in session/index.tsx:1375, and the onCleanup at form.tsx:236 stores a draft for a form id that never comes back — it sits in the module-level map for the whole TUI process. Small per abandoned form, but it adds up. The test fixture works around exactly this by giving each test a unique form id (formSequence in form.test.tsx:18), which is a hint the map could use an owner for eviction — e.g. dropping drafts whose ids fall out of the session's form list, or clearing them on session switch.
Smaller thing: the settle dance between onCleanup and reply/cancel is correct but only because both halves cooperate — cleanup snapshots the draft while settled is still false, then the .then() at form.tsx:299 deletes it after the fact. If that delete ever gets dropped, a stale draft quietly comes back for an already-answered form. Might deserve a one-line comment, or a settle() helper both paths call, since the settled = true + drafts.delete pair is duplicated in reply and cancel anyway.
The cursor restore with the gotoLineEnd fallback kept for the no-draft case is a nice touch, and asserting that no replies were sent during the round-trip is the right test shape.
Replace the app-lifetime FormDraftProvider and its event-driven settlement bookkeeping with a module-level Map, mirroring component/prompt/draft-stash. A settled form is already gone from data when FormPrompt unmounts, so a membership check in onCleanup is enough to skip stashing it.
Co-authored-by: Shoubhit Dash <shoubhit2005@gmail.com>
Issue for this PR
Type of change
What does this PR do?
How did you verify your code works?
bun test test/cli/tui/form.test.tsxfrompackages/tui: 30 passed.bun typecheckfrompackages/tui: passed.bun run checkfrom the repository root: passed.Screenshots / recordings
Screen.Recording.2026-09-14.at.10.49.34.mov
Checklist