diff --git a/.changeset/fix-undo-step-id-leak-undefined.md b/.changeset/fix-undo-step-id-leak-undefined.md new file mode 100644 index 000000000..096d7f4f2 --- /dev/null +++ b/.changeset/fix-undo-step-id-leak-undefined.md @@ -0,0 +1,5 @@ +--- +'@portabletext/editor': patch +--- + +fix: merge consecutive text ops when one undoStepId is undefined diff --git a/packages/editor/src/editor/undo-step.ts b/packages/editor/src/editor/undo-step.ts index 3ac5ecbd2..ea79b183d 100644 --- a/packages/editor/src/editor/undo-step.ts +++ b/packages/editor/src/editor/undo-step.ts @@ -117,6 +117,40 @@ export function createUndoSteps({ } } + // Handle case when the current operation has no undoStepId but the previous + // one did. This can happen when a forward-only behavior intercepts an event, + // causing the operation to be applied with an undoStepId, followed by a + // subsequent operation without one. We still want to merge consecutive text + // operations in this case. + // + // Note: the reverse case (current has ID, previous doesn't) is intentionally + // not merged. When a behavior claims an event with an undoStepId, it signals + // an intentional undo step boundary (e.g., input rules). + if (currentUndoStepId === undefined && previousUndoStepId !== undefined) { + const lastOp = lastStep.operations.at(-1) + + if ( + lastOp && + op.type === 'insert_text' && + lastOp.type === 'insert_text' && + op.offset === lastOp.offset + lastOp.text.length && + Path.equals(op.path, lastOp.path) && + op.text !== ' ' + ) { + return mergeIntoLastStep(steps, lastStep, op) + } + + if ( + lastOp && + op.type === 'remove_text' && + lastOp.type === 'remove_text' && + op.offset + op.text.length === lastOp.offset && + Path.equals(op.path, lastOp.path) + ) { + return mergeIntoLastStep(steps, lastStep, op) + } + } + return createNewStep(steps, op, editor) } diff --git a/packages/editor/tests/event.history.undo.test.tsx b/packages/editor/tests/event.history.undo.test.tsx index 55addcad0..4307188ee 100644 --- a/packages/editor/tests/event.history.undo.test.tsx +++ b/packages/editor/tests/event.history.undo.test.tsx @@ -853,7 +853,7 @@ describe('event.history.undo', () => { decorator: 'strong', }) - // After adding: "f" + "oob" (strong) + "ar" — three spans + // After adding: "f" + "oob" (strong) + "ar" - three spans await vi.waitFor(() => { expect(getTersePt(editor.getSnapshot().context)).toEqual(['f,oob,ar']) }) @@ -919,7 +919,7 @@ describe('event.history.undo', () => { }, }) - // After adding: "f" + "oob" (link) + "ar" — three spans + // After adding: "f" + "oob" (link) + "ar" - three spans await vi.waitFor(() => { expect(getTersePt(editor.getSnapshot().context)).toEqual(['f,oob,ar']) }) @@ -931,4 +931,35 @@ describe('event.history.undo', () => { expect(editor.getSnapshot().context.value).toEqual(initialValue) }) }) + + test('Scenario: forward-only behavior should not break consecutive typing merge', async () => { + const {editor, locator} = await createTestEditor({ + children: ( + event.text === 'a', + actions: [({event}) => [forward(event)]], + }), + ]} + /> + ), + }) + + await userEvent.type(locator, 'a') + await userEvent.type(locator, 'b') + + await vi.waitFor(() => { + expect(getTersePt(editor.getSnapshot().context)).toEqual(['ab']) + }) + + // If the undoStepId leak is fixed, these should be one undo step + // (consecutive insert_text, same path, adjacent offsets) + editor.send({type: 'history.undo'}) + + await vi.waitFor(() => { + expect(getTersePt(editor.getSnapshot().context)).toEqual(['']) + }) + }) })