Save a card as you leave each field, and Undo instead of confirmations #242
No reviewers
Labels
No labels
No milestone
No project
No assignees
5 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
Grandiras/Ponente!242
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "ui-v2-live"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Two pieces of the UI language v2: saving as you leave a field, and Undo in place of "are you sure?".
Save on leave. An existing card's dialog saves each part when it is finished: title and description on change (blur), tags and assignees on the pick, the date on the pick or Clear. Save and Cancel are gone; the footer is Delete, a quiet "Saved" status and Done. A new card keeps Cancel + Create. Each save writes only its part (
CardModal.CardFields), so somebody else's card still sends only the date and assignees, and saves queue so quick edits land in order. Escape blurs the focused field first (modal-escape.js), so every way out saves, and switching cards in the inspector loses nothing.Undo. Deleting a card, a comment or a board is one click, with a toast carrying Undo for six seconds (the toast's only clickable part; the word takes a light board accent). Cards and comments are hidden at once and deleted when the toast has gone (
UndoService), because they cascade to tags, people, threads and files and could not be rebuilt; a delete that fails in the window brings the thing back with a toast. Boards are soft-deleted already, so they go at once and Undo restores them (settings page and the super-admin record).Docs:
ui.md(card dialog, the inspector, somebody else's card, a new Undo section),boards-and-permissions.md(a save writes only its part),frontend.md(uploads gate Create/Done; the v2 region's status),orientation.md(UndoService),testing.md. One changelog line, both locales.Verified:
tools/verify.sh(584 unit tests); driven in a dev instance (title saved by clicking another card, "Saved" after a date pick, Escape saves, delete hides at once, Undo restores, the row is gone after the window; light and dark); full e2e on an isolated stack: 158/158 passed (specs updated in this PR: board, permissions, calendar-all, share, moderation, helpers/boards).🤖 Generated with Claude Code
🤖 Reviewing this PR — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the task runs.
💬 Verdict: comment — notes only, nothing blocking.
The design is coherent: each save writes only its part, saves queue so they land in order, and delete is deferred until the Undo window closes. The docs and both locales are updated. I found two real issues and did not run anything.
1. "Saved" can show for a failed save, and a throw stops later saves (inline on
CardModal.razor).BoardPage.SaveCardandCalendar.SaveCardcatch their own errors and return normally, so_saved = truestill runs after a refused or failed write. IfOnSaveever does throw, the faulted_savingtask makes every later save in that dialog rethrow.2. A late Undo click is silently ignored (inline on
UndoService.cs). The toast is shown a render after the commit timer starts, so Undo can still be on screen when the delete runs.Smaller notes, not blocking
modal-escape.jsblurs every.modal inputexcepttype=search. If the assignee picker's text field handles Escape itself to close its menu, blurring it first may change that. I couldn't tell without reading the picker.BoardRecordPagepasses its instanceRestoreAsyncto the toast, so it may run on a disposed page.BoardSettingscorrectly makes its restore touch services only.🧠 claude-sonnet-5-5 · ⏱ 0m 40s · 2 turns · tokens: 43.9k in / 4.7k out (+12.2k cache-read)
📜 Full session transcript
@ -382,0 +424,4 @@{await previous;await OnSave.InvokeAsync(args);_saved = true;_saved = trueis set wheneverOnSavereturns, butBoardPage.SaveCardandCalendar.SaveCardcatch their own errors and return normally. A refused or failed write therefore still shows "Saved". Also, ifOnSaveever throws,_savingstays faulted, and every laterawait previousrethrows, so no later save in that dialog can succeed. Wrapawait previousin try/catch, or chain with a continuation that ignores the earlier fault. HaveOnSavereport success (a bool or an exception) before showing Saved.@ -0,0 +37,4 @@lock (_lock) _pending.Add(key);Changed?.Invoke();toasts.Show(message, ToastKind.Success, new ToastAction(undoLabel, () =>Race: the commit timer starts here, but the toast's 6 s timer starts only after
InvokeAsyncrenders it. The toast can therefore still be on screen for a short time after the delete has run. A click on Undo in that gap cancels a spent token, callsSettle, and shows nothing, while the thing stays deleted. Consider a committed flag so Undo is refused or disabled once the delete has started, or make the toast's lifetime end before the commit by starting the delay from the toast.✅ Verdict: approve — no blocking issues found.
This is a careful, well-tested change and I found nothing that blocks merging.
What holds up under scrutiny
CardFieldsflags are produced only from controls the dialog actually renders (CardModal.razor): title/description inputs exist only whenCanEdit, theTagPickeronly whenCanEdit, and the reader's card offers only the date and theAssigneePicker.OnlyDid. So theTextbranch ofBoardPage.SaveCard/Calendar.SaveCardis unreachable on somebody else's card, and the reader's commits send exactlySetDueDateAsyncand/orSetCardAssigneesAsync— same writes as before, now split. The server re-checks every call (CardService.DeleteCardAsync→RequireCardDelete, etc.), so nothing new rides on client trust.CommitAsyncsnapshots into a freshCardModalSaveArgsbefore chaining_saving = SaveAfter(_saving, args), so a later edit cannot change what an earlier save writes, and two quick edits land in order. It also means an in-flight save survives the inspector switching to another card, which is exactly what the docs claim.SaveAfteronly sets_saved = trueafterOnSavecompletes; the failure paths in both pages close the dialog before_savedcan be shown, so the "Saved" status doesn't lie about a refused write.UndoServiceis handled.SettleraisesChangedfrom a thread-pool thread, and every subscriber (BoardPage.Reload,Calendar.Reload,CommentList.Refresh) marshals throughInvokeAsync;ToastContainer.Showwas moved to build the item off-list (Interlocked.Increment, add insideInvokeAsync) precisely because failure toasts now arrive fromTask.Run. The_toasts.Contains(toast)guard also prevents the removal timer from racing an Undo click that already ran the action. Disposal unsubscribesChangedin all three components.modal-escape.jsblurs in the capture phase before the dialog's own Escape handler, and the exclude of[type=search]correctly leaves the assignee menu's own Escape handling (AssigneePicker) untouched.BoardViewercaptured at delete time is still re-checked server-side, so a mid-window revocation is caught, not bypassed.ui.md,boards-and-permissions.md,frontend.md,orientation.md,testing.md), the changelog line in both locales, and the e2e updates (including the DB-level "row is gone after the window" check viapsql) all match the implementation; the removed locale keys (board.card.saved, the confirm strings) have no remaining references.Minor notes, none blocking
CommitAsync(CardFields.Text)builds its args withDueDate = _dueDate, andUpdateCardAsyncwrites title, description and due date. So a text-only save can revert a due date that another member changed while the dialog was open — the same clobber window the old single-write Save had, so it is not a regression, but the docs' claim "a save writes only the part just changed" is slightly optimistic for text saves.ui.md's Undo section someday.UndoService, if the 6 s delay completes in the same instant the Undo button is clicked, the cancel can lose the race and the delete still commits (button is disabled only at ~the same boundary). Aif (undone.IsCancellationRequested) return;after the delay would close the sliver; genuinely rare, so fine to leave.toasts.Showinside theTask.Runcatch) can throw out of the catch into an unobserved task fault — harmless noise today, but wrapping that oneShowin a try/catch would make the remark "the safe way to fail" literally quiet.⏱ 26m 14s · 30 steps · $0.1072 · tokens: 243.7k in / 3.4k out (+1062.0k cache-read) · Go: 5h 4% · wk 9% · mo 87%
📜 Full session transcript
Addressed in
2b3ab52:ActionLifetime+ 1 s, so the toast (whose clock starts a render later, plus its 200 ms exit) is gone first, and Undo and the delete each claim the item once viaInterlocked.Exchange. A click that still loses (only with over a second of lag) shows "Already deleted" rather than nothing.UpdateCardAsyncis nowUpdateTextAsyncand writes only title and description; the date hasSetDueDateAsync, so the doc claim holds. Move calls both.SaveAfterwaits withTask.WhenAny(previous). "Saved" after a failed save cannot show, because both pages close the dialog on failure (now said in a comment)._recordcould name another board by the time Undo was clicked.ui.md's Undo section.docs/calendar.md, which still described the old save split.Not changed:
modal-escape.jsand the assignee picker: its field istype=search, which the selector excludes, so its own Escape (closing the menu) is untouched.ToastContainerunsubscribes on dispose, soShowthen has no handler and cannot throw.🤖 Reviewing this PR — the review will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the review runs.
🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.
📜 Live transcript — updating every ~5 minutes while the task runs.
✅ Verdict: approve — no blocking issues found.
Both issues from my last pass are resolved.
SaveAfternow usesTask.WhenAny(previous), so an earlier fault no longer blocks later saves. A failed write makesBoardPage.SaveCardandCalendar.SaveCardclose the dialog and show a toast, so a "Saved" flash on a closing dialog is harmless. The code comment says this. A refused or failed write still hits_saved = truein the dialog, but the dialog is already gone by then.UndoServicenow delays the commit by 1 s more than the toast. Undo and the commit also claim the item once through anInterlockedflag, and a click that loses shows "Already deleted".I did not run the app or the tests.
Non-blocking notes:
modal-escape.jsblurs any.modal inputexcepttype=searchon Escape. I did not check whether the assignee picker's text input handles Escape itself to close its menu.BoardRecordPage.RestoreAsyncis an instance method and now captures the board id, but it still touches_busyandReloadAsyncon a page that may have been disposed. That would only matter if someone navigated away before pressing Undo, and the failure is minor.🧠 claude-sonnet-5-5 · ⏱ 0m 10s · 3 turns · tokens: 49.4k in / 886 out (+72.6k cache-read)
📜 Full session transcript
✅ Verdict: approve — no blocking issues found.
Re-review: both earlier blockers are resolved
1. "Saved" after a failed save, and a poisoned save queue — fixed.
SaveAfternow chains withawait Task.WhenAny(previous), which waits out the predecessor without inheriting its fault, so one throwing save no longer blocks every later save in the dialog. And_saved = trueonly runs whenOnSavereturned normally: bothBoardPage.SaveCardandCalendar.SaveCardclose the dialog and show the danger toast inside theirPermissionDeniedException/ general catches before returning, so a refused or failed write never renders the "Saved" status.2. The late-Undo race — fixed on both ends. The commit timer now waits
ActionLifetime + Margin(one second longer than the toast), and the toast's clock starts only a render later with a 200 ms exit, so Undo is off screen before the delete begins. The button is also disabled as soon as the toast starts exiting, and a click that still loses the race is no longer silent: theclaimedflag routes it to the "Already deleted" danger toast rather than a no-op.Everything else from the last pass still holds: the per-part writes send only what the dialog rendered (a reader's card offers only the date and the menu-less picker, so
Text/Tagscommits cannot happen there, and every service re-checks server-side); theCommitAsyncsnapshot keeps a later edit out of an earlier save and lets an in-flight save survive the inspector switching cards;UpdateTextAsyncno longer touchesDueDate, so the text save cannot revert a date;ToastContainerbuilds items off-list withInterlocked.Incrementand guards removal with_toasts.Contains, because failure toasts now arrive fromTask.Run;modal-escape.jsblurs in the capture phase and excludestype=search, leavingAssigneePicker's own Escape intact; everyUndoService.Changedsubscriber unsubscribes on dispose and marshals throughInvokeAsync; boards soft-delete and restore throughBoardSettings/BoardRecordPageclosures that touch services only or catch their own errors.Docs and tests are consistent:
ui.md's Undo section states plainly that search, Home and comment counts catch up when the delete lands, both locales carry every new string, no stale reference toUpdateCardAsync,board.card.savedor the confirm keys remains, and the e2e suite pins the whole loop including the DB-level check that the row survives the window and goes after it.Minor, non-blocking: the
toasts.Show(failedMessage, …)insideUndoService's catch is itself unwrapped, so if the circuit died inside the window the failure toast becomes an unobserved-task exception instead of silence — noise only, worth a try/catch someday.⏱ 3m 17s · 15 steps · $0.0363 · tokens: 122.8k in / 2.4k out (+440.7k cache-read) · Go: 5h 4% · wk 9% · mo 87%
📜 Full session transcript