diff --git a/apps/staged/src/lib/features/sessions/ChatComposer.component.test.ts b/apps/staged/src/lib/features/sessions/ChatComposer.component.test.ts index ae0142dc5..28156ca19 100644 --- a/apps/staged/src/lib/features/sessions/ChatComposer.component.test.ts +++ b/apps/staged/src/lib/features/sessions/ChatComposer.component.test.ts @@ -1,17 +1,23 @@ -import { flushSync, mount, unmount } from 'svelte'; +import { flushSync, mount, tick, unmount } from 'svelte'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import ChatComposer from './ChatComposer.svelte'; -import type { DraftQuote } from './composerDraft'; +import type { ComposerDraft, DraftQuote } from './composerDraft'; import { composerSendControl } from './composerSendControl'; +import ComposerDraftHost from './testStubs/ComposerDraftHost.svelte'; import { reactiveProps } from './testStubs/reactiveProps.svelte'; +import { + installNativeUndoStack, + type NativeUndoStack, +} from '../../shared/testStubs/nativeUndoStack'; // The composer's field: one box holding the quote blocks, the text between // them and the main input, which the user works as one editor. Covered here // are the ways the field ties its inputs together — the arrows walking from // one into the next, a press on its surface landing the caret in an input, -// the single placeholder, the keyboard cap on the whole field and the -// focused input kept in view by the field's own scroll — while the inputs' -// own behaviour is under ComposerDraftQuotes.component.test.ts. +// the single placeholder, the keyboard cap on the whole field, the focused +// input kept in view by the field's own scroll, and a quote's removal taking +// its place in the browser's undo history with the typing around it — while +// the inputs' own behaviour is under ComposerDraftQuotes.component.test.ts. // The on-screen-keyboard cap reads the visible viewport; stand it in with a // keyboard up, so the cap is a share of the 400px left. @@ -210,25 +216,6 @@ describe('ChatComposer field', () => { else delete (Range.prototype as Partial).getClientRects; } }); - - it('scrolls the field to a quote block on request without moving focus', () => { - render([quote('a'), quote('b')]); - const fieldEl = layOutField(); - const block = target.querySelector('[data-quote-entry="b"] .composer-quote'); - if (!block) throw new Error('no block'); - block.getBoundingClientRect = () => rect(320, 380); - const ancestors = vi.spyOn(Element.prototype, 'scrollIntoView'); - const focused = document.activeElement; - - expect(component!.revealQuote('b')).toBe(true); - expect(fieldEl.scrollTop).toBe(79); - expect(document.activeElement).toBe(focused); - expect(ancestors).not.toHaveBeenCalled(); - // A block that isn't shown is reported, not scrolled to. - expect(component!.revealQuote('missing')).toBe(false); - expect(fieldEl.scrollTop).toBe(79); - ancestors.mockRestore(); - }); }); describe('ChatComposer placeholder', () => { @@ -344,3 +331,260 @@ describe('ChatComposer arrows between inputs', () => { expect(document.activeElement).toBe(replyOf('a')); }); }); + +// Removing a quote is a step on the browser's undo stack, in order with the +// typing in the field's inputs. jsdom has no such stack; the stand-in walks +// edits the way a browser does (testStubs/nativeUndoStack.ts), and the host +// keeps the draft as the pane would. +describe('ChatComposer quote removal', () => { + let stack: NativeUndoStack; + let host: ReturnType; + + function mountHost() { + return mount(ComposerDraftHost, { target }); + } + + function renderDraft(draft: ComposerDraft) { + target = document.createElement('div'); + document.body.appendChild(target); + host = mountHost(); + component = host; + host.setDraft(draft); + flushSync(); + } + + /** Let the composer's own follow-up to a change (the caret, the field's scroll) run. */ + async function settle() { + flushSync(); + await tick(); + flushSync(); + } + + async function pressRemove(id: string) { + const button = target.querySelector( + `[data-quote-entry="${id}"] .composer-quote-remove` + ); + if (!button) throw new Error(`no remove button for ${id}`); + button.click(); + await settle(); + } + + async function undo() { + expect(stack.undo()).toBe(true); + await settle(); + } + + async function redo() { + expect(stack.redo()).toBe(true); + await settle(); + } + + function quoteIds(): string[] { + return host.draft().quotes.map((q) => q.id); + } + + beforeEach(() => { + stack = installNativeUndoStack(); + }); + + afterEach(() => { + stack.uninstall(); + }); + + it('puts the quote back where it was on undo, and takes it out again on redo', async () => { + const draft = { + quotes: [quote('a', '', 'first reply'), quote('b', 'lead-in', 'second reply'), quote('c')], + text: 'after', + }; + renderDraft(draft); + + await pressRemove('b'); + // The text before the removed quote joins what follows it. + expect(host.draft()).toEqual({ + quotes: [quote('a', '', 'first reply'), quote('c', 'lead-in')], + text: 'after', + }); + + await undo(); + expect(host.draft()).toEqual(draft); + + await redo(); + expect(quoteIds()).toEqual(['a', 'c']); + expect(host.removedIds()).toEqual(['b', 'b']); + + await undo(); + expect(host.draft()).toEqual(draft); + }); + + it('undoes an unanswered quote removed with Backspace too, with the caret back in its gap', async () => { + renderDraft({ quotes: [quote('a', '', 'a reply'), quote('b')], text: '' }); + focusWithCaret(gapOf('b')); + + gapOf('b').dispatchEvent( + new KeyboardEvent('keydown', { key: 'Backspace', bubbles: true, cancelable: true }) + ); + await settle(); + expect(quoteIds()).toEqual(['a']); + expect(document.activeElement).toBe(mainInput()); + + await undo(); + expect(quoteIds()).toEqual(['a', 'b']); + expect(document.activeElement).toBe(gapOf('b')); + + await redo(); + expect(quoteIds()).toEqual(['a']); + expect(document.activeElement).toBe(mainInput()); + }); + + it('undoes in order with the typing before and after the removal', async () => { + renderDraft({ quotes: [quote('a', '', 'a reply')], text: '' }); + + stack.type(mainInput(), 'abc'); + await pressRemove('a'); + stack.type(mainInput(), 'def'); + await settle(); + expect(host.draft()).toEqual({ quotes: [], text: 'abcdef' }); + + await undo(); + expect(host.draft()).toEqual({ quotes: [], text: 'abc' }); + await undo(); + expect(host.draft()).toEqual({ quotes: [quote('a', '', 'a reply')], text: 'abc' }); + await undo(); + expect(host.draft()).toEqual({ quotes: [quote('a', '', 'a reply')], text: '' }); + + await redo(); + expect(host.draft()).toEqual({ quotes: [quote('a', '', 'a reply')], text: 'abc' }); + await redo(); + expect(host.draft()).toEqual({ quotes: [], text: 'abc' }); + await redo(); + expect(host.draft()).toEqual({ quotes: [], text: 'abcdef' }); + // Typing was the browser's own to undo and redo: the quote left once + // by hand and once by redo, and nothing else touched it. + expect(host.removedIds()).toEqual(['a', 'a']); + }); + + it('returns the caret to the reply it was taken from', async () => { + renderDraft({ quotes: [quote('a', '', 'a reply'), quote('b')], text: '' }); + focusWithCaret(replyOf('a')); + + await pressRemove('a'); + // Focus in the removed block moved to the input its text joined. + expect(document.activeElement).toBe(gapOf('b')); + + await undo(); + expect(document.activeElement).toBe(replyOf('a')); + + await redo(); + expect(document.activeElement).toBe(gapOf('b')); + }); + + it('leaves a caret that was elsewhere where it is, and scrolls the field to the restored block', async () => { + renderDraft({ quotes: [quote('a'), quote('b', '', 'b reply')], text: 'typing here' }); + const fieldEl = layOutField(); + // The main input is in the field's view; the block will come back below it. + mainInput().getBoundingClientRect = () => rect(210, 240); + focusWithCaret(mainInput()); + await pressRemove('b'); + expect(document.activeElement).toBe(mainInput()); + + stack.undo(); + flushSync(); + const block = target.querySelector('[data-quote-entry="b"] .composer-quote'); + if (!block) throw new Error('no block'); + block.getBoundingClientRect = () => rect(320, 380); + await settle(); + + expect(document.activeElement).toBe(mainInput()); + expect(fieldEl.scrollTop).toBe(79); + }); + + it('leaves the field scrolled where it was as the removal is recorded', async () => { + renderDraft({ quotes: [quote('a', '', 'a reply')], text: 'typing here' }); + const fieldEl = layOutField(); + mainInput().getBoundingClientRect = () => rect(320, 350); + focusWithCaret(mainInput()); + expect(fieldEl.scrollTop).toBe(49); + // Scrolled back up to the quote, the caret still in the main input: + // recording borrows focus and hands it back, which is not a move to + // follow. + fieldEl.scrollTop = 0; + + await pressRemove('a'); + + expect(document.activeElement).toBe(mainInput()); + expect(fieldEl.scrollTop).toBe(0); + }); + + it('leaves the quote out when the parent refuses the undo, and restores it on a later one', async () => { + const draft = { quotes: [quote('a', '', 'a reply')], text: '' }; + renderDraft(draft); + await pressRemove('a'); + + host.refuseUndo(true); + await undo(); + expect(quoteIds()).toEqual([]); + // Nothing came back, so there is nothing for redo to take out again. + await redo(); + expect(host.removedIds()).toEqual(['a']); + + host.refuseUndo(false); + await undo(); + expect(host.draft()).toEqual(draft); + }); + + it('withdraws an undo the parent is still holding on redo, rather than letting it land', async () => { + const draft = { quotes: [quote('a', '', 'a reply')], text: '' }; + renderDraft(draft); + await pressRemove('a'); + + // The parent accepts the undo but has not put the quote back yet (the + // pane's draft is loading): redo finds no quote in the box to take out. + host.holdUndo(true); + await undo(); + expect(quoteIds()).toEqual([]); + await redo(); + + host.landHeldUndo(); + await settle(); + expect(quoteIds()).toEqual([]); + // Taken back, not removed a second time. + expect(host.removedIds()).toEqual(['a']); + + // The removal is the step's to undo still. + host.holdUndo(false); + await undo(); + expect(host.draft()).toEqual(draft); + await redo(); + expect(quoteIds()).toEqual([]); + expect(host.removedIds()).toEqual(['a', 'a']); + }); + + it('takes the quote out again on redo once a held undo has landed', async () => { + const draft = { quotes: [quote('a', '', 'a reply')], text: '' }; + renderDraft(draft); + await pressRemove('a'); + + host.holdUndo(true); + await undo(); + host.landHeldUndo(); + await settle(); + expect(host.draft()).toEqual(draft); + + await redo(); + expect(quoteIds()).toEqual([]); + expect(host.removedIds()).toEqual(['a', 'a']); + }); + + it('records nothing for a removal the parent gives no way back from', async () => { + const props = render([quote('a')]); + const anchor = target.querySelector('.native-undo-anchor'); + if (!anchor) throw new Error('no undo anchor'); + + target.querySelector('.composer-quote-remove')!.click(); + await settle(); + + expect(props.onRemoveQuote).toHaveBeenCalledWith('a'); + expect(anchor.textContent).toBe(''); + expect(stack.undo()).toBe(false); + }); +}); diff --git a/apps/staged/src/lib/features/sessions/ChatComposer.svelte b/apps/staged/src/lib/features/sessions/ChatComposer.svelte index c68c7688f..6abce22e9 100644 --- a/apps/staged/src/lib/features/sessions/ChatComposer.svelte +++ b/apps/staged/src/lib/features/sessions/ChatComposer.svelte @@ -38,7 +38,9 @@ the neighbouring input (composerCaretNav.ts), a click on the field's own surface puts the caret in the nearest input, and only the last input shows a placeholder, while nothing has been typed anywhere. Removing a quote - moves focus that was in it to the input its text joined. + moves focus that was in it to the input its text joined, and is a step on + the browser's undo stack like the typing around it: Cmd+Z puts the quote + back where it was, and redo takes it out again. -->
+ {#if canAttachImages && attachedImageIds.length > 0}
{#each attachedImageIds as imageId (imageId)} @@ -471,7 +569,7 @@ {hashtagItems} {onkeydown} {oninput} - onRemoveQuote={(id) => void removeQuote(id)} + onRemoveQuote={removeQuote} /> {/if} { loadDraftMock.mockImplementation(async () => null); }); - type UndoToast = { onClick: () => void }; - - /** The Undo action of the newest "Quote removed" toast, with the toast's id. */ - function lastUndo(): { id: string; action: UndoToast } { - const call = toastMock.mock.calls[toastMock.mock.calls.length - 1] as unknown as [ - string, - { action: UndoToast }, - ]; - const id = toastMock.mock.results[toastMock.mock.results.length - 1].value as string; - return { id, action: call[1].action }; - } - /** * Mount a pane whose draft holds three quotes, then remove the middle one - * (which has a reply, so Undo is offered). Returns the draft as saved - * before the removal, for Undo to be measured against. + * as the composer's X would. Returns the draft as saved before the + * removal, for undo to be measured against. */ async function removeMiddleQuote(sessionId: string) { const { pane, props } = mountPane(sessionId); @@ -245,27 +233,52 @@ describe('SessionChatPane quoted replies', () => { await settle(); const before = composerDraftFromContent(lastSaved(replyKey(sessionId))); const [, second] = before.quotes; - document.querySelector(`[data-testid="remove-quote-${second.id}"]`)!.click(); - await settle(); + await removeQuote(second.id); expect(quoteCount()).toBe(2); return { pane, props, before, second }; } - function quoteCount(): number { + async function removeQuote(id: string) { + document.querySelector(`[data-testid="remove-quote-${id}"]`)!.click(); + await settle(); + } + + /** + * Undo the newest removal, as the browser's undo reaches the composer's + * step for it (the stub keeps the steps; see PromptInputStub.svelte). + */ + async function undoRemoval() { + document.querySelector('[data-testid="undo-remove-quote"]')!.click(); + await settle(); + } + + /** + * Redo the removal last undone, as the browser's redo reaches the + * composer's step for it. + */ + async function redoRemoval() { + document.querySelector('[data-testid="redo-remove-quote"]')!.click(); + await settle(); + } + + /** Whether the pane took each undo so far, oldest first. */ + function undoResults(): boolean[] { const input = document.querySelector('[data-testid="prompt-input"]'); - return Number(input?.dataset.quoteCount ?? NaN); + return (input?.dataset.undoResults ?? '') + .split(' ') + .filter(Boolean) + .map((result) => result === 'true'); } - /** The quote replies the pane asked the composer to put the caret in. */ - function focusedQuotes(): string[] { + /** The quotes the pane reported back in the composer after an undo. */ + function restoredQuotes(): string[] { const input = document.querySelector('[data-testid="prompt-input"]'); - return (input?.dataset.focusedQuotes ?? '').split(' ').filter(Boolean); + return (input?.dataset.restoredQuotes ?? '').split(' ').filter(Boolean); } - /** The quote blocks the pane asked the composer to scroll to without focusing. */ - function revealedQuotes(): string[] { + function quoteCount(): number { const input = document.querySelector('[data-testid="prompt-input"]'); - return (input?.dataset.revealedQuotes ?? '').split(' ').filter(Boolean); + return Number(input?.dataset.quoteCount ?? NaN); } function lastSaved(key: string) { @@ -293,88 +306,84 @@ describe('SessionChatPane quoted replies', () => { unmount(pane); }); - it('offers to undo removing a quote with a reply, putting it back in its place', async () => { - toastMock.mockClear(); - const { pane } = mountPane('session-undo'); - flushSync(); - await settle(); - - await addQuoteToReplyDraft('session-undo', { ...quote, quote: 'First.' }); - await addQuoteToReplyDraft('session-undo', { ...quote, quote: 'Second.' }); - await addQuoteToReplyDraft('session-undo', { ...quote, quote: 'Third.', reply: '' }); - await settle(); - const before = composerDraftFromContent(lastSaved(replyKey('session-undo'))); - const [, second] = before.quotes; + it('removes a quote without a toast, and undo puts it back in its place', async () => { + const { pane, before, second } = await removeMiddleQuote('session-undo'); + const removed = composerDraftFromContent(lastSaved(replyKey('session-undo'))); + expect(removed.quotes.map((q) => q.quote)).toEqual(['First.', 'Third.']); + expect(toastMock).not.toHaveBeenCalled(); - document.querySelector(`[data-testid="remove-quote-${second.id}"]`)!.click(); - await settle(); - - expect(quoteCount()).toBe(2); - expect(toastMock).toHaveBeenCalledTimes(1); - const [, options] = toastMock.mock.calls[0] as [string, { action: { onClick: () => void } }]; - expect(options.action).toMatchObject({ label: 'Undo' }); - - options.action.onClick(); - await settle(); + await undoRemoval(); + expect(undoResults()).toEqual([true]); expect(quoteCount()).toBe(3); const restored = composerDraftFromContent(lastSaved(replyKey('session-undo'))); expect(restored.quotes.map((q) => q.quote)).toEqual(['First.', 'Second.', 'Third.']); expect(restored.quotes[1]).toMatchObject({ id: second.id, reply: 'Which one?' }); expect(restored).toEqual(before); + // The composer is told once the block is back in it, to place the caret. + expect(restoredQuotes()).toEqual([second.id]); - // An unanswered quote just goes. - const third = restored.quotes[2]; - document.querySelector(`[data-testid="remove-quote-${third.id}"]`)!.click(); - await settle(); - expect(quoteCount()).toBe(2); - expect(toastMock).toHaveBeenCalledTimes(1); + // Redo is the composer removing the quote again, which undo reverses again. + await removeQuote(second.id); + expect(composerDraftFromContent(lastSaved(replyKey('session-undo')))).toEqual(removed); + await undoRemoval(); + expect(composerDraftFromContent(lastSaved(replyKey('session-undo')))).toEqual(before); + + expect(toastMock).not.toHaveBeenCalled(); + expect(toastMock.error).not.toHaveBeenCalled(); unmount(pane); }); - it('puts the caret in the restored reply for a pointer, and only scrolls to the block on a touch screen', async () => { - const first = await removeMiddleQuote('session-caret'); - lastUndo().action.onClick(); + it('removes an unanswered quote the same way: no toast, and undo puts it back', async () => { + const { pane } = mountPane('session-unanswered'); + flushSync(); await settle(); - expect(quoteCount()).toBe(3); - // The focus brings the block into the field's view itself. - expect(focusedQuotes()).toEqual([first.second.id]); - expect(revealedQuotes()).toEqual([]); - unmount(first.pane); - - // A touch screen's keyboard would rise over the restored draft, so the - // field is scrolled to the block and nothing is focused. - viewportMock.hasCoarsePointer = true; - try { - const second = await removeMiddleQuote('session-caret-touch'); - lastUndo().action.onClick(); - await settle(); - expect(quoteCount()).toBe(3); - expect(focusedQuotes()).toEqual([]); - expect(revealedQuotes()).toEqual([second.second.id]); - unmount(second.pane); - } finally { - viewportMock.hasCoarsePointer = false; - } + await addQuoteToReplyDraft('session-unanswered', { ...quote, reply: '' }); + await settle(); + const before = composerDraftFromContent(lastSaved(replyKey('session-unanswered'))); + + await removeQuote(before.quotes[0].id); + expect(quoteCount()).toBe(0); + expect(toastMock).not.toHaveBeenCalled(); + + await undoRemoval(); + expect(quoteCount()).toBe(1); + expect(composerDraftFromContent(lastSaved(replyKey('session-unanswered')))).toEqual(before); + + unmount(pane); }); - it('takes the Undo toast down with a send, and a late Undo puts nothing back', async () => { + it('undoes removals newest first, each quote going back to its own place', async () => { + const { pane, before } = await removeMiddleQuote('session-two'); + const [first] = before.quotes; + await removeQuote(first.id); + expect(quoteCount()).toBe(1); + + await undoRemoval(); + expect( + composerDraftFromContent(lastSaved(replyKey('session-two'))).quotes.map((q) => q.quote) + ).toEqual(['First.', 'Third.']); + await undoRemoval(); + expect(composerDraftFromContent(lastSaved(replyKey('session-two')))).toEqual(before); + + unmount(pane); + }); + + it('puts nothing back once the draft the quote came from has been sent', async () => { const { pane } = await removeMiddleQuote('session-sent'); - const undo = lastUndo(); document.querySelector('[data-testid="composer-send"]')!.click(); await settle(); expect(quoteCount()).toBe(0); expect(writer.clearNow).toHaveBeenCalledWith(replyKey('session-sent')); - expect(toastMock.dismiss).toHaveBeenCalledWith(undo.id); - // The toast's leaving animation can still take a click: the draft the - // block came from has gone with the message, so nothing is restored — - // neither into the empty composer nor into the cleared stored draft. + // The draft the block came from has gone with the message, so an undo + // that reaches the removal now restores nothing — neither into the empty + // composer nor into the cleared stored draft. writer.scheduleSave.mockClear(); - undo.action.onClick(); - await settle(); + await undoRemoval(); + expect(undoResults()).toEqual([false]); expect(quoteCount()).toBe(0); expect(lastSaved(replyKey('session-sent'))).toBeUndefined(); expect(saveDraftMock).not.toHaveBeenCalled(); @@ -382,52 +391,47 @@ describe('SessionChatPane quoted replies', () => { unmount(pane); }); - it('keeps the Undo toast through a failed send, whose draft is put back', async () => { + it('still undoes the removal after a failed send, whose draft is put back', async () => { const { pane, before } = await removeMiddleQuote('session-failed'); - const undo = lastUndo(); resumeSessionMock.mockRejectedValueOnce(new Error('agent gone')); document.querySelector('[data-testid="composer-send"]')!.click(); await settle(); // The draft is back in the box, the block it lost still restorable. expect(quoteCount()).toBe(2); - expect(toastMock.dismiss).not.toHaveBeenCalled(); - undo.action.onClick(); - await settle(); + await undoRemoval(); + expect(undoResults()).toEqual([true]); expect(quoteCount()).toBe(3); expect(composerDraftFromContent(lastSaved(replyKey('session-failed')))).toEqual(before); - // Pressed, the toast is gone with the press; the send that follows has - // nothing left to take down. - document.querySelector('[data-testid="composer-send"]')!.click(); - await settle(); - expect(quoteCount()).toBe(0); - expect(toastMock.dismiss).not.toHaveBeenCalled(); - unmount(pane); }); - it('takes the Undo toast down when the pane turns to another session', async () => { + it('puts nothing back once the pane has turned to another session', async () => { const { pane, props } = await removeMiddleQuote('session-left'); - const undo = lastUndo(); props.sessionId = 'session-next'; flushSync(); await settle(); - expect(toastMock.dismiss).toHaveBeenCalledWith(undo.id); + writer.scheduleSave.mockClear(); + + await undoRemoval(); + expect(undoResults()).toEqual([false]); + expect(quoteCount()).toBe(0); + expect(lastSaved(replyKey('session-next'))).toBeUndefined(); + expect(saveDraftMock).not.toHaveBeenCalled(); unmount(pane); }); - it('holds an Undo that lands while the draft is reloading until the draft has landed', async () => { + it('holds an undo that lands while the draft is reloading until the draft has landed', async () => { // The pane goes inactive (the note's chat closed) and comes back for the - // same session within the toast's lifetime. Its draft is loading again - // when Undo is pressed: the restore waits for the load, rather than - // being applied to the composer and then replaced by the stored draft. + // same session. Its draft is loading again when the removal is undone: + // the restore waits for the load, rather than being applied to the + // composer and then replaced by the stored draft. const { pane, props, before, second } = await removeMiddleQuote('session-reload'); const stored = lastSaved(replyKey('session-reload')); - const undo = lastUndo(); props.active = false; flushSync(); @@ -443,48 +447,107 @@ describe('SessionChatPane quoted replies', () => { props.active = true; flushSync(); await settle(); - expect(toastMock.dismiss).not.toHaveBeenCalled(); - undo.action.onClick(); - await settle(); + await undoRemoval(); + expect(undoResults()).toEqual([true]); expect(saveDraftMock).not.toHaveBeenCalled(); - // The caret follows the block into the composer: not yet, while the - // block is still held with the restore, or it would be looked for - // before it is rendered and the ask dropped. - expect(focusedQuotes()).toEqual([]); - expect(revealedQuotes()).toEqual([]); + // The composer hears the block is back only once it is: not yet, while + // it is still held with the restore, or the caret would be sent to a + // block that isn't rendered. + expect(restoredQuotes()).toEqual([]); land(stored); await settle(); expect(quoteCount()).toBe(3); expect(composerDraftFromContent(lastSaved(replyKey('session-reload')))).toEqual(before); - expect(focusedQuotes()).toEqual([second.id]); - expect(revealedQuotes()).toEqual([]); + expect(restoredQuotes()).toEqual([second.id]); + + unmount(pane); + }); + + it('leaves the quote out when the removal is redone while its undo is still held for the reloading draft', async () => { + // Undo, then redo, both inside the reload: the held restore has put no + // block in the box for the redo to take out, so the redo withdraws it. + // Left queued, it would bring the block back once the draft landed. + const { pane, props, before, second } = await removeMiddleQuote('session-reload-redo'); + const stored = lastSaved(replyKey('session-reload-redo')); + + props.active = false; + flushSync(); + await settle(); + + let land!: (value: typeof stored) => void; + loadDraftMock.mockImplementation( + () => + new Promise((resolve) => { + land = resolve; + }) + ); + props.active = true; + flushSync(); + await settle(); + + await undoRemoval(); + expect(undoResults()).toEqual([true]); + await redoRemoval(); + + land(stored); + await settle(); + expect(quoteCount()).toBe(2); + // The composer is never told of a block that did not come back. + expect(restoredQuotes()).toEqual([]); + + // The removal is still there to undo, and puts the block back as it was. + await undoRemoval(); + expect(undoResults()).toEqual([true, true]); + expect(quoteCount()).toBe(3); + expect(composerDraftFromContent(lastSaved(replyKey('session-reload-redo')))).toEqual(before); + expect(restoredQuotes()).toEqual([second.id]); unmount(pane); }); - it('restores into the stored draft when the pane no longer shows the session', async () => { - const { pane, props, before } = await removeMiddleQuote('session-away'); + it('sends no held edit to the stored draft when the pane moves on after a withdrawn undo', async () => { + const { pane, props } = await removeMiddleQuote('session-reload-away'); + const stored = lastSaved(replyKey('session-reload-away')); + + props.active = false; + flushSync(); + await settle(); + loadDraftMock.mockImplementation(() => new Promise(() => {})); + props.active = true; + flushSync(); + await settle(); + + await undoRemoval(); + await redoRemoval(); + writer.run.mockClear(); + + props.active = false; + flushSync(); + await settle(); + expect(writer.run).not.toHaveBeenCalled(); + + unmount(pane); + }); + + it('puts nothing back, in the composer or the stored draft, while the pane shows no session', async () => { + const { pane, props } = await removeMiddleQuote('session-away'); const stored = lastSaved(replyKey('session-away')); - const undo = lastUndo(); props.active = false; flushSync(); await settle(); loadDraftMock.mockImplementation(async () => stored); + writer.scheduleSave.mockClear(); + writer.run.mockClear(); - undo.action.onClick(); - await settle(); + await undoRemoval(); - // Through the shared writer, so it queues behind any pending save. - expect(writer.run).toHaveBeenCalledTimes(1); - expect(saveDraftMock).toHaveBeenCalledTimes(1); - const [key, content] = saveDraftMock.mock.calls[0]; - expect(key).toBe(replyKey('session-away')); - expect( - composerDraftFromContent(content as Parameters[0]) - ).toEqual(before); + expect(undoResults()).toEqual([false]); + expect(writer.run).not.toHaveBeenCalled(); + expect(saveDraftMock).not.toHaveBeenCalled(); + expect(lastSaved(replyKey('session-away'))).toBeUndefined(); unmount(pane); }); diff --git a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte index 08d4955cb..de290ab80 100644 --- a/apps/staged/src/lib/features/sessions/SessionChatPane.svelte +++ b/apps/staged/src/lib/features/sessions/SessionChatPane.svelte @@ -107,7 +107,7 @@ setAcpConfigPref, setComposerSendMode, } from '../settings/preferences.svelte'; - import ChatComposer, { composerControlClass } from './ChatComposer.svelte'; + import ChatComposer, { composerControlClass, type RemovedQuote } from './ChatComposer.svelte'; import ComposerError from './ComposerError.svelte'; import { composerSendControl, @@ -236,7 +236,6 @@ import { loadReplyDraft as loadStoredReplyDraft, amendStoredReplyDraft, - restoreQuoteInReplyDraft, registerReplyComposer, replyDraftKey, replyDraftWriter, @@ -256,7 +255,6 @@ takeDraftQuote, type ComposerDraft, type DraftQuote, - type RemovedDraftQuote, } from './composerDraft'; import QuoteExcerpt from './QuoteExcerpt.svelte'; import NoteActivityCard from './NoteActivityCard.svelte'; @@ -455,26 +453,22 @@ // exactly that run so inherited text isn't filed under this session's key. let suppressNextReplySave = false; /** - * Edits a note made to this session's draft (a quote added, its reply - * typed, a removed block put back) while the draft's load was still in - * flight: they are applied once the stored draft has landed, rather than - * being replaced by it, and what was to follow each in the composer (the - * caret into the block, or the field scrolled to it) runs then too, once - * the block is rendered. Keyed to the session they were made for. + * Edits made to this session's draft from outside the composer's inputs + * (a quote a note added, its reply typed there, a removed block put back + * by undo) while the draft's load was still in flight: they are applied + * once the stored draft has landed, rather than being replaced by it, and + * what was to follow each in the composer (the caret into the block, or + * the field scrolled to it) runs then too, once the block is rendered. + * Keyed to the session they were made for. */ - let pendingDraftEdits: { - sessionId: string; - edits: Array<{ edit: (draft: ComposerDraft) => ComposerDraft; then?: () => void }>; - } | null = null; + type PendingDraftEdit = { edit: (draft: ComposerDraft) => ComposerDraft; then?: () => void }; + let pendingDraftEdits: { sessionId: string; edits: PendingDraftEdit[] } | null = null; /** - * Undo toasts for removed quote blocks still on screen (svelte-sonner - * keeps one up for seconds). A send the backend accepts clears the draft - * they were taken from, and a switch shows another session's; either - * dismisses them, so Undo can't put a block into a draft it never belonged - * to. `draftEpoch` counts the sends, for a click that lands as the toast - * is leaving. + * Counts the drafts sent from this composer. A removed quote block can be + * put back by undo (see `removeQuote`) only into the draft it was taken + * from; a send the backend accepts clears that draft, and an undo made + * after it must not put the block into the next one. */ - let quoteUndoToasts: Array<{ toastId: string | number; sessionId: string }> = []; let draftEpoch = 0; /** The composer's draft as one value (live `$state`; clone before keeping it). */ @@ -537,22 +531,40 @@ * the held edits land — so a caret or scroll meant for a block the edit * adds finds it rendered rather than running against the loading draft. * It is dropped with the edits that go to the stored draft instead when - * the pane moves on, there being no composer to act in. + * the pane moves on, there being no composer to act in. Returns the edit + * as it is held (for `withdrawPendingDraftEdit`), or null once applied. */ function receiveDraftEdit( id: string, edit: (draft: ComposerDraft) => ComposerDraft, then?: () => void - ) { + ): PendingDraftEdit | null { if (loadedDraftSessionId !== id) { if (pendingDraftEdits?.sessionId !== id) pendingDraftEdits = { sessionId: id, edits: [] }; - pendingDraftEdits.edits.push({ edit, then }); - return; + const held = { edit, then }; + pendingDraftEdits.edits.push(held); + return held; } // Quoting is a draft edit: the last failure line has been read. sendError = null; setDraft(edit(currentDraft())); then?.(); + return null; + } + + /** + * Take a held edit back before the draft's load lands, so it is never + * applied and its `then` never runs. False when it is no longer held: it + * went into the composer with the loaded draft, or to the stored draft + * when the pane moved on. + */ + function withdrawPendingDraftEdit(held: PendingDraftEdit): boolean { + const index = pendingDraftEdits?.edits.indexOf(held) ?? -1; + if (!pendingDraftEdits || index === -1) return false; + pendingDraftEdits.edits.splice(index, 1); + // Nothing left to hold: a pane moving on has no edit to send to the store. + if (pendingDraftEdits.edits.length === 0) pendingDraftEdits = null; + return true; } function applyPendingDraftEdits(id: string) { @@ -584,96 +596,58 @@ pendingDraftEdits.edits.push({ edit: (draft) => draft, then }); } - /** Scroll the composer's field to a quote block, once it is rendered, leaving focus where it is. */ - function revealDraftQuote(quoteId: string) { - tick().then(() => { - if (!closed && active) composerView?.revealQuote(quoteId); - }); - } - /** - * Take a quote block out of the draft. One with a reply typed under it is - * offered back in a toast (there is no other undo for a block): Undo puts - * it back where it was, reply and lead-in text included, and — with a - * pointer, not a touch screen — the caret in its reply; on a touch screen - * the field is scrolled to show it instead. An unanswered block just goes. + * Take a quote block out of the draft, with no notice: the composer makes + * the removal a step of its undo history, and what is returned is how + * that undo puts the block back where it was, reply and lead-in text + * included. The block goes back only into the draft it was taken from, + * while this pane shows it — after a send the backend accepted, or with + * another session in the box, the undo does nothing — and by the same way + * in as a quote a note adds, so one made while the draft is loading again + * is held until it has landed rather than being replaced by it. A redo + * that comes while it is still held finds no block in the box to take + * out: it withdraws the held undo instead, and the block stays out when + * the draft lands, still this removal's to put back. */ - function removeQuote(id: string) { + function removeQuote(id: string): RemovedQuote | null { const taken = takeDraftQuote(currentDraft(), id); - if (!taken) return; + if (!taken) return null; sendError = null; setDraft(taken.draft); - if (taken.removed.quote.reply.trim() !== '') offerQuoteUndo(sessionId, taken.removed); - } - - function offerQuoteUndo(sid: string | null, removed: RemovedDraftQuote) { - if (!sid) return; + // Quotes only come with a session's draft; without one there is no + // draft to tell the block's own from, so the removal just stands. + const sid = sessionId; + if (!sid) return null; const epoch = draftEpoch; - const forget = () => { - quoteUndoToasts = quoteUndoToasts.filter((entry) => entry.toastId !== toastId); - }; - const toastId = toast('Quote removed from your reply', { - onDismiss: forget, - onAutoClose: forget, - action: { - label: 'Undo', - onClick: () => { - // The toast goes with the press (svelte-sonner deletes it without - // `onDismiss`), so it is forgotten here. - forget(); - // The draft the block was taken from has been sent since. - if (epoch !== draftEpoch) return; - if (active && sessionId === sid) { - // The same way in as a quote a note adds: applied to the - // composer, or held until this session's draft has loaded, so a - // load in flight can't land over it. - // The caret goes back into the reply for a keyboard user; with - // a touch screen that would raise the on-screen keyboard over - // the restored draft, which the user hasn't asked to type in, - // so there the field is only scrolled to the block, which in a - // capped field that has scrolled on may otherwise come back - // out of view. Either follows the block into the composer, so - // a restore held for the load isn't looked for before it lands. - receiveDraftEdit( - sid, - (draft) => restoreDraftQuote(draft, removed), - () => { - if (!viewport.hasCoarsePointer) focusDraftQuoteReply(removed.quote.id); - else revealDraftQuote(removed.quote.id); - } - ); - return; - } - // This pane has moved on: another pane showing the draft takes it, - // or else the stored draft. - restoreQuoteInReplyDraft(sid, removed).catch((err) => { - console.error('[SessionChatPane] Failed to restore the quote:', err); - toast.error('Could not restore the quote'); - }); - }, + // The undo while it waits on the draft's load, for a redo to withdraw. + let heldRestore: PendingDraftEdit | null = null; + return { + restore(then) { + if (closed || !active || sessionId !== sid || epoch !== draftEpoch) return false; + heldRestore = receiveDraftEdit( + sid, + (draft) => restoreDraftQuote(draft, taken.removed), + then + ); + return true; }, - }); - quoteUndoToasts = [...quoteUndoToasts, { toastId, sessionId: sid }]; - } - - /** Take down the Undo toasts whose draft is gone or off screen (all, by default). */ - function dismissQuoteUndo(keep: (sessionId: string) => boolean = () => false) { - const going = quoteUndoToasts.filter((entry) => !keep(entry.sessionId)); - if (going.length === 0) return; - quoteUndoToasts = quoteUndoToasts.filter((entry) => keep(entry.sessionId)); - for (const entry of going) toast.dismiss(entry.toastId); + withdraw() { + const held = heldRestore; + heldRestore = null; + return held !== null && withdrawPendingDraftEdit(held); + }, + }; } /** * The draft has gone with a message the backend accepted: a block removed * from it has nothing to go back into. Not before — a send that fails puts - * the draft back in the box, and the block's Undo should still hold then. - * (Pressed while the send is in flight, Undo puts the block into the + * the draft back in the box, and undoing the block's removal should still + * hold then. (Undone while the send is in flight, the block goes into the * emptied box, where it is a new draft.) */ function draftSent() { draftEpoch += 1; - dismissQuoteUndo(); } let discoveredFollowupConfig = $state(null); @@ -1212,9 +1186,6 @@ const draftWhenLoadStarted = untrack(() => cloneComposerDraft(currentDraft())); suppressNextReplySave = false; loadedDraftSessionId = null; - // Another session's draft is coming up: an Undo for a block removed from - // the previous one would read as this one's. - dismissQuoteUndo((sid) => sid === id); replyDraftLoad = loadReplyDraft(id, generation, draftWhenLoadStarted); }); @@ -1258,7 +1229,6 @@ receiveDraftEdit(id, (draft) => appendDraftQuote(draft, quote, quoteId)), setQuoteReply: (quoteId, reply) => receiveDraftEdit(id, (draft) => setDraftQuoteReply(draft, quoteId, reply)), - restoreQuote: (removed) => receiveDraftEdit(id, (draft) => restoreDraftQuote(draft, removed)), focusQuoteReply: (quoteId) => whenDraftLoaded(id, () => focusDraftQuoteReply(quoteId)), }); return () => { diff --git a/apps/staged/src/lib/features/sessions/replyDrafts.test.ts b/apps/staged/src/lib/features/sessions/replyDrafts.test.ts index 19a3ac3b9..6dba6cb3e 100644 --- a/apps/staged/src/lib/features/sessions/replyDrafts.test.ts +++ b/apps/staged/src/lib/features/sessions/replyDrafts.test.ts @@ -26,12 +26,7 @@ vi.mock('../../shared/persistentStore', () => ({ import { setStoreValue } from '../../shared/persistentStore'; import { saveDraft, DRAFT_TTL_MS } from '../../shared/draftCache'; -import { - appendDraftQuote, - composerDraftContent, - serializeComposerDraft, - takeDraftQuote, -} from './composerDraft'; +import { serializeComposerDraft } from './composerDraft'; import { addQuoteToReplyDraft, focusReplyDraftQuote, @@ -40,7 +35,6 @@ import { replyDraftKey, replyDraftPresence, replyDraftWriter, - restoreQuoteInReplyDraft, setReplyDraftQuoteReply, startLiveQuoteReply, type ReplyComposerTarget, @@ -147,16 +141,10 @@ describe('addQuoteToReplyDraft', () => { const id = await addQuoteToReplyDraft('s6', quote); await setReplyDraftQuoteReply('s6', id, 'Because.'); - const removed = takeDraftQuote( - appendDraftQuote({ quotes: [], text: '' }, quote, id), - id - )!.removed; - await restoreQuoteInReplyDraft('s6', removed); expect(newer.addQuote).not.toHaveBeenCalled(); expect(older.addQuote).toHaveBeenCalledWith(quote, id); expect(older.setQuoteReply).toHaveBeenCalledWith(id, 'Because.'); - expect(older.restoreQuote).toHaveBeenCalledWith(removed); expect(focusReplyDraftQuote('s6', id)).toBe(true); expect(older.focusQuoteReply).toHaveBeenCalledWith(id); expect(newer.focusQuoteReply).not.toHaveBeenCalled(); @@ -195,45 +183,10 @@ function fakeComposer(): { [K in keyof ReplyComposerTarget]: Mock(), setQuoteReply: vi.fn(), - restoreQuote: vi.fn(), focusQuoteReply: vi.fn(), }; } -describe('restoreQuoteInReplyDraft', () => { - beforeEach(() => { - store.clear(); - }); - - it('puts a removed block back into the stored draft when no composer shows it', async () => { - await addQuoteToReplyDraft('u1', quote, 'first'); - await addQuoteToReplyDraft('u1', { ...quote, quote: 'Second.' }, 'second'); - const before = await loadReplyDraft('u1'); - const taken = takeDraftQuote(before, 'first')!; - await saveDraft(replyDraftKey('u1'), composerDraftContent(taken.draft)); - expect((await loadReplyDraft('u1')).quotes.map((q) => q.id)).toEqual(['second']); - - await restoreQuoteInReplyDraft('u1', taken.removed); - - expect(await loadReplyDraft('u1')).toEqual(before); - }); - - it('goes to the open composer instead of the store', async () => { - const composer = fakeComposer(); - const unregister = registerReplyComposer('u2', composer); - const removed = takeDraftQuote( - { quotes: [{ id: 'q', before: '', ...quote }], text: '' }, - 'q' - )!.removed; - - await restoreQuoteInReplyDraft('u2', removed); - - expect(composer.restoreQuote).toHaveBeenCalledWith(removed); - expect(store.has(replyDraftKey('u2'))).toBe(false); - unregister(); - }); -}); - describe('setReplyDraftQuoteReply', () => { beforeEach(() => { store.clear(); diff --git a/apps/staged/src/lib/features/sessions/replyDrafts.ts b/apps/staged/src/lib/features/sessions/replyDrafts.ts index 5b97a9e0a..120dcea31 100644 --- a/apps/staged/src/lib/features/sessions/replyDrafts.ts +++ b/apps/staged/src/lib/features/sessions/replyDrafts.ts @@ -29,13 +29,11 @@ import { composerDraftContent, composerDraftFromContent, newDraftQuoteId, - restoreDraftQuote, setDraftQuoteReply, summarizeComposerDraft, type ComposerDraft, type ComposerDraftSummary, type NewDraftQuote, - type RemovedDraftQuote, } from './composerDraft'; const REPLY_DRAFT_KEY_PREFIX = 'draft:session-reply:'; @@ -66,8 +64,6 @@ export type ReplyComposerTarget = { addQuote(quote: NewDraftQuote, id: string): void; /** Replace the reply of the quote with this id; nothing if it has been removed. */ setQuoteReply(id: string, reply: string): void; - /** Put a removed quote block back where it was (the composer's Undo). */ - restoreQuote(removed: RemovedDraftQuote): void; /** Put the caret in that quote's reply input, if the block is shown. */ focusQuoteReply(id: string): void; }; @@ -146,28 +142,6 @@ export function setReplyDraftQuoteReply( return amendStoredReplyDraft(sessionId, (draft) => setDraftQuoteReply(draft, id, reply)); } -/** - * Put a quote block a composer removed back into the session's reply draft - * (its Undo; see `takeDraftQuote`), wherever the draft is now: the composer - * showing it — this one, mid-load or not, or another pane's with the same - * session — or else the stored draft. A write to the store under an open - * composer would be overwritten by its next save, as with - * `addQuoteToReplyDraft`. Rejects when the stored draft couldn't be written. - */ -export function restoreQuoteInReplyDraft( - sessionId: string, - removed: RemovedDraftQuote -): Promise { - const live = liveComposers(sessionId); - if (live.length > 0) { - // A composer that didn't remove the block still holds it; its restore is - // a no-op (see `restoreDraftQuote`). - for (const composer of live) composer.restoreQuote(removed); - return Promise.resolve(); - } - return amendStoredReplyDraft(sessionId, (draft) => restoreDraftQuote(draft, removed)); -} - /** * Put the caret in a quote block's reply input. Only a composer showing the * draft can; returns whether one did. With several showing it, the newest diff --git a/apps/staged/src/lib/features/sessions/testStubs/ComposerDraftHost.svelte b/apps/staged/src/lib/features/sessions/testStubs/ComposerDraftHost.svelte new file mode 100644 index 000000000..474f12404 --- /dev/null +++ b/apps/staged/src/lib/features/sessions/testStubs/ComposerDraftHost.svelte @@ -0,0 +1,103 @@ + + + + diff --git a/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte b/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte index bde56f55e..1dcf82574 100644 --- a/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte +++ b/apps/staged/src/lib/features/sessions/testStubs/PromptInputStub.svelte @@ -9,11 +9,20 @@ count it shows as `data-quote-count`, and offers a remove button per quote (`data-testid="remove-quote-"`) that calls `onRemoveQuote` as the composer's X would, a Send button (`data-testid="composer-send"`) that - calls `onSend`, a `focusQuoteReply` that places no caret but records the - ids asked for on the input (`data-focused-quotes`), and a `revealQuote` - that scrolls nothing but records likewise (`data-revealed-quotes`). + calls `onSend`, and a `focusQuoteReply` that places no caret. + + The composer's undo history is stood in for by a stack of the removals + `onRemoveQuote` returned: an undo button (`data-testid="undo-remove-quote"`) + restores the newest, as the browser's undo would, records on the input + whether the pane took it (`data-undo-results`), and the ids of the quotes + the pane then reported back in the composer (`data-restored-quotes`). A + redo button (`data-testid="redo-remove-quote"`) takes the quote last + undone out again as the composer's redo does: it withdraws an undo the + pane is still holding, and otherwise removes the quote from the box. --> @@ -60,8 +88,8 @@ data-value={value} data-send-split={sendControl?.split} data-quote-count={quotes.length} - data-focused-quotes={focusedQuotes.join(' ')} - data-revealed-quotes={revealedQuotes.join(' ')} + data-undo-results={undoResults.join(' ')} + data-restored-quotes={restoredQuotes.join(' ')} oninput={(e) => (value = e.currentTarget.textContent ?? '')} >
{#each quotes as quote (quote.id)} @@ -69,9 +97,11 @@ type="button" aria-label="Remove quote" data-testid="remove-quote-{quote.id}" - onclick={() => onRemoveQuote?.(quote.id)} + onclick={() => removeQuote(quote.id)} > {/each} + + {#if sendError} diff --git a/apps/staged/src/lib/shared/NativeUndoAnchor.component.test.ts b/apps/staged/src/lib/shared/NativeUndoAnchor.component.test.ts new file mode 100644 index 000000000..7b8e57070 --- /dev/null +++ b/apps/staged/src/lib/shared/NativeUndoAnchor.component.test.ts @@ -0,0 +1,209 @@ +import { flushSync, mount, unmount } from 'svelte'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import NativeUndoAnchor from './NativeUndoAnchor.svelte'; +import { installNativeUndoStack, type NativeUndoStack } from './testStubs/nativeUndoStack'; + +// The anchor's side of the browser's undo stack: what `record` puts on it, +// and which steps run as undo and redo walk it. jsdom has no such stack, so +// one is stood in (testStubs/nativeUndoStack.ts); that a real browser keeps +// the anchor's edits in order with those of the inputs around it is what +// the component is built on, and isn't something jsdom can show. + +let component: ReturnType, { record: Recorder }>> | null = null; +let target: HTMLElement; +let stack: NativeUndoStack; + +type Recorder = (step: { undo: () => void; redo: () => void }) => boolean; + +function render() { + target = document.createElement('div'); + document.body.appendChild(target); + component = mount(NativeUndoAnchor, { target }); + flushSync(); +} + +function record(name: string, log: string[]): boolean { + return component!.record({ + undo: () => log.push(`undo ${name}`), + redo: () => log.push(`redo ${name}`), + }); +} + +function anchor(): HTMLElement { + const el = target.querySelector('.native-undo-anchor'); + if (!el) throw new Error('no anchor'); + return el; +} + +/** An editable of the page's own, for focus and typing around the anchor's steps. */ +function editor(text = ''): HTMLElement { + const el = document.createElement('div'); + el.setAttribute('contenteditable', 'true'); + el.textContent = text; + document.body.appendChild(el); + return el; +} + +beforeEach(() => { + stack = installNativeUndoStack(); +}); + +afterEach(() => { + if (component) unmount(component); + component = null; + stack.uninstall(); + document.body.innerHTML = ''; + window.getSelection()?.removeAllRanges(); +}); + +describe('NativeUndoAnchor', () => { + it('stays out of the tab order and the accessibility tree', () => { + render(); + + expect(anchor().getAttribute('contenteditable')).toBe('true'); + expect(anchor().tabIndex).toBe(-1); + expect(anchor().getAttribute('aria-hidden')).toBe('true'); + }); + + it('records a step as one edit of its own, and runs it as undo and redo walk the stack', () => { + render(); + const log: string[] = []; + + expect(record('a', log)).toBe(true); + expect(anchor().textContent).toHaveLength(1); + // Recording is not an undo or a redo. + expect(log).toEqual([]); + + expect(stack.undo()).toBe(true); + expect(anchor().textContent).toBe(''); + expect(stack.redo()).toBe(true); + expect(anchor().textContent).toHaveLength(1); + expect(log).toEqual(['undo a', 'redo a']); + }); + + it('keeps its steps in order with the typing before and after them', () => { + render(); + const input = editor(); + const log: string[] = []; + + stack.type(input, 'abc'); + record('a', log); + stack.type(input, 'def'); + + stack.undo(); + expect(input.textContent).toBe('abc'); + expect(log).toEqual([]); + stack.undo(); + expect(input.textContent).toBe('abc'); + expect(log).toEqual(['undo a']); + stack.undo(); + expect(input.textContent).toBe(''); + + stack.redo(); + expect(input.textContent).toBe('abc'); + expect(log).toEqual(['undo a']); + stack.redo(); + expect(log).toEqual(['undo a', 'redo a']); + stack.redo(); + expect(input.textContent).toBe('abcdef'); + }); + + it('undoes the newest step first, and drops the undone ones when another is recorded', () => { + render(); + const log: string[] = []; + record('a', log); + record('b', log); + + stack.undo(); + stack.undo(); + expect(log).toEqual(['undo b', 'undo a']); + stack.redo(); + expect(log).toEqual(['undo b', 'undo a', 'redo a']); + + // Recorded over the undone `b`, as a new edit drops the browser's redo. + record('c', log); + expect(stack.redo()).toBe(false); + stack.undo(); + stack.undo(); + expect(log).toEqual(['undo b', 'undo a', 'redo a', 'undo c', 'undo a']); + }); + + it('runs every step between where the stack stood and where it stands', () => { + render(); + const log: string[] = []; + record('a', log); + record('b', log); + + // A browser that walks both edits as one (WebKit groups those made + // while handling the same event) reports the pair in a single change. + anchor().textContent = ''; + anchor().dispatchEvent(new InputEvent('input', { bubbles: true, inputType: 'historyUndo' })); + expect(log).toEqual(['undo b', 'undo a']); + }); + + it('gives focus and the caret back after recording', () => { + render(); + const input = editor('some text'); + input.focus(); + const text = input.firstChild!; + window.getSelection()!.setBaseAndExtent(text, 4, text, 4); + + record('a', []); + + expect(document.activeElement).toBe(input); + const selection = window.getSelection()!; + expect(selection.anchorNode).toBe(text); + expect(selection.anchorOffset).toBe(4); + }); + + it('takes no focus to keep, and leaves no selection of its own, when recording with none', () => { + render(); + + record('a', []); + + expect(document.activeElement).toBe(document.body); + const selection = window.getSelection()!; + expect(anchor().contains(selection.anchorNode)).toBe(false); + }); + + it('gives focus back before a step runs, so the step finds it where the user had it', () => { + render(); + const input = editor('some text'); + let focusedDuringUndo: Element | null = null; + component!.record({ + undo: () => (focusedDuringUndo = document.activeElement), + redo: () => {}, + }); + input.focus(); + const text = input.firstChild!; + window.getSelection()!.setBaseAndExtent(text, 2, text, 2); + + // The browser moves focus to the anchor as it undoes the edit there. + stack.undo(); + + expect(focusedDuringUndo).toBe(input); + expect(document.activeElement).toBe(input); + expect(window.getSelection()!.anchorOffset).toBe(2); + }); + + it('keeps its edits to itself', () => { + render(); + const heard = vi.fn(); + target.addEventListener('input', heard); + + record('a', []); + stack.undo(); + + expect(heard).not.toHaveBeenCalled(); + }); + + it('reports a step it could not record, and runs nothing for it', () => { + stack.uninstall(); + render(); + const log: string[] = []; + + expect(record('a', log)).toBe(false); + expect(anchor().textContent).toBe(''); + expect(log).toEqual([]); + }); +}); diff --git a/apps/staged/src/lib/shared/NativeUndoAnchor.svelte b/apps/staged/src/lib/shared/NativeUndoAnchor.svelte new file mode 100644 index 000000000..3f8d979b3 --- /dev/null +++ b/apps/staged/src/lib/shared/NativeUndoAnchor.svelte @@ -0,0 +1,174 @@ + + + + + + + + + diff --git a/apps/staged/src/lib/shared/testStubs/nativeUndoStack.ts b/apps/staged/src/lib/shared/testStubs/nativeUndoStack.ts new file mode 100644 index 000000000..74074c064 --- /dev/null +++ b/apps/staged/src/lib/shared/testStubs/nativeUndoStack.ts @@ -0,0 +1,105 @@ +/** + * A stand-in for the browser's undo stack, which jsdom doesn't have. + * + * A browser keeps one stack per document for the edits made to its editable + * content: `document.execCommand('insertText')` and typing put an edit on + * it, and undo and redo walk it, each time changing the edited element and + * raising `beforeinput` and `input` there with `historyUndo` or + * `historyRedo`. This models as much of that as the app's undo wiring + * leans on (see NativeUndoAnchor.svelte), so component tests can type, + * undo and redo across several editable elements in the order a browser + * would: + * + * - `execCommand('insertText')` appends to the editable element holding the + * selection, and drops whatever had been undone; + * - undo and redo move focus and the caret to the element they change, as + * a browser does, between its `beforeinput` and its `input`; + * - an edit to an element since taken out of the document is still walked, + * but changes nothing and raises no event. + */ + +type Edit = { host: HTMLElement; before: string; after: string }; + +export type NativeUndoStack = { + /** Type `text` at the end of `host`, as one edit. */ + type(host: HTMLElement, text: string): void; + /** Undo the newest edit; false when there is none. */ + undo(): boolean; + /** Redo the edit undone last; false when there is none. */ + redo(): boolean; + /** Put back the `document.execCommand` that was there before. */ + uninstall(): void; +}; + +function caretToEnd(host: HTMLElement) { + const selection = window.getSelection(); + selection?.selectAllChildren(host); + selection?.collapseToEnd(); +} + +function editableHostOf(node: Node | null): HTMLElement | null { + const element = node instanceof HTMLElement ? node : (node?.parentElement ?? null); + return element?.closest('[contenteditable="true"]') ?? null; +} + +export function installNativeUndoStack(): NativeUndoStack { + const done: Edit[] = []; + let undone: Edit[] = []; + const original = Object.getOwnPropertyDescriptor(document, 'execCommand'); + + function insertText(text: string): boolean { + const host = editableHostOf(window.getSelection()?.anchorNode ?? null); + if (!host) return false; + const before = host.textContent ?? ''; + host.dispatchEvent(new InputEvent('beforeinput', { bubbles: true, inputType: 'insertText' })); + host.textContent = before + text; + caretToEnd(host); + done.push({ host, before, after: before + text }); + undone = []; + host.dispatchEvent(new InputEvent('input', { bubbles: true, inputType: 'insertText' })); + return true; + } + + function walk(edit: Edit, text: string, inputType: 'historyUndo' | 'historyRedo') { + const { host } = edit; + if (!host.isConnected) return; + host.dispatchEvent(new InputEvent('beforeinput', { bubbles: true, inputType })); + host.focus(); + host.textContent = text; + caretToEnd(host); + host.dispatchEvent(new InputEvent('input', { bubbles: true, inputType })); + } + + Object.defineProperty(document, 'execCommand', { + configurable: true, + writable: true, + value: (command: string, _showUi?: boolean, value?: string) => + command === 'insertText' && insertText(value ?? ''), + }); + + return { + type(host, text) { + host.focus(); + caretToEnd(host); + insertText(text); + }, + undo() { + const edit = done.pop(); + if (!edit) return false; + undone.push(edit); + walk(edit, edit.before, 'historyUndo'); + return true; + }, + redo() { + const edit = undone.pop(); + if (!edit) return false; + done.push(edit); + walk(edit, edit.after, 'historyRedo'); + return true; + }, + uninstall() { + if (original) Object.defineProperty(document, 'execCommand', original); + else delete (document as Partial).execCommand; + }, + }; +}