From 5cfdd9eb5a4d3e4535d989407b382e4db1f042f9 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 30 Sep 2026 09:29:04 -0600 Subject: [PATCH 01/17] Add an undo history that replays re-anchoring on restore (#184) --- src/__tests__/utils/undo-history.test.ts | 113 +++++++++++++++++++++ src/utils/undo-history.ts | 122 +++++++++++++++++++++++ 2 files changed, 235 insertions(+) create mode 100644 src/__tests__/utils/undo-history.test.ts create mode 100644 src/utils/undo-history.ts diff --git a/src/__tests__/utils/undo-history.test.ts b/src/__tests__/utils/undo-history.test.ts new file mode 100644 index 00000000..f7e767b8 --- /dev/null +++ b/src/__tests__/utils/undo-history.test.ts @@ -0,0 +1,113 @@ +/// + +import { + canRedo, + canUndo, + emptyHistory, + MAX_UNDO_STEPS, + recordBookPass, + recordStep, + redo, + undo, +} from '../../utils/undo-history'; + +describe('undo history', () => { + it('undoes a step back to the content before it', () => { + const history = recordStep(emptyHistory(), 'before'); + expect(undo(history, 'after')?.content).toBe('before'); + }); + + it('redoes an undone step back to the content it undid', () => { + const undone = undo(recordStep(emptyHistory(), 'before'), 'after'); + expect(undone && redo(undone.history, undone.content)?.content).toBe('after'); + }); + + it('undoes steps latest first', () => { + const history = recordStep(recordStep(emptyHistory(), 'first'), 'second'); + const once = undo(history, 'third'); + expect(once && undo(once.history, once.content)?.content).toBe('first'); + }); + + it('has nothing to undo before any step', () => { + const history = emptyHistory(); + expect(canUndo(history)).toBe(false); + expect(undo(history, 'present')).toBeUndefined(); + }); + + it('can undo once a step is recorded', () => { + expect(canUndo(recordStep(emptyHistory(), 'before'))).toBe(true); + }); + + it('has nothing to redo until a step is undone', () => { + const history = recordStep(emptyHistory(), 'before'); + expect(canRedo(history)).toBe(false); + expect(redo(history, 'after')).toBeUndefined(); + }); + + it('can redo once a step is undone', () => { + const undone = undo(recordStep(emptyHistory(), 'before'), 'after'); + expect(undone && canRedo(undone.history)).toBe(true); + }); + + it('discards the undone steps when a new step is recorded', () => { + const undone = undo(recordStep(emptyHistory(), 'before'), 'after'); + expect(undone && canRedo(recordStep(undone.history, undone.content))).toBe(false); + }); + + it('forgets the oldest step beyond the cap', () => { + let history = emptyHistory(); + for (let before = 0; before <= MAX_UNDO_STEPS; before += 1) + history = recordStep(history, before); + + const restored: number[] = []; + let move = undo(history, MAX_UNDO_STEPS + 1); + while (move) { + restored.push(move.content); + move = undo(move.history, move.content); + } + expect(restored).toHaveLength(MAX_UNDO_STEPS); + expect(restored.at(-1)).toBe(1); + }); + + describe('re-anchor passes', () => { + /** A pass that marks the content with a label, so a test can see which passes ran. */ + const tagWith = (bookCode: string) => (content: string) => `${content}|${bookCode}`; + + it('replays a pass that ran after the restored step', () => { + const history = recordBookPass( + recordStep(emptyHistory(), 'before'), + 'GEN', + tagWith('GEN'), + ); + expect(undo(history, 'after|GEN')?.content).toBe('before|GEN'); + }); + + it('does not replay a pass the restored content already had', () => { + const history = recordStep( + recordBookPass(emptyHistory(), 'GEN', tagWith('GEN')), + 'before|GEN', + ); + expect(undo(history, 'after|GEN')?.content).toBe('before|GEN'); + }); + + it("replays only a book's latest pass", () => { + let history = recordStep(emptyHistory(), 'before'); + history = recordBookPass(history, 'GEN', tagWith('GEN-old')); + history = recordBookPass(history, 'GEN', tagWith('GEN-new')); + expect(undo(history, 'after|GEN-new')?.content).toBe('before|GEN-new'); + }); + + it('replays the latest pass of every book', () => { + let history = recordStep(emptyHistory(), 'before'); + history = recordBookPass(history, 'GEN', tagWith('GEN')); + history = recordBookPass(history, 'EXO', tagWith('EXO')); + expect(undo(history, 'after|GEN|EXO')?.content).toBe('before|GEN|EXO'); + }); + + it('replays on redo a pass that ran while the step was undone', () => { + const undone = undo(recordStep(emptyHistory(), 'before'), 'after'); + const history = undone && recordBookPass(undone.history, 'GEN', tagWith('GEN')); + expect(history && redo(history, 'before|GEN')?.content).toBe('after|GEN'); + }); + }); +}); diff --git a/src/utils/undo-history.ts b/src/utils/undo-history.ts new file mode 100644 index 00000000..a2730044 --- /dev/null +++ b/src/utils/undo-history.ts @@ -0,0 +1,122 @@ +/** How many of the most recent steps stay undoable; older ones are forgotten. */ +export const MAX_UNDO_STEPS = 100; + +/** Re-anchors content to one book's current text. */ +export type BookPass = (content: T) => T; + +/** Content the history can restore. */ +type Snapshot = Readonly<{ + content: T; + /** How many passes had been recorded when the content was current, all of which it reflects. */ + passesSeen: number; +}>; + +/** A draft's undo history: the content each undo or redo can return to. Immutable. */ +export type UndoHistory = Readonly<{ + /** Content before each undoable step, oldest first. */ + past: readonly Snapshot[]; + /** Content each undone step left, most recently undone last. */ + future: readonly Snapshot[]; + /** The latest re-anchor pass per book code, with its position among all recorded passes. */ + passes: ReadonlyMap; ordinal: number }>>; + /** How many passes have ever been recorded, superseded ones included. */ + passCount: number; +}>; + +/** The outcome of an undo or redo. */ +export type HistoryMove = Readonly<{ + /** The history as it stands after the move. */ + history: UndoHistory; + /** The content the move restores, re-anchored to the current text of every re-anchored book. */ + content: T; +}>; + +/** Returns a history with nothing to undo or redo. */ +export function emptyHistory(): UndoHistory { + return { past: [], future: [], passes: new Map(), passCount: 0 }; +} + +/** Whether the history holds a step to undo. */ +export function canUndo(history: UndoHistory): boolean { + return history.past.length > 0; +} + +/** Whether the history holds an undone step to redo. */ +export function canRedo(history: UndoHistory): boolean { + return history.future.length > 0; +} + +/** + * Records an undo step, given the content as it stood before the step. Any undone steps become + * unreachable. + */ +export function recordStep(history: UndoHistory, before: T): UndoHistory { + return { + ...history, + past: [...history.past, snapshot(history, before)].slice(-MAX_UNDO_STEPS), + future: [], + }; +} + +/** + * Records a re-anchor pass that has just run over the content for `bookCode`, so that content an + * undo or redo restores is re-anchored to that book's text too. + */ +export function recordBookPass( + history: UndoHistory, + bookCode: string, + pass: BookPass, +): UndoHistory { + const ordinal = history.passCount + 1; + return { + ...history, + passes: new Map(history.passes).set(bookCode, { pass, ordinal }), + passCount: ordinal, + }; +} + +/** Captures content that reflects every pass recorded so far. */ +function snapshot(history: UndoHistory, content: T): Snapshot { + return { content, passesSeen: history.passCount }; +} + +/** Re-anchors a restored snapshot to the text of every book re-anchored since it was current. */ +function restore(history: UndoHistory, { content, passesSeen }: Snapshot): T { + return [...history.passes.values()] + .filter(({ ordinal }) => ordinal > passesSeen) + .reduce((restored, { pass }) => pass(restored), content); +} + +/** + * Undoes the latest step, given the content as it stands now. + * + * @returns The move, or `undefined` when there is no step to undo. + */ +export function undo(history: UndoHistory, present: T): HistoryMove | undefined { + if (!canUndo(history)) return undefined; + return { + history: { + ...history, + past: history.past.slice(0, -1), + future: [...history.future, snapshot(history, present)], + }, + content: restore(history, history.past[history.past.length - 1]), + }; +} + +/** + * Redoes the most recently undone step, given the content as it stands now. + * + * @returns The move, or `undefined` when there is no undone step to redo. + */ +export function redo(history: UndoHistory, present: T): HistoryMove | undefined { + if (!canRedo(history)) return undefined; + return { + history: { + ...history, + past: [...history.past, snapshot(history, present)], + future: history.future.slice(0, -1), + }, + content: restore(history, history.future[history.future.length - 1]), + }; +} From 5e64b263ee9de9b7bae443534a36df9f64ff0c73 Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Fri, 2 Oct 2026 11:05:48 -0600 Subject: [PATCH 02/17] Record draft edits as undo steps; re-anchor in the loader (#184) --- .../components/AnalysisStore.test.tsx | 157 +++++-------- .../components/Interlinearizer.test.tsx | 50 ++++- .../components/InterlinearizerLoader.test.tsx | 74 ++++++- src/__tests__/hooks/useDraftProject.test.ts | 209 +++++++++++++++++- src/__tests__/store/analysisSlice.test.ts | 30 ++- src/__tests__/utils/reanchor-draft.test.ts | 98 ++++++++ src/components/AnalysisStore.tsx | 42 +--- src/components/Interlinearizer.tsx | 43 ++-- src/components/InterlinearizerLoader.tsx | 27 ++- src/components/__mocks__/AnalysisStore.tsx | 11 +- src/hooks/useDraftProject.ts | 177 +++++++++++++-- src/store/analysisSlice.ts | 29 +-- src/utils/reanchor-draft.ts | 22 ++ 13 files changed, 730 insertions(+), 239 deletions(-) create mode 100644 src/__tests__/utils/reanchor-draft.test.ts create mode 100644 src/utils/reanchor-draft.ts diff --git a/src/__tests__/components/AnalysisStore.test.tsx b/src/__tests__/components/AnalysisStore.test.tsx index 4cc3c5ee..d3c1a2e6 100644 --- a/src/__tests__/components/AnalysisStore.test.tsx +++ b/src/__tests__/components/AnalysisStore.test.tsx @@ -6,8 +6,7 @@ import userEvent from '@testing-library/user-event'; import type { TextAnalysis, TokenAnalysis, TokenAnalysisLink } from 'interlinearizer'; import type { ReactNode } from 'react'; import { emptyAnalysis } from '../../types/empty-factories'; -import { resegmentBook } from '../../parsers/papi/resegmentBook'; -import { FIXTURE_STAMPS, makeVerseBook } from '../test-helpers'; +import { FIXTURE_STAMPS } from '../test-helpers'; import type { AnalysisEditOutcome } from '../../components/AnalysisStore'; import { AnalysisStoreProvider, @@ -31,7 +30,6 @@ import { usePhraseDispatch, usePhraseGloss, usePhraseGlossDispatch, - useReanchorToBook, useReportGlossEditing, useResolvedTokenAnalysis, useSuggestionAfterClearing, @@ -147,6 +145,7 @@ function renderStoreHook( onGlossChange?: (tokenRef: string, value: string) => void; showSuggestions?: boolean; readOnly?: boolean; + subscribeToReplacements?: (listener: (analysis: TextAnalysis) => void) => () => void; }> = {}, ) { const { analysisLanguage = 'und', ...rest } = options; @@ -328,6 +327,55 @@ describe('useAnalysis', () => { }); }); +describe('analysis replacements', () => { + /** Stands in for the draft's feed of replaced analyses, letting a test push one. */ + function makeReplacementFeed() { + let listener: ((analysis: TextAnalysis) => void) | undefined; + const unsubscribe = jest.fn(); + return { + subscribe: (next: (analysis: TextAnalysis) => void) => { + listener = next; + return unsubscribe; + }, + replace: (analysis: TextAnalysis) => listener?.(analysis), + unsubscribe, + }; + } + + it('follows an analysis the draft replaces', () => { + const feed = makeReplacementFeed(); + const { result } = renderStoreHook(() => useAnalysis(), { + subscribeToReplacements: feed.subscribe, + }); + const replaced = makeAnalysisWithGloss('tok-1', 'restored'); + + act(() => feed.replace(replaced)); + + expect(result.current).toBe(replaced); + }); + + it('does not save back an analysis the draft replaced', () => { + const feed = makeReplacementFeed(); + const onSave = jest.fn(); + renderStoreHook(() => useAnalysis(), { subscribeToReplacements: feed.subscribe, onSave }); + + act(() => feed.replace(makeAnalysisWithGloss('tok-1', 'restored'))); + + expect(onSave).not.toHaveBeenCalled(); + }); + + it('stops following the draft when it unmounts', () => { + const feed = makeReplacementFeed(); + const { unmount } = renderStoreHook(() => useAnalysis(), { + subscribeToReplacements: feed.subscribe, + }); + + unmount(); + + expect(feed.unsubscribe).toHaveBeenCalled(); + }); +}); + describe('useGlossDispatch', () => { it('replaces the existing approved analysis on subsequent writes for the same token', () => { const { result } = renderStoreHook(() => ({ @@ -1873,106 +1921,3 @@ describe('useAnalysisDeletionOutcome', () => { ); }); }); - -describe('useReanchorToBook', () => { - /** Seeds an approved gloss on the sole occurrence of `surfaceText` in a one-verse book. */ - function glossedAnalysis(text: string, surfaceText: string, gloss: string): TextAnalysis { - const token = makeVerseBook([{ sid: 'GEN 1:1', text }]).segments[0].tokens.find( - (t) => t.surfaceText === surfaceText, - ); - if (!token) throw new Error('fixture missing token'); - return makeAnalysisWithGloss(token.ref, gloss, surfaceText); - } - - it('re-points a link when the loaded book shifted its token', () => { - const initialAnalysis = glossedAnalysis('it was unbelievable', 'unbelievable', 'incroyable'); - const book = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); - - const { result } = renderStoreHook( - () => { - useReanchorToBook(book); - return useAnalysis(); - }, - { initialAnalysis }, - ); - - const moved = book.segments[0].tokens.find((t) => t.surfaceText === 'unbelievable'); - expect(result.current.tokenAnalysisLinks[0].token.tokenRef).toBe(moved?.ref); - }); - - it('moves a split piece translation along with its stored split', () => { - const storedSplits = [{ tokenRef: 'GEN 1:1:6', surfaceText: 'beta' }]; - const book = resegmentBook(makeVerseBook([{ sid: 'GEN 1:1', text: 'alpha and beta' }]), { - removedVerseStarts: [], - addedStarts: [{ tokenRef: 'GEN 1:1:10', surfaceText: 'beta' }], - }); - const initialAnalysis: TextAnalysis = { - ...emptyAnalysis(), - segmentAnalyses: [{ id: 'sa-1', ...FIXTURE_STAMPS, surfaceText: 'beta' }], - segmentAnalysisLinks: [ - { analysisId: 'sa-1', ...FIXTURE_STAMPS, status: 'approved', segmentId: 'GEN 1:1:6' }, - ], - }; - - const { result } = renderStoreHook( - () => { - useReanchorToBook(book, storedSplits); - return useAnalysis(); - }, - { initialAnalysis }, - ); - - expect(result.current.segmentAnalysisLinks[0].segmentId).toBe('GEN 1:1:10'); - }); - - it('persists the healed analysis through onSave', () => { - const initialAnalysis = glossedAnalysis('it was unbelievable', 'unbelievable', 'incroyable'); - const book = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); - const onSave = jest.fn(); - - renderStoreHook(() => useReanchorToBook(book), { initialAnalysis, onSave }); - - expect(onSave).toHaveBeenCalledTimes(1); - }); - - it('does not save when the book still matches the stored refs', () => { - const initialAnalysis = glossedAnalysis('it was unbelievable', 'unbelievable', 'incroyable'); - const book = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was unbelievable' }]); - const onSave = jest.fn(); - - renderStoreHook(() => useReanchorToBook(book), { initialAnalysis, onSave }); - - expect(onSave).not.toHaveBeenCalled(); - }); - - it('leaves a read-only store alone so an import is never rewritten', () => { - const initialAnalysis = glossedAnalysis('it was unbelievable', 'unbelievable', 'incroyable'); - const book = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); - - const { result } = renderStoreHook( - () => { - useReanchorToBook(book); - return useAnalysis(); - }, - { initialAnalysis, readOnly: true }, - ); - - expect(result.current).toBe(initialAnalysis); - }); - - it('waits for a book rather than re-anchoring against nothing', () => { - const initialAnalysis = glossedAnalysis('it was unbelievable', 'unbelievable', 'incroyable'); - const onSave = jest.fn(); - - const { result } = renderStoreHook( - () => { - useReanchorToBook(undefined); - return useAnalysis(); - }, - { initialAnalysis, onSave }, - ); - - expect(result.current).toBe(initialAnalysis); - expect(onSave).not.toHaveBeenCalled(); - }); -}); diff --git a/src/__tests__/components/Interlinearizer.test.tsx b/src/__tests__/components/Interlinearizer.test.tsx index 66c87382..da16ba59 100644 --- a/src/__tests__/components/Interlinearizer.test.tsx +++ b/src/__tests__/components/Interlinearizer.test.tsx @@ -140,7 +140,6 @@ jest.mock('../../components/AnalysisStore', () => ({ deletePhrase: (...args: Parameters) => mockDeletePhrase(...args), }), /** No-op: these tests render no store, and re-anchoring is covered against the real one. */ - useReanchorToBook: () => {}, })); jest.mock('../../components/ContinuousView', () => ({ @@ -413,6 +412,7 @@ function renderInterlinearizer({ segmentationDispatch, formerBoundaries, unmergeableStarts, + asOneStep, }: { book?: Book; continuousScroll?: boolean; @@ -426,6 +426,7 @@ function renderInterlinearizer({ segmentationDispatch?: SegmentationDispatch; formerBoundaries?: ReadonlyMap; unmergeableStarts?: ReadonlySet; + asOneStep?: (action: () => void) => void; } = {}) { return render( withNav( @@ -435,6 +436,7 @@ function renderInterlinearizer({ segmentationDispatch={segmentationDispatch} formerBoundaries={formerBoundaries} unmergeableStarts={unmergeableStarts} + asOneStep={asOneStep} scrRef={scrRef} phraseMode={{ kind: 'view' }} setPhraseMode={() => {}} @@ -1912,8 +1914,12 @@ describe('segmentation dispatch force-break', () => { * Renders with continuous scroll on (so the stubbed ContinuousView captures the segmentation * context) and returns the wrapped dispatch. */ - function renderAndCaptureDispatch(raw: SegmentationDispatch, book: Book): SegmentationDispatch { - renderInterlinearizer({ book, continuousScroll: true, segmentationDispatch: raw }); + function renderAndCaptureDispatch( + raw: SegmentationDispatch, + book: Book, + asOneStep?: (action: () => void) => void, + ): SegmentationDispatch { + renderInterlinearizer({ book, continuousScroll: true, segmentationDispatch: raw, asOneStep }); const dispatch = capturedSegmentation?.dispatch; if (!dispatch) throw new Error('expected a captured segmentation dispatch'); return dispatch; @@ -1983,6 +1989,44 @@ describe('segmentation dispatch force-break', () => { expect(raw.move).toHaveBeenCalledWith('GEN 1:1:0', 'GEN 1:2:0'); }); + it("makes a split's force-break and boundary write one undo step", () => { + const raw = makeRawDispatch(); + const writesInStep: string[] = []; + let stepOpen = false; + const asOneStep = (action: () => void) => { + stepOpen = true; + action(); + stepOpen = false; + }; + mockDeletePhrase.mockImplementation(() => stepOpen && writesInStep.push('break')); + jest.mocked(raw.split).mockImplementation(() => stepOpen && writesInStep.push('split')); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK, asOneStep); + + dispatch.split('GEN 1:2:0'); + + expect(writesInStep).toEqual(['break', 'split']); + }); + + it("makes a move's force-break and boundary write one undo step", () => { + const raw = makeRawDispatch(); + const writesInStep: string[] = []; + let stepOpen = false; + const asOneStep = (action: () => void) => { + stepOpen = true; + action(); + stepOpen = false; + }; + mockDeletePhrase.mockImplementation(() => stepOpen && writesInStep.push('break')); + jest.mocked(raw.move).mockImplementation(() => stepOpen && writesInStep.push('move')); + mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); + const dispatch = renderAndCaptureDispatch(raw, GEN_1_MULTI_BOOK, asOneStep); + + dispatch.move('GEN 1:1:0', 'GEN 1:2:0'); + + expect(writesInStep).toEqual(['break', 'move']); + }); + it('skips the force-break when the boundary ref is unknown to the book', () => { const raw = makeRawDispatch(); mockPhraseLinkById.set('p1', makePhraseLink('p1', ['GEN 1:1:0', 'GEN 1:2:0'])); diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index e96b1846..0dc7cf59 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -10,7 +10,7 @@ import type { Book, DraftProject, PhraseAnalysisLink, TextAnalysis } from 'inter import { useState as useReactState } from 'react'; import type { Dispatch, ReactNode, SetStateAction } from 'react'; import { useStore } from 'react-redux'; -import { useGlossDispatch } from '../../components/AnalysisStore'; +import { useAnalysis, useGlossDispatch } from '../../components/AnalysisStore'; import InterlinearizerLoader from '../../components/InterlinearizerLoader'; import { RECENTER_FADE_MS } from '../../components/recenter-fade'; import useConcordanceIndex, { type ConcordanceIndex } from '../../hooks/useConcordanceIndex'; @@ -244,6 +244,9 @@ let mountStoreProbe = false; /** The Redux store the probe is mounted in, captured so a test can compare store identity. */ let probeStore: unknown; +/** The analysis the store the probe is mounted in holds. */ +let probeAnalysis: TextAnalysis | undefined; + /** Writes a gloss through the store the probe is mounted in. */ let probeWriteGloss: ((tokenRef: string, surfaceText: string, value: string) => void) | undefined; @@ -253,6 +256,7 @@ let probeWriteGloss: ((tokenRef: string, surfaceText: string, value: string) => */ function StoreProbe() { probeStore = useStore(); + probeAnalysis = useAnalysis(); probeWriteGloss = useGlossDispatch(); return undefined; } @@ -276,6 +280,22 @@ jest.mock('../../components/Interlinearizer', () => { }; }); +/** An approved analysis of `surfaceText`, written against the token at `tokenRef`. */ +function analysisApprovingAt(tokenRef: string, surfaceText: string): TextAnalysis { + return { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText }], + tokenAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef, surfaceText }, + }, + ], + }; +} + /** Minimal project summary used across modal interaction tests. */ type MockProject = { id: string; @@ -2684,10 +2704,19 @@ describe('InterlinearizerLoader', () => { * @returns The persisted delta, or `undefined` when no draft has been saved or it carried none. */ function lastPersistedSegmentation(): DraftProject['segmentation'] { + return lastPersistedDraft()?.segmentation; + } + + /** + * Reads the draft back out of the most recent `saveDraft` call. + * + * @returns The persisted draft, or `undefined` when none has been saved. + */ + function lastPersistedDraft(): DraftProject | undefined { const calls = mockSendCommand.mock.calls.filter(([c]) => c === 'interlinearizer.saveDraft'); const last = calls[calls.length - 1]; const json = last?.[2]; - return typeof json === 'string' ? JSON.parse(json).segmentation : undefined; + return typeof json === 'string' ? JSON.parse(json) : undefined; } /** @@ -2843,6 +2872,22 @@ describe('InterlinearizerLoader', () => { ); }); + it("re-anchors the draft's analyses to the loaded book, and persists them", async () => { + // Written against "Al beta.", where "beta" began at offset 3. + const analysis = analysisApprovingAt('GEN 1:1:3', 'beta'); + mockSendCommand.mockResolvedValue(JSON.stringify({ ...emptyDraft(testProjectId), analysis })); + mockBookData({ book: TWO_VERSE_BOOK }); + await act(async () => { + renderLoader(); + }); + + await waitFor(() => + expect(lastPersistedDraft()?.analysis.tokenAnalysisLinks[0].token.tokenRef).toBe( + 'GEN 1:1:6', + ), + ); + }); + it('clears the segmentation field when an edit restores the default segmentation', async () => { mockBookData({ book: TWO_VERSE_BOOK }); await act(async () => { @@ -4202,6 +4247,7 @@ describe('analysis store lifetime', () => { beforeEach(() => { mountStoreProbe = true; probeStore = undefined; + probeAnalysis = undefined; probeWriteGloss = undefined; capturedInterlinearizerProps = undefined; capturedStoreProps = undefined; @@ -4258,6 +4304,30 @@ describe('analysis store lifetime', () => { expect(probeStore).toBe(storeBefore); }); + it('shows the store what re-anchoring made of the draft', async () => { + mockBookData({ + book: { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + duplicateVerseIds: [], + segments: [ + makeSegment('GEN 1:1', 'Alpha beta.', [ + makeWordToken('GEN 1:1:0', 'Alpha'), + makeWordToken('GEN 1:1:6', 'beta', 6), + ]), + ], + }, + }); + // Written against "Al beta.", where "beta" began at offset 3. + const analysis = analysisApprovingAt('GEN 1:1:3', 'beta'); + mockSendCommand.mockResolvedValue(JSON.stringify({ ...emptyDraft(testProjectId), analysis })); + + await act(async () => renderLoader()); + + expect(probeAnalysis?.tokenAnalysisLinks[0].token.tokenRef).toBe('GEN 1:1:6'); + }); + it('rebuilds the store when the draft is replaced wholesale', async () => { // The store's seed is not reactive, so a replacement (New / Open / Wipe) reseeds by remounting // the provider. Hoisting it above the book key must not cost that: a wiped draft whose store diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index 82b85573..849e96e5 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -4,7 +4,7 @@ import papi, { logger } from '@papi/frontend'; import { act, renderHook, waitFor } from '@testing-library/react'; import type { DraftProject, TextAnalysis } from 'interlinearizer'; import { FIXTURE_STAMPS } from '../test-helpers'; -import useDraftProject from '../../hooks/useDraftProject'; +import useDraftProject, { type DraftContent } from '../../hooks/useDraftProject'; import { emptyAnalysis } from '../../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../../types/model-version'; @@ -620,6 +620,213 @@ describe('useDraftProject', () => { }); }); + describe('undo history', () => { + it('undoes an analysis edit back to the analysis before it', async () => { + const loaded = analysisWithToken('tok-loaded'); + mockGetDraftResolves(makeDraft({ analysis: loaded })); + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis).toEqual(loaded); + }); + + it('hands the analysis an undo restores to subscribers', async () => { + const loaded = analysisWithToken('tok-loaded'); + mockGetDraftResolves(makeDraft({ analysis: loaded })); + const { result } = await renderLoaded(); + const listener = jest.fn(); + result.current.subscribeToAnalysisReplacements(listener); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(listener).toHaveBeenCalledWith(loaded); + }); + + it('does not hand subscribers an edit the analysis store made itself', async () => { + const { result } = await renderLoaded(); + const listener = jest.fn(); + result.current.subscribeToAnalysisReplacements(listener); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('stops handing analyses to a listener once it unsubscribes', async () => { + const { result } = await renderLoaded(); + const listener = jest.fn(); + const unsubscribe = result.current.subscribeToAnalysisReplacements(listener); + + unsubscribe(); + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('undoes a boundary edit back to the boundaries before it', async () => { + const { result } = await renderLoaded(); + + act(() => + result.current.autosaveSegmentation({ removedVerseStarts: ['GEN 1:2:0'], addedStarts: [] }), + ); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.segmentation).toBeUndefined(); + }); + + it('redoes an undone boundary edit', async () => { + const { result } = await renderLoaded(); + const delta = { removedVerseStarts: ['GEN 1:2:0'], addedStarts: [] }; + + act(() => result.current.autosaveSegmentation(delta)); + act(() => result.current.undo()); + act(() => result.current.redo()); + + expect(result.current.getDraftSnapshot()?.segmentation).toBe(delta); + }); + + it('bumps segmentationVersion when an undo changes the boundaries', async () => { + const { result } = await renderLoaded(); + act(() => + result.current.autosaveSegmentation({ removedVerseStarts: ['GEN 1:2:0'], addedStarts: [] }), + ); + const versionBefore = result.current.segmentationVersion; + + act(() => result.current.undo()); + + expect(result.current.segmentationVersion).toBe(versionBefore + 1); + }); + + it('redoes an undone edit', async () => { + const { result } = await renderLoaded(); + const edited = analysisWithToken('tok-edited'); + + act(() => result.current.autosaveAnalysis(edited)); + act(() => result.current.undo()); + act(() => result.current.redo()); + + expect(result.current.getDraftSnapshot()?.analysis).toBe(edited); + }); + + it('records no step for a save that changes nothing', async () => { + const loaded = analysisWithToken('tok-loaded'); + mockGetDraftResolves(makeDraft({ analysis: loaded })); + const { result } = await renderLoaded(); + const edited = analysisWithToken('tok-edited'); + + act(() => result.current.autosaveAnalysis(edited)); + act(() => result.current.autosaveAnalysis(edited)); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis).toEqual(loaded); + }); + + it('undoes the saves made as one step together', async () => { + const loaded = analysisWithToken('tok-loaded'); + mockGetDraftResolves(makeDraft({ analysis: loaded })); + const { result } = await renderLoaded(); + + act(() => + result.current.asOneStep(() => { + result.current.autosaveAnalysis(analysisWithToken('tok-edited')); + result.current.autosaveSegmentation({ + removedVerseStarts: ['GEN 1:2:0'], + addedStarts: [], + }); + }), + ); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis).toEqual(loaded); + expect(result.current.getDraftSnapshot()?.segmentation).toBeUndefined(); + }); + + it('leaves the draft clean when there is nothing to undo', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(false); + }); + + it('forgets the history when a project is opened', async () => { + const { result } = await renderLoaded(); + const opened = analysisWithToken('tok-open'); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.loadFromProject({ analysis: opened, analysisLanguages: ['de'] })); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis).toBe(opened); + }); + + it('forgets the history when a new draft is started', async () => { + mockGetDraftResolves(makeDraft({ analysis: analysisWithToken('tok-loaded') })); + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.newDraft({ analysisLanguages: ['sw'] })); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis).toEqual(emptyAnalysis()); + }); + + describe('re-anchoring', () => { + /** A pass that renames the content's token analysis, so a test can see where it ran. */ + const renameToken = + (suffix: string) => + (content: DraftContent): DraftContent => ({ + ...content, + analysis: analysisWithToken(`${content.analysis.tokenAnalyses[0]?.id}${suffix}`), + }); + + it("applies a book's pass to the draft", async () => { + mockGetDraftResolves(makeDraft({ analysis: analysisWithToken('tok') })); + const { result } = await renderLoaded(); + + act(() => result.current.reanchorBook('GEN', renameToken('+GEN'))); + + expect(result.current.getDraftSnapshot()?.analysis.tokenAnalyses[0].id).toBe('tok+GEN'); + }); + + it('keeps a re-anchor when the edit before it is undone', async () => { + mockGetDraftResolves(makeDraft({ analysis: analysisWithToken('tok-loaded') })); + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.reanchorBook('GEN', renameToken('+GEN'))); + act(() => result.current.undo()); + + expect(result.current.getDraftSnapshot()?.analysis.tokenAnalyses[0].id).toBe( + 'tok-loaded+GEN', + ); + }); + + it('hands a re-anchored analysis to subscribers', async () => { + mockGetDraftResolves(makeDraft({ analysis: analysisWithToken('tok') })); + const { result } = await renderLoaded(); + const listener = jest.fn(); + result.current.subscribeToAnalysisReplacements(listener); + + act(() => result.current.reanchorBook('GEN', renameToken('+GEN'))); + + expect(listener).toHaveBeenCalledWith(analysisWithToken('tok+GEN')); + }); + + it('leaves the draft clean when a pass moves nothing', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.reanchorBook('GEN', (content) => content)); + + expect(result.current.dirty).toBe(false); + }); + }); + }); + it('does not update state or throw when unmounted before getDraft resolves', async () => { let resolveGetDraft: (json: string) => void = () => {}; const deferred = new Promise((resolve) => { diff --git a/src/__tests__/store/analysisSlice.test.ts b/src/__tests__/store/analysisSlice.test.ts index 53cf0638..9513f856 100644 --- a/src/__tests__/store/analysisSlice.test.ts +++ b/src/__tests__/store/analysisSlice.test.ts @@ -2,6 +2,7 @@ import type { AssignmentStatus, + Book, Confidence, MorphemeAnalysis, PhraseAnalysis, @@ -13,7 +14,7 @@ import type { TokenAnalysisLink, TokenSnapshot, } from 'interlinearizer'; -import { createAnalysisStore } from '../../store'; +import { createAnalysisStore, type AnalysisStore } from '../../store'; import { approveAnalysisForToken, approvePhrase, @@ -51,11 +52,12 @@ import { writeMorphemes, writePhraseGloss, writeSegmentFreeTranslation, - reanchorToBook, + replaceAnalysis, type AnalysisState, } from '../../store/analysisSlice'; import { emptyAnalysis } from '../../types/empty-factories'; import { deriveMergeContent } from '../../utils/merge-content'; +import { reanchorAnalysisToBook } from '../../utils/reanchor-analysis'; import { makePhraseLink, makeVerseBook, FIXTURE_STAMPS } from '../test-helpers'; /** @@ -6022,7 +6024,15 @@ describe('analysis-keyed reducers', () => { }); }); -describe('reanchorToBook', () => { +describe('a re-anchored analysis', () => { + /** Re-anchors the store's analysis to `book` and hands it back, as the draft does on a load. */ + function reanchor(store: AnalysisStore, book: Book): void { + const { analysis } = store.getState().analysis; + store.dispatch( + replaceAnalysis(reanchorAnalysisToBook(analysis, book, FIXTURE_STAMPS.updatedAt)), + ); + } + it('moves a gloss onto the token that kept its text when a word is inserted before it', () => { const store = createAnalysisStore({ analysis: { analysis: emptyAnalysis(), analysisLanguage: 'fr' }, @@ -6033,7 +6043,7 @@ describe('reanchorToBook', () => { store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); const after = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); - store.dispatch(reanchorToBook({ book: after })); + reanchor(store, after); const moved = after.segments[0].tokens.find((t) => t.surfaceText === 'unbelievable'); if (!moved) throw new Error('fixture missing moved token'); @@ -6050,7 +6060,7 @@ describe('reanchorToBook', () => { store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); const after = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); - store.dispatch(reanchorToBook({ book: after })); + reanchor(store, after); const inserted = after.segments[0].tokens.find((t) => t.surfaceText === 'and'); if (!inserted) throw new Error('fixture missing inserted token'); @@ -6066,7 +6076,7 @@ describe('reanchorToBook', () => { if (!target) throw new Error('fixture missing target token'); store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); - store.dispatch(reanchorToBook({ book: makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }]) })); + reanchor(store, makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }])); const { tokenAnalysisLinks } = store.getState().analysis.analysis; expect(tokenAnalysisLinks[0].status).toBe('stale'); @@ -6083,7 +6093,7 @@ describe('reanchorToBook', () => { store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); const after = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was indeed' }]); - store.dispatch(reanchorToBook({ book: after })); + reanchor(store, after); const successor = after.segments[0].tokens.find((t) => t.surfaceText === 'indeed'); if (!successor) throw new Error('fixture missing successor token'); @@ -6102,7 +6112,7 @@ describe('reanchorToBook', () => { if (!target) throw new Error('fixture missing target token'); store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); - store.dispatch(reanchorToBook({ book: makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }]) })); + reanchor(store, makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }])); const rows = selectCatalogRows(store.getState().analysis, 'GEN'); expect(rows).toHaveLength(1); @@ -6118,7 +6128,7 @@ describe('reanchorToBook', () => { if (!target) throw new Error('fixture missing target token'); store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); - store.dispatch(reanchorToBook({ book: makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }]) })); + reanchor(store, makeVerseBook([{ sid: 'GEN 1:1', text: 'it was' }])); const [row] = selectCatalogRows(store.getState().analysis, 'GEN'); expect(row.usageCount).toBe(0); @@ -6135,7 +6145,7 @@ describe('reanchorToBook', () => { store.dispatch(writeGloss(target.ref, target.surfaceText, 'incroyable')); const before = store.getState().analysis.analysis; - store.dispatch(reanchorToBook({ book })); + reanchor(store, book); expect(store.getState().analysis.analysis).toBe(before); }); diff --git a/src/__tests__/utils/reanchor-draft.test.ts b/src/__tests__/utils/reanchor-draft.test.ts new file mode 100644 index 00000000..be2d3259 --- /dev/null +++ b/src/__tests__/utils/reanchor-draft.test.ts @@ -0,0 +1,98 @@ +/// + +import type { TextAnalysis } from 'interlinearizer'; +import { emptyAnalysis } from '../../types/empty-factories'; +import { reanchorDraftToBook } from '../../utils/reanchor-draft'; +import { FIXTURE_STAMPS, makeVerseBook } from '../test-helpers'; + +/** An analysis approving `gloss` on the sole occurrence of `surfaceText` in a one-verse `text`. */ +function glossedOn(text: string, surfaceText: string, gloss: string): TextAnalysis { + const token = makeVerseBook([{ sid: 'GEN 1:1', text }]).segments[0].tokens.find( + (t) => t.surfaceText === surfaceText, + ); + if (!token) throw new Error('fixture missing token'); + return { + ...emptyAnalysis(), + tokenAnalyses: [{ ...FIXTURE_STAMPS, id: 'ta-1', surfaceText, gloss: { und: gloss } }], + tokenAnalysisLinks: [ + { + ...FIXTURE_STAMPS, + analysisId: 'ta-1', + status: 'approved', + token: { tokenRef: token.ref, surfaceText }, + }, + ], + }; +} + +describe('reanchorDraftToBook', () => { + it('re-points a gloss whose token the book shifted', () => { + const verseBook = makeVerseBook([{ sid: 'GEN 1:1', text: 'it was and unbelievable' }]); + const content = { + analysis: glossedOn('it was unbelievable', 'unbelievable', 'incroyable'), + segmentation: undefined, + }; + + const reanchored = reanchorDraftToBook(verseBook)(content); + + const moved = verseBook.segments[0].tokens.find((t) => t.surfaceText === 'unbelievable'); + expect(reanchored.analysis.tokenAnalysisLinks[0].token.tokenRef).toBe(moved?.ref); + }); + + it('moves a stored split with the word it starts at', () => { + const verseBook = makeVerseBook([{ sid: 'GEN 1:1', text: 'alpha and beta' }]); + const content = { + analysis: emptyAnalysis(), + segmentation: { + removedVerseStarts: [], + addedStarts: [{ tokenRef: 'GEN 1:1:6', surfaceText: 'beta' }], + }, + }; + + const reanchored = reanchorDraftToBook(verseBook)(content); + + expect(reanchored.segmentation?.addedStarts).toEqual([ + { tokenRef: 'GEN 1:1:10', surfaceText: 'beta' }, + ]); + }); + + it("moves a split piece's translation along with its split", () => { + const verseBook = makeVerseBook([{ sid: 'GEN 1:1', text: 'alpha and beta' }]); + const content = { + analysis: { + ...emptyAnalysis(), + segmentAnalyses: [{ id: 'sa-1', ...FIXTURE_STAMPS, surfaceText: 'beta' }], + segmentAnalysisLinks: [ + { + analysisId: 'sa-1', + ...FIXTURE_STAMPS, + status: 'approved' as const, + segmentId: 'GEN 1:1:6', + }, + ], + }, + segmentation: { + removedVerseStarts: [], + addedStarts: [{ tokenRef: 'GEN 1:1:6', surfaceText: 'beta' }], + }, + }; + + const reanchored = reanchorDraftToBook(verseBook)(content); + + expect(reanchored.analysis.segmentAnalysisLinks[0].segmentId).toBe('GEN 1:1:10'); + }); + + it('returns the content itself when the book still reads as stored', () => { + const verseBook = makeVerseBook([{ sid: 'GEN 1:1', text: 'alpha beta' }]); + const analysis = glossedOn('alpha beta', 'beta', 'bêta'); + const segmentation = { + removedVerseStarts: [], + addedStarts: [{ tokenRef: 'GEN 1:1:6', surfaceText: 'beta' }], + }; + + const reanchored = reanchorDraftToBook(verseBook)({ analysis, segmentation }); + + expect(reanchored.analysis).toBe(analysis); + expect(reanchored.segmentation).toBe(segmentation); + }); +}); diff --git a/src/components/AnalysisStore.tsx b/src/components/AnalysisStore.tsx index c8ddc504..380dfb3f 100644 --- a/src/components/AnalysisStore.tsx +++ b/src/components/AnalysisStore.tsx @@ -1,5 +1,4 @@ import type { - Book, MorphemeAnalysis, PhraseAnalysisLink, TextAnalysis, @@ -42,8 +41,8 @@ import analysisReducer, { writeMorphemes, writePhraseGloss, writeSegmentFreeTranslation, - reanchorToBook, reapplyStaleAnalysis, + replaceAnalysis, type AnalysisDeletionOutcome, type MergedContent, } from '../store/analysisSlice'; @@ -140,6 +139,11 @@ type AnalysisStoreProviderProps = Readonly<{ * render. Used for a Paratext 9 import, whose analysis only sync may change. */ readOnly?: boolean; + /** + * Registers a listener for each analysis the draft takes from somewhere other than this store's + * edits, such as an undo, so the store follows it in place; returns the unregistering function. + */ + subscribeToReplacements?: (listener: (analysis: TextAnalysis) => void) => () => void; }>; /** @@ -156,6 +160,7 @@ export function AnalysisStoreProvider({ onPendingEditsChange, showSuggestions = false, readOnly = false, + subscribeToReplacements, }: AnalysisStoreProviderProps) { // Lazy initialization: useRef(createStore()) would create and discard a store on every render const storeRef = useRef | undefined>(undefined); @@ -166,6 +171,11 @@ export function AnalysisStoreProvider({ } const store = storeRef.current; + useEffect( + () => subscribeToReplacements?.((analysis) => store.dispatch(replaceAnalysis(analysis))), + [subscribeToReplacements, store], + ); + // Use refs so the dispatch callback never needs to re-create when parent re-renders const onSaveRef = useRef(onSave); onSaveRef.current = onSave; @@ -261,34 +271,6 @@ function useAnalysisSave(hookName: string): { return { callbacks, dispatch, save }; } -/** - * Re-points the stored analysis at `book`'s tokens whenever a newly tokenized book arrives, so an - * upstream text edit moves each link with the word it was written for instead of stranding it on a - * character offset that now belongs to a different word. - * - * Runs per book rather than once per mount, since the store outlives any one book and a book - * re-tokenizes whenever its text changes — exactly when offsets move. Only a pass that actually - * moved a link saves, so merely opening a book neither dirties the draft nor rewrites storage. A - * read-only store is left alone entirely: an import is a record of what was imported, not a draft - * to heal. - * - * @param book - The freshly tokenized book to re-anchor against; `undefined` while one loads, which - * defers the pass rather than clearing anything. - * @param storedSplits - The draft's splits as stored before `book` re-anchored them. - * @throws When called outside an {@link AnalysisStoreProvider}. - */ -export function useReanchorToBook(book: Book | undefined, storedSplits?: TokenSnapshot[]): void { - const { callbacks, dispatch, save } = useAnalysisSave('useReanchorToBook'); - const store = useStore(); - - useEffect(() => { - if (!book || callbacks.readOnly) return; - const before = store.getState().analysis.analysis; - dispatch(reanchorToBook({ book, storedSplits })); - if (store.getState().analysis.analysis !== before) save(); - }, [book, storedSplits, callbacks.readOnly, dispatch, save, store]); -} - /** * Registers a gloss input as currently holding uncommitted text whenever `isEditing` is true, * unregistering it when `isEditing` turns false or the input unmounts. The provider aggregates diff --git a/src/components/Interlinearizer.tsx b/src/components/Interlinearizer.tsx index 670519da..cfa9a70a 100644 --- a/src/components/Interlinearizer.tsx +++ b/src/components/Interlinearizer.tsx @@ -1,14 +1,9 @@ import type { SerializedVerseRef } from '@sillsdev/scripture'; -import type { Book, TokenSnapshot } from 'interlinearizer'; +import type { Book } from 'interlinearizer'; import { TooltipProvider } from 'platform-bible-react'; import { useCallback, useEffect, useMemo, useState } from 'react'; import type { Dispatch, SetStateAction } from 'react'; -import { - usePhraseDispatch, - usePhraseLinkByIdGetter, - usePhraseLinkByIdMap, - useReanchorToBook, -} from './AnalysisStore'; +import { usePhraseDispatch, usePhraseLinkByIdGetter, usePhraseLinkByIdMap } from './AnalysisStore'; import { NO_OP_SEGMENTATION_DISPATCH, SegmentationProvider, @@ -35,6 +30,9 @@ const EMPTY_FORMER_BOUNDARIES: ReadonlyMap = new Map(); /** Stable empty set used as the `unmergeableStarts` default so memoization holds. */ const EMPTY_UNMERGEABLE_STARTS: ReadonlySet = new Set(); +/** Runs an action with no undo history to group its edits in. */ +const RUN_UNGROUPED = (action: () => void) => action(); + /** Props for {@link Interlinearizer}. */ type InterlinearizerProps = Readonly<{ /** Tokenized book whose segments are rendered. */ @@ -73,11 +71,8 @@ type InterlinearizerProps = Readonly<{ * re-tokenization. */ segmentationVersion?: number; - /** - * The draft's splits as stored before `book` re-anchored them, so a split piece's translation - * follows its boundary; without them, no translation follows a moved split. - */ - storedSplits?: TokenSnapshot[]; + /** Runs an action so every edit it makes undoes as one step; ungrouped when omitted. */ + asOneStep?: (action: () => void) => void; }>; /** @@ -96,15 +91,13 @@ export default function Interlinearizer({ formerBoundaries = EMPTY_FORMER_BOUNDARIES, unmergeableStarts = EMPTY_UNMERGEABLE_STARTS, segmentationVersion = 0, - storedSplits, + asOneStep = RUN_UNGROUPED, }: InterlinearizerProps) { // Navigation surface from the context: `consumeInternalNav` lets the segment window suppress the // fade for internal moves, and `reportSettled` lifts the cross-book curtain once the new book is // laid out. const { consumeInternalNav, reportSettled } = useInterlinearNav(); - useReanchorToBook(book, storedSplits); - useAltHeldAttribute(); // Book-wide lookup indexes. @@ -171,16 +164,18 @@ export default function Interlinearizer({ const dispatch = useMemo( () => ({ merge: segmentationDispatch.merge, - split: (tokenRef) => { - forceBreakStraddledPhrases(tokenRef); - segmentationDispatch.split(tokenRef); - }, - move: (fromRef, toRef) => { - forceBreakStraddledPhrases(toRef); - segmentationDispatch.move(fromRef, toRef); - }, + split: (tokenRef) => + asOneStep(() => { + forceBreakStraddledPhrases(tokenRef); + segmentationDispatch.split(tokenRef); + }), + move: (fromRef, toRef) => + asOneStep(() => { + forceBreakStraddledPhrases(toRef); + segmentationDispatch.move(fromRef, toRef); + }), }), - [segmentationDispatch, forceBreakStraddledPhrases], + [segmentationDispatch, forceBreakStraddledPhrases, asOneStep], ); /** Segmentation context value — the dispatch paired with the lookups it operates over. */ diff --git a/src/components/InterlinearizerLoader.tsx b/src/components/InterlinearizerLoader.tsx index 863bc41f..794bbb16 100644 --- a/src/components/InterlinearizerLoader.tsx +++ b/src/components/InterlinearizerLoader.tsx @@ -34,6 +34,7 @@ import { splitSegmentBefore, unmergeableVerseStarts, } from '../utils/segmentation'; +import { reanchorDraftToBook } from '../utils/reanchor-draft'; import { isInterlinearProjectSummary, isTextAnalysis, isWordToken } from '../types/type-guards'; import { isPt9ImportReport, isPt9UnreadableFileList } from '../converters/pt9'; import { toProjectSummary } from '../types/interlinear-project-summary'; @@ -363,6 +364,9 @@ function InterlinearizerLoaderInner({ markSynced, wipeBook, wipeAll, + reanchorBook, + asOneStep, + subscribeToAnalysisReplacements, } = useDraftProject(projectId, platformLanguage); /** @@ -564,25 +568,19 @@ function InterlinearizerLoaderInner({ * touching the boundaries, so keying on it would re-run the full re-segmentation after every * gloss edit. `isDraftLoading` covers the one replacement that bumps neither counter: the initial * draft load. - * - * `storedSplits` are the draft's splits as stored, before this re-anchoring moved any. */ - const { segmentation, storedSplits } = useMemo( - () => ({ - segmentation: verseBook - ? reanchorSegmentation(verseBook, draft?.segmentation) - : draft?.segmentation, - storedSplits: draft?.segmentation?.addedStarts, - }), + const segmentation = useMemo( + () => (verseBook ? reanchorSegmentation(verseBook, draft?.segmentation) : draft?.segmentation), // eslint-disable-next-line react-hooks/exhaustive-deps -- the version counters track draft?.segmentation, a ref value [verseBook, segmentationVersion, draftVersion, isDraftLoading], ); + // Re-anchors the draft whenever the loaded book's text, the boundaries, or the draft itself + // changes. An import is read-only and does not show the draft, so the draft waits until it does. useEffect(() => { - // An import is read-only and does not show the draft, so its boundaries wait until it does. - if (isImportView || isDraftLoading) return; - if (segmentation !== getDraftSnapshot()?.segmentation) autosaveSegmentation(segmentation); - }, [autosaveSegmentation, getDraftSnapshot, isDraftLoading, isImportView, segmentation]); + if (isImportView || isDraftLoading || !verseBook) return; + reanchorBook(verseBook.bookRef, reanchorDraftToBook(verseBook)); + }, [reanchorBook, verseBook, isImportView, isDraftLoading, segmentationVersion, draftVersion]); /** * The book the views render: the verse-tokenized book re-grouped into the user's custom segments. @@ -1323,7 +1321,7 @@ function InterlinearizerLoaderInner({ formerBoundaries={formerBoundaries} unmergeableStarts={unmergeableStarts} segmentationVersion={segmentationVersion} - storedSplits={storedSplits} + asOneStep={asOneStep} /> ); @@ -1432,6 +1430,7 @@ function InterlinearizerLoaderInner({ onSave={autosaveAnalysis} onPendingEditsChange={setPendingEdits} showSuggestions={showSuggestions} + subscribeToReplacements={subscribeToAnalysisReplacements} > {panelGroup} diff --git a/src/components/__mocks__/AnalysisStore.tsx b/src/components/__mocks__/AnalysisStore.tsx index b39a6d88..13ea8c4d 100644 --- a/src/components/__mocks__/AnalysisStore.tsx +++ b/src/components/__mocks__/AnalysisStore.tsx @@ -1,7 +1,7 @@ import { createContext, useCallback, useContext, useMemo, useState } from 'react'; import type { ReactNode } from 'react'; -import type { AssignmentStatus, Book, MorphemeAnalysis, TokenSnapshot } from 'interlinearizer'; +import type { AssignmentStatus, MorphemeAnalysis } from 'interlinearizer'; import type { ResolvedTokenAnalysis } from '../../utils/suggestion-engine'; type GlossMap = Record; @@ -144,15 +144,6 @@ export function useMorphemeGlossDispatch(): ( */ export function useReportGlossEditing(_isEditing: boolean): void {} -/** - * No-op stand-in for the real re-anchoring pass. The mock seeds glosses by token ref and never - * re-tokenizes, so there are no offsets to heal. Re-anchoring is covered against the real store. - */ -export function useReanchorToBook( - _book: Book | undefined, - _storedSplits?: TokenSnapshot[], -): void {} - /** * Returns the merged token analysis in mock context. The mock pool is empty, so it never derives * a suggestion — always `undefined`. Suggestion behavior is covered against the real store. diff --git a/src/hooks/useDraftProject.ts b/src/hooks/useDraftProject.ts index 55fff4e9..782a012d 100644 --- a/src/hooks/useDraftProject.ts +++ b/src/hooks/useDraftProject.ts @@ -10,10 +10,42 @@ import { emptyAnalysis, emptyDraft } from '../types/empty-factories'; import { CURRENT_MODEL_VERSION } from '../types/model-version'; import { removeBookFromAnalysis, removeBookFromSegmentation } from '../utils/analysis-book'; import { isEmptyDelta } from '../utils/segmentation'; +import { + emptyHistory, + recordBookPass, + recordStep, + redo as redoStep, + undo as undoStep, + type BookPass, + type HistoryMove, + type UndoHistory, +} from '../utils/undo-history'; /** Milliseconds to wait after the last keystroke before flushing an autosave write. */ const AUTOSAVE_DEBOUNCE_MS = 300; +/** The part of a draft its undo history covers. */ +export type DraftContent = Readonly<{ + analysis: TextAnalysis; + segmentation: SegmentationDelta | undefined; +}>; + +function contentOf(draft: DraftProject): DraftContent { + return { analysis: draft.analysis, segmentation: draft.segmentation }; +} + +function sameContent(a: DraftContent, b: DraftContent): boolean { + return a.analysis === b.analysis && a.segmentation === b.segmentation; +} + +/** Returns `draft` holding `content`, marked dirty. */ +function withContent(draft: DraftProject, { analysis, segmentation }: DraftContent): DraftProject { + const next: DraftProject = { ...draft, analysis, dirty: true }; + if (segmentation !== undefined) next.segmentation = segmentation; + else delete next.segmentation; + return next; +} + /** The subset of an {@link InterlinearProject} needed to open it into the draft as a working copy. */ export type OpenableProject = Pick< InterlinearProject, @@ -106,12 +138,30 @@ export type UseDraftProjectResult = { savedAnalysis: TextAnalysis, savedSegmentation: SegmentationDelta | undefined, ) => void; + /** Returns the draft's content to how it stood before the latest undo step. */ + undo: () => void; + /** Reapplies the most recently undone step. */ + redo: () => void; + /** Runs `action`, recording every edit it auto-saves as a single undo step. */ + asOneStep: (action: () => void) => void; + /** + * Runs `pass` over the draft's content to re-anchor it to the book `bookCode` names. Bookkeeping + * rather than an edit: never an undo step, and never undone. + */ + reanchorBook: (bookCode: string, pass: BookPass) => void; + /** + * Registers `listener` to receive each analysis the draft takes from somewhere other than the + * analysis store's own edits. + * + * @returns A function that unregisters the listener. + */ + subscribeToAnalysisReplacements: (listener: (analysis: TextAnalysis) => void) => () => void; }; /** * Owns the always-present, auto-saved draft for one source project. Loads the draft on mount, seeds - * a gloss language when none is stored, and exposes callbacks to auto-save edits and to replace the - * draft wholesale (New / Open / Wipe). + * a gloss language when none is stored, and exposes callbacks to auto-save edits, to undo and redo + * them, and to replace the draft wholesale (New / Open / Wipe). * * The full draft lives in a ref — the synchronous source of truth for persistence and Save — while * a small amount of state (`isDraftLoading`, `draftVersion`, `dirty`) drives re-renders, so @@ -129,6 +179,11 @@ export default function useDraftProject( const [draftVersion, setDraftVersion] = useState(0); const [segmentationVersion, setSegmentationVersion] = useState(0); const [dirty, setDirty] = useState(false); + const historyRef = useRef>(emptyHistory()); + const replacementListenersRef = useRef(new Set<(analysis: TextAnalysis) => void>()); + // Whether an asOneStep action is running, and whether the step it shares is recorded yet. + const stepGroupOpenRef = useRef(false); + const stepGroupRecordedRef = useRef(false); // Read the latest platform language via a ref so the load effect (keyed on sourceProjectId) // does not re-run when the UI language changes after the draft has loaded. @@ -213,6 +268,7 @@ export default function useDraftProject( autosaveTimeoutRef.current = undefined; } draftRef.current = next; + historyRef.current = emptyHistory(); persist(next); setDirty(next.dirty); setDraftVersion((v) => v + 1); @@ -221,9 +277,27 @@ export default function useDraftProject( ); /** - * Shared per-edit auto-save pipeline: swaps the mutated draft into the ref, debounces the - * persistence write, and marks the draft dirty. There is no version bump and so no remount, and - * re-marking an already-dirty draft is a no-op, so editing does not re-render. + * Swaps `next` into the ref, debounces the persistence write, and marks the draft dirty. There is + * no version bump and so no remount, and re-marking an already-dirty draft is a no-op, so writing + * does not re-render. + */ + const writeDraft = useCallback( + (next: DraftProject) => { + draftRef.current = next; + // Debounce writes so rapid keystrokes don't queue unbounded commands to the backend. + if (autosaveTimeoutRef.current !== undefined) clearTimeout(autosaveTimeoutRef.current); + autosaveTimeoutRef.current = setTimeout(() => { + autosaveTimeoutRef.current = undefined; + persist(next); + }, AUTOSAVE_DEBOUNCE_MS); + setDirty(true); + }, + [persist], + ); + + /** + * Shared per-edit auto-save pipeline: writes the mutated draft and records the edit in the undo + * history. * * @param mutate - Produces the next draft from the current one; must set `dirty: true`. * @returns `true` when the edit was applied; `false` when no draft has loaded yet (nothing is @@ -236,17 +310,14 @@ export default function useDraftProject( if (!current) return false; const next = mutate(current); - draftRef.current = next; - // Debounce writes so rapid keystrokes don't queue unbounded commands to the backend. - if (autosaveTimeoutRef.current !== undefined) clearTimeout(autosaveTimeoutRef.current); - autosaveTimeoutRef.current = setTimeout(() => { - autosaveTimeoutRef.current = undefined; - persist(next); - }, AUTOSAVE_DEBOUNCE_MS); - setDirty(true); + if (!sameContent(contentOf(next), contentOf(current)) && !stepGroupRecordedRef.current) { + historyRef.current = recordStep(historyRef.current, contentOf(current)); + stepGroupRecordedRef.current = stepGroupOpenRef.current; + } + writeDraft(next); return true; }, - [persist], + [writeDraft], ); const autosaveAnalysis = useCallback( @@ -374,6 +445,79 @@ export default function useDraftProject( [persist], ); + /** + * Writes content that did not come from the analysis store's own edits, bringing the store and + * the boundary consumers along with it. + */ + const replaceContent = useCallback( + (current: DraftProject, content: DraftContent) => { + writeDraft(withContent(current, content)); + if (content.analysis !== current.analysis) + replacementListenersRef.current.forEach((listener) => listener(content.analysis)); + if (content.segmentation !== current.segmentation) setSegmentationVersion((v) => v + 1); + }, + [writeDraft], + ); + + /** + * Moves through the undo history, bringing the draft and its analysis store to the content moved + * to. + */ + const moveThroughHistory = useCallback( + ( + step: ( + history: UndoHistory, + present: DraftContent, + ) => HistoryMove | undefined, + ) => { + const { current } = draftRef; + /* v8 ignore next -- undo and redo are unavailable until the draft loads */ + if (!current) return; + const move = step(historyRef.current, contentOf(current)); + if (!move) return; + historyRef.current = move.history; + replaceContent(current, move.content); + }, + [replaceContent], + ); + + const undo = useCallback(() => moveThroughHistory(undoStep), [moveThroughHistory]); + + const redo = useCallback(() => moveThroughHistory(redoStep), [moveThroughHistory]); + + const reanchorBook = useCallback( + (bookCode: string, pass: BookPass) => { + const { current } = draftRef; + /* v8 ignore next -- books are re-anchored only once the draft has loaded */ + if (!current) return; + historyRef.current = recordBookPass(historyRef.current, bookCode, pass); + const before = contentOf(current); + const after = pass(before); + if (!sameContent(after, before)) replaceContent(current, after); + }, + [replaceContent], + ); + + const asOneStep = useCallback((action: () => void) => { + stepGroupOpenRef.current = true; + try { + action(); + } finally { + stepGroupOpenRef.current = false; + stepGroupRecordedRef.current = false; + } + }, []); + + const subscribeToAnalysisReplacements = useCallback( + (listener: (analysis: TextAnalysis) => void) => { + replacementListenersRef.current.add(listener); + return () => { + replacementListenersRef.current.delete(listener); + }; + }, + [], + ); + return { isDraftLoading, draft: draftRef.current, @@ -388,5 +532,10 @@ export default function useDraftProject( wipeBook, wipeAll, markSynced, + undo, + redo, + asOneStep, + reanchorBook, + subscribeToAnalysisReplacements, }; } diff --git a/src/store/analysisSlice.ts b/src/store/analysisSlice.ts index bd29501b..a86707de 100644 --- a/src/store/analysisSlice.ts +++ b/src/store/analysisSlice.ts @@ -1,6 +1,5 @@ import { createSelector, createSlice, current, type PayloadAction } from '@reduxjs/toolkit'; import type { - Book, Confidence, MorphemeAnalysis, PhraseAnalysis, @@ -20,7 +19,6 @@ import { phraseAnalysesAreIdentical, reconcileMorphemes, } from '../utils/analysis-identity'; -import { reanchorAnalysisToBook } from '../utils/reanchor-analysis'; import { buildCatalogRows, type HeadingPlacement } from '../utils/analysis-query'; import { isEmptyMultiString } from '../utils/multi-string'; import { @@ -1689,28 +1687,9 @@ const analysisSlice = createSlice({ }, }, - reanchorToBook: { - /** Reads the clock before the action reaches the reducer, keeping the reducer pure. */ - prepare(arg: { book: Book; storedSplits?: TokenSnapshot[] }) { - return { payload: { book: arg.book, storedSplits: arg.storedSplits, now: nowIso() } }; - }, - /** - * Re-points the analysis at a freshly tokenized book, healing links whose tokens an upstream - * text edit re-keyed and staling those it cannot place. State is replaced only when something - * actually moved, so loading a book whose text is unchanged is not a write. - */ - reducer( - state, - action: PayloadAction<{ book: Book; storedSplits?: TokenSnapshot[]; now: string }>, - ) { - const reanchored = reanchorAnalysisToBook( - state.analysis, - action.payload.book, - action.payload.now, - action.payload.storedSplits, - ); - if (reanchored !== state.analysis) state.analysis = reanchored; - }, + /** Takes an analysis the draft holds that did not come from this store's own edits. */ + replaceAnalysis(state, action: PayloadAction) { + state.analysis = action.payload; }, }, }); @@ -1735,7 +1714,7 @@ export const { writePhraseGloss, approvePhrase, writeSegmentFreeTranslation, - reanchorToBook, + replaceAnalysis, } = analysisSlice.actions; export default analysisSlice.reducer; diff --git a/src/utils/reanchor-draft.ts b/src/utils/reanchor-draft.ts new file mode 100644 index 00000000..b1cbab25 --- /dev/null +++ b/src/utils/reanchor-draft.ts @@ -0,0 +1,22 @@ +import type { Book } from 'interlinearizer'; +import type { DraftContent } from '../hooks/useDraftProject'; +import { resegmentBook } from '../parsers/papi/resegmentBook'; +import { reanchorAnalysisToBook } from './reanchor-analysis'; +import { reanchorSegmentation } from './segmentation'; +import type { BookPass } from './undo-history'; + +/** Builds the pass that re-anchors a draft's content to `verseBook`'s current text. */ +export function reanchorDraftToBook(verseBook: Book): BookPass { + return (content) => { + const segmentation = reanchorSegmentation(verseBook, content.segmentation); + return { + analysis: reanchorAnalysisToBook( + content.analysis, + resegmentBook(verseBook, segmentation), + new Date().toISOString(), + content.segmentation?.addedStarts, + ), + segmentation, + }; + }; +} From 849c9a2f723adc4793c57f48d55bd5f4455d6d2b Mon Sep 17 00:00:00 2001 From: Alex Rawlings Date: Wed, 30 Sep 2026 13:48:40 -0600 Subject: [PATCH 03/17] Undo and redo draft edits from the keys, toolbar, and Edit menu (#184) --- __mocks__/platform-bible-react.tsx | 69 ++++++ contributions/localizedStrings.json | 4 + contributions/menus.json | 25 +- .../components/InterlinearizerLoader.test.tsx | 215 ++++++++++++++++-- src/__tests__/hooks/useDraftProject.test.ts | 106 +++++++++ src/__tests__/hooks/useUndoRedoKeys.test.tsx | 116 ++++++++++ src/__tests__/main.test.ts | 2 + src/components/CatalogRowEditor.tsx | 1 + src/components/InterlinearizerLoader.tsx | 85 +++++-- src/components/MorphemeBox.tsx | 1 + src/components/PhraseBox.tsx | 1 + .../SegmentFreeTranslationInput.tsx | 1 + src/components/TokenChip.tsx | 1 + src/hooks/useDraftProject.ts | 59 +++-- src/hooks/useUndoRedoKeys.ts | 43 ++++ src/main.ts | 28 +++ src/types/interlinearizer.d.ts | 14 ++ 17 files changed, 710 insertions(+), 61 deletions(-) create mode 100644 src/__tests__/hooks/useUndoRedoKeys.test.tsx create mode 100644 src/hooks/useUndoRedoKeys.ts diff --git a/__mocks__/platform-bible-react.tsx b/__mocks__/platform-bible-react.tsx index 65757fcd..65ba5e52 100644 --- a/__mocks__/platform-bible-react.tsx +++ b/__mocks__/platform-bible-react.tsx @@ -116,6 +116,24 @@ export const MOCK_WIPE_MENU_ITEM: MenuItemContainingCommand = { localizeNotes: '', }; +/** Sentinel menu item passed by the mock toolbar when the undo button is clicked. */ +export const MOCK_UNDO_MENU_ITEM: MenuItemContainingCommand = { + label: '%interlinearizer_undo%', + command: 'interlinearizer.undo', + group: 'interlinearizer.editActions', + order: 1, + localizeNotes: '', +}; + +/** Sentinel menu item passed by the mock toolbar when the redo button is clicked. */ +export const MOCK_REDO_MENU_ITEM: MenuItemContainingCommand = { + label: '%interlinearizer_redo%', + command: 'interlinearizer.redo', + group: 'interlinearizer.editActions', + order: 2, + localizeNotes: '', +}; + /** Sentinel menu item passed by the mock toolbar when the analysis-catalog button is clicked. */ export const MOCK_OPEN_ANALYSIS_CATALOG_MENU_ITEM: MenuItemContainingCommand = { label: '%interlinearizer_openAnalysisCatalog%', @@ -247,6 +265,24 @@ export function TabToolbar({ Wipe )} + {onSelectProjectMenuItem && ( + + )} + {onSelectProjectMenuItem && ( + + )} {onSelectProjectMenuItem && ( + {onRedoClick && ( + + )} + + ); +} + /** * Stub toggle switch rendered as a native checkbox so tests can read and change the checked state * without the real Radix UI implementation. diff --git a/contributions/localizedStrings.json b/contributions/localizedStrings.json index 7915705b..bf9281a3 100644 --- a/contributions/localizedStrings.json +++ b/contributions/localizedStrings.json @@ -17,6 +17,10 @@ "%interlinearizer_openLexiconChooser%": "Connect a Lexicon…", "%interlinearizer_error_openLexiconChooser_failed%": "The lexicon chooser could not be opened.", + "%interlinearizer_menu_column_edit%": "Edit", + "%interlinearizer_undo%": "Undo", + "%interlinearizer_redo%": "Redo", + "%interlinearizer_menu_column_view%": "View", "%interlinearizer_openAnalysisCatalog%": "Analysis Catalog", "%interlinearizer_openConcordance%": "Concordance", diff --git a/contributions/menus.json b/contributions/menus.json index 67d7967c..061871fc 100644 --- a/contributions/menus.json +++ b/contributions/menus.json @@ -22,10 +22,15 @@ "localizeNotes": "Interlinearizer top menu column for project actions", "order": 1 }, + "interlinearizer.edit": { + "label": "%interlinearizer_menu_column_edit%", + "localizeNotes": "Interlinearizer top menu column for undoing and redoing edits", + "order": 2 + }, "interlinearizer.view": { "label": "%interlinearizer_menu_column_view%", "localizeNotes": "Interlinearizer top menu column for opening views of the analysis", - "order": 2 + "order": 3 } }, "groups": { @@ -45,6 +50,10 @@ "column": "interlinearizer.project", "order": 4 }, + "interlinearizer.editActions": { + "column": "interlinearizer.edit", + "order": 1 + }, "interlinearizer.viewActions": { "column": "interlinearizer.view", "order": 1 @@ -100,6 +109,20 @@ "order": 1, "command": "interlinearizer.openLexiconChooser" }, + { + "label": "%interlinearizer_undo%", + "localizeNotes": "Interlinearizer top menu > Edit > Undo the latest edit to the draft", + "group": "interlinearizer.editActions", + "order": 1, + "command": "interlinearizer.undo" + }, + { + "label": "%interlinearizer_redo%", + "localizeNotes": "Interlinearizer top menu > Edit > Redo the most recently undone edit", + "group": "interlinearizer.editActions", + "order": 2, + "command": "interlinearizer.redo" + }, { "label": "%interlinearizer_openAnalysisCatalog%", "localizeNotes": "Interlinearizer top menu > Open the analysis catalog, listing every analysis recorded in the draft with its usage counts and locations", diff --git a/src/__tests__/components/InterlinearizerLoader.test.tsx b/src/__tests__/components/InterlinearizerLoader.test.tsx index 0dc7cf59..17c076f4 100644 --- a/src/__tests__/components/InterlinearizerLoader.test.tsx +++ b/src/__tests__/components/InterlinearizerLoader.test.tsx @@ -194,6 +194,7 @@ type CapturedInterlinearizerProps = { formerBoundaries: ReadonlyMap; unmergeableStarts?: ReadonlySet; segmentationVersion: number; + asOneStep: (action: () => void) => void; }; let capturedInterlinearizerProps: CapturedInterlinearizerProps | undefined; let interlinearizerMountCount = 0; @@ -296,6 +297,20 @@ function analysisApprovingAt(tokenRef: string, surfaceText: string): TextAnalysi }; } +/** A one-verse book with a second word to split before. */ +const ALPHA_BETA_BOOK: Book = { + id: 'GEN', + bookRef: 'GEN', + textVersion: 'v1', + duplicateVerseIds: [], + segments: [ + makeSegment('GEN 1:1', 'Alpha beta.', [ + makeWordToken('GEN 1:1:0', 'Alpha'), + makeWordToken('GEN 1:1:6', 'beta', 6), + ]), + ], +}; + /** Minimal project summary used across modal interaction tests. */ type MockProject = { id: string; @@ -1997,6 +2012,42 @@ describe('InterlinearizerLoader', () => { }); }); + it('offers no undo or redo buttons in the import view', async () => { + mockImportCommands(); + await renderImportView(); + + expect(screen.queryByRole('button', { name: '%undoButton_tooltip%' })).toBeNull(); + expect(screen.queryByRole('button', { name: '%redoButton_tooltip%' })).toBeNull(); + }); + + it('leaves the draft alone on Ctrl+Z while an import is showing', async () => { + mockImportCommands(); + mockPdpGet.mockResolvedValue({ + getPt9InterlinearManifest: async () => probeOf({ 'Lexicon.xml': 'aaaa1111' }), + }); + mockBookData({ book: ALPHA_BETA_BOOK }); + jest.useFakeTimers(); + await act(async () => renderLoader()); + act(() => capturedInterlinearizerProps?.segmentationDispatch.split('GEN 1:1:6')); + act(() => jest.advanceTimersByTime(300)); + fireEvent.click(screen.getByTestId('tab-toolbar-project-menu')); + await act(async () => fireEvent.click(screen.getByTestId('select-modal-open-import'))); + await screen.findByTestId('pt9-import-banner'); + mockSendCommand.mockClear(); + + act(() => { + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + }); + act(() => jest.advanceTimersByTime(300)); + jest.useRealTimers(); + + expect(mockSendCommand).not.toHaveBeenCalledWith( + 'interlinearizer.saveDraft', + expect.anything(), + expect.anything(), + ); + }); + it('hands the import view a segmentation dispatch that cannot write the draft', async () => { mockImportCommands(); await renderImportView(); @@ -4243,30 +4294,33 @@ const LUK_1_1_BOOK: Book = { ], }; +/** Resets the loader's mocks and captures for a test that reads the store through the probe. */ +function prepareStoreProbeTest(): void { + mountStoreProbe = true; + probeStore = undefined; + probeAnalysis = undefined; + probeWriteGloss = undefined; + capturedInterlinearizerProps = undefined; + capturedStoreProps = undefined; + interlinearizerMountCount = 0; + mockBookData(); + mockLexiconRegistry(); + mockOptimisticSetting(); + mockLostBoundaries([]); + mockProjectBookIds(undefined); + mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); + jest + .mocked(useData) + .mockReturnValue( + new Proxy({}, { get: () => jest.fn().mockReturnValue([undefined, jest.fn(), false]) }), + ); + jest.mocked(useLocalizedStrings).mockReturnValue([{}, false]); + mockSettings(); + mockSourceShortName(''); +} + describe('analysis store lifetime', () => { - beforeEach(() => { - mountStoreProbe = true; - probeStore = undefined; - probeAnalysis = undefined; - probeWriteGloss = undefined; - capturedInterlinearizerProps = undefined; - capturedStoreProps = undefined; - interlinearizerMountCount = 0; - mockBookData(); - mockLexiconRegistry(); - mockOptimisticSetting(); - mockLostBoundaries([]); - mockProjectBookIds(undefined); - mockSendCommand.mockResolvedValue(JSON.stringify(emptyDraft(testProjectId))); - jest - .mocked(useData) - .mockReturnValue( - new Proxy({}, { get: () => jest.fn().mockReturnValue([undefined, jest.fn(), false]) }), - ); - jest.mocked(useLocalizedStrings).mockReturnValue([{}, false]); - mockSettings(); - mockSourceShortName(''); - }); + beforeEach(prepareStoreProbeTest); afterEach(() => { mountStoreProbe = false; @@ -4347,3 +4401,118 @@ describe('analysis store lifetime', () => { expect(capturedStoreProps?.initialAnalysis).toEqual(emptyAnalysis()); }); }); + +describe('undo and redo', () => { + beforeEach(() => { + prepareStoreProbeTest(); + mockBookData({ book: ALPHA_BETA_BOOK }); + }); + + afterEach(() => { + mountStoreProbe = false; + }); + + /** Renders the loader and glosses "Alpha" through the store. */ + async function renderAndGloss(): Promise { + await act(async () => renderLoader()); + act(() => probeWriteGloss?.('GEN 1:1:0', 'Alpha', 'alpha')); + } + + it('undoes the last edit on Ctrl+Z', async () => { + await renderAndGloss(); + + act(() => { + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + }); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(0); + }); + + it('redoes an undone edit on Ctrl+Y', async () => { + await renderAndGloss(); + + act(() => { + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + }); + act(() => { + fireEvent.keyDown(document.body, { key: 'y', ctrlKey: true }); + }); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(1); + }); + + it("undoes an editor action's analysis and boundary edits as one step", async () => { + await act(async () => renderLoader()); + + act(() => + capturedInterlinearizerProps?.asOneStep(() => { + probeWriteGloss?.('GEN 1:1:0', 'Alpha', 'alpha'); + capturedInterlinearizerProps?.segmentationDispatch.split('GEN 1:1:6'); + }), + ); + act(() => { + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + }); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(0); + expect(capturedInterlinearizerProps?.book.segments).toHaveLength(1); + }); + + it('leaves the draft alone on Ctrl+Z while a dialog is open over it', async () => { + await renderAndGloss(); + render(
); + + act(() => { + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + }); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(1); + }); + + it('undoes from the Edit menu', async () => { + await renderAndGloss(); + + act(() => screen.getByTestId('tab-toolbar-undo').click()); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(0); + }); + + it('redoes from the Edit menu', async () => { + await renderAndGloss(); + + act(() => screen.getByTestId('tab-toolbar-undo').click()); + act(() => screen.getByTestId('tab-toolbar-redo').click()); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(1); + }); + + it("undoes from the toolbar's undo button", async () => { + await renderAndGloss(); + + act(() => screen.getByRole('button', { name: '%undoButton_tooltip%' }).click()); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(0); + }); + + it("redoes from the toolbar's redo button", async () => { + await renderAndGloss(); + + act(() => screen.getByRole('button', { name: '%undoButton_tooltip%' }).click()); + act(() => screen.getByRole('button', { name: '%redoButton_tooltip%' }).click()); + + expect(probeAnalysis?.tokenAnalysisLinks).toHaveLength(1); + }); + + it('disables the toolbar buttons while there is nothing to undo or redo', async () => { + await act(async () => renderLoader()); + + expect(screen.getByRole('button', { name: '%undoButton_tooltip%' })).toBeDisabled(); + expect(screen.getByRole('button', { name: '%redoButton_tooltip%' })).toBeDisabled(); + }); + + it('enables the toolbar undo button once there is an edit to undo', async () => { + await renderAndGloss(); + + expect(screen.getByRole('button', { name: '%undoButton_tooltip%' })).toBeEnabled(); + }); +}); diff --git a/src/__tests__/hooks/useDraftProject.test.ts b/src/__tests__/hooks/useDraftProject.test.ts index 849e96e5..fe6f6424 100644 --- a/src/__tests__/hooks/useDraftProject.test.ts +++ b/src/__tests__/hooks/useDraftProject.test.ts @@ -775,6 +775,112 @@ describe('useDraftProject', () => { expect(result.current.getDraftSnapshot()?.analysis).toEqual(emptyAnalysis()); }); + it('has nothing to undo or redo before any edit', async () => { + const { result } = await renderLoaded(); + + expect(result.current.canUndo).toBe(false); + expect(result.current.canRedo).toBe(false); + }); + + it('can undo once an edit is made', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + + expect(result.current.canUndo).toBe(true); + }); + + it('can redo once an edit is undone', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(result.current.canRedo).toBe(true); + expect(result.current.canUndo).toBe(false); + }); + + it('has nothing to undo once a project is opened', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => + result.current.loadFromProject({ analysis: emptyAnalysis(), analysisLanguages: [] }), + ); + + expect(result.current.canUndo).toBe(false); + }); + + describe('dirty baseline', () => { + it('is clean again when an undo returns to the loaded content', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(false); + }); + + it('stays dirty when an undo lands on content that was never synced', async () => { + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-first'))); + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-second'))); + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(true); + }); + + it('is clean again when an undo returns to the last saved content', async () => { + const { result } = await renderLoaded(); + const saved = analysisWithToken('tok-saved'); + + act(() => result.current.autosaveAnalysis(saved)); + act(() => result.current.markSynced(saved, undefined)); + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-after-save'))); + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(false); + }); + + it('is clean again when an undo returns to an opened project', async () => { + const { result } = await renderLoaded(); + + act(() => + result.current.loadFromProject({ + analysis: analysisWithToken('tok-open'), + analysisLanguages: ['de'], + }), + ); + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(false); + }); + + it('stays dirty after undoing into a draft that loaded unsaved', async () => { + mockGetDraftResolves(makeDraft({ dirty: true })); + const { result } = await renderLoaded(); + + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + + expect(result.current.dirty).toBe(true); + }); + + it('persists the draft as clean once an undo returns it to the baseline', async () => { + const { result } = await renderLoaded(); + + jest.useFakeTimers(); + act(() => result.current.autosaveAnalysis(analysisWithToken('tok-edited'))); + act(() => result.current.undo()); + act(() => jest.advanceTimersByTime(300)); + jest.useRealTimers(); + + expect(lastSavedDraft().dirty).toBe(false); + }); + }); + describe('re-anchoring', () => { /** A pass that renames the content's token analysis, so a test can see where it ran. */ const renameToken = diff --git a/src/__tests__/hooks/useUndoRedoKeys.test.tsx b/src/__tests__/hooks/useUndoRedoKeys.test.tsx new file mode 100644 index 00000000..2bd700ad --- /dev/null +++ b/src/__tests__/hooks/useUndoRedoKeys.test.tsx @@ -0,0 +1,116 @@ +/// + +import { fireEvent, render, renderHook, screen } from '@testing-library/react'; +import useUndoRedoKeys from '../../hooks/useUndoRedoKeys'; +import { pretendMacOs } from '../test-helpers'; + +/** Binds the hook to fresh undo and redo spies. */ +function renderKeys({ hasPendingEdits = false } = {}) { + const undo = jest.fn(); + const redo = jest.fn(); + const view = renderHook(() => useUndoRedoKeys({ undo, redo, hasPendingEdits })); + return { undo, redo, ...view }; +} + +describe('useUndoRedoKeys', () => { + it('undoes on Ctrl+Z', () => { + const { undo } = renderKeys(); + + fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + + expect(undo).toHaveBeenCalledTimes(1); + }); + + it('redoes on Ctrl+Y', () => { + const { redo } = renderKeys(); + + fireEvent.keyDown(document.body, { key: 'y', ctrlKey: true }); + + expect(redo).toHaveBeenCalledTimes(1); + }); + + it('redoes rather than undoes on Ctrl+Shift+Z', () => { + const { undo, redo } = renderKeys(); + + fireEvent.keyDown(document.body, { key: 'Z', ctrlKey: true, shiftKey: true }); + + expect(redo).toHaveBeenCalledTimes(1); + expect(undo).not.toHaveBeenCalled(); + }); + + it('claims the shortcut from the browser', () => { + renderKeys(); + + const notCanceled = fireEvent.keyDown(document.body, { key: 'z', ctrlKey: true }); + + expect(notCanceled).toBe(false); + }); + + it('leaves Ctrl+Z to a text field that holds no draft content', () => { + const { undo } = renderKeys(); + render(); + + const notCanceled = fireEvent.keyDown(screen.getByLabelText('search'), { + key: 'z', + ctrlKey: true, + }); + + expect(undo).not.toHaveBeenCalled(); + expect(notCanceled).toBe(true); + }); + + it('leaves Ctrl+Z to a draft field holding uncommitted text', () => { + const { undo } = renderKeys({ hasPendingEdits: true }); + render(); + + fireEvent.keyDown(screen.getByLabelText('gloss'), { key: 'z', ctrlKey: true }); + + expect(undo).not.toHaveBeenCalled(); + }); + + it('undoes from a draft field with nothing uncommitted', () => { + const { undo } = renderKeys(); + render(