Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-undo-step-id-leak-undefined.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@portabletext/editor': patch
---

fix: merge consecutive text ops when one undoStepId is undefined
34 changes: 34 additions & 0 deletions packages/editor/src/editor/undo-step.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
35 changes: 33 additions & 2 deletions packages/editor/tests/event.history.undo.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'])
})
Expand Down Expand Up @@ -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'])
})
Expand All @@ -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: (
<BehaviorPlugin
behaviors={[
defineBehavior({
on: 'insert.text',
guard: ({event}) => 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([''])
})
})
})