From d4de12d451095d65b7541550cb1ec88aecd22cd7 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Tue, 19 May 2026 23:41:31 +0100 Subject: [PATCH 1/5] Imlement simple history manager in core package --- .../src/BatchedOperation.ts | 1 + .../src/CollaborationManager.ts | 16 +- .../src/api/DocumentAPI/DocumentAPI.spec.ts | 13 +- .../core/src/api/DocumentAPI/DocumentAPI.ts | 28 +- .../src/components/UndoRedoManager.spec.ts | 986 ++++++++++++++++++ .../core/src/components/UndoRedoManager.ts | 331 ++++++ packages/core/src/index.ts | 2 + packages/model/src/EventBus/events/index.ts | 1 + packages/model/src/EventBus/index.ts | 1 - .../src/EventBus/types/EventPayloadBase.ts | 23 - packages/sdk/src/api/DocumentAPI.ts | 10 + .../EventBus/events/core/CoreEventBase.ts | 4 +- .../EventBus/events/core/RedoCoreEvent.ts | 2 +- .../EventBus/events/core/UndoCoreEvent.ts | 2 +- packages/ui/src/Blocks/Blocks.ts | 18 +- 15 files changed, 1395 insertions(+), 43 deletions(-) create mode 100644 packages/core/src/components/UndoRedoManager.spec.ts create mode 100644 packages/core/src/components/UndoRedoManager.ts diff --git a/packages/collaboration-manager/src/BatchedOperation.ts b/packages/collaboration-manager/src/BatchedOperation.ts index 1d3c7dbe..191be094 100644 --- a/packages/collaboration-manager/src/BatchedOperation.ts +++ b/packages/collaboration-manager/src/BatchedOperation.ts @@ -107,6 +107,7 @@ export class BatchedOperation extends O * Checks if operation can be added to the batch * * Only text operations with the same type (Insert/Delete) on the same block and data key could be added + * @todo delete operations are not being batched properly * @param op - operation to check */ public canAdd(op: Operation): boolean { diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index c6f8f58a..5ee4ba14 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -11,7 +11,7 @@ import { type EditorAPI, type EditorjsPlugin, type EditorjsPluginParams, - PluginType + PluginType, UndoCoreEvent } from '@editorjs/sdk'; import { OTClient } from './client/index.js'; import { BatchedOperation } from './BatchedOperation.js'; @@ -97,10 +97,14 @@ export class CollaborationManager implements EditorjsPlugin { this.#config = config; this.#undoRedoManager = new UndoRedoManager(); - const onUndo = (): void => { + const onUndo = (e: UndoCoreEvent): void => { + e.preventDefault(); + this.undo(); }; - const onRedo = (): void => { + const onRedo = (e: UndoCoreEvent): void => { + e.preventDefault(); + this.redo(); }; const onReady = (): void => { @@ -110,13 +114,13 @@ export class CollaborationManager implements EditorjsPlugin { this.#unsubscribeDocumentUpdates = api.document.onUpdate(this.#handleEvent.bind(this)); eventBus.addEventListener(`core:${CoreEventType.Undo}`, onUndo); - this.#unsubscribeUndo = () => eventBus.removeEventListener(`core:${CoreEventType.Undo}`, onUndo); + this.#unsubscribeUndo = () => void eventBus.removeEventListener(`core:${CoreEventType.Undo}`, onUndo); eventBus.addEventListener(`core:${CoreEventType.Redo}`, onRedo); - this.#unsubscribeRedo = () => eventBus.removeEventListener(`core:${CoreEventType.Redo}`, onRedo); + this.#unsubscribeRedo = () => void eventBus.removeEventListener(`core:${CoreEventType.Redo}`, onRedo); eventBus.addEventListener(`core:${CoreEventType.Ready}`, onReady); - this.#unsubscribeReady = () => eventBus.removeEventListener(`core:${CoreEventType.Ready}`, onReady); + this.#unsubscribeReady = () => void eventBus.removeEventListener(`core:${CoreEventType.Ready}`, onReady); } /** diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts index 6182eeb4..08deb238 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts @@ -1,6 +1,11 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { beforeEach, describe, expect, jest } from '@jest/globals'; -import type { CoreConfigValidated } from '@editorjs/sdk'; +import type { CoreConfigValidated, EventBus } from '@editorjs/sdk'; + +jest.unstable_mockModule('@editorjs/sdk', () => ({ + UndoCoreEvent: jest.fn(), + RedoCoreEvent: jest.fn(), +})); jest.unstable_mockModule('@editorjs/model', () => { const EditorJSModel = jest.fn(() => ({ @@ -22,7 +27,11 @@ describe('DocumentAPI', () => { // @ts-expect-error - mock object, don't need to pass any arguments const model = new EditorJSModel(); - const documentAPI = new DocumentAPI(model, {} as unknown as CoreConfigValidated); + const documentAPI = new DocumentAPI( + model, + {} as unknown as CoreConfigValidated, + { dispatchEvent: jest.fn() } as unknown as EventBus + ); beforeEach(() => { jest.resetAllMocks(); diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.ts index 777559c8..2e1ba81f 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.ts @@ -3,9 +3,9 @@ import 'reflect-metadata'; import { type EditorDocumentSerialized, EditorJSModel, EventType, type ModelEvents } from '@editorjs/model'; import { CoreConfigValidated, - DocumentAPI as DocumentApiInterface, + DocumentAPI as DocumentApiInterface, EventBus, type InsertRemoveDataParams, - type ModifyDataParams + type ModifyDataParams, RedoCoreEvent, UndoCoreEvent } from '@editorjs/sdk'; import { inject, injectable } from 'inversify'; import { TOKENS } from '../../tokens.js'; @@ -26,18 +26,26 @@ export class DocumentAPI implements DocumentApiInterface { */ #config: CoreConfigValidated; + /** + * Editor's event bus instance + */ + #eventBus: EventBus; + /** * DocumentAPI constructor * All parameters are injected through the IoC container * @param model - Editor's Document Model instance * @param config - Editor's config + * @param eventBus - Editor's event bus instance */ constructor( model: EditorJSModel, - @inject(TOKENS.EditorConfig) config: CoreConfigValidated + @inject(TOKENS.EditorConfig) config: CoreConfigValidated, + eventBus: EventBus ) { this.#model = model; this.#config = config; + this.#eventBus = eventBus; } /** @@ -91,4 +99,18 @@ export class DocumentAPI implements DocumentApiInterface { public modifyData({ userId = this.#config.userId, index, data }: ModifyDataParams): void { this.#model.modifyData(userId, index, data); } + + /** + * Undoes the last change in the document + */ + public undo(): void { + this.#eventBus.dispatchEvent(new UndoCoreEvent()); + } + + /** + * Redoes the last undone change in the document + */ + public redo(): void { + this.#eventBus.dispatchEvent(new RedoCoreEvent()); + } } diff --git a/packages/core/src/components/UndoRedoManager.spec.ts b/packages/core/src/components/UndoRedoManager.spec.ts new file mode 100644 index 00000000..fa69fcf0 --- /dev/null +++ b/packages/core/src/components/UndoRedoManager.spec.ts @@ -0,0 +1,986 @@ +/* eslint-disable @typescript-eslint/no-magic-numbers, jsdoc/require-jsdoc, @typescript-eslint/naming-convention */ +import { afterEach, beforeEach, describe, expect, it, jest } from '@jest/globals'; +import type { CoreConfigValidated } from '@editorjs/sdk'; +// @ts-expect-error -- jest module mock doesn't import type +import type { EventType } from '@editorjs/model'; + +const USER_ID = 'user'; +const OTHER_USER_ID = 'other-user'; + +// Register ESM mocks before importing the module under test +jest.unstable_mockModule('@editorjs/model', () => { + const EditorJSModel = jest.fn(() => ({ + addEventListener: jest.fn(), + insertData: jest.fn(), + removeData: jest.fn(), + modifyData: jest.fn(), + })); + + const EventType = { Changed: 'model:changed' }; + + const EventAction = { + Added: 'added', + Removed: 'removed', + Modified: 'modified', + }; + + return { + EditorJSModel, + EventType, + EventAction, + }; +}); + +jest.unstable_mockModule('@editorjs/sdk', () => ({ + CoreEventType: { + Undo: 'undo', + Redo: 'redo', + }, + EventBus: jest.fn(() => ({ + addEventListener: jest.fn(), + removeEventListener: jest.fn(), + dispatchEvent: jest.fn(), + })), +})); + +const { EditorJSModel, EventType, EventAction } = await import('@editorjs/model'); +const { EventBus } = await import('@editorjs/sdk'); +const { UndoRedoManager } = await import('./UndoRedoManager.js'); + +// ─── helpers ──────────────────────────────────────────────────────────────── + +type MockModel = { + addEventListener: jest.Mock; + insertData: jest.Mock; + removeData: jest.Mock; + modifyData: jest.Mock; +}; + +type MockEventBus = { + addEventListener: jest.Mock; + removeEventListener: jest.Mock; + dispatchEvent: jest.Mock; +}; + +/** + * Creates Index mock + * @param options - index mock options + */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +function createIndex(options: { + isTextIndex?: boolean; + blockIndex?: number; + dataKey?: string; + textRange?: [number, number] | undefined; +} = {}) { + return { + isTextIndex: options.isTextIndex ?? true, + blockIndex: options.blockIndex ?? 0, + dataKey: options.dataKey ?? 'text', + textRange: 'textRange' in options ? options.textRange : ([0, 0] as [number, number]), + }; +} + +/** + * Creates model event payload mock + * @param action - event action + * @param options - payload options (index, data, userId) + */ +// eslint-disable-next-line @typescript-eslint/explicit-function-return-type +function createPayload( + action: string, + options: { + index?: ReturnType; + data?: unknown; + userId?: string; + } = {} +) { + return { + action, + index: options.index ?? createIndex(), + data: options.data ?? 'a', + userId: options.userId ?? USER_ID, + }; +} + +function fireModelEvent(listener: (e: unknown) => void, payload: ReturnType): void { + listener({ detail: payload }); +} + +describe('UndoRedoManager', () => { + let model: MockModel; + let eventBus: MockEventBus; + let manager: InstanceType; + let modelChangedListener: (e: unknown) => void; + let undoEventListener: (options: { defaultPrevented: boolean }) => void; + let redoEventListener: (options: { defaultPrevented: boolean }) => void; + + beforeEach(() => { + jest.useFakeTimers(); + + model = new (EditorJSModel as unknown as new () => MockModel)(); + eventBus = new (EventBus as unknown as new () => MockEventBus)(); + + // Intercept the model Changed listener so tests can trigger it directly + model.addEventListener = jest.fn((...args: unknown[]) => { + const [type, callback] = args as [EventType, (e: unknown) => void]; + + if (type === EventType.Changed) { + modelChangedListener = callback; + } + }); + + // Intercept the eventBus undo/redo listeners + eventBus.addEventListener = jest.fn((...args: unknown[]) => { + const [type, callback] = args as [string, () => void]; + + if (type === 'core:undo') { + undoEventListener = callback; + } else if (type === 'core:redo') { + redoEventListener = callback; + } + }); + + manager = new UndoRedoManager( + model as unknown as InstanceType, + eventBus as unknown as InstanceType, + { userId: USER_ID } as CoreConfigValidated + ); + }); + + afterEach(() => { + jest.useRealTimers(); + }); + + describe('constructor', () => { + it('should register a listener for model Changed events', () => { + expect(model.addEventListener).toHaveBeenCalledWith( + EventType.Changed, + expect.any(Function) + ); + }); + + it('should register an undo listener on the eventBus', () => { + expect(eventBus.addEventListener).toHaveBeenCalledWith( + 'core:undo', + expect.any(Function) + ); + }); + + it('should register a redo listener on the eventBus', () => { + expect(eventBus.addEventListener).toHaveBeenCalledWith( + 'core:redo', + expect.any(Function) + ); + }); + }); + + describe('.destroy()', () => { + it('should remove the undo listener from eventBus', () => { + manager.destroy(); + + expect(eventBus.removeEventListener).toHaveBeenCalledWith( + 'core:undo', + expect.any(Function) + ); + }); + + it('should remove the redo listener from eventBus', () => { + manager.destroy(); + + expect(eventBus.removeEventListener).toHaveBeenCalledWith( + 'core:redo', + expect.any(Function) + ); + }); + + it('should clear the debounce timer', () => { + const clearTimeoutSpy = jest.spyOn(globalThis, 'clearTimeout'); + + fireModelEvent(modelChangedListener, createPayload(EventAction.Added)); + + manager.destroy(); + + expect(clearTimeoutSpy).toHaveBeenCalled(); + }); + }); + + describe('EventBus integration', () => { + it('should undo when the core:undo event fires on EventBus', () => { + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: createIndex({ textRange: [0, 0] }), + data: 'a', + } + ) + ); + jest.runAllTimers(); + + undoEventListener({ defaultPrevented: false }); + + expect(model.removeData).toHaveBeenCalled(); + }); + + it('should redo when the core:redo event fires on EventBus', () => { + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: createIndex({ textRange: [0, 0] }), + data: 'a', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + redoEventListener({ defaultPrevented: false }); + + expect(model.insertData).toHaveBeenCalled(); + }); + + it('should not undo if the event was cancelled', () => { + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: createIndex({ textRange: [0, 0] }), + data: 'a', + } + ) + ); + jest.runAllTimers(); + + undoEventListener({ defaultPrevented: true }); + + expect(model.removeData).not.toHaveBeenCalled(); + }); + + it('should not redo when the event was cancelled', () => { + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: createIndex({ textRange: [0, 0] }), + data: 'a', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + redoEventListener({ defaultPrevented: true }); + + expect(model.insertData).not.toHaveBeenCalled(); + }); + }); + + describe('.undo()', () => { + it('should do nothing when the undo stack is empty', () => { + manager.undo(); + + expect(model.removeData).not.toHaveBeenCalled(); + expect(model.insertData).not.toHaveBeenCalled(); + expect(model.modifyData).not.toHaveBeenCalled(); + }); + + it('should apply the inverse of an Added event by calling model.removeData', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'hello', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledWith(USER_ID, index, 'hello'); + }); + + it('should apply the inverse of a Removed event by calling model.insertData', () => { + const index = createIndex({ textRange: [0, 5] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index, + data: 'hello', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.insertData).toHaveBeenCalledWith(USER_ID, index, 'hello'); + }); + + it('should apply the inverse of a Modified event with swapped value/previous', () => { + const index = createIndex({ + isTextIndex: false, + textRange: undefined, + }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Modified, + { + index, + data: { + value: 'new', + previous: 'old', + }, + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.modifyData).toHaveBeenCalledWith(USER_ID, index, { + value: 'old', + previous: 'new', + }); + }); + + it('should flush the current batch before undoing', () => { + const index = createIndex({ textRange: [0, 0] }); + + // Fire event but do NOT advance timers – batch is still pending + fireModelEvent(modelChangedListener, createPayload(EventAction.Added, { index, + data: 'a' })); + + // undo must flush the batch and then undo + manager.undo(); + + expect(model.removeData).toHaveBeenCalledWith(USER_ID, index, 'a'); + }); + + it('should push the inverse events to the redo stack after undoing', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent(modelChangedListener, createPayload(EventAction.Added, { index, + data: 'a' })); + jest.runAllTimers(); + + manager.undo(); + // redo must be possible now + manager.redo(); + + expect(model.insertData).toHaveBeenCalled(); + }); + + it('should undo multiple batched events in one step', () => { + const index1 = createIndex({ textRange: [0, 0] }); + const index2 = createIndex({ textRange: [1, 1] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledTimes(2); + }); + + it('should ignore model events triggered during undo application', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'a', + } + ) + ); + jest.runAllTimers(); + + // While applying, the model fires a Removed event – it should be swallowed + model.removeData = jest.fn(() => { + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index, + data: 'a', + } + ) + ); + }); + + manager.undo(); + jest.runAllTimers(); + + // The undo stack should be empty – the inner event was ignored + const insertDataMock = jest.fn(); + + model.insertData = insertDataMock; + manager.undo(); + + expect(insertDataMock).not.toHaveBeenCalled(); + }); + }); + + describe('.redo()', () => { + it('should do nothing when the redo stack is empty', () => { + manager.redo(); + + expect(model.removeData).not.toHaveBeenCalled(); + expect(model.insertData).not.toHaveBeenCalled(); + expect(model.modifyData).not.toHaveBeenCalled(); + }); + + it('should re-apply an undone Added event by calling model.insertData', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'hello', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + manager.redo(); + + expect(model.insertData).toHaveBeenCalledWith(USER_ID, index, 'hello'); + }); + + it('should re-apply an undone Removed event by calling model.removeData', () => { + const index = createIndex({ textRange: [0, 5] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index, + data: 'hello', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + manager.redo(); + + expect(model.removeData).toHaveBeenCalledWith(USER_ID, index, 'hello'); + }); + + it('should flush the current batch (from an undo triggered mid-typing) and then redo', () => { + const index = createIndex({ textRange: [0, 0] }); + + // User starts typing – batch accumulates but debounce has NOT fired yet + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'a', + } + ) + ); + + // User presses Cmd+Z immediately (undo flushes the pending batch) + manager.undo(); + + // Now redo() should flush any leftover batch (empty here) and re-apply + manager.redo(); + + expect(model.insertData).toHaveBeenCalledWith(USER_ID, index, 'a'); + }); + + it('should push the inverse events back to the undo stack so they can be undone again', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'a', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); // removeData × 1 + manager.redo(); // insertData × 1 + manager.undo(); // removeData × 2 + + expect(model.removeData).toHaveBeenCalledTimes(2); + }); + }); + + describe('event handling', () => { + it('should ignore events from other users', () => { + fireModelEvent(modelChangedListener, createPayload(EventAction.Added, { userId: OTHER_USER_ID })); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).not.toHaveBeenCalled(); + }); + + it('should clear the redo stack when a new event is received', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'a', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); // 'a' is now in redo stack + + // New user action – must clear the redo stack + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: createIndex({ + blockIndex: 1, + textRange: [0, 0], + }), + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.redo(); // redo stack should be empty + + expect(model.insertData).not.toHaveBeenCalled(); + }); + }); + + describe('batching', () => { + it('should batch consecutive Added events at sequential character positions', () => { + // 'a' at [0,0], next char must start at lastRange[1] + len('a') = 0 + 1 = 1 + const index1 = createIndex({ textRange: [0, 0] }); + const index2 = createIndex({ textRange: [1, 1] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); // both events should be undone in a single step + + expect(model.removeData).toHaveBeenCalledTimes(2); + }); + + it('should NOT batch Added events at non-sequential positions', () => { + const index1 = createIndex({ textRange: [0, 0] }); + const index2 = createIndex({ textRange: [5, 5] }); // gap – not consecutive + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); // only the second event + + expect(model.removeData).toHaveBeenCalledTimes(1); + model.removeData.mockClear(); + + manager.undo(); // now the first event + + expect(model.removeData).toHaveBeenCalledTimes(1); + }); + + it('should NOT batch a Modified event with preceding text events', () => { + const addedIndex = createIndex({ textRange: [0, 0] }); + const modifiedIndex = createIndex({ + isTextIndex: false, + textRange: undefined, + }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: addedIndex, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Modified, + { + index: modifiedIndex, + data: { + value: 'new', + previous: 'old', + }, + } + ) + ); + jest.runAllTimers(); + + // First undo targets the Modified event (on top of the stack) + manager.undo(); + + expect(model.modifyData).toHaveBeenCalledTimes(1); + expect(model.removeData).not.toHaveBeenCalled(); + }); + + it('should NOT batch events with different actions', () => { + const index1 = createIndex({ textRange: [0, 0] }); + const index2 = createIndex({ textRange: [1, 1] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + // First undo should only undo the Removed event (top of stack) + manager.undo(); + + expect(model.insertData).toHaveBeenCalledTimes(1); + expect(model.removeData).not.toHaveBeenCalled(); + }); + + it('should NOT batch events from different block indices', () => { + const index1 = createIndex({ + blockIndex: 0, + textRange: [0, 0], + }); + const index2 = createIndex({ + blockIndex: 1, + textRange: [1, 1], + }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledTimes(1); + }); + + it('should NOT batch events with different data keys', () => { + const index1 = createIndex({ + dataKey: 'text', + textRange: [0, 0], + }); + const index2 = createIndex({ + dataKey: 'caption', + textRange: [1, 1], + }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledTimes(1); + }); + + it('should NOT batch events on non-text indices', () => { + const index1 = createIndex({ + isTextIndex: false, + textRange: undefined, + }); + const index2 = createIndex({ + isTextIndex: false, + textRange: undefined, + }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledTimes(1); + }); + + it('should batch consecutive Removed events at the same start position (forward Delete)', () => { + // Pressing Delete repeatedly keeps the cursor at the same position + const index1 = createIndex({ textRange: [0, 1] }); + const index2 = createIndex({ textRange: [0, 1] }); // newRange[0] === lastRange[0] ✓ + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index: index1, + data: 'a', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index: index2, + data: 'b', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.insertData).toHaveBeenCalledTimes(2); + }); + + it('should batch consecutive Removed events at decreasing positions (Backspace)', () => { + // Pressing Backspace: newRange[1] === lastRange[0] + const index1 = createIndex({ textRange: [1, 2] }); + const index2 = createIndex({ textRange: [0, 1] }); // newRange[1]=1 === lastRange[0]=1 ✓ + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index: index1, + data: 'b', + } + ) + ); + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Removed, + { + index: index2, + data: 'a', + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + + expect(model.insertData).toHaveBeenCalledTimes(2); + }); + + it('should flush the batch to the undo stack after the debounce timeout', () => { + const index = createIndex({ textRange: [0, 0] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index, + data: 'a', + } + ) + ); + + jest.runAllTimers(); // debounce fires → batch pushed to undoStack + + manager.undo(); + + expect(model.removeData).toHaveBeenCalledWith(USER_ID, index, 'a'); + }); + + it('should restart the debounce timer on each new event', () => { + const index1 = createIndex({ textRange: [0, 0] }); + const index2 = createIndex({ textRange: [1, 1] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index1, + data: 'a', + } + ) + ); + + // Advance to 400 ms – timer has not fired yet (fires at 500 ms) + jest.advanceTimersByTime(400); + + // Second event resets the timer + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Added, + { + index: index2, + data: 'b', + } + ) + ); + + // Advance another 400 ms – total 800 ms but timer was reset at 400 ms, fires at 900 ms + jest.advanceTimersByTime(400); + + // Batch has NOT been flushed automatically yet; undo() will flush it + manager.undo(); + + // Both events must be undone in one step (same batch) + expect(model.removeData).toHaveBeenCalledTimes(2); + }); + }); +}); diff --git a/packages/core/src/components/UndoRedoManager.ts b/packages/core/src/components/UndoRedoManager.ts new file mode 100644 index 00000000..5584e221 --- /dev/null +++ b/packages/core/src/components/UndoRedoManager.ts @@ -0,0 +1,331 @@ +import 'reflect-metadata'; + +import { inject, injectable } from 'inversify'; +import { + EditorJSModel, + EventAction, + EventPayloadBase, + EventType, + type ModelEvents, + ModifiedEventData +} from '@editorjs/model'; +import { type CoreConfigValidated, CoreEventType, EventBus, RedoCoreEvent, UndoCoreEvent } from '@editorjs/sdk'; +import { TOKENS } from '../tokens.js'; + +/** + * Timeout in milliseconds to debounce grouping of consecutive operations. + * All events that occur within this window are merged into a single undo step. + */ +const DEBOUNCE_TIMEOUT = 500; + +/** + * UndoRedoManager is a core component that provides local undo/redo history + * for single-user editing. + * + * It listens to document model events directly, groups consecutive changes into + * debounced steps, and re-applies or inverts them through the Editor API when + * undo/redo is requested. + * + * This component is intentionally designed for single-user scenarios only and + * has no dependency on Operational-Transformation (OT) infrastructure. + */ +@injectable() +export class UndoRedoManager { + /** + * Editor Model instance to apply undo/redo operations back to the document. + */ + #model: EditorJSModel; + + /** + * Editor's EventBus instance + */ + #eventBus: EventBus; + + /** + * Editor configuration (provides the current user id). + */ + #config: CoreConfigValidated; + + /** + * Stack of events or batches that can be undone. + * Each entry is an ordered list of events applied in one debounce window. + */ + #undoStack: EventPayloadBase[][] = []; + + /** + * Stack of events or batches that can be redone. + */ + #redoStack: EventPayloadBase[][] = []; + + /** + * Array that stores batched events + */ + #batch: EventPayloadBase[] | null = null; + + /** + * Timer handle for the debounce flush. + */ + #debounceTimer?: ReturnType; + + /** + * Flag to properly handling events that are triggered by undo/redo operations + */ + #isApplying = false; + + /** + * Undo Core Event listener. Stored to be removed on destroy + * @param event - undo core event + */ + #undoListener = (event: UndoCoreEvent): void => { + if (event.defaultPrevented) { + return; + } + + this.undo(); + }; + + /** + * Reddo Core Event listener. Stored to be removed on destroy + * @param event - redo core event + */ + #redoListener = (event: RedoCoreEvent): void => { + if (event.defaultPrevented) { + return; + } + + this.redo(); + }; + + /** + * UndoRedoManager constructor. + * All parameters are injected through the IoC container. + * @param model - Editor's Document Model instance + * @param eventBus - Editor's EventBus instance + * @param config - Editor validated configuration + */ + constructor( + model: EditorJSModel, + eventBus: EventBus, + @inject(TOKENS.EditorConfig) config: CoreConfigValidated + ) { + this.#config = config; + this.#model = model; + this.#eventBus = eventBus; + + model.addEventListener(EventType.Changed, (e: ModelEvents) => { + this.#handleEvent(e); + }); + + eventBus.addEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); + eventBus.addEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); + } + + /** + * Undoes the last recorded group of events. + * Flushes the current debounce group first so it is included in the step. + */ + public undo(): void { + this.#flushBatch(); + + const events = this.#undoStack.pop(); + + if (events === undefined) { + return; + } + + this.#isApplying = true; + + try { + events.forEach(e => this.#apply(e)); + } finally { + this.#isApplying = false; + } + + this.#redoStack.push(events.map(this.#inverse).reverse()); + } + + /** + * Redoes the last undone group of events. + */ + public redo(): void { + this.#flushBatch(); + + const events = this.#redoStack.pop(); + + if (events === undefined) { + return; + } + + this.#isApplying = true; + + try { + events.forEach(e => this.#apply(e)); + } finally { + this.#isApplying = false; + } + + this.#undoStack.push(events.map(this.#inverse).reverse()); + } + + /** + * Releases resources held by the manager (the pending debounce timer). + */ + public destroy(): void { + clearTimeout(this.#debounceTimer); + this.#eventBus.removeEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); + this.#eventBus.removeEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); + } + + /** + * Applies inverted event back to the document + * @param event - event to apply + */ + #apply(event: EventPayloadBase): void { + switch (event.action) { + case EventAction.Added: + case EventAction.Removed: + /** + * @todo currently undo of deleting forward sets the caret in the wrong place + */ + this.#model[event.action === EventAction.Removed ? 'insertData' : 'removeData']( + event.userId, + event.index, + event.data as string + ); + break; + case EventAction.Modified: + this.#model.modifyData( + event.userId, + event.index, + { + value: (event as EventPayloadBase).data.previous, + previous: (event as EventPayloadBase).data.value, + } + ); + break; + } + } + + /** + * Inverse event action type + * @param event - event to inverse + */ + #inverse(event: EventPayloadBase): EventPayloadBase { + let newAction; + + switch (event.action) { + case EventAction.Added: + newAction = EventAction.Removed; + break; + case EventAction.Removed: + newAction = EventAction.Added; + break; + case EventAction.Modified: + newAction = EventAction.Modified; + break; + } + + return { + ...event, + action: newAction, + }; + } + + /** + * Receives a raw model event, converts it to a RecordedEvent, and adds it + * to the current debounce group if it belongs to the current user. + * @param e - model event emitted by EditorJSModel + */ + #handleEvent(e: ModelEvents): void { + if (this.#isApplying) { + return; + } + + const { detail } = e; + + /** + * Internal history manager doesn't support remote changes + */ + if (detail.userId !== this.#config.userId) { + return; + } + + this.#redoStack = []; + + if (this.#canAddToBatch(detail)) { + this.#batch!.push(detail); + } else { + this.#flushBatch(); + this.#batch = [detail]; + } + + this.#debounce(); + } + + /** + * Checks if the given event can be added to the current batch + * Events are batched only for insert and remove text operations if they: + * 1. The same type (text inserted or removed) + * 2. Have consecutive indexes + * @param payload - event to check + */ + #canAddToBatch(payload: EventPayloadBase): boolean { + const lastEvent = this.#batch?.[this.#batch?.length - 1]; + + if (lastEvent === undefined) { + return false; + } + + if (payload.action === EventAction.Modified) { + return false; + } + + if (lastEvent.action !== payload.action) { + return false; + } + + const index = payload.index; + const prevIndex = lastEvent.index; + + if (!index.isTextIndex || !prevIndex.isTextIndex) { + return false; + } + + if (index.blockIndex !== prevIndex.blockIndex || index.dataKey !== prevIndex.dataKey) { + return false; + } + + // Check consecutive text ranges + const lastRange = prevIndex.textRange!; + const newRange = index.textRange!; + + const lastPayload = lastEvent.data as string; + + if (payload.action === EventAction.Added) { + return newRange[0] === lastRange[1] + lastPayload.length; + } else { + return (newRange[1] === lastRange[0]) || (newRange[0] === lastRange[0]); + } + } + + /** + * Moves the batch to the undo stack and cancels the debounce timer. + */ + #flushBatch(): void { + clearTimeout(this.#debounceTimer); + + if (this.#batch !== null) { + this.#undoStack.push(this.#batch.reverse()); + this.#batch = null; + } + } + + /** + * Restarts the debounce timer so that the current group is flushed after + * DEBOUNCE_TIMEOUT milliseconds of inactivity. + */ + #debounce(): void { + clearTimeout(this.#debounceTimer); + this.#debounceTimer = setTimeout(() => this.#flushBatch(), DEBOUNCE_TIMEOUT); + } +} diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7c16d0c9..f617061e 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -23,6 +23,7 @@ import { BlocksManager } from './components/BlockManager.js'; import { BlockRenderer } from './components/BlockRenderer.js'; import { SelectionManager } from './components/SelectionManager.js'; import { TOKENS } from './tokens.js'; +import { UndoRedoManager } from './components/UndoRedoManager.js'; /** * If no holder is provided via config, the editor will be appended to the element with this id */ @@ -166,6 +167,7 @@ export default class Core { this.#iocContainer.get(SelectionManager); this.#iocContainer.get(BlocksManager); this.#iocContainer.get(BlockRenderer); + this.#iocContainer.get(UndoRedoManager); this.#model.initializeDocument({ blocks }); diff --git a/packages/model/src/EventBus/events/index.ts b/packages/model/src/EventBus/events/index.ts index e218bee3..23940e25 100644 --- a/packages/model/src/EventBus/events/index.ts +++ b/packages/model/src/EventBus/events/index.ts @@ -12,3 +12,4 @@ export * from './CaretManagerCaretAddedEvent.js'; export * from './CaretManagerCaretRemovedEvent.js'; export * from './DataNodeAddedEvent.js'; export * from './DataNodeRemovedEvent.js'; +export * from './BaseEvent.js'; diff --git a/packages/model/src/EventBus/index.ts b/packages/model/src/EventBus/index.ts index 8469ff8a..bbf8cb26 100644 --- a/packages/model/src/EventBus/index.ts +++ b/packages/model/src/EventBus/index.ts @@ -2,7 +2,6 @@ export * from './EventBus.js'; export * from './events/index.js'; export * from './types/EventAction.js'; export type * from './types/EventMap.js'; -export type * from './types/EventPayloadBase.js'; export type * from './types/EventTarget.d.ts'; export * from './types/EventType.js'; export type * from './types/indexing.js'; diff --git a/packages/model/src/EventBus/types/EventPayloadBase.ts b/packages/model/src/EventBus/types/EventPayloadBase.ts index 2a9e3c8c..136a8c2d 100644 --- a/packages/model/src/EventBus/types/EventPayloadBase.ts +++ b/packages/model/src/EventBus/types/EventPayloadBase.ts @@ -1,26 +1,3 @@ -import type { Index } from '../../entities/index.js'; -import type { EventAction } from './EventAction.js'; - -/** - * Common fields for all events related to the document model - */ -export interface EventPayloadBase { - /** - * The index of changed information - */ - index: Index; - - /** - * The action that was performed on the information - */ - action: Action; - - /** - * The data of the changed information - */ - data: unknown; -} - /** * Base data interface for Modified event with new and previous values */ diff --git a/packages/sdk/src/api/DocumentAPI.ts b/packages/sdk/src/api/DocumentAPI.ts index 2d783f23..5ca3c972 100644 --- a/packages/sdk/src/api/DocumentAPI.ts +++ b/packages/sdk/src/api/DocumentAPI.ts @@ -57,4 +57,14 @@ export interface DocumentAPI { * @param params - modify operation parameters */ modifyData(params: ModifyDataParams): void; + + /** + * Undoes the last change in the document + */ + undo(): void; + + /** + * Redoes the last undone change in the document + */ + redo(): void; } diff --git a/packages/sdk/src/entities/EventBus/events/core/CoreEventBase.ts b/packages/sdk/src/entities/EventBus/events/core/CoreEventBase.ts index 6fc38f84..6da27357 100644 --- a/packages/sdk/src/entities/EventBus/events/core/CoreEventBase.ts +++ b/packages/sdk/src/entities/EventBus/events/core/CoreEventBase.ts @@ -7,10 +7,12 @@ export class CoreEventBase extends CustomEvent { * CoreEventBase constructor function * @param name - type of the core event * @param payload - payload of the core event, can contain any data + * @param options - any additional event options */ - constructor(name: string, payload: Payload) { + constructor(name: string, payload: Payload, options: Omit = {}) { super(`core:${name}`, { detail: payload, + ...options, }); } } diff --git a/packages/sdk/src/entities/EventBus/events/core/RedoCoreEvent.ts b/packages/sdk/src/entities/EventBus/events/core/RedoCoreEvent.ts index b177c43b..843056b8 100644 --- a/packages/sdk/src/entities/EventBus/events/core/RedoCoreEvent.ts +++ b/packages/sdk/src/entities/EventBus/events/core/RedoCoreEvent.ts @@ -9,6 +9,6 @@ export class RedoCoreEvent extends CoreEventBase { * RedoCoreEvent constructor function */ constructor() { - super(CoreEventType.Redo, undefined); + super(CoreEventType.Redo, undefined, { cancelable: true }); } } diff --git a/packages/sdk/src/entities/EventBus/events/core/UndoCoreEvent.ts b/packages/sdk/src/entities/EventBus/events/core/UndoCoreEvent.ts index f5fd82f0..8a4bb418 100644 --- a/packages/sdk/src/entities/EventBus/events/core/UndoCoreEvent.ts +++ b/packages/sdk/src/entities/EventBus/events/core/UndoCoreEvent.ts @@ -9,6 +9,6 @@ export class UndoCoreEvent extends CoreEventBase { * UndoCoreEvent constructor function */ constructor() { - super(CoreEventType.Undo, undefined); + super(CoreEventType.Undo, undefined, { cancelable: true }); } } diff --git a/packages/ui/src/Blocks/Blocks.ts b/packages/ui/src/Blocks/Blocks.ts index 30df927a..7e6313ea 100644 --- a/packages/ui/src/Blocks/Blocks.ts +++ b/packages/ui/src/Blocks/Blocks.ts @@ -1,7 +1,9 @@ -import type { BlockAddedCoreEvent, - BlockRemovedCoreEvent, +import type { + BlockAddedCoreEvent, + BlockRemovedCoreEvent, EditorAPI, EditorjsPlugin, - EditorjsPluginParams } from '@editorjs/sdk'; + EditorjsPluginParams +} from '@editorjs/sdk'; import { CoreEventType, UiComponentType, @@ -40,12 +42,18 @@ export class BlocksUI implements EditorjsPlugin { */ #eventBus: EventBus; + /** + * Editor's API + */ + #api: EditorAPI; + /** * EditorUI constructor method * @param params - Plugin parameters */ constructor(params: EditorjsPluginParams) { this.#eventBus = params.eventBus; + this.#api = params.api; this.#blocksHolder = this.#prepareBlocksHolder(); this.#eventBus.addEventListener(`core:${CoreEventType.BlockAdded}`, (event: BlockAddedCoreEvent) => { @@ -118,14 +126,14 @@ export class BlocksUI implements EditorjsPlugin { } if (e.shiftKey) { - this.#eventBus.dispatchEvent(new Event('core:redo')); + this.#api.document.redo(); e.preventDefault(); return; } - this.#eventBus.dispatchEvent(new Event('core:undo')); + this.#api.document.undo(); e.preventDefault(); }); From 2996449b8ffd116fbedab39f40500fc2a21cdd6b Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 20 May 2026 00:02:32 +0100 Subject: [PATCH 2/5] Fix lint & tests --- .../src/CollaborationManager.ts | 11 +++++++---- .../test/mocks/createManager.ts | 12 +++++++----- .../core/src/api/DocumentAPI/DocumentAPI.spec.ts | 1 + 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 5ee4ba14..8ce156cd 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -6,12 +6,15 @@ import { TextFormattedEvent, TextRemovedEvent, TextUnformattedEvent } from '@editorjs/model'; +import type { + UndoCoreEvent, + EditorAPI, + EditorjsPlugin, + EditorjsPluginParams +} from '@editorjs/sdk'; import { CoreEventType, - type EditorAPI, - type EditorjsPlugin, - type EditorjsPluginParams, - PluginType, UndoCoreEvent + PluginType } from '@editorjs/sdk'; import { OTClient } from './client/index.js'; import { BatchedOperation } from './BatchedOperation.js'; diff --git a/packages/collaboration-manager/test/mocks/createManager.ts b/packages/collaboration-manager/test/mocks/createManager.ts index b018faf1..a3b105e8 100644 --- a/packages/collaboration-manager/test/mocks/createManager.ts +++ b/packages/collaboration-manager/test/mocks/createManager.ts @@ -2,7 +2,7 @@ import type { EditorDocumentSerialized, ModelEvents } from '@editorjs/model'; import { EventType } from '@editorjs/model'; import type { EditorJSModel } from '@editorjs/model'; import { EventBus } from '@editorjs/sdk'; -import type { CoreConfigValidated, DocumentAPI, EditorAPI, InsertRemoveDataParams, ModifyDataParams } from '@editorjs/sdk'; +import type { CoreConfigValidated, DocumentAPI, EditorAPI, InsertRemoveDataParams, ModifyDataParams, BlocksAPI } from '@editorjs/sdk'; import { CollaborationManager } from '../../src/CollaborationManager.js'; /** @@ -28,7 +28,7 @@ function createMockDocumentAPI(model: EditorJSModel): DocumentAPI { modifyData({ userId, index, data }: ModifyDataParams): void { model.modifyData(userId, index, data); }, - }; + } as DocumentAPI; } /** @@ -37,8 +37,10 @@ function createMockDocumentAPI(model: EditorJSModel): DocumentAPI { * @param model - the EditorJS model instance * @returns an object containing the manager and the eventBus used */ -export function createManager(config: CoreConfigValidated, model: EditorJSModel): { manager: CollaborationManager; - eventBus: EventBus; } { +export function createManager(config: CoreConfigValidated, model: EditorJSModel): { + manager: CollaborationManager; + eventBus: EventBus; +} { const eventBus = new EventBus(); const api: EditorAPI = { @@ -46,7 +48,7 @@ export function createManager(config: CoreConfigValidated, model: EditorJSModel) // eslint-disable-next-line @typescript-eslint/no-explicit-any blocks: { render: () => undefined, - } as any, + } as unknown as BlocksAPI, // eslint-disable-next-line @typescript-eslint/no-explicit-any selection: {} as any, // eslint-disable-next-line @typescript-eslint/no-explicit-any diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts index 08deb238..7f219970 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts @@ -5,6 +5,7 @@ import type { CoreConfigValidated, EventBus } from '@editorjs/sdk'; jest.unstable_mockModule('@editorjs/sdk', () => ({ UndoCoreEvent: jest.fn(), RedoCoreEvent: jest.fn(), + EventBus: jest.fn(), })); jest.unstable_mockModule('@editorjs/model', () => { From 6485ec892b63e5ac84ecb8116d955260f1f8f0b9 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 20 May 2026 00:05:23 +0100 Subject: [PATCH 3/5] Add explanation why undo/redo api dispatches an event instead of a direct call --- packages/core/src/api/DocumentAPI/DocumentAPI.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.ts index 2e1ba81f..c7863ba8 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.ts @@ -104,6 +104,10 @@ export class DocumentAPI implements DocumentApiInterface { * Undoes the last change in the document */ public undo(): void { + /** + * To enable Plugins to cancel the default undo/redo behavior, + * we have to dispatch event here instead of a direct call + */ this.#eventBus.dispatchEvent(new UndoCoreEvent()); } @@ -111,6 +115,10 @@ export class DocumentAPI implements DocumentApiInterface { * Redoes the last undone change in the document */ public redo(): void { + /** + * To enable Plugins to cancel the default undo/redo behavior, + * we have to dispatch event here instead of a direct call + */ this.#eventBus.dispatchEvent(new RedoCoreEvent()); } } From 3bc2098ef315f862669fc2b518a554362f64c317 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 20 May 2026 00:22:50 +0100 Subject: [PATCH 4/5] Review comments fixes --- .../src/CollaborationManager.ts | 5 ++- .../src/api/DocumentAPI/DocumentAPI.spec.ts | 28 +++++++++++-- .../src/components/UndoRedoManager.spec.ts | 42 +++++++++++++++++++ .../core/src/components/UndoRedoManager.ts | 23 +++++++--- 4 files changed, 88 insertions(+), 10 deletions(-) diff --git a/packages/collaboration-manager/src/CollaborationManager.ts b/packages/collaboration-manager/src/CollaborationManager.ts index 8ce156cd..7f9dfa10 100644 --- a/packages/collaboration-manager/src/CollaborationManager.ts +++ b/packages/collaboration-manager/src/CollaborationManager.ts @@ -10,7 +10,8 @@ import type { UndoCoreEvent, EditorAPI, EditorjsPlugin, - EditorjsPluginParams + EditorjsPluginParams, + RedoCoreEvent } from '@editorjs/sdk'; import { CoreEventType, @@ -105,7 +106,7 @@ export class CollaborationManager implements EditorjsPlugin { this.undo(); }; - const onRedo = (e: UndoCoreEvent): void => { + const onRedo = (e: RedoCoreEvent): void => { e.preventDefault(); this.redo(); diff --git a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts index 7f219970..77945ef7 100644 --- a/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts +++ b/packages/core/src/api/DocumentAPI/DocumentAPI.spec.ts @@ -3,8 +3,12 @@ import { beforeEach, describe, expect, jest } from '@jest/globals'; import type { CoreConfigValidated, EventBus } from '@editorjs/sdk'; jest.unstable_mockModule('@editorjs/sdk', () => ({ - UndoCoreEvent: jest.fn(), - RedoCoreEvent: jest.fn(), + UndoCoreEvent: class UndoCoreEvent { + public name = 'undo'; + }, + RedoCoreEvent: class RedoCoreEvent { + public name = 'redo'; + }, EventBus: jest.fn(), })); @@ -28,10 +32,12 @@ describe('DocumentAPI', () => { // @ts-expect-error - mock object, don't need to pass any arguments const model = new EditorJSModel(); + const dispatchEvent = jest.fn(); + const documentAPI = new DocumentAPI( model, {} as unknown as CoreConfigValidated, - { dispatchEvent: jest.fn() } as unknown as EventBus + { dispatchEvent } as unknown as EventBus ); beforeEach(() => { @@ -62,4 +68,20 @@ describe('DocumentAPI', () => { expect(data).toEqual(mockedSerializedModel); }); }); + + describe('.undo()', () => { + it('should dispatch an undo core event', () => { + documentAPI.undo(); + + expect(dispatchEvent).toBeCalledWith(expect.objectContaining({ name: 'undo' })); + }); + }); + + describe('.redo()', () => { + it('should dispatch an redo core event', () => { + documentAPI.redo(); + + expect(dispatchEvent).toBeCalledWith(expect.objectContaining({ name: 'redo' })); + }); + }); }); diff --git a/packages/core/src/components/UndoRedoManager.spec.ts b/packages/core/src/components/UndoRedoManager.spec.ts index fa69fcf0..d4d08d22 100644 --- a/packages/core/src/components/UndoRedoManager.spec.ts +++ b/packages/core/src/components/UndoRedoManager.spec.ts @@ -11,6 +11,7 @@ const OTHER_USER_ID = 'other-user'; jest.unstable_mockModule('@editorjs/model', () => { const EditorJSModel = jest.fn(() => ({ addEventListener: jest.fn(), + removeEventListener: jest.fn(), insertData: jest.fn(), removeData: jest.fn(), modifyData: jest.fn(), @@ -51,6 +52,7 @@ const { UndoRedoManager } = await import('./UndoRedoManager.js'); type MockModel = { addEventListener: jest.Mock; + removeEventListener: jest.Mock; insertData: jest.Mock; removeData: jest.Mock; modifyData: jest.Mock; @@ -203,6 +205,15 @@ describe('UndoRedoManager', () => { expect(clearTimeoutSpy).toHaveBeenCalled(); }); + + it('should remove the model updates listener', () => { + manager.destroy(); + + expect(model.removeEventListener).toHaveBeenCalledWith( + 'model:changed', + expect.any(Function) + ); + }); }); describe('EventBus integration', () => { @@ -510,6 +521,37 @@ describe('UndoRedoManager', () => { expect(model.removeData).toHaveBeenCalledWith(USER_ID, index, 'hello'); }); + it('should re-apply an undone Modified event by calling model.modifyData with the same values', () => { + const index = createIndex({ textRange: [0, 5] }); + + fireModelEvent( + modelChangedListener, + createPayload( + EventAction.Modified, + { + index, + data: { + value: 'new', + previous: 'old', + }, + } + ) + ); + jest.runAllTimers(); + + manager.undo(); + manager.redo(); + + expect(model.modifyData).toHaveBeenCalledWith( + USER_ID, + index, + { + value: 'new', + previous: 'old', + } + ); + }); + it('should flush the current batch (from an undo triggered mid-typing) and then redo', () => { const index = createIndex({ textRange: [0, 0] }); diff --git a/packages/core/src/components/UndoRedoManager.ts b/packages/core/src/components/UndoRedoManager.ts index 5584e221..97ba90a8 100644 --- a/packages/core/src/components/UndoRedoManager.ts +++ b/packages/core/src/components/UndoRedoManager.ts @@ -85,7 +85,7 @@ export class UndoRedoManager { }; /** - * Reddo Core Event listener. Stored to be removed on destroy + * Redo Core Event listener. Stored to be removed on destroy * @param event - redo core event */ #redoListener = (event: RedoCoreEvent): void => { @@ -96,6 +96,14 @@ export class UndoRedoManager { this.redo(); }; + /** + * Model updates listener. Stored to be removed on destroy + * @param e - model event + */ + #modelUpdatesListener = (e: ModelEvents): void => { + this.#handleEvent(e); + }; + /** * UndoRedoManager constructor. * All parameters are injected through the IoC container. @@ -112,9 +120,7 @@ export class UndoRedoManager { this.#model = model; this.#eventBus = eventBus; - model.addEventListener(EventType.Changed, (e: ModelEvents) => { - this.#handleEvent(e); - }); + model.addEventListener(EventType.Changed, this.#modelUpdatesListener); eventBus.addEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); eventBus.addEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); @@ -141,7 +147,7 @@ export class UndoRedoManager { this.#isApplying = false; } - this.#redoStack.push(events.map(this.#inverse).reverse()); + this.#redoStack.push(events.map(e => this.#inverse(e)).reverse()); } /** @@ -174,6 +180,7 @@ export class UndoRedoManager { clearTimeout(this.#debounceTimer); this.#eventBus.removeEventListener(`core:${CoreEventType.Undo}`, this.#undoListener); this.#eventBus.removeEventListener(`core:${CoreEventType.Redo}`, this.#redoListener); + this.#model.removeEventListener(EventType.Changed, this.#modelUpdatesListener); } /** @@ -212,6 +219,7 @@ export class UndoRedoManager { */ #inverse(event: EventPayloadBase): EventPayloadBase { let newAction; + let newPayload = event.data; switch (event.action) { case EventAction.Added: @@ -222,12 +230,17 @@ export class UndoRedoManager { break; case EventAction.Modified: newAction = EventAction.Modified; + newPayload = { + previous: (event.data as ModifiedEventData).value, + value: (event.data as ModifiedEventData).previous, + } as ModifiedEventData; break; } return { ...event, action: newAction, + data: newPayload, }; } From 969637ae81a9a37850c6dabf196082c9ab8d0777 Mon Sep 17 00:00:00 2001 From: gohabereg Date: Wed, 20 May 2026 22:08:35 +0100 Subject: [PATCH 5/5] Review comments resolved --- .../collaboration-manager/test/mocks/createManager.ts | 1 - packages/core/src/components/UndoRedoManager.ts | 10 +++++----- .../model/src/EventBus/events/PropertyModifiedEvent.ts | 2 +- .../model/src/EventBus/events/TuneModifiedEvent.ts | 2 +- .../model/src/EventBus/events/ValueModifiedEvent.ts | 2 +- packages/model/src/EventBus/types/EventPayloadBase.ts | 7 ------- 6 files changed, 8 insertions(+), 16 deletions(-) delete mode 100644 packages/model/src/EventBus/types/EventPayloadBase.ts diff --git a/packages/collaboration-manager/test/mocks/createManager.ts b/packages/collaboration-manager/test/mocks/createManager.ts index a3b105e8..7487a221 100644 --- a/packages/collaboration-manager/test/mocks/createManager.ts +++ b/packages/collaboration-manager/test/mocks/createManager.ts @@ -45,7 +45,6 @@ export function createManager(config: CoreConfigValidated, model: EditorJSModel) const api: EditorAPI = { document: createMockDocumentAPI(model), - // eslint-disable-next-line @typescript-eslint/no-explicit-any blocks: { render: () => undefined, } as unknown as BlocksAPI, diff --git a/packages/core/src/components/UndoRedoManager.ts b/packages/core/src/components/UndoRedoManager.ts index 97ba90a8..4c0e24ee 100644 --- a/packages/core/src/components/UndoRedoManager.ts +++ b/packages/core/src/components/UndoRedoManager.ts @@ -131,7 +131,7 @@ export class UndoRedoManager { * Flushes the current debounce group first so it is included in the step. */ public undo(): void { - this.#flushBatch(); + this.#putBatchToUndo(); const events = this.#undoStack.pop(); @@ -154,7 +154,7 @@ export class UndoRedoManager { * Redoes the last undone group of events. */ public redo(): void { - this.#flushBatch(); + this.#putBatchToUndo(); const events = this.#redoStack.pop(); @@ -268,7 +268,7 @@ export class UndoRedoManager { if (this.#canAddToBatch(detail)) { this.#batch!.push(detail); } else { - this.#flushBatch(); + this.#putBatchToUndo(); this.#batch = [detail]; } @@ -324,7 +324,7 @@ export class UndoRedoManager { /** * Moves the batch to the undo stack and cancels the debounce timer. */ - #flushBatch(): void { + #putBatchToUndo(): void { clearTimeout(this.#debounceTimer); if (this.#batch !== null) { @@ -339,6 +339,6 @@ export class UndoRedoManager { */ #debounce(): void { clearTimeout(this.#debounceTimer); - this.#debounceTimer = setTimeout(() => this.#flushBatch(), DEBOUNCE_TIMEOUT); + this.#debounceTimer = setTimeout(() => this.#putBatchToUndo(), DEBOUNCE_TIMEOUT); } } diff --git a/packages/model/src/EventBus/events/PropertyModifiedEvent.ts b/packages/model/src/EventBus/events/PropertyModifiedEvent.ts index 0b6e156f..aa285649 100644 --- a/packages/model/src/EventBus/events/PropertyModifiedEvent.ts +++ b/packages/model/src/EventBus/events/PropertyModifiedEvent.ts @@ -1,5 +1,5 @@ import type { Index } from '../../entities/Index/index.js'; -import type { ModifiedEventData } from '../types/EventPayloadBase.js'; +import type { ModifiedEventData } from './BaseEvent.js'; import { EventAction } from '../types/EventAction.js'; import { BaseDocumentEvent } from './BaseEvent.js'; diff --git a/packages/model/src/EventBus/events/TuneModifiedEvent.ts b/packages/model/src/EventBus/events/TuneModifiedEvent.ts index 449b71e2..6eff85b1 100644 --- a/packages/model/src/EventBus/events/TuneModifiedEvent.ts +++ b/packages/model/src/EventBus/events/TuneModifiedEvent.ts @@ -1,5 +1,5 @@ import type { Index } from '../../entities/Index/index.js'; -import type { ModifiedEventData } from '../types/EventPayloadBase.js'; +import type { ModifiedEventData } from './BaseEvent.js'; import { EventAction } from '../types/EventAction.js'; import { BaseDocumentEvent } from './BaseEvent.js'; diff --git a/packages/model/src/EventBus/events/ValueModifiedEvent.ts b/packages/model/src/EventBus/events/ValueModifiedEvent.ts index cb7c4609..a0cf8027 100644 --- a/packages/model/src/EventBus/events/ValueModifiedEvent.ts +++ b/packages/model/src/EventBus/events/ValueModifiedEvent.ts @@ -1,5 +1,5 @@ import type { Index } from '../../entities/Index/index.js'; -import type { ModifiedEventData } from '../types/EventPayloadBase.js'; +import type { ModifiedEventData } from './BaseEvent.js'; import { EventAction } from '../types/EventAction.js'; import { BaseDocumentEvent } from './BaseEvent.js'; diff --git a/packages/model/src/EventBus/types/EventPayloadBase.ts b/packages/model/src/EventBus/types/EventPayloadBase.ts deleted file mode 100644 index 136a8c2d..00000000 --- a/packages/model/src/EventBus/types/EventPayloadBase.ts +++ /dev/null @@ -1,7 +0,0 @@ -/** - * Base data interface for Modified event with new and previous values - */ -export interface ModifiedEventData { - value: T; - previous: T; -}