Repository navigation
fix(staged): undo quote removal with native undo instead of a toast - #1068
Merged
Merged
Conversation
Removing a quote from the chat composer must not raise a toast, and the
removal must be undoable and redoable with the platform's own undo and
redo, in order with the typing around it.
Root cause: `removeQuote` in SessionChatPane.svelte called
`offerQuoteUndo`, which raised a svelte-sonner toast ("Quote removed from
your reply") with an Undo action whenever the removed quote had a reply.
That action was the only way to get a quote back. Both ways of removing
a quote (the block's X button, and Backspace or Delete in the empty gap
above an unanswered quote) go through that one function, and
SessionChatPane is the only composer that takes quotes.
How undo is wired: the composer has no editor framework. Its inputs are
separate contenteditable elements that rely on the browser's undo stack,
and quotes live in Svelte state outside it. The new shared
NativeUndoAnchor component is a hidden editable element: recording a
step types one invisible character into it with
`execCommand('insertText')`, which puts an entry on the same
document-wide undo stack as the user's typing. When native undo or redo
reaches that entry the browser removes or re-adds the character, and the
component runs the step's `undo` or `redo` to match. No keydown handler
is involved, so Cmd+Z, Ctrl+Z, the Edit menu and plain text undo are all
left to the browser. Focus and the caret are borrowed for the edit and
handed back before the step runs.
ChatComposer records each removal as such a step. `onRemoveQuote` now
returns a restore function; SessionChatPane's puts the quote back where
it was with its reply and lead-in text, and refuses once the draft has
been sent, another session is in the box, or the pane is inactive or
closed. Undo returns the caret to the block's input it was taken from,
or scrolls the field to the block when the caret was elsewhere.
Removed as dead: `offerQuoteUndo`, `dismissQuoteUndo`, the toast
bookkeeping, `revealDraftQuote`, `restoreQuoteInReplyDraft` and
`ReplyComposerTarget.restoreQuote`. Toasts for other actions are
untouched.
Verified: vitest (3712 tests) and svelte-check pass. The real
ChatComposer was also driven in a standalone harness in Chromium with
real Cmd+Z key presses, and in an offscreen WKWebView through its
NSUndoManager, the path the Edit menu's Undo and Redo take. Typing, then
removing a quote, then typing again undid in reverse order and redid in
order in both.
Not verified: the running Tauri app itself, hardware key events in
WKWebView, iOS (on-screen keyboard behaviour while the anchor borrows
focus, shake and gesture undo), and Firefox.
Known limitation: removing or restoring a quote re-renders the inputs
whose text it merges or splits, so typing steps recorded earlier in
those inputs point at replaced nodes. Undo presses that reach them do
nothing visible before the stack moves on. Ordering is exact when the
removed quote had no lead-in text above it.
Signed-off-by: Matt Toohey <contact@matttoohey.com>
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96a349b94e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+370
to
+373
| redo() { | ||
| // Only a quote undo put back: one a refused undo left out is still | ||
| // out, and still this step's to restore. | ||
| if (taken === null) taken = takeQuote(id); |
There was a problem hiding this comment.
If the pane is reopened and its draft is still loading, undo queues the restoration through SessionChatPane.receiveDraftEdit and returns true before the quote actually reappears. This clears taken, but a redo before the load completes calls takeQuote(id) against a draft that still lacks the quote, so it returns null without cancelling the queued restoration. When the load lands, the quote reappears despite the redo, and subsequent undo cannot restore the expected history because taken is still null. Queue the redo alongside the restoration or explicitly cancel the pending restore.
Useful? React with 👍 / 👎.
Review feedback on PR #1068: redoing a quote removal while the undo before it was still waiting on the draft's load did nothing, so the quote came back once the load landed. Root cause: when a pane is reopened its draft loads again, and an undo made in that window is queued by `receiveDraftEdit` in SessionChatPane.svelte rather than applied. The pane answered that it took the undo, so the composer's step marked the quote as back. A redo before the load landed looked for the quote in the box, found none, and gave up without touching the queued restore. The removal handle `onRemoveQuote` returns is now an object, `RemovedQuote`, with `restore` as before and a new `withdraw`. The pane's `restore` keeps the queue entry `receiveDraftEdit` now returns for a held edit, and `withdraw` takes that entry out of the queue (`withdrawPendingDraftEdit`), so it is never applied and its follow-up never moves the caret. ChatComposer's redo first asks the removal undo last handed back to withdraw. When that succeeds the step is as it stood before the undo, with its original restore and caret position, and a later undo puts the quote back. When there is nothing to withdraw, the restore already landed, and redo takes the quote out of the box as before. A queue the withdrawal empties is dropped, so a pane that then moves on sends no no-op amend to the stored draft. Tests: the pane test undoes and redoes inside a held draft load and checks the quote stays out, the composer is never told it returned, and a later undo restores the original draft. A second pane test checks no stored-draft write follows a withdrawn undo. Two composer tests cover redo against a held undo and against one that has landed. The two test stubs speak the new handle; PromptInputStub gains a redo button. Verified: ChatComposer and SessionChatPane component tests pass (36), and the three tests for the fix fail with the withdrawal disabled. svelte-check passes. The full vitest suite was not run locally (machine under heavy load); CI covers it. Not verified: the running app. The window is the draft's load after a pane is reopened, which was only driven through the mocked load in tests. Not covered: an undo held during one load, flushed to the stored draft because the pane moved on, then redone during the next load. That redo still does nothing until the load lands. The step's state stays consistent with the draft, and a redo after the load works. Signed-off-by: Matt Toohey <contact@matttoohey.com> Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
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.
Summary
Removing a quote from the chat composer no longer raises a toast. The removal is instead undoable and redoable with the platform's own undo and redo (Cmd+Z, the Edit menu, text undo), in order with the typing around it.
Problem
removeQuoteinSessionChatPane.sveltecalledofferQuoteUndo, which showed a svelte-sonner toast ("Quote removed from your reply") with an Undo action whenever the removed quote had a reply. That action was the only way to get the quote back. Both removal paths (the block's X button, and Backspace/Delete in the empty gap above an unanswered quote) go through this one function.Approach
The composer has no editor framework: its inputs are separate contenteditable elements relying on the browser's undo stack, while quotes live in Svelte state outside it. The new shared
NativeUndoAnchorcomponent bridges the two:execCommand('insertText'), placing an entry on the same document-wide undo stack as the user's typing.undoorredoto match.ChatComposerrecords each quote removal as such a step.onRemoveQuotenow returns a restore function;SessionChatPane's puts the quote back where it was with its reply and lead-in text, and refuses once the draft has been sent, another session is in the box, or the pane is inactive or closed. Undo returns the caret to the block's input it was taken from, or scrolls the field to the block when the caret was elsewhere.Removed as dead
offerQuoteUndo,dismissQuoteUndo, the toast bookkeeping,revealDraftQuote,restoreQuoteInReplyDraft, andReplyComposerTarget.restoreQuote. Toasts for other actions are untouched.Testing
NativeUndoAnchor.component.test.tsand extendedChatComposer/SessionChatPanecomponent tests, backed by anativeUndoStacktest stub.ChatComposerwas driven in a standalone harness in Chromium with real Cmd+Z key presses, and in an offscreen WKWebView through its NSUndoManager (the path the Edit menu's Undo/Redo take). Typing, removing a quote, then typing again undid in reverse order and redid in order in both.Not verified: the running Tauri app itself, hardware key events in WKWebView, iOS (on-screen keyboard behaviour while the anchor borrows focus, shake/gesture undo), and Firefox.
Known limitation: removing or restoring a quote re-renders the inputs whose text it merges or splits, so earlier typing steps in those inputs point at replaced nodes. Undo presses that reach them do nothing visible before the stack moves on. Ordering is exact when the removed quote had no lead-in text above it.
🤖 Generated with Claude Code