From b149e414ed204b214a03b4293a9e05e357df5a5c Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 12:12:36 +0100 Subject: [PATCH 1/4] feat(track-changes): block-level structural tracked changes for tables Adds a new StructuralTrackChanges extension that gives block-level (table) add/remove operations the same review pipeline as inline tracked changes - same data-track-change rendering, same accept/reject command surface, plus block-level entry registration in the comments-plugin. Consumer pattern: compute structural hunks via computeStructuralDiff (or construct them yourself), dispatch via editor.commands.setStructuralDiff (hunks). The extension stamps a trackChange PM attribute on each affected tableRow. Rendering is handled by the painter (reads row.trackedChange, stamps data-track-change* on cells). Accept/reject operates on PM attrs via getBlockTrackedChanges + applyRowTrackedChangeResolution. Pipeline pieces: - Shared blockTrackedChangeAttrSpec helper + TableRow.addAttributes spread - TableRow.trackedChange contract field - pm-adapter populates TableRow.trackedChange from PM attr - painter renderTableRow stamps data-track-change on cells - CSS rules for [data-track-change="insert" | "delete"] - getBlockTrackedChanges walks the doc for block-level entries - applyRowTrackedChangeResolution handles accept/reject by id - acceptTrackedChangeById / rejectTrackedChangeById extended to route inline marks, row attrs, or operationId - trackedTransaction passes through ReplaceSteps that already carry block-level metadata so inline marks don't double-track - comments-plugin walks block-level entries alongside inline marks Existing Diffing extension, compareDocuments, replayDifferences, and inline TrackChanges are untouched. The new extension is opt-in via editorExtensions: [StructuralTrackChanges] (not in starter extensions). Tests: ~30 new unit tests across the touched modules, plus an end-to-end test using a real .docx fixture pair that exercises compute -> set -> accept-all and compute -> set -> reject-all. Out of scope (separate follow-ups): - DOCX round-trip of block-level tracked changes - Block-level entries surfacing in the default SuperDoc bubble (requires mirroring inline's commentsUpdate event payload pipeline) - Cell-level / column-level tracking - A combined acceptAllChanges that batches inline + structural into one transaction (currently two sequential commands; two undo steps) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../layout-engine/contracts/src/index.test.ts | 16 +- packages/layout-engine/contracts/src/index.ts | 12 ++ .../dom/src/table/renderTableRow.test.ts | 51 ++++++ .../painters/dom/src/table/renderTableRow.ts | 15 ++ .../pm-adapter/src/converters/table.test.ts | 89 +++++++++++ .../pm-adapter/src/converters/table.ts | 34 +++- .../v1/assets/styles/elements/prosemirror.css | 33 ++++ .../comment/blockTrackedChangeBubble.test.js | 53 +++++++ .../v1/extensions/comment/comments-plugin.js | 82 +++++++++- .../structural-track-changes/index.js | 2 + .../structural-track-changes-e2e.test.js | 69 ++++++++ .../structural-track-changes.js | 147 ++++++++++++++++++ .../structural-track-changes.test.js | 65 ++++++++ .../applyHunks.js | 86 ++++++++++ .../applyHunks.test.js | 86 ++++++++++ .../computeStructuralDiff.js | 77 +++++++++ .../computeStructuralDiff.test.js | 97 ++++++++++++ .../v1/extensions/table-row/table-row.js | 3 + .../track-changes/blockTrackedChangeAttr.js | 26 ++++ .../blockTrackedChangeAttr.test.js | 38 +++++ .../extensions/track-changes/track-changes.js | 67 ++++++-- .../acceptRejectRowTrackedChange.js | 70 +++++++++ .../acceptRejectRowTrackedChange.test.js | 101 ++++++++++++ .../blockTrackedChangePassthrough.test.js | 71 +++++++++ .../getBlockTrackedChanges.js | 37 +++++ .../getBlockTrackedChanges.test.js | 50 ++++++ .../trackChangesHelpers/trackedTransaction.js | 33 ++++ packages/super-editor/src/editors/v1/index.js | 4 + .../data/diffing/diff_after_table_remove.docx | Bin 0 -> 6757 bytes .../diffing/diff_before_table_remove.docx | Bin 0 -> 7235 bytes 30 files changed, 1501 insertions(+), 13 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/index.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes-e2e.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/blockTrackedChangePassthrough.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js create mode 100644 packages/super-editor/src/editors/v1/tests/data/diffing/diff_after_table_remove.docx create mode 100644 packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx diff --git a/packages/layout-engine/contracts/src/index.test.ts b/packages/layout-engine/contracts/src/index.test.ts index 51286c7172..9496677ac9 100644 --- a/packages/layout-engine/contracts/src/index.test.ts +++ b/packages/layout-engine/contracts/src/index.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import { cloneColumnLayout, extractHeaderFooterSpace, normalizeColumnLayout, widthsEqual } from './index.js'; -import type { FlowBlock, Layout } from './index.js'; +import type { FlowBlock, Layout, TableRow, TrackedChangeMeta } from './index.js'; describe('contracts', () => { it('accepts a basic FlowBlock structure', () => { @@ -85,4 +85,18 @@ describe('contracts', () => { }); expect(normalizeColumnLayout({ count: 2, gap: 24 }, 624).widths).toEqual([300, 300]); }); + + it('TrackedChangeMeta accepts an optional operationId', () => { + const meta: TrackedChangeMeta = { kind: 'delete', id: 'r1', operationId: 'op-table-1' }; + expect(meta.operationId).toBe('op-table-1'); + }); + + it('TableRow accepts an optional trackedChange field carrying row-level metadata', () => { + const row: TableRow = { + id: 'row-1', + cells: [], + trackedChange: { kind: 'insert', id: 'r1', operationId: 'op-table-1' }, + }; + expect(row.trackedChange?.kind).toBe('insert'); + }); }); diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 4ee7c1d164..65227ec21f 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -236,6 +236,12 @@ export type RunMark = { export type TrackedChangeMeta = { kind: TrackedChangeKind; id: string; + /** + * Optional grouping key. Tracked changes sharing a non-empty `operationId` + * are resolved together — e.g. every row of a deleted table shares one + * `operationId` and the review surface collapses them into one entry. + */ + operationId?: string; /** * Internal story key identifying which content story owns this tracked * change (`'body'`, `'hf:part:…'`, `'fn:…'`, `'en:…'`). @@ -697,6 +703,12 @@ export type TableRow = { cells: TableCell[]; attrs?: TableRowAttrs; sourceAnchor?: SourceAnchor; + /** + * Row-level tracked-change metadata. Populated by pm-adapter from the + * ProseMirror `trackChange` node attribute. The painter reads this and + * stamps `data-track-change*` on the cell DOM elements. + */ + trackedChange?: TrackedChangeMeta; }; export type TableBlock = { diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts index 4c1661ced4..e68693a3ed 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.test.ts @@ -345,4 +345,55 @@ describe('renderTableRow', () => { expect(calls[1].x).toBe(4); }); }); + + describe('block-level tracked-change data attributes on cells', () => { + it('stamps data-track-change attrs on every cell when row.trackedChange is set', () => { + const deps = createDeps({ + row: { + id: 'row-1', + cells: [ + { id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }, + { id: 'cell-2', blocks: [{ kind: 'paragraph', id: 'p2', runs: [] }] }, + ], + trackedChange: { kind: 'delete', id: 'row-rev-1', operationId: 'op-1' }, + }, + rowMeasure: { + height: 20, + cells: [ + { width: 100, height: 20, gridColumnStart: 0, colSpan: 1, rowSpan: 1 }, + { width: 100, height: 20, gridColumnStart: 1, colSpan: 1, rowSpan: 1 }, + ], + }, + columnWidths: [100, 100], + }); + renderTableRow(deps as never); + + const cells = container.querySelectorAll('[data-track-change]'); + expect(cells.length).toBe(2); + cells.forEach((el) => { + expect(el.getAttribute('data-track-change')).toBe('delete'); + expect(el.getAttribute('data-track-change-id')).toBe('row-rev-1'); + expect(el.getAttribute('data-track-change-operation')).toBe('op-1'); + }); + }); + + it('does not stamp data-track-change when row.trackedChange is undefined', () => { + renderTableRow(createDeps() as never); + expect(container.querySelector('[data-track-change]')).toBeNull(); + }); + + it('omits operation attr when operationId is missing', () => { + const deps = createDeps({ + row: { + id: 'row-1', + cells: [{ id: 'cell-1', blocks: [{ kind: 'paragraph', id: 'p1', runs: [] }] }], + trackedChange: { kind: 'insert', id: 'row-rev-2' }, + }, + }); + renderTableRow(deps as never); + const cell = container.querySelector('[data-track-change]'); + expect(cell?.getAttribute('data-track-change')).toBe('insert'); + expect(cell?.hasAttribute('data-track-change-operation')).toBe(false); + }); + }); }); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index 764f5873d2..d06cd5c6c0 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -434,6 +434,21 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { cellWidth: computedCellWidth > 0 ? computedCellWidth : undefined, }); + // Row-level tracked change → stamp data attrs on each cell element. + // The painter renders cells as absolutely-positioned divs (no ), so + // every cell of a tracked row carries the attribute. CSS targets the cell + // divs and produces the visible red/green strip across the row. + const trackedChange = row?.trackedChange; + if (trackedChange) { + cellElement.setAttribute('data-track-change', trackedChange.kind); + if (trackedChange.id) { + cellElement.setAttribute('data-track-change-id', trackedChange.id); + } + if (trackedChange.operationId) { + cellElement.setAttribute('data-track-change-operation', trackedChange.operationId); + } + } + container.appendChild(cellElement); } }; diff --git a/packages/layout-engine/pm-adapter/src/converters/table.test.ts b/packages/layout-engine/pm-adapter/src/converters/table.test.ts index 86f7e24496..b52ac44331 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.test.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.test.ts @@ -2683,4 +2683,93 @@ describe('tableCellNodeToBlock — SD-2516: documentPartObject children', () => expect(result?.attrs?.tableDirectionContext?.parentSection).toBe(customSectionContext); }); }); + + describe('block-level tracked-change passthrough on TableRow', () => { + const mockBlockIdGenerator: BlockIdGenerator = vi.fn((kind) => `test-${kind}`); + const mockPositionMap: PositionMap = new Map(); + const mockParagraphConverter = vi.fn((params) => [ + { + kind: 'paragraph', + id: 'p1', + runs: [{ text: params.para.content?.[0]?.text || 'text', fontFamily: 'Arial', fontSize: 12 }], + } as ParagraphBlock, + ]); + + const tableWithRowTrackChange = (trackChange: object | null): PMNode => ({ + type: 'table', + content: [ + { + type: 'tableRow', + attrs: { trackChange }, + content: [ + { + type: 'tableCell', + content: [{ type: 'paragraph', content: [{ type: 'text', text: 'Cell 1' }] }], + }, + ], + }, + ], + }); + + it('populates TableRow.trackedChange when PM row has trackChange attr', () => { + const node = tableWithRowTrackChange({ + kind: 'delete', + id: 'row-rev-1', + operationId: 'op-table-1', + author: 'Alice', + date: '2026-05-15T00:00:00Z', + }); + const result = tableNodeToBlock( + node, + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + expect(result.rows[0].trackedChange).toMatchObject({ + kind: 'delete', + id: 'row-rev-1', + operationId: 'op-table-1', + author: 'Alice', + date: '2026-05-15T00:00:00Z', + }); + }); + + it('omits TableRow.trackedChange when PM row has no trackChange attr', () => { + const result = tableNodeToBlock( + tableWithRowTrackChange(null), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + expect(result.rows[0].trackedChange).toBeUndefined(); + }); + + it('rejects unknown kinds defensively', () => { + const result = tableNodeToBlock( + tableWithRowTrackChange({ kind: 'bogus', id: 'r1' }), + mockBlockIdGenerator, + mockPositionMap, + 'Arial', + 16, + undefined, + undefined, + undefined, + undefined, + mockParagraphConverter, + ) as TableBlock; + expect(result.rows[0].trackedChange).toBeUndefined(); + }); + }); }); diff --git a/packages/layout-engine/pm-adapter/src/converters/table.ts b/packages/layout-engine/pm-adapter/src/converters/table.ts index ca0aab0f0e..29b4a51250 100644 --- a/packages/layout-engine/pm-adapter/src/converters/table.ts +++ b/packages/layout-engine/pm-adapter/src/converters/table.ts @@ -731,12 +731,44 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { // The PM table-row extension has both cantSplit as a top-level attr AND within tableRowProperties // For layout engine, we only need to read from tableRowProperties.cantSplit - return { + const row: TableRow = { id: context.nextBlockId(`row-${rowIndex}`), cells, attrs, sourceAnchor: sourceAnchorFromNode(rowNode), }; + + // Row-level tracked change (block-level diff replay sets this on the PM node). + // See packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js + const trackChangeAttr = rowNode.attrs?.trackChange as + | { + kind?: 'insert' | 'delete'; + id?: string; + operationId?: string; + author?: string; + authorEmail?: string; + date?: string; + storyKey?: string; + } + | null + | undefined; + if ( + trackChangeAttr && + (trackChangeAttr.kind === 'insert' || trackChangeAttr.kind === 'delete') && + trackChangeAttr.id + ) { + row.trackedChange = { + kind: trackChangeAttr.kind, + id: trackChangeAttr.id, + operationId: trackChangeAttr.operationId, + author: trackChangeAttr.author, + authorEmail: trackChangeAttr.authorEmail, + date: trackChangeAttr.date, + storyKey: trackChangeAttr.storyKey, + }; + } + + return row; }; /** diff --git a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css index 090dbbbff2..6112f125ae 100644 --- a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css +++ b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css @@ -285,6 +285,39 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html } /* Track changes - end */ +/* + * Block-level tracked changes (row granularity for v1). + * + * The `data-track-change` attribute is stamped in two places: + * 1. ProseMirror's contenteditable via the schema's renderDOM + * (hidden in layout-engine mode but kept consistent). + * 2. The DomPainter's per-cell
elements (no exists in layout + * mode; cells are absolutely positioned). Every cell of a tracked row + * carries the attribute → a continuous red/green strip across the row. + * + * Selectors are intentionally minimal so the rule applies wherever the painter + * places the cell — across re-renders and table-fragment boundaries. + */ +[data-track-change='delete'] { + background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)) !important; + outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47) !important; + outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px) !important; +} + +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] td, +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th, +.superdoc-table-fragment [data-track-change='delete'] { + text-decoration: line-through; + text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); +} + +[data-track-change='insert'] { + background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)) !important; + outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d) !important; + outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px) !important; +} +/* Block-level tracked changes - end */ + /* Collaboration cursors */ .sd-editor-scoped .ProseMirror > .ProseMirror-yjs-cursor:first-child { margin-top: 16px; diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js new file mode 100644 index 0000000000..a453195e06 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -0,0 +1,53 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { CommentsPluginKey } from './comments-plugin.js'; + +describe('comments-plugin — block-level tracked-change registration', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const makeRow = (kind, id, operationId) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + return schema.nodes.tableRow.create({ trackChange: { kind, id, operationId } }, [cell]); + }; + + const installDoc = (children) => { + const doc = schema.nodes.doc.create(null, children); + const newState = EditorState.create({ schema, doc, plugins: editor.state.plugins }); + editor.setState(newState); + }; + + it('registers one entry per operationId, collapsing multi-row deletes', () => { + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [ + makeRow('delete', 'r1', 'OP1'), + makeRow('delete', 'r2', 'OP1'), + makeRow('delete', 'r3', 'OP1'), + ]); + installDoc([table]); + const pluginState = CommentsPluginKey.getState(editor.state); + const tracked = pluginState?.trackedChanges ?? {}; + expect(Object.keys(tracked)).toContain('OP1'); + }); + + it('registers individual rows when they have no operationId', () => { + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('insert', 'r1', undefined)]); + installDoc([table]); + const pluginState = CommentsPluginKey.getState(editor.state); + const tracked = pluginState?.trackedChanges ?? {}; + expect(Object.keys(tracked)).toContain('r1'); + }); + + it('does not register entries for block-level when there are no tracked rows', () => { + installDoc([schema.nodes.paragraph.create(null, schema.text('plain'))]); + const pluginState = CommentsPluginKey.getState(editor.state); + const tracked = pluginState?.trackedChanges ?? {}; + // None of the block-level test ids should be present + expect(tracked['OP1']).toBeUndefined(); + expect(tracked['r1']).toBeUndefined(); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index 52a7dabd1a..5ef21e2a10 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -17,6 +17,7 @@ import { v4 as uuidv4 } from 'uuid'; import { TrackDeleteMarkName, TrackFormatMarkName, TrackInsertMarkName } from '../track-changes/constants.js'; import { TrackChangesBasePluginKey } from '../track-changes/plugins/index.js'; import { getTrackChanges } from '../track-changes/trackChangesHelpers/getTrackChanges.js'; +import { getBlockTrackedChanges } from '../track-changes/trackChangesHelpers/getBlockTrackedChanges.js'; import { normalizeCommentEventPayload, updatePosition } from './helpers/index.js'; const TRACK_CHANGE_MARKS = [TrackInsertMarkName, TrackDeleteMarkName, TrackFormatMarkName]; @@ -452,8 +453,28 @@ export const CommentsPlugin = Extension.create({ key: CommentsPluginKey, state: { - init() { + init(_config, editorState) { const highlightColors = editor.options.comments?.highlightColors || {}; + // Pick up any block-level tracked changes already present in the + // initial doc (e.g. when a docx is loaded with pre-marked rows or + // when EditorState is reconstructed via setState during tests). + const trackedChanges = {}; + if (editorState?.doc) { + const blockEntries = getBlockTrackedChanges(editorState); + const seenOperations = new Set(); + for (const entry of blockEntries) { + const key = entry.operationId || entry.id; + if (entry.operationId) { + if (seenOperations.has(entry.operationId)) continue; + seenOperations.add(entry.operationId); + } + trackedChanges[key] = { + type: entry.kind === 'insert' ? 'trackedInsert' : 'trackedDelete', + range: { from: entry.from, to: entry.to }, + isBlockLevel: true, + }; + } + } return { activeThreadId: null, externalColor: highlightColors.external ?? '#B1124B', @@ -461,7 +482,7 @@ export const CommentsPlugin = Extension.create({ decorations: DecorationSet.empty, allCommentPositions: {}, allCommentIds: [], - trackedChanges: {}, + trackedChanges, }; }, @@ -513,6 +534,33 @@ export const CommentsPlugin = Extension.create({ ); } + // Block-level tracked changes don't fire TrackChangesBasePluginKey meta + // (they're applied as PM node attrs via setNodeMarkup). On any doc + // change, refresh the block-level entries in trackedChanges. Rows that + // share an operationId collapse into a single entry so a multi-row + // delete surfaces as one bubble. + if (tr.docChanged || trackedChangeMeta) { + const blockEntries = getBlockTrackedChanges(newEditorState); + const blockTracked = {}; + const seenOperations = new Set(); + for (const entry of blockEntries) { + const key = entry.operationId || entry.id; + if (entry.operationId) { + if (seenOperations.has(entry.operationId)) continue; + seenOperations.add(entry.operationId); + } + blockTracked[key] = { + type: entry.kind === 'insert' ? 'trackedInsert' : 'trackedDelete', + range: { from: entry.from, to: entry.to }, + isBlockLevel: true, + }; + } + // Merge: keep existing inline entries, add block-level entries. + // Block-level keys (operationId / row-id) shouldn't collide with + // inline ones (mark-id), but if they ever do, inline wins. + pluginState.trackedChanges = { ...blockTracked, ...pluginState.trackedChanges }; + } + // Check for changes in the actively selected comment if (!tr.docChanged && tr.selectionSet) { const { selection } = tr; @@ -695,6 +743,36 @@ export const CommentsPlugin = Extension.create({ decorations.push(trackedChangeDeco); } } + + // Block-level tracked changes (e.g. tableRow with trackChange attr). + // Surface the same bubble UX inline tracked changes have. Rows that + // share an operationId collapse to one entry (one bubble per + // operation, not per row). + const blockTrackChange = node?.attrs?.trackChange; + if ( + blockTrackChange && + (blockTrackChange.kind === 'insert' || blockTrackChange.kind === 'delete') && + blockTrackChange.id + ) { + const threadId = blockTrackChange.operationId || blockTrackChange.id; + if (!onlyActiveThreadChanged) { + let currentBounds; + try { + currentBounds = view.coordsAtPos(pos); + } catch { + currentBounds = null; + } + if (currentBounds && !allCommentPositions[threadId]) { + updatePosition({ + allCommentPositions, + threadId, + pos, + currentBounds, + node, + }); + } + } + } }); const decorationSet = DecorationSet.create(doc, decorations); diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/index.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/index.js new file mode 100644 index 0000000000..1896f0432c --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/index.js @@ -0,0 +1,2 @@ +export { StructuralTrackChanges, computeStructuralDiff } from './structural-track-changes.js'; +export { applyHunks } from './structuralTrackChangesHelpers/applyHunks.js'; diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes-e2e.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes-e2e.test.js new file mode 100644 index 0000000000..7e04f64d4d --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes-e2e.test.js @@ -0,0 +1,69 @@ +import { describe, it, expect } from 'vitest'; +import { Editor } from '@core/Editor.js'; +import { getStarterExtensions } from '@extensions/index.js'; +import { getTestDataAsBuffer } from '@tests/export/export-helpers/export-helpers.js'; +import { StructuralTrackChanges, computeStructuralDiff } from './structural-track-changes.js'; +import { getBlockTrackedChanges } from '../track-changes/trackChangesHelpers/getBlockTrackedChanges.js'; + +const editorFromFixture = async (name, user) => { + const buffer = await getTestDataAsBuffer(`diffing/${name}`); + const [docx, media, mediaFiles, fonts] = await Editor.loadXmlData(buffer, true); + return new Editor({ + isHeadless: true, + extensions: [...getStarterExtensions(), StructuralTrackChanges], + documentId: `test-${name}`, + content: docx, + mode: 'docx', + media, + mediaFiles, + fonts, + annotations: true, + user, + }); +}; + +describe('StructuralTrackChanges — end-to-end with real docx fixtures', () => { + it('compute → set → accept-all on a table-removal pair removes the table from the doc', async () => { + const testUser = { name: 'Tester', email: 'test@example.com' }; + const baseEditor = await editorFromFixture('diff_before_table_remove.docx', testUser); + const afterEditor = await editorFromFixture('diff_after_table_remove.docx'); + try { + const hunks = computeStructuralDiff(baseEditor.state.doc, afterEditor.state.doc); + expect(hunks.length).toBeGreaterThan(0); + expect(baseEditor.commands.setStructuralDiff(hunks)).toBe(true); + expect(getBlockTrackedChanges(baseEditor.state).length).toBeGreaterThan(0); + expect(baseEditor.commands.acceptAllStructuralChanges()).toBe(true); + expect(getBlockTrackedChanges(baseEditor.state).length).toBe(0); + let hasTable = false; + baseEditor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(false); + } finally { + baseEditor.destroy?.(); + afterEditor.destroy?.(); + } + }); + + it('compute → set → reject-all on a table-removal pair restores the base shape', async () => { + const testUser = { name: 'Tester', email: 'test@example.com' }; + const baseEditor = await editorFromFixture('diff_before_table_remove.docx', testUser); + const afterEditor = await editorFromFixture('diff_after_table_remove.docx'); + try { + const beforeText = baseEditor.state.doc.textContent; + const hunks = computeStructuralDiff(baseEditor.state.doc, afterEditor.state.doc); + baseEditor.commands.setStructuralDiff(hunks); + baseEditor.commands.rejectAllStructuralChanges(); + expect(getBlockTrackedChanges(baseEditor.state).length).toBe(0); + let hasTable = false; + baseEditor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(true); + expect(baseEditor.state.doc.textContent).toBe(beforeText); + } finally { + baseEditor.destroy?.(); + afterEditor.destroy?.(); + } + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js new file mode 100644 index 0000000000..c6c13c02f4 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js @@ -0,0 +1,147 @@ +import { Extension } from '@core/Extension.js'; +import { applyHunks } from './structuralTrackChangesHelpers/applyHunks.js'; +import { computeStructuralDiff } from './structuralTrackChangesHelpers/computeStructuralDiff.js'; +import { getBlockTrackedChanges } from '../track-changes/trackChangesHelpers/getBlockTrackedChanges.js'; +import { applyRowTrackedChangeResolution } from '../track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js'; + +/** + * StructuralTrackChanges — block-level (table) tracked-change extension. + * + * Pattern: consumer computes `StructuralHunk[]` (e.g. via `computeStructuralDiff`) + * and dispatches via `editor.commands.setStructuralDiff(hunks)`. The extension + * stamps a `trackChange` PM attribute on each affected `tableRow`. Rendering is + * handled by the painter (reads `data-track-change` data attrs from row.trackedChange). + * The review bubble appears via the comments-plugin's block-level walk. + * + * Accept/reject is identity-agnostic — operates on PM attrs, not an in-memory + * hunk store. The same `acceptTrackedChangeById` entry point inline tracked + * changes use is extended in `track-changes.js` to also handle row-attr ids. + * + * Not registered in `getStarterExtensions()`; consumers opt in via + * `editorExtensions: [StructuralTrackChanges]`. + */ +export const StructuralTrackChanges = Extension.create({ + name: 'structuralTrackChanges', + + addCommands() { + return { + setStructuralDiff: + (hunks) => + ({ state, dispatch }) => { + if (!Array.isArray(hunks) || hunks.length === 0) return true; + const tr = state.tr; + tr.setMeta('addToHistory', false); + const { applied } = applyHunks({ tr, state, hunks }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + acceptStructuralChange: + (id) => + ({ state, dispatch }) => { + if (!id) return false; + const entries = getBlockTrackedChanges(state); + const ids = entries.filter((e) => e.id === id || e.operationId === id).map((e) => e.id); + if (ids.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ tr, state, ids, decision: 'accept' }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + rejectStructuralChange: + (id) => + ({ state, dispatch }) => { + if (!id) return false; + const entries = getBlockTrackedChanges(state); + const ids = entries.filter((e) => e.id === id || e.operationId === id).map((e) => e.id); + if (ids.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ tr, state, ids, decision: 'reject' }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + acceptAllStructuralChanges: + () => + ({ state, dispatch }) => { + const entries = getBlockTrackedChanges(state); + if (entries.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr, + state, + ids: entries.map((e) => e.id), + decision: 'accept', + }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + rejectAllStructuralChanges: + () => + ({ state, dispatch }) => { + const entries = getBlockTrackedChanges(state); + if (entries.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr, + state, + ids: entries.map((e) => e.id), + decision: 'reject', + }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + acceptTrackedChangeOperation: + (operationId) => + ({ state, dispatch }) => { + if (!operationId) return false; + const entries = getBlockTrackedChanges(state).filter((e) => e.operationId === operationId); + if (entries.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr, + state, + ids: entries.map((e) => e.id), + decision: 'accept', + }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + + rejectTrackedChangeOperation: + (operationId) => + ({ state, dispatch }) => { + if (!operationId) return false; + const entries = getBlockTrackedChanges(state).filter((e) => e.operationId === operationId); + if (entries.length === 0) return false; + const tr = state.tr; + tr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr, + state, + ids: entries.map((e) => e.id), + decision: 'reject', + }); + if (applied === 0) return false; + if (dispatch) dispatch(tr); + return true; + }, + }; + }, +}); + +export { computeStructuralDiff }; diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js new file mode 100644 index 0000000000..c675cddcb6 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { getStarterExtensions } from '@extensions/index.js'; +import { StructuralTrackChanges } from './structural-track-changes.js'; +import { getBlockTrackedChanges } from '../track-changes/trackChangesHelpers/getBlockTrackedChanges.js'; + +describe('StructuralTrackChanges extension', () => { + let editor; + beforeEach(() => { + ({ editor } = initTestEditor({ + mode: 'text', + content: '

hi

after

', + extensions: [...getStarterExtensions(), StructuralTrackChanges], + })); + }); + afterEach(() => editor?.destroy()); + + it('setStructuralDiff stamps trackChange on every row of the matching table', () => { + const table = editor.state.doc.firstChild; + const ok = editor.commands.setStructuralDiff([ + { kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }, + ]); + expect(ok).toBe(true); + const block = getBlockTrackedChanges(editor.state); + expect(block.length).toBeGreaterThanOrEqual(1); + block.forEach((b) => { + expect(b.kind).toBe('delete'); + }); + }); + + it('acceptStructuralChange removes the table when a remove hunk is staged and accepted', () => { + const table = editor.state.doc.firstChild; + editor.commands.setStructuralDiff([{ kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }]); + expect(editor.commands.acceptStructuralChange('t1')).toBe(true); + let hasTable = false; + editor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(false); + }); + + it('rejectStructuralChange clears the trackChange attrs and keeps the table', () => { + const table = editor.state.doc.firstChild; + editor.commands.setStructuralDiff([{ kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }]); + expect(editor.commands.rejectStructuralChange('t1')).toBe(true); + expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); + let hasTable = false; + editor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(true); + }); + + it('acceptAllStructuralChanges resolves every staged hunk', () => { + const table = editor.state.doc.firstChild; + editor.commands.setStructuralDiff([{ kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }]); + expect(editor.commands.acceptAllStructuralChanges()).toBe(true); + expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); + }); + + it('is NOT registered in default starter extensions (opt-in only)', () => { + const names = getStarterExtensions().map((e) => e.name); + expect(names).not.toContain('structuralTrackChanges'); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.js new file mode 100644 index 0000000000..24d6c4e8e0 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.js @@ -0,0 +1,86 @@ +import { Fragment } from 'prosemirror-model'; +import { v4 as uuidv4 } from 'uuid'; + +/** + * Apply structural hunks to a transaction as row-level tracked-change PM attrs. + * + * For each hunk: + * remove → walk to each row of the existing table at basePos, + * setNodeMarkup with trackChange.delete (one shared operationId). + * insert → reconstruct the proposal table with every row pre-marked + * trackChange.insert (one shared operationId), insert at + * anchorBasePos. + * + * Sets inputType meta so the trackedTransaction interceptor's existing + * notAllowedMeta short-circuit skips this transaction (the rows already + * carry block-level metadata; inline wrapping would double-track). + * + * @param {{ + * tr: import('prosemirror-state').Transaction, + * state: import('prosemirror-state').EditorState, + * hunks: object[], + * }} args + * @returns {{ applied: number, warnings: string[] }} + */ +export const applyHunks = ({ tr, state, hunks }) => { + const warnings = []; + let applied = 0; + tr.setMeta('inputType', 'acceptReject'); + + for (const hunk of hunks) { + if (!hunk || !hunk.changeId) continue; + const operationId = hunk.changeId; + + if (hunk.kind === 'remove') { + const livePos = tr.mapping.map(hunk.basePos); + const tableNode = tr.doc.nodeAt(livePos); + if (!tableNode || tableNode.type.name !== 'table') { + warnings.push(`No table at pos ${hunk.basePos} for remove hunk ${hunk.changeId}`); + continue; + } + let rowPos = livePos + 1; + for (let i = 0; i < tableNode.childCount; i += 1) { + const row = tableNode.child(i); + if (row.type.name === 'tableRow') { + tr.setNodeMarkup(rowPos, null, { + ...row.attrs, + trackChange: { kind: 'delete', id: uuidv4(), operationId }, + }); + } + rowPos += row.nodeSize; + } + applied += 1; + continue; + } + + if (hunk.kind === 'insert') { + const proposal = hunk.proposalNode; + if (!proposal) { + warnings.push(`Missing proposalNode for insert hunk ${hunk.changeId}`); + continue; + } + const trackedRows = []; + proposal.content.forEach((row) => { + if (row.type.name !== 'tableRow') { + trackedRows.push(row); + return; + } + trackedRows.push( + row.type.create( + { ...row.attrs, trackChange: { kind: 'insert', id: uuidv4(), operationId } }, + row.content, + row.marks, + ), + ); + }); + const trackedTable = proposal.type.create(proposal.attrs, Fragment.fromArray(trackedRows), proposal.marks); + tr.insert(tr.mapping.map(hunk.anchorBasePos), trackedTable); + applied += 1; + continue; + } + + warnings.push(`Unsupported hunk kind: ${hunk.kind}`); + } + + return { applied, warnings }; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.test.js new file mode 100644 index 0000000000..cb205af91a --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.test.js @@ -0,0 +1,86 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { applyHunks } from './applyHunks.js'; + +describe('applyHunks', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const buildTable = (id, rowCount = 2) => { + const rows = []; + for (let i = 0; i < rowCount; i += 1) { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text(`r${i}`))); + rows.push(schema.nodes.tableRow.create(null, [cell])); + } + return schema.nodes.table.create({ sdBlockId: id }, rows); + }; + + it('stamps trackChange.delete on every row of an existing table for a remove hunk', () => { + const table = buildTable('tbl-del', 3); + const doc = schema.nodes.doc.create(null, [table]); + const state = EditorState.create({ schema, doc }); + const tr = state.tr; + const result = applyHunks({ + tr, + state, + hunks: [{ kind: 'remove', changeId: 'tbl-del', basePos: 0, baseNodeSize: table.nodeSize }], + }); + expect(result.applied).toBe(1); + const nextDoc = state.apply(tr).doc; + const next = nextDoc.firstChild; + expect(next.type.name).toBe('table'); + expect(next.childCount).toBe(3); + const opIds = new Set(); + next.forEach((row) => { + expect(row.attrs.trackChange?.kind).toBe('delete'); + opIds.add(row.attrs.trackChange?.operationId); + }); + expect(opIds.size).toBe(1); + }); + + it('inserts a proposal table with trackChange.insert on every row at the anchor position', () => { + const proposalTable = buildTable('tbl-add', 2); + const doc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('before'))]); + const state = EditorState.create({ schema, doc }); + const tr = state.tr; + const insertPos = doc.content.size; + const result = applyHunks({ + tr, + state, + hunks: [{ kind: 'insert', changeId: 'tbl-add', proposalNode: proposalTable, anchorBasePos: insertPos }], + }); + expect(result.applied).toBe(1); + const nextDoc = state.apply(tr).doc; + const insertedTable = nextDoc.child(nextDoc.childCount - 1); + expect(insertedTable.type.name).toBe('table'); + insertedTable.forEach((row) => { + expect(row.attrs.trackChange?.kind).toBe('insert'); + }); + }); + + it('rows in one hunk share an operationId but have unique row ids', () => { + const table = buildTable('tbl-ids', 4); + const doc = schema.nodes.doc.create(null, [table]); + const state = EditorState.create({ schema, doc }); + const tr = state.tr; + applyHunks({ + tr, + state, + hunks: [{ kind: 'remove', changeId: 'tbl-ids', basePos: 0, baseNodeSize: table.nodeSize }], + }); + const ids = new Set(); + let operationId; + state.apply(tr).doc.firstChild.forEach((row) => { + ids.add(row.attrs.trackChange.id); + operationId = row.attrs.trackChange.operationId; + }); + expect(ids.size).toBe(4); + expect(typeof operationId).toBe('string'); + expect(operationId.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js new file mode 100644 index 0000000000..9da9c19846 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js @@ -0,0 +1,77 @@ +/** + * Compute structural hunks describing whole-block add/removes between two docs. + * + * Walks both docs at depth 1, matches blocks by `identityKey` (default: + * `sdBlockId`). For each base block absent from proposal: emit + * `{ kind: 'remove' }`. For each proposal block (of allowed type) absent from + * base: emit `{ kind: 'insert' }`. + * + * Identity matching is intentionally simple. AI integrations whose proposal + * docx has different `sdBlockId`s across imports can provide a structural + * fingerprint via `opts.identityKey`. + * + * @param {import('prosemirror-model').Node} baseDoc + * @param {import('prosemirror-model').Node} proposalDoc + * @param {{ blockTypes?: readonly string[], identityKey?: (n: any) => string | null }} [opts] + * @returns {Array<{ + * kind: 'remove' | 'insert', + * changeId: string, + * basePos?: number, + * baseNodeSize?: number, + * proposalNode?: import('prosemirror-model').Node, + * anchorBasePos?: number, + * }>} + */ +const DEFAULT_BLOCK_TYPES = Object.freeze(['table']); +const defaultIdentityKey = (node) => node?.attrs?.sdBlockId ?? null; + +export const computeStructuralDiff = (baseDoc, proposalDoc, opts = {}) => { + const blockTypes = opts.blockTypes ?? DEFAULT_BLOCK_TYPES; + const identityKey = opts.identityKey ?? defaultIdentityKey; + const blockTypeSet = new Set(blockTypes); + + const collectTopLevel = (doc) => { + const entries = []; + doc.forEach((node, offset) => { + const id = identityKey(node); + if (id != null) entries.push({ id, node, pos: offset, end: offset + node.nodeSize }); + }); + return entries; + }; + + const baseEntries = collectTopLevel(baseDoc); + const proposalEntries = collectTopLevel(proposalDoc); + const baseById = new Map(baseEntries.map((e) => [e.id, e])); + const proposalIds = new Set(proposalEntries.map((e) => e.id)); + + const hunks = []; + + for (const entry of baseEntries) { + if (!proposalIds.has(entry.id) && blockTypeSet.has(entry.node.type.name)) { + hunks.push({ + kind: 'remove', + changeId: entry.id, + basePos: entry.pos, + baseNodeSize: entry.node.nodeSize, + }); + } + } + + let lastSharedBaseEnd = null; + for (const entry of proposalEntries) { + const sharedInBase = baseById.get(entry.id); + if (sharedInBase) { + lastSharedBaseEnd = sharedInBase.end; + continue; + } + if (!blockTypeSet.has(entry.node.type.name)) continue; + hunks.push({ + kind: 'insert', + changeId: entry.id, + proposalNode: entry.node, + anchorBasePos: lastSharedBaseEnd ?? 0, + }); + } + + return hunks; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js new file mode 100644 index 0000000000..ce5057d6a3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js @@ -0,0 +1,97 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { computeStructuralDiff } from './computeStructuralDiff.js'; + +const idAttr = (id) => ({ sdBlockId: id }); + +describe('computeStructuralDiff', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const makeDoc = (blocks) => + schema.nodes.doc.create( + null, + blocks.map((b) => b(schema)), + ); + const p = (id, text) => (schema) => schema.nodes.paragraph.create(idAttr(id), schema.text(text)); + const table = (id) => (schema) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + const row = schema.nodes.tableRow.create(null, [cell]); + return schema.nodes.table.create(idAttr(id), [row]); + }; + + it('emits a remove hunk for a base table absent from proposal', () => { + const base = makeDoc([p('p1', 'a'), table('t1'), p('p2', 'b')]); + const proposal = makeDoc([p('p1', 'a'), p('p2', 'b')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'remove', changeId: 't1' }); + }); + + it('emits an insert hunk for a proposal table absent from base', () => { + const base = makeDoc([p('p1', 'a')]); + const proposal = makeDoc([p('p1', 'a'), table('t1')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'insert', changeId: 't1' }); + }); + + it('emits no hunks when both sides have matching tables', () => { + const base = makeDoc([table('t1')]); + const proposal = makeDoc([table('t1')]); + expect(computeStructuralDiff(base, proposal)).toHaveLength(0); + }); + + it('anchor for insert is end of last shared base block', () => { + const base = makeDoc([p('p1', 'first'), p('p2', 'second')]); + const proposal = makeDoc([p('p1', 'first'), table('t-new'), p('p2', 'second')]); + const hunks = computeStructuralDiff(base, proposal); + const insert = hunks.find((h) => h.kind === 'insert'); + expect(insert).toBeDefined(); + expect(insert.anchorBasePos).toBe(base.child(0).nodeSize); + }); + + it('insert with no shared base block anchors at 0', () => { + const base = makeDoc([p('p1', 'only')]); + const proposal = makeDoc([table('t-new')]); + const hunks = computeStructuralDiff(base, proposal); + const insert = hunks.find((h) => h.kind === 'insert'); + expect(insert?.anchorBasePos).toBe(0); + }); + + it('blockTypes filter (default ["table"]) restricts insert events', () => { + const base = makeDoc([]); + const proposal = makeDoc([table('t1'), p('p1', 'extra')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks.filter((h) => h.kind === 'insert')).toHaveLength(1); + expect(hunks.find((h) => h.kind === 'insert')?.changeId).toBe('t1'); + }); + + it('custom identityKey override', () => { + const fingerprint = (node) => `${node.type.name}:${node.textContent}`; + const noId = (text) => (schema) => schema.nodes.paragraph.create(null, schema.text(text)); + const base = makeDoc([noId('A'), noId('B')]); + const proposal = makeDoc([noId('A')]); + const hunks = computeStructuralDiff(base, proposal, { + identityKey: fingerprint, + blockTypes: ['paragraph'], + }); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'remove', changeId: 'paragraph:B' }); + }); + + it('does not descend into matched tables (nested table inside matched outer is not double-counted)', () => { + const outerWithCellPara = (id) => (schema) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('inner'))); + const row = schema.nodes.tableRow.create(null, [cell]); + return schema.nodes.table.create(idAttr(id), [row]); + }; + const base = makeDoc([outerWithCellPara('t-outer')]); + const proposal = makeDoc([outerWithCellPara('t-outer')]); + expect(computeStructuralDiff(base, proposal)).toHaveLength(0); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js index 623a5ca113..c6d575d098 100644 --- a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js +++ b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js @@ -3,6 +3,7 @@ import { Node } from '@core/Node.js'; import { Attribute } from '@core/Attribute.js'; import { pixelsToTwips } from '@core/super-converter/helpers.js'; import { parseRowHeight } from './helpers/parseRowHeight.js'; +import { blockTrackedChangeAttrSpec } from '../track-changes/blockTrackedChangeAttr.js'; /** * @typedef {Object} CnfStyle @@ -98,6 +99,8 @@ export const TableRow = Node.create({ }, }, + ...blockTrackedChangeAttrSpec, + rowHeight: { renderDOM({ rowHeight }) { if (!rowHeight) return {}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js new file mode 100644 index 0000000000..a893c77bf2 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -0,0 +1,26 @@ +/** + * Shared ProseMirror addAttributes-compatible spec for block-level tracked + * changes. Apply to block-unit nodes (TableRow for v1; TableCell, Image, + * PageBreak in future) by spreading into addAttributes(). + * + * Value shape: + * { kind: 'insert' | 'delete', id: string, operationId?: string } | null + */ +export const blockTrackedChangeAttrSpec = { + trackChange: { + default: null, + parseDOM: (el) => { + const kind = el.getAttribute('data-track-change'); + if (kind !== 'insert' && kind !== 'delete') return null; + return { + kind, + id: el.getAttribute('data-track-change-id') ?? null, + operationId: el.getAttribute('data-track-change-operation') ?? undefined, + }; + }, + renderDOM: (attrs) => { + const tc = attrs?.trackChange; + return tc ? { 'data-track-change': tc.kind } : {}; + }, + }, +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js new file mode 100644 index 0000000000..629fa391b7 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js @@ -0,0 +1,38 @@ +import { describe, it, expect } from 'vitest'; +import { blockTrackedChangeAttrSpec } from './blockTrackedChangeAttr.js'; + +describe('blockTrackedChangeAttrSpec', () => { + it('declares a trackChange attribute with null default', () => { + expect(blockTrackedChangeAttrSpec.trackChange).toBeDefined(); + expect(blockTrackedChangeAttrSpec.trackChange.default).toBeNull(); + }); + + it('parseDOM reads data-track-change* into an attribute value', () => { + const el = document.createElement('tr'); + el.setAttribute('data-track-change', 'delete'); + el.setAttribute('data-track-change-id', 'row-abc'); + el.setAttribute('data-track-change-operation', 'op-xyz'); + const parsed = blockTrackedChangeAttrSpec.trackChange.parseDOM(el); + expect(parsed).toMatchObject({ kind: 'delete', id: 'row-abc', operationId: 'op-xyz' }); + }); + + it('parseDOM returns null when data-track-change is missing', () => { + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(document.createElement('tr'))).toBeNull(); + }); + + it('parseDOM rejects unknown kinds', () => { + const el = document.createElement('tr'); + el.setAttribute('data-track-change', 'foo'); + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toBeNull(); + }); + + it('renderDOM emits data-track-change when set; nothing when null', () => { + expect( + blockTrackedChangeAttrSpec.trackChange.renderDOM({ + trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' }, + }), + ).toEqual({ 'data-track-change': 'insert' }); + expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: null })).toEqual({}); + expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({})).toEqual({}); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js index f33f7040bd..59b69fe41e 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js @@ -11,6 +11,8 @@ import { collectTrackedChanges, isTrackedChangeActionAllowed } from './permissio import { CommentsPluginKey, createOrUpdateTrackedChangeComment } from '../comment/comments-plugin.js'; import { findMarkInRangeBySnapshot } from './trackChangesHelpers/markSnapshotHelpers.js'; import { hasExpandedSelection } from '@utils/selectionUtils.js'; +import { getBlockTrackedChanges } from './trackChangesHelpers/getBlockTrackedChanges.js'; +import { applyRowTrackedChangeResolution } from './trackChangesHelpers/acceptRejectRowTrackedChange.js'; /** * Reads the `replacements` mode from editor.options.trackedChanges. @@ -188,16 +190,40 @@ export const TrackChanges = Extension.create({ acceptTrackedChangeById: (id) => - ({ state, tr, commands }) => { + ({ state, tr, dispatch, commands }) => { const toResolve = getChangesByIdToResolve(state, id) || []; - return toResolve - .map(({ from, to }) => { - let mappedFrom = tr.mapping.map(from); - let mappedTo = tr.mapping.map(to); - return commands.acceptTrackedChangesBetween(mappedFrom, mappedTo); - }) - .every((result) => result); + if (toResolve.length > 0) { + return toResolve + .map(({ from, to }) => { + let mappedFrom = tr.mapping.map(from); + let mappedTo = tr.mapping.map(to); + return commands.acceptTrackedChangesBetween(mappedFrom, mappedTo); + }) + .every((result) => result); + } + + // No inline-mark match — try row-attr (by row id, then by operationId). + const blockEntries = getBlockTrackedChanges(state); + const byRowId = blockEntries.find((e) => e.id === id); + if (byRowId) { + const blockTr = state.tr; + blockTr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr: blockTr, + state, + ids: [id], + decision: 'accept', + }); + if (applied === 0) return false; + if (dispatch) dispatch(blockTr); + return true; + } + const byOperation = blockEntries.filter((e) => e.operationId === id); + if (byOperation.length > 0 && commands.acceptTrackedChangeOperation) { + return commands.acceptTrackedChangeOperation(id); + } + return false; }, acceptAllTrackedChanges: @@ -210,9 +236,32 @@ export const TrackChanges = Extension.create({ rejectTrackedChangeById: (id) => - ({ state, tr, commands }) => { + ({ state, tr, dispatch, commands }) => { const toReject = getChangesByIdToResolve(state, id) || []; + if (toReject.length === 0) { + const blockEntries = getBlockTrackedChanges(state); + const byRowId = blockEntries.find((e) => e.id === id); + if (byRowId) { + const blockTr = state.tr; + blockTr.setMeta('inputType', 'acceptReject'); + const { applied } = applyRowTrackedChangeResolution({ + tr: blockTr, + state, + ids: [id], + decision: 'reject', + }); + if (applied === 0) return false; + if (dispatch) dispatch(blockTr); + return true; + } + const byOperation = blockEntries.filter((e) => e.operationId === id); + if (byOperation.length > 0 && commands.rejectTrackedChangeOperation) { + return commands.rejectTrackedChangeOperation(id); + } + return false; + } + return toReject .map(({ from, to }) => { let mappedFrom = tr.mapping.map(from); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js new file mode 100644 index 0000000000..f39c059a0a --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js @@ -0,0 +1,70 @@ +import { getBlockTrackedChanges } from './getBlockTrackedChanges.js'; + +/** + * Apply accept or reject to one or more row-level tracked changes by id. + * + * Resolution rules: + * insert + accept → strip attr (row stays in the doc) + * insert + reject → delete the row + * delete + accept → delete the row + * delete + reject → strip attr (row stays in the doc) + * + * If a deletion leaves the parent table with zero rows, the parent table is + * also removed (matches OOXML / Google Docs row-delete semantics). + * + * Steps are appended to the provided transaction in descending position order + * so deletions don't invalidate earlier positions in the same batch. + * + * @param {{ + * tr: import('prosemirror-state').Transaction, + * state: import('prosemirror-state').EditorState, + * ids: string[], + * decision: 'accept' | 'reject', + * }} args + * @returns {{ applied: number, notFound: string[] }} + */ +export const applyRowTrackedChangeResolution = ({ tr, state, ids, decision }) => { + const all = getBlockTrackedChanges(state); + const wanted = new Set(ids); + const entries = all.filter((e) => wanted.has(e.id)); + const foundIds = new Set(entries.map((e) => e.id)); + const notFound = ids.filter((id) => !foundIds.has(id)); + if (!entries.length) return { applied: 0, notFound }; + + // Descending position order so earlier deletions don't invalidate later + // positions within this single transaction. + const sorted = [...entries].sort((a, b) => b.from - a.from); + + let applied = 0; + for (const entry of sorted) { + const stripAttr = + (entry.kind === 'insert' && decision === 'accept') || (entry.kind === 'delete' && decision === 'reject'); + + const livePos = tr.mapping.map(entry.from); + const liveNode = tr.doc.nodeAt(livePos); + if (!liveNode || liveNode.type.name !== entry.nodeType) continue; + + if (stripAttr) { + tr.setNodeMarkup(livePos, null, { ...liveNode.attrs, trackChange: null }); + applied += 1; + continue; + } + + // Delete the row. If it's the only row in its parent table, delete the + // table too (PM's tableRow+ content schema would otherwise reject an + // empty table). + const $pos = tr.doc.resolve(livePos); + const parent = $pos.node($pos.depth); + const parentPos = $pos.before($pos.depth); + const isLastRowInTable = parent.type.name === 'table' && parent.childCount === 1; + + if (isLastRowInTable) { + tr.delete(parentPos, parentPos + parent.nodeSize); + } else { + tr.delete(livePos, livePos + liveNode.nodeSize); + } + applied += 1; + } + + return { applied, notFound }; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.test.js new file mode 100644 index 0000000000..cd149c62ef --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.test.js @@ -0,0 +1,101 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { applyRowTrackedChangeResolution } from './acceptRejectRowTrackedChange.js'; + +describe('applyRowTrackedChangeResolution', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const makeRow = (kind, id, operationId) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + return schema.nodes.tableRow.create({ trackChange: { kind, id, operationId } }, [cell]); + }; + const stateWith = (children) => EditorState.create({ schema, doc: schema.nodes.doc.create(null, children) }); + const tableWith = (rows) => schema.nodes.table.create({ sdBlockId: 't1' }, rows); + + it('accept on a deleted row deletes it (and the parent table when last row)', () => { + const state = stateWith([ + schema.nodes.paragraph.create(null, schema.text('before')), + tableWith([makeRow('delete', 'r1', 'op1')]), + schema.nodes.paragraph.create(null, schema.text('after')), + ]); + const tr = state.tr; + const result = applyRowTrackedChangeResolution({ tr, state, ids: ['r1'], decision: 'accept' }); + expect(result.applied).toBe(1); + const nextDoc = state.apply(tr).doc; + let hasTable = false; + nextDoc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(false); + expect(nextDoc.textContent).toBe('beforeafter'); + }); + + it('accept on a deleted row leaves other rows when more remain', () => { + const otherCell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('keep'))); + const state = stateWith([ + tableWith([makeRow('delete', 'r1', 'op1'), schema.nodes.tableRow.create(null, [otherCell])]), + ]); + const tr = state.tr; + applyRowTrackedChangeResolution({ tr, state, ids: ['r1'], decision: 'accept' }); + const table = state.apply(tr).doc.firstChild; + expect(table.type.name).toBe('table'); + expect(table.childCount).toBe(1); + expect(table.firstChild.attrs.trackChange).toBeFalsy(); + }); + + it('accept on inserted row strips the attr; row stays', () => { + const state = stateWith([tableWith([makeRow('insert', 'r1', 'op1')])]); + const tr = state.tr; + applyRowTrackedChangeResolution({ tr, state, ids: ['r1'], decision: 'accept' }); + const row = state.apply(tr).doc.firstChild.firstChild; + expect(row.attrs.trackChange).toBeFalsy(); + }); + + it('reject on a deleted row strips the attr; row stays', () => { + const state = stateWith([tableWith([makeRow('delete', 'r1', 'op1')])]); + const tr = state.tr; + applyRowTrackedChangeResolution({ tr, state, ids: ['r1'], decision: 'reject' }); + const row = state.apply(tr).doc.firstChild.firstChild; + expect(row.attrs.trackChange).toBeFalsy(); + }); + + it('reject on an inserted row deletes it (and the table if last row)', () => { + const state = stateWith([ + schema.nodes.paragraph.create(null, schema.text('a')), + tableWith([makeRow('insert', 'r1', 'op1')]), + schema.nodes.paragraph.create(null, schema.text('b')), + ]); + const tr = state.tr; + applyRowTrackedChangeResolution({ tr, state, ids: ['r1'], decision: 'reject' }); + let hasTable = false; + state.apply(tr).doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(false); + }); + + it('returns notFound for unknown ids', () => { + const state = stateWith([tableWith([makeRow('delete', 'r1', 'op1')])]); + const tr = state.tr; + const result = applyRowTrackedChangeResolution({ tr, state, ids: ['r1', 'missing'], decision: 'reject' }); + expect(result.applied).toBe(1); + expect(result.notFound).toEqual(['missing']); + }); + + it('processes multiple deletes back-to-front so positions stay valid', () => { + const state = stateWith([ + tableWith([makeRow('delete', 'r1', 'op1'), makeRow('delete', 'r2', 'op1'), makeRow('delete', 'r3', 'op1')]), + schema.nodes.paragraph.create(null, schema.text('after')), + ]); + const tr = state.tr; + const result = applyRowTrackedChangeResolution({ tr, state, ids: ['r1', 'r2', 'r3'], decision: 'accept' }); + expect(result.applied).toBe(3); + expect(state.apply(tr).doc.textContent).toBe('after'); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/blockTrackedChangePassthrough.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/blockTrackedChangePassthrough.test.js new file mode 100644 index 0000000000..6b0b433340 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/blockTrackedChangePassthrough.test.js @@ -0,0 +1,71 @@ +import { beforeEach, afterEach, describe, expect, it, vi } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { Slice, Fragment } from 'prosemirror-model'; +import { ReplaceStep } from 'prosemirror-transform'; +import { trackedTransaction } from './trackedTransaction.js'; +import { TrackInsertMarkName, TrackDeleteMarkName } from '../constants.js'; +import { initTestEditor } from '@tests/helpers/helpers.js'; + +describe('trackedTransaction — pass-through for pre-marked block content', () => { + let editor, schema, basePlugins; + const user = { name: 'Block Tester', email: 'block@example.com' }; + + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + basePlugins = editor.state.plugins; + }); + afterEach(() => { + vi.restoreAllMocks(); + editor?.destroy(); + editor = null; + }); + + const createState = (doc) => EditorState.create({ schema, doc, plugins: basePlugins }); + + const buildPreMarkedTable = (operationId, rowCount = 2) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('hello'))); + const rows = []; + for (let i = 0; i < rowCount; i += 1) { + rows.push(schema.nodes.tableRow.create({ trackChange: { kind: 'insert', id: `row-${i}`, operationId } }, [cell])); + } + return schema.nodes.table.create({ sdBlockId: 'tbl-1' }, rows); + }; + + const inlineTrackedMarksOnText = (doc) => { + let count = 0; + doc.descendants((node) => { + if (!node.isText) return; + node.marks.forEach((m) => { + if (m.type.name === TrackInsertMarkName || m.type.name === TrackDeleteMarkName) count += 1; + }); + }); + return count; + }; + + it('ReplaceStep inserting a pre-marked table passes through without inline-mark wrapping', () => { + const doc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('before'))]); + const state = createState(doc); + const tr = state.tr; + const preMarkedTable = buildPreMarkedTable('op-1', 2); + const insertPos = state.doc.content.size; + tr.step(new ReplaceStep(insertPos, insertPos, new Slice(Fragment.from(preMarkedTable), 0, 0))); + const result = trackedTransaction({ tr, state, user }); + const nextDoc = state.apply(result).doc; + const insertedTable = nextDoc.child(nextDoc.childCount - 1); + expect(insertedTable.type.name).toBe('table'); + insertedTable.forEach((row) => { + expect(row.attrs.trackChange?.kind).toBe('insert'); + }); + expect(inlineTrackedMarksOnText(nextDoc)).toBe(0); + }); + + it('normal text insertion still gets wrapped with inline trackInsert marks (regression guard)', () => { + const doc = schema.nodes.doc.create(null, [schema.nodes.paragraph.create(null, schema.text('hello world'))]); + const state = createState(doc); + const tr = state.tr.insertText('X', 1); + const result = trackedTransaction({ tr, state, user }); + const nextDoc = state.apply(result).doc; + expect(inlineTrackedMarksOnText(nextDoc)).toBeGreaterThan(0); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js new file mode 100644 index 0000000000..99ccd18451 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js @@ -0,0 +1,37 @@ +/** + * Walk the doc collecting block-level tracked changes — PM nodes whose + * `trackChange` attribute is set. Mirror of `getTrackChanges` (mark-based). + * + * Returned entries: `{ id, kind, operationId?, nodeType, from, to, node }`. + * + * @param {{ doc: import('prosemirror-model').Node } | import('prosemirror-state').EditorState | null | undefined} stateOrDoc + * @returns {Array<{ + * id: string, + * kind: 'insert' | 'delete', + * operationId?: string, + * nodeType: string, + * from: number, + * to: number, + * node: import('prosemirror-model').Node, + * }>} + */ +export const getBlockTrackedChanges = (stateOrDoc) => { + const doc = stateOrDoc?.doc ?? stateOrDoc; + const out = []; + if (!doc || typeof doc.descendants !== 'function') return out; + doc.descendants((node, pos) => { + const tc = node?.attrs?.trackChange; + if (tc && tc.id && (tc.kind === 'insert' || tc.kind === 'delete')) { + out.push({ + id: tc.id, + kind: tc.kind, + operationId: tc.operationId, + nodeType: node.type.name, + from: pos, + to: pos + node.nodeSize, + node, + }); + } + }); + return out; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js new file mode 100644 index 0000000000..a512a5fa6a --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js @@ -0,0 +1,50 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { getBlockTrackedChanges } from './getBlockTrackedChanges.js'; + +describe('getBlockTrackedChanges', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const trackedRow = (kind, id, operationId) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + return schema.nodes.tableRow.create({ trackChange: { kind, id, operationId } }, [cell]); + }; + const tableWith = (rows) => schema.nodes.table.create({ sdBlockId: 't1' }, rows); + + it('returns an entry per node with trackChange attr set', () => { + const doc = schema.nodes.doc.create(null, [ + tableWith([trackedRow('delete', 'r1', 'op1'), trackedRow('delete', 'r2', 'op1')]), + ]); + const items = getBlockTrackedChanges({ doc }); + expect(items).toHaveLength(2); + items.forEach((it) => { + expect(it.kind).toBe('delete'); + expect(it.operationId).toBe('op1'); + expect(it.nodeType).toBe('tableRow'); + expect(it.to).toBeGreaterThan(it.from); + }); + expect(items.map((it) => it.id)).toEqual(['r1', 'r2']); + }); + + it('returns [] when no node carries trackChange attr', () => { + expect(getBlockTrackedChanges({ doc: schema.nodes.doc.create(null, [schema.nodes.paragraph.create()]) })).toEqual( + [], + ); + }); + + it('skips defensively when kind is not insert/delete', () => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + const row = schema.nodes.tableRow.create({ trackChange: { kind: 'format', id: 'r1' } }, [cell]); + expect(getBlockTrackedChanges({ doc: schema.nodes.doc.create(null, [tableWith([row])]) })).toEqual([]); + }); + + it('returns [] for null/undefined input', () => { + expect(getBlockTrackedChanges(null)).toEqual([]); + expect(getBlockTrackedChanges(undefined)).toEqual([]); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js index 3b3af70cc1..de6b87d224 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js @@ -307,6 +307,30 @@ const getPendingDeadKeyPlaceholder = ({ tr, newTr, user }) => { }; }; +/** + * Detects whether a ReplaceStep's slice already carries block-level + * `trackChange` metadata via PM node attributes (e.g. a table whose rows + * have `trackChange.insert` stamped by `applyHunks`). When true, the slice + * should be applied as-is — wrapping its inline content with `trackInsert` + * marks would double-track. + * + * @param {import('prosemirror-model').Slice} slice + * @returns {boolean} + */ +const sliceContainsPreMarkedBlockTrackedChange = (slice) => { + if (!slice || !slice.content || slice.content.size === 0) return false; + let found = false; + slice.content.descendants((node) => { + if (found) return false; + if (node?.attrs?.trackChange?.kind) { + found = true; + return false; + } + return undefined; + }); + return found; +}; + /** * Tracked transaction to track changes. * @param {{ tr: import('prosemirror-state').Transaction; state: import('prosemirror-state').EditorState; user: import('@core/types/EditorConfig.js').User; replacements?: 'paired' | 'independent' }} params @@ -349,6 +373,15 @@ export const trackedTransaction = ({ tr, state, user, replacements = 'paired' }) step = normalizeCompositionInsertStep({ step, doc, tr, user, pendingDeadKeyPlaceholder }); + // Block-level tracked-change replay path: the inserted slice already + // carries row-level tracked metadata via PM node attrs (see applyHunks). + // Wrapping the inner cell content with inline trackInsert marks would + // double-track. Apply such steps as-is. + if (step instanceof ReplaceStep && sliceContainsPreMarkedBlockTrackedChange(step.slice)) { + newTr.step(step); + return; + } + if (step instanceof ReplaceStep) { replaceStep({ state, diff --git a/packages/super-editor/src/editors/v1/index.js b/packages/super-editor/src/editors/v1/index.js index 0979a94485..8e1346d8ac 100644 --- a/packages/super-editor/src/editors/v1/index.js +++ b/packages/super-editor/src/editors/v1/index.js @@ -41,6 +41,7 @@ import AIWriter from './components/toolbar/AIWriter.vue'; import * as fieldAnnotationHelpers from './extensions/field-annotation/fieldAnnotationHelpers/index.js'; import * as trackChangesHelpers from './extensions/track-changes/trackChangesHelpers/index.js'; import { TrackChangesBasePluginKey } from './extensions/track-changes/plugins/index.js'; +import { StructuralTrackChanges, computeStructuralDiff } from './extensions/structural-track-changes/index.js'; import { CommentsPluginKey, createOrUpdateTrackedChangeComment } from './extensions/comment/comments-plugin.js'; import { AnnotatorHelpers } from '@helpers/annotator.js'; import { SectionHelpers } from '@extensions/structured-content/document-section/index.js'; @@ -108,6 +109,9 @@ export { helpers, fieldAnnotationHelpers, trackChangesHelpers, + // Structural (block-level) tracked changes (opt-in; not in starter extensions) + StructuralTrackChanges, + computeStructuralDiff, /** @internal */ AnnotatorHelpers, SectionHelpers, diff --git a/packages/super-editor/src/editors/v1/tests/data/diffing/diff_after_table_remove.docx b/packages/super-editor/src/editors/v1/tests/data/diffing/diff_after_table_remove.docx new file mode 100644 index 0000000000000000000000000000000000000000..1904c3fd0fcebb5a6ee2054e069d1aca87395f49 GIT binary patch literal 6757 zcmaJ`1z1#Fw;sB?q!}9NloAA_1eB0Y$DxOAq!DQtVE{q8Lz*EZq@=sUkq+q+Q0ju; zfB(V7U$%CobxAPIcc{U=!un$Cnhbf?v%kSnl)?(a)#Os~sRd%eLx-k%7Ym<#> z6UW{-bj2?Sn$p2&c0|QrxIXF1d52q{{{|~9Z*}|FO_T{`Lq-s)a15hqVCw(e;ghi` zTPpU(H#lH~M?;9E;Y=i)QPHKWH9Bfd9Q6S@+4(`ZelDOcgjO?$%d?VP^Zcby_zFi6 zA1e#mR<0~=LZ(&f?73##yFmqYlZQ5F?!B)PKj3ncPG7_LWqIWA6dH~6bjP2Y8epyq zgU_ac?oXyJPHR;}-c%6y>Kvt7IY9M=U=kEMx*KM4G~KjRdW=-^NyL8ZA2>A_T!T3f zPG~mHv9f!`)@;vTL2D_Vx^t}3cvIM-N`BQrdic|;s4hD`vK&)p4F`fdiy`Ck$P@)k zA0*`3KZ~5=d&HGNbK`IDB0Ko!n9wlaV=^3*^q@| zHeLcFN-rb2AS1N%IgIkW**`-%IJr{YtS6U&Qj`eAq|VeNB^o#-&MWAG`a*=pJhMv2 zF1sf)!qJLclV~X_WP-+^ZqmFnOKQRjLQ)~;L)fEH~OdLy^j4B)Z$UAD(PRYqh1wA605CP0Ep|Xw!F7B(-ER77* zr%nv%GONBQ0@A;q`0{<2^-#NU=YC<266a0qsa(H7^lppKYxh%h){+arH#{?6`o$mQ z0X-Wpqk81iUD#V7T?RA)ES``UzbQj&(ZxU7#rKWPmX7IpabAhe$qQN|=?Zp~HEV^XLA6wHw~;B65+j6g&hla(VWJ5y^UEsW z`G(b}w?DmKJG|bdzdMa^S(69PkO2TI%>Q;8{|z`64)(6kUw|Oyzm8*qiv9dwfP?vE z9u_^Jgw`)J%7Ad4J_V4(#bF+Y0L?$w@tSWh&B-uuaw^kYE#UTMbnK45*XfnXOqyB+ z!&Q|TajT6|1laspv|fG*T$COc+^A zn!+Lp4w1(L`hk>%7hBwMEa1Tmrg?EoO4HUKfwW!gxeD=aVIhgQrFHUFEXmy1y^qf} z$wXQ9^^YL>r6E`aQKTUe!g?#5-+Z2pyIAwXzu`>V_prbOBICq(Z#;|FO6WMLGFp_ir?{(UnYKn`Y>f; z0bX5l(#P|O(-D-Lkdbrs*syVQkHE!>g3a;!cd6E&=oJkF2c9@)%XW0Z3TrHiM;D^+ z1DP)T`0xl@_kC5)6}+a=p2>9()!X&$J1^Uo*lBDFiGa-t-l)1G*5ZVck!U6QJz$xb z2s>+!?VamiAe8ws)vfLpgzEk)2od}ngj~HK=D#2ak+f?+$4C06-0Upi=#v^IlwU~P zt@ISQt77PZ#1;-DkFlG@hZ$e(bYcOq*wu64w*CRJzI61Tq`E~M79Qqiw3L8TQ zb1vv%3S3>TVjh0dKJ_W$C`AEXkM7C4SVra^@2yYH52_Y(%Q$CV6{Y5leo=-k3r;6! z1ZhI2qnL^%C{%MZUA4Fnq@DLybcs~N*l9pPlT+Ux@;Y+88B9|hNtQRt6D3xy3K8HD ztkqk@oQ)lmJ4vo{Y_QZwWGp_SG!6`l3Dih7<54m2=27!tWdZv_h_+#{6_KW4NM%6u z*C-e%iQT)f1~Z~KO~d`jp$O3vZ%iw~1jP)((+w^rWu$TJmzp_9#p^9oE{3Dd6ZSv} zZsu<5%(Jz4jh z5Cez*UW1g*`l3^rm$VdkuH^jX;=!)Luc=x`T*owX{jpl0doC>!84TUoM@{zm! z$!>PrnH3VH#+cA&f*0>=6a0P5=3Z4K2S)LwOVl9;>$nR>f=dG*6&r?USv;{X`q0&g z&T?aQOm)JsdXcyGRKyN-v#JJY11ZDnvU}6>zBIKL*e(`i<^uXzj?yjSXYrVtL2Cfi z@OetcUM1<0cnTA~HfJoO1O=|I4~x^&MXkLrI`vs{S}NxKX#~dRxIe8$pUkThk{f2= z-yrweCnWoGz5D1{*U0gJZC0v;A1WuYLE#FMY(A+67<#r^ixHX(#JI@F-Ru1k5# zM97R7*XS!_NXeO!DT=mey4uBk`4?GGp4F@jTl)3jjiH(2m;#7)T4Nm5tKGQ!5crXp z;`5u#!s|$4H_{U_VFt8dfp^k*ggtBtn28StJu`vLtf)lz9`v3(t_%(a zVcZnV)bn$<*rFdjt6f3jyefk<-*efn{u*lLK>yUgK~+3ARZ_XnRKHMs@`HFfJd^2$ zOP=>!1@D1uXZQZ{9xs)AVt^f9y4jiA zyWU~?ZqlIhya-`HM&^FN5+b{>udJgICil=gS36fo^ST z)Cam{Imz@ufa9mDCFja|uN%GlI>Ek&)s6({vDL&I_X5 zHpTrUX~d4^i>hMO=qSv&0`uB1?8ooBDu=Pz8#3|BFjj$Ltr@F9wgy{q~B1qiCh?WY7Qw2{vr-^dgBpcAz{8YI`>6)@kubK6XOVEL%su znxso-i5m0sjw1Wd$qWLy7yC}-nGd|VHDvfMB|5E=M0WMIxQ90-Yw;!Psj}Dll82$d z3yhl1AOur)U$sO<*9&OZ1M-C*9%}INH%`V%!@WEpF-oCZ>Gr@T^h(Oi!N@{qBn6&8-3e-UvqeIG$?)| z6{RBxzpV-L@nT+FN{tqa|IGbrrj;7A1hQx~_zBrJoGTIdewu=FtDKf~@Rk8KrtH9o zaAsnKEkH9zFxw`BF%-IUGTGJfN!#)5rq2;M$;Odl^pN)9;3U3Fl+2iODdv?WREdfW@u@i4-}u5xY~23d9%wLy_)s{@!Cb zgp+_nhQiD4@F4COLAK*%GXwRV0Gey_Db>0zGwEHSOXGy?X1uoi_tiMrX~)gGP78?6 z6=ClmXm=@FZSZ!%^)_WAZ)@&9cLg;6TaZni%^@zlzb7uw-JQ}nHJc7T!jRh*Fl=_Z zYMlN_c&ZvE*zgMq`_Xx*0TWQ|iSI=Oj2RzwJS1s9-0OHSYU(|`M3@6^rAKaNk29fC z*W^Puwp0mXg*vP!#pa=?EEb+X zij;WV(vvx#Z{%zDhfU4jSv>NJ=QxE2Rg~ipPHrAb&z4zx2OC<(kcaQrKvQk_(Y8hy zIYoP=t9vx>^<3bVuuw7z6s&b|cFiWsc^VLBIdvvT;l^tleGm)VU;%Dg5!@U5q8k<| zjB!}C$?r+y=p7L+RhRT2qv`Tj=%9d-?Zex^aNUOQpF@Xp`;Wh(_A64+!xH%Ie7NX~ zB#xmy5^DVcgf?SnKsWg{fW~q@(V(2N=gBG94A9wXb#Z3duuJF8`XH*;vpciMhG8>T zx62xRW2yctlCr)ii^V>)EeK1z#pao4q>=j=Se6mZDZCBlGNYBU`dTNq55v`5;HQ;5 zkNJcUrWX;_xBRj?p%&^^Y1J`K^h#Yk=nJg#*xl|?B}q>&_~ub8iD@`vWFfy`{Y7D+ zH@u`Bd5!zZ^R4tR*2Zl$%vgE5j{CRF?VneA3;KVl1Flx)cILdlKKXxPyS|FUya>%F zf*B76R|oT1oh?+ih?&ayQZ;I3`%pUT@WdJQ>_=nDKGbt70Z1JmUxbp)*oD3@3?!e) zyAYZchtQ6cSA3#NoF+yLI{6u zYFFk)wqI6rX{I}>JJ(JvtfuUP|9P8vQ@$)Q5$Z{$QHLGp^ZWYMcHi0r5-Gf7L7pPZ zjP2O@j1|6t1OX16JKm+WAjNlGvxAZGZPhq-!1Z17?m}H7$|mcj{7yU(H2sd5b{?N)QG(&X!}D4oH*y#d6NkiWEp|73MRL6usnn~Wm_6x-A8-0cgE17({6*A_JDf7L zn$NEzKhgU}TzelOB127+4>h}QuE#AuUH#0XEOcEwE~mdaJUKjekh(bz4MjhH(<(VO5lYUxVGZ>kN}0ipZ;3p61@d;`yPJ)kx~bC;SooD6~)2HCll%DO_15%;WO| zLl5;(L6Qr#DpPq`Z8Hmyvfk%t-9x>loBQB zDTW|TjWh8Ye3zNJ8RlZfknC5it9SzqF<(|+AqW-2H7uz@#ZQ?UFgPMcMRXNdJmIoq zVx36cDjmL|#BQ~7Eo#9dsSMxZEBO*omnOYaD~MQ~Mwd97#j_8cq9uY_qK6xu>h6W3vCT(Lb}nz>($mbFdq9hTKns7y@l0cRmgvFuV!7*!9Jyag?OM5vl2 z64c6D96&GPK|SEF#LkMUhT@SPJ(wCdj#N`(vE(-=u=hWn@-V=gnIWmFdW)9tm6VO7 zlk0QeeLZ}#yhCmYYPE=S@u6F*l21$r9d6g}koLVdwix(2{&9yi&>JnOsAI)g$Fom_ zg-+}_vPXfxvCQ>cHO%-1DdZ7JHqgtHybijs4Oc*CvjV8$crlZC{u=BZ5Gc993`Cm6fHJnXyIs# zXAf!LJ{HsaEV2~B%V3D&pS;Rs5$`+^(BrXT9R8@YU2cZkfq5G?( z9F2h{3>HT=Idr$3#4=gR*%cLnT! zIseX#|8)PIFWwRAzwHO!zuf;TZWr|%+x*k<_sqJZYkwOZ;lHNv oCwcqx5x*afyA=G}qRIc?gj82SyA3S>fOUJx-YUL$_wIiE5BH(JivR!s literal 0 HcmV?d00001 diff --git a/packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx b/packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx new file mode 100644 index 0000000000000000000000000000000000000000..6d38931b1c5d6b354cfc63ac717502c984e50426 GIT binary patch literal 7235 zcmaJ`1z1#Fw;p21p-W1-9J(6;X;8XLVt}E$r8|Y8QMyGML_$hZx@BdBGZ-?zm?uL~mbArV|0M(E@N8bZib^O!xV2vsjkxg%DYO4#Y? zfZO>JprmZe%-IXogw#Q4WkV4t(5=rq`5lM_fBF{2A;Bt%A>E{}tu?N0q>J*M$LxHX z%1wUi>a1RYH?$JVSMxa2(mq~?8zw}gskLD&N!CM7qD@UAm4f@~`#Z1-rArXgf+G;> z6eqD?YQ>=L9bZrM+>L3C%!|kxQTUrCoXAhll7`&Gs0tMEb#xohEV8)sQzHa3ia>6c zfm!rTa&8Djhe62bb*eu|ypG=0{bttD$2=vDjM?65_RiAY5^a9&rV49s1t~UbA3@O$ zFn$H7(F3f9y^3aJWD^Kn=t<)xQyC7j17O+h$agq*l`?tgLf^OTDc>gH87kODq<6if zL=eF+!)0lOQ&vDm33eDJ&VvI0(vbiF<^Nh@oPU+r+1$nDzQE4EDx9p4F!xa5cp>Gv zAPNLCiI_C0LV6uD&3J^~1e>yvGFoD+hBz}l-Z=ICc!IIT1)~9_ruCHR7ef%4T#O`Y zgg$B{F6#I$br|tQi(i&lP+FC;ac@2)F+UE1VS|xjdJNT+ARCu6;!9pK^XzI(+uYvl zNC!(oRh;GMkO?x~hDr0T9MK7AEUnhaSSBa*g$hZJ^@yn%5|o8k#AJwRY^=BAXM7AZ zTBPh99`z4cvq5jhGaiPHdSV)-RG}dJ!rXpD5=ZjUod9NfN<1e9n!cmgC(CJMcc=MN8-JgozA7KRaS$d>)37j z#RbRN(yfbMG!*$`O`HfDj>;MMNI9s~Pf1FO20kU2;04UkAkvQpEFCD5EsuQpkU24= zMXUI-go+|#;_K-!{gFn~?vvtPS@2!lndE?O%wDTcu-h3Dec2UY3&Yr#V(F65zjwnl zx>xG0Gh=I9&0~J>EDF%NE3CezH3>oV_dZ@h{oncPq4h3& z{rDkPl$QRRy0u_Sx~RF!>%^@$Skx5CC9JlF^u)%s?`&2sr}iqFFz1yF3tL!hZLKV< z`LL<9Xf_tuq**ZL(iYW5-b&jN*RB_r2iB9+xsFU3mgz6> zdi~QY*gklV;$b${Nf`3IfCm69QT}N*{u^*0_I57nFHLOBe@(|E1-tpb00-skJS=8H z7C0b2O1a=NeFnfwNI*FWp)&v6z-GR)JSR>G2FsD%EQ0#7I`_siH2cJ}Q>NC;>J;Uu zK{fj6oD6<<&CHMGRuCRx_LIC$%sX7OIyav;3*l*JP_gqV)OlWttuI|-kuJ+h7Hq|-j_kF^>U=jw-_aRzpXaZrc`m}x_ z4%gI)>cJW?qfMy5-~WOo{F~pM5=7WI`lv{Ua@@~B)v{2b9}|05kPbG ziX8*H?Z8*zLfUg0_(HN1=Ywtkfzyh0nXSqWH?Ns-QK+KZ!g>OC8XURMfIBQ3Wx>|U zeP{Rf7YJpWX1X@qgHXdif)LifLCD3^#{3rqEu`$(t?=WARv7QYt@~wQQx0V3nur_{9+@o9B=0>D*ul#qo-!?&8Z^~0vP5*GCwK9UjjeI?gt92DFYJ@jU| zd;^3XqaD2k-M>D$UWaUh^5IDkUg8hFAx{|EXu`^>=PGeS>AS7*4L6o zl^6fU8C$%s&-prVOsUFA4SOeg@fq`awnI3orqJ|xb6eFTUUyD$3qr@l63lBMlHT|tiP7hi7f87Z~;;aB1xMEHZogn6>u-4!Qux?QfBh=$r{#uAyGgZ zS58$`#aeUf?}1XX^0vOrg5Z^r9ja_`^~b&v{Fyb;v_M60G*b*xar%|!u&va9$`#S9 zHI-b})5!M~{P>h^dyGr)B0INa=cOx1;@%~S>OfCaYWb(dxKCNBunEgmngJgJN>f)g zDxB=%O+a9{tybA}oiuW<7$;A8|2z@fQ^C2cKyx3X{Z$3aIs*R8Oyom7|0UtJT8!|K z0dtU+_mlv?XdIq8nf^ZV4FGLTm!w06ufECHt~B|nx7z;Nrp&6fa%&NvS@4v+n>=pfzMJ?4ZuB>0kjy3ll{#-eJH0}|ME z`5@^?Q~W_na)`|w%7})QHREpT^w5TRaS%FeB+RORDxMXkNF}v?H6inztxQ7oSvdt^ zc)Y!G?W1YdpI;w&y=`QHIrp#`OGzlWdOhofnQU!wz*i(BK00S-Z$O)=t5{493u5;s zeSt~4U4U60EI@Qe1n-;%rAf>+Wv|Wke|ChWdp5--J{q-MJzuIz)AeaOR6Co)Z!(FF z)D08-Bb#XcC)}24Hd;@Oxls}$Gr<_T=jO&-*yU0q*IDy8onHCSO#+Gc9-YPvbYKHl z^DpthD9*w$0FRBS%TP?bfg_E!Ou~hYto5WR6o~;$nB)Zd`U8I{(v~ael?k*M2 zwBM=Od`%@}OAoka*z)L>Uj*y%b$|U?c!Eu~wtSs2kT2QaSC;S7FELpHnLH{4l3x-r zd3|Q%`HF&*B^uR~(p$VLEx7GTBXay&L;sM}Q|MCV)a=@?Y7b%L0`*Qh<_nC5Fxn+{ z^XCdv*4o0S2jHKA0?#gy5-!`VNGo|TzNU2Wm8M%2M~xK-YN2~{Fu86|6eZ4_CyekCl^TVs zH#a9mw+yM(J{O-88k-Lv@P?~|3tClzCzkh3=A z?ITIGV!fs{Ww6N0%eRIE zo0xK(kDWbV%g#Ad;(D$MLAh1Ut)fjPHcCK5w1iU8D~a3CMNoALLky79)COsFNi!P3 z_>2r~IlbO8W(n{rNzRJF8C)+-STD&yQ_D;QICh@2GDnE}1eT$n1+X(#=u`7CaV)#j zo^GY~9YFM1l?8AT^*@I^cZN8@Hnwe<7r_zya0pU`uDKh`NIlPJA0vDR5dr~tngjf7 zGRwit+g8UV0e;SWQqPKH3_zLV8)O=4!>w=fzQMUNG*?<%Z6h%E$#l1CM{CybwC=*R z;?S)6wZ_@rI2F@}`ufq3$MJ6z&#L598a%-#UL1Go-6)2vvDaYCTfd7sRMxM}KQX0k zw1I3nJXshxXJc$H3(!x}Jpf?3`0ba}YC^6~tdsM_DBq&Gur^<*_cJEW!HM_B_8{j!uh>rwvR~>8C%(T#7G$%Ai$F63RHA(N=Gc@FZo55ekBha5} z`K6#Q-6gGw0e(z(y+2#A78r-Vky4-0``_Y^Z2o@cDhNB+)WnA=^jky zjI6)YAm>w|{@6t@>F(QL;sSpGlB}#~<>vDth@Sj#ENk=hLajyJyQQs1$E6veyT~^+ zh7M!s9UBur5^0Pl22n27?EL6G=(~eP$910jxs0bC0Ew;BjJ@YiC1h+b3p47O)yAz3 z_&uI0iSfaAWM|+UhNAZOq2Q+|xaZuMC98YP40|oIN(vCbuN0Z~biTs7j+^+AOZ3)_ zf#-5?{WQSW%||(_-0+@bs7w+?uAQNC=6k@zM8ud5i7vh!q#TU-UUh7JWJ^x3-Yzwy zqbM2w(9G1c%&=|=)|hulIkYYNfN!5+RfP^}A-c&GNF19Ql<&y$hqovc3ge z?s~rCr+GMk7uj@;_5ij$PLdX^Kmb6O;h(^k>|b=t$jRKsneBJTWqr6)o}dKnWXBG< z-{6POPFIgp$VX%2-BsZQj+(*iiFW(B94co97K4Y3`S37PzZ(FgR0!~ zvwNMeWxFRu>d-{X7FJ2ad()vJ{1QMK=@}T-4i*`AQToiOXe7Rbig?fNVnKAFWSQxA zVO1|H{mS%2Q2CEJpDn`mC&NbOsgS3hiA-m8ft3~L*pr(_Vzck9yn^&BVhJM-YU4AZ z9Kh`nYA}DFSWU0$quwh}867b-XVH2$xO+BD(nA+F$FVC(6qKl;|4tx$gN|y`66?{} zSFP|U9^|9yO%4w-2d~IP(S{VhtmbRrL+auIWjjariDAC)RsVgvjDG)*zq0l#Q!&Fr zm>ujOq$NCuuwEgh0e@`h7?8?UY8{}mQiwAsDd+Ly%*+_j)n<8hZqc~+*o~eqy3eC0 zy97$PnXlDtg|xB!;TxQs4nG~_Aig~iRj?KMf%ZOlx98t53&4s)JSOJ57t%i#{R-1GAP{-*cs`Y-Lr#nRl?obA^q$FHue zqhLSJOZEwC#+}l|-h5Vb8__j#rfR-iiImnZ?6Flu@{Drs(=j<8(z#WCxXzC+!w6<< z!(QqI5Y7}_anB0ckdIVUetMkDlrfa2)PPxz#VdDndw?u=d7RW4p5seRyY4ve#5%?N z%p~rL8*^@IPy9||KtgqSrYE{5-&QHS_We7*i+1zoLJ3?P#2;1qowi{0CptB@TkV|5 zM4l2R9=t2m9cYErmA(O3{`OtFUgh;BGO68!k&cKq_A$~EG?wwzwNpi3yg{udbmPFd z2m`jnVl^{uzhODQwP$W=Pp?wzSmIYtg5U(y2~D)0c#PCTXM3ur+2-kR)D$dvKE66* z+C+o*t9UyaVXTcSc=LU`U@*Z?WaCr#&Z*bJTeaf>z|2XTss-HqZbRk}tO%(;EULMQ zr(x>d3eRl}@ETe)?x+R6=mst7w>3b)gE3yf#mwaesT(2Eg?)M8!O$TX{o=M@G6_PE z6(AwK+5&VT8Rq1qGwYh=p=kylPI1FQ81)9;XNqbLII(n`f7nEzL}%6HmWC$8f%K)ezM00w;lyj}H55jo-hr|EVj>^!21bLBWzPM0 zl?^%_v(;KIZlgX?_(tA(9WO+M8KoVm_T1f$TYS3tSwLLuvUE~Gad-6N=)_+1?j$S> z=_0gEl3X+!=k+GNz*8Y%_)ZXfuX1LL3ARb8noUsNG%k;YDm zkP{Dg|IDZ7t{ldNccoNqBqgC?3^9?@{v4xqq`ll*%n_R!+Z>=_sp{lT-5l9j5Gacr z>^N}}0i#dXGf`DJ7p%o}o@tn&Eu{|0^`>9L_|h2rbDy)wSKNuDQF~A1+Q&f#* zyKp@0RWn?8ok~nv`47QmA$2f!H?aa^=lbu@*^>Obp;1?tPLiK&l5MJWC|$*4VC_~R zJghBm*%^@Es-~}|7c}tu}6gZ<6&W zndd6AoAjvAo1Si{r>_pkFdsG9wXzK&uPxVT*w^jtl$$4Mw?Yk7fxkAJvtmbIsb4So z+LmbTIJBbStSP|bp4Lu)6A=%^8(=T$f2?Fw(8YDLW5;#cdHKlLlD=p>chu=tl4bXD zBw;k%1KQVbqbs~>D%EATNs&!O8buEB;yc+rE(v~al=U2O&8V$bBkiV_}Q zTd=mT2*L9jIO~;dNYu{$*c*#iWlBrQ0b^{IU)4Qq+=R&w$-_mToO>chc;slnj2ih7 z-{_?~GL`x5;7{n{9-IgZL5J*^9nX$d3>RkKw@>dMy{{)yn3&q9nzdPuV-#&huI5|z z;(}a8DjFwa)k{GR{*N2}Zg1qNv-HZGT#Bb+nU;dH#eC1UjCUU|EG3?ci(RGB=1$f`@Nr$ zx~GG=v%y2Nse>s!|3yXG$f@mBR+E7e#a2X5Q`&OL+Ugj}L+;knU}tBFW4yQ;ud(Bm z%r*v_&tq6T1s6TFY^d^La&Qx!n9)!KNl2%XJoH$l-E*I+wlfkTsGDFhey=Ipa>@dr z+hJ-)V5odd!97K4?%9&I+~$H>0(#zV%^RqxA@@LX3L3!=+DioZvyF-~No8pWRc z6eBRC2XQdKu(RpdIT6tQ%)1=IMyZG3m$pU&NpzaHoW` z`>=)o+g9&K{N?}qxx%0BzpI1)IlSY2u)o~@LkRxU|95TSK|}o8Ebk}WU&H@fQT)^S zcdq=Ou|v`R*ZFs5{HOcxeDQ%$|83Lvq5K;AztHPHpY%H)d*GLU8}5Bvzp>3f9e-b0 z4|MHsyS{h)6~dq7?axR2J~J`i$_l{y)B*sg_m{-Ip1(u<@aum7ku;+z literal 0 HcmV?d00001 From 70003e4666483bbff9196707b72279586f0b61bd Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 13:05:05 +0100 Subject: [PATCH 2/4] fix(track-changes): address PR review on block-level structural tracked changes - Move painter-targeting CSS into BLOCK_TRACK_CHANGE_STYLES in the painter's styles module so document visuals live behind the painter boundary; keep contenteditable-side rules scoped under .sd-editor-scoped .ProseMirror tr. - Drop stale isBlockLevel entries from previous state before merging in freshly-computed block entries, so accept/reject clears the bubble. - renderDOM now emits data-track-change-id and data-track-change-operation alongside data-track-change so HTML round-trips preserve enough state for getBlockTrackedChanges and acceptTrackedChangeById to resolve the row. Adds regression tests: stale-entry pruning in comments-plugin apply(), renderDOM/parseDOM round-trip in blockTrackedChangeAttr, and a styles.ts guard that fails if a bare [data-track-change=...] selector regresses. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../painters/dom/src/styles.test.ts | 15 ++++++++ .../layout-engine/painters/dom/src/styles.ts | 28 +++++++++++++++ .../v1/assets/styles/elements/prosemirror.css | 35 ++++++++----------- .../comment/blockTrackedChangeBubble.test.js | 27 ++++++++++++++ .../v1/extensions/comment/comments-plugin.js | 14 +++++--- .../track-changes/blockTrackedChangeAttr.js | 9 ++++- .../blockTrackedChangeAttr.test.js | 27 ++++++++++++-- 7 files changed, 128 insertions(+), 27 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index 14f084b300..4a9f1a4238 100644 --- a/packages/layout-engine/painters/dom/src/styles.test.ts +++ b/packages/layout-engine/painters/dom/src/styles.test.ts @@ -70,4 +70,19 @@ describe('ensureTrackChangeStyles', () => { ); expect(cssText).not.toMatch(/track-format-dec\.highlighted\.track-change-focused\s*\{[\s\S]*border-bottom-width:/); }); + + it('block-level tracked-change rules are scoped to .superdoc-layout, not bare selectors', () => { + ensureTrackChangeStyles(document); + + const styleEl = document.querySelector('[data-superdoc-track-change-styles="true"]'); + const cssText = styleEl?.textContent ?? ''; + + // Scoped selectors present + expect(cssText).toContain(".superdoc-layout [data-track-change='delete']"); + expect(cssText).toContain(".superdoc-layout [data-track-change='insert']"); + + // No bare `[data-track-change=...] {` selector — would leak to PM mirror + // and any host page that uses the same attribute name. + expect(cssText).not.toMatch(/^\s*\[data-track-change=/m); + }); }); diff --git a/packages/layout-engine/painters/dom/src/styles.ts b/packages/layout-engine/painters/dom/src/styles.ts index cd916debff..12fd09b215 100644 --- a/packages/layout-engine/painters/dom/src/styles.ts +++ b/packages/layout-engine/painters/dom/src/styles.ts @@ -313,6 +313,34 @@ const TRACK_CHANGE_STYLES = ` .superdoc-layout .track-format-dec.highlighted.track-change-focused { background-color: var(--sd-tracked-changes-format-background-focused, #ffd70033); } + +/* + * Block-level tracked changes (row granularity for v1). + * + * The DomPainter stamps data-track-change on each cell
of a tracked + * row (no exists in layout mode; cells are absolutely positioned). + * Scoping to .superdoc-layout keeps these rules out of the contenteditable + * mirror; the editor stylesheet handles the PM path separately. + */ +.superdoc-layout [data-track-change='delete'] { + background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)); + outline: + var(--sd-block-tracked-deleted-outline-width, 2px) + dashed + var(--sd-block-tracked-deleted-outline, #cb0e47); + outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px); + text-decoration: line-through; + text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); +} + +.superdoc-layout [data-track-change='insert'] { + background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)); + outline: + var(--sd-block-tracked-inserted-outline-width, 2px) + dashed + var(--sd-block-tracked-inserted-outline, #00853d); + outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px); +} `; const FORMATTING_MARKS_STYLES = ` diff --git a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css index 6112f125ae..9b23656946 100644 --- a/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css +++ b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css @@ -286,35 +286,30 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html /* Track changes - end */ /* - * Block-level tracked changes (row granularity for v1). + * Block-level tracked changes (row granularity for v1) — contenteditable path. * - * The `data-track-change` attribute is stamped in two places: - * 1. ProseMirror's contenteditable via the schema's renderDOM - * (hidden in layout-engine mode but kept consistent). - * 2. The DomPainter's per-cell
elements (no exists in layout - * mode; cells are absolutely positioned). Every cell of a tracked row - * carries the attribute → a continuous red/green strip across the row. - * - * Selectors are intentionally minimal so the rule applies wherever the painter - * places the cell — across re-renders and table-fragment boundaries. + * The data-track-change attribute is stamped on PM's via the schema's + * renderDOM. The painter-side stamping (per-cell
in layout mode) is + * styled by BLOCK_TRACK_CHANGE_STYLES in + * packages/layout-engine/painters/dom/src/styles.ts to keep document visuals + * inside the painter's stylesheet boundary. */ -[data-track-change='delete'] { - background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)) !important; - outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47) !important; - outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px) !important; +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] { + background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)); + outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47); + outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px); } .sd-editor-scoped .ProseMirror tr[data-track-change='delete'] td, -.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th, -.superdoc-table-fragment [data-track-change='delete'] { +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th { text-decoration: line-through; text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); } -[data-track-change='insert'] { - background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)) !important; - outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d) !important; - outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px) !important; +.sd-editor-scoped .ProseMirror tr[data-track-change='insert'] { + background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)); + outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d); + outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px); } /* Block-level tracked changes - end */ diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js index a453195e06..40363f61ee 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -50,4 +50,31 @@ describe('comments-plugin — block-level tracked-change registration', () => { expect(tracked['OP1']).toBeUndefined(); expect(tracked['r1']).toBeUndefined(); }); + + it('drops a block-level entry once its row is no longer tracked (resolved op)', () => { + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); + installDoc([table]); + expect(CommentsPluginKey.getState(editor.state).trackedChanges['OP1']).toBeDefined(); + + // Walk the doc to find the tracked row, then clear its trackChange attr + // through a transaction. This exercises the apply() reducer's stale-entry + // pruning rather than recomputing state from init(). + let rowPos = null; + let rowNode = null; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === 'tableRow' && node.attrs.trackChange) { + rowPos = pos; + rowNode = node; + return false; + } + return true; + }); + expect(rowPos).not.toBeNull(); + + const tr = editor.state.tr.setNodeMarkup(rowPos, undefined, { ...rowNode.attrs, trackChange: null }); + editor.view.dispatch(tr); + + const pluginState = CommentsPluginKey.getState(editor.state); + expect(pluginState.trackedChanges['OP1']).toBeUndefined(); + }); }); diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index 5ef21e2a10..a0042638df 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -555,10 +555,16 @@ export const CommentsPlugin = Extension.create({ isBlockLevel: true, }; } - // Merge: keep existing inline entries, add block-level entries. - // Block-level keys (operationId / row-id) shouldn't collide with - // inline ones (mark-id), but if they ever do, inline wins. - pluginState.trackedChanges = { ...blockTracked, ...pluginState.trackedChanges }; + // Drop stale block-level entries from the previous state first. + // getBlockTrackedChanges is the source of truth for block-level + // tracking, so anything no longer in the doc (accepted/rejected) + // must not leak back via the merge. Inline entries are owned by + // handleTrackedChangeTransaction and are kept as-is. + const previousInline = {}; + for (const [k, v] of Object.entries(pluginState.trackedChanges || {})) { + if (!v?.isBlockLevel) previousInline[k] = v; + } + pluginState.trackedChanges = { ...previousInline, ...blockTracked }; } // Check for changes in the actively selected comment diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js index a893c77bf2..5106987f92 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -20,7 +20,14 @@ export const blockTrackedChangeAttrSpec = { }, renderDOM: (attrs) => { const tc = attrs?.trackChange; - return tc ? { 'data-track-change': tc.kind } : {}; + if (!tc) return {}; + const out = { 'data-track-change': tc.kind }; + // Emit id + operationId so HTML round-trips (clipboard, getHTML/setContent, + // collaboration patches) preserve enough to resolve the change via + // getBlockTrackedChanges and accept/reject it. + if (tc.id) out['data-track-change-id'] = tc.id; + if (tc.operationId) out['data-track-change-operation'] = tc.operationId; + return out; }, }, }; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js index 629fa391b7..1e1221e1d4 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js @@ -26,13 +26,36 @@ describe('blockTrackedChangeAttrSpec', () => { expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toBeNull(); }); - it('renderDOM emits data-track-change when set; nothing when null', () => { + it('renderDOM emits kind, id, and operationId when present', () => { expect( blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' }, }), - ).toEqual({ 'data-track-change': 'insert' }); + ).toEqual({ + 'data-track-change': 'insert', + 'data-track-change-id': 'r1', + 'data-track-change-operation': 'op1', + }); + }); + + it('renderDOM omits optional id / operationId when missing', () => { + expect( + blockTrackedChangeAttrSpec.trackChange.renderDOM({ + trackChange: { kind: 'delete' }, + }), + ).toEqual({ 'data-track-change': 'delete' }); + }); + + it('renderDOM emits nothing when trackChange is null or absent', () => { expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: null })).toEqual({}); expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({})).toEqual({}); }); + + it('renderDOM and parseDOM round-trip preserve id and operationId', () => { + const original = { kind: 'delete', id: 'row-42', operationId: 'op-99' }; + const rendered = blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: original }); + const el = document.createElement('tr'); + for (const [k, v] of Object.entries(rendered)) el.setAttribute(k, v); + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toEqual(original); + }); }); From 2a7887582c5b1d9d19d6eb41483929e5c06a4301 Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 14:25:20 +0100 Subject: [PATCH 3/4] feat(track-changes): register StructuralTrackChanges in getStarterExtensions Consumers (Superdoc Vue host, @superdoc-dev/react wrapper) take their extension list from getStarterExtensions(); without this entry the new block-level extension was exported but never loaded into the editor. Updates the dedicated test that previously asserted opt-in-only behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/super-editor/src/editors/v1/extensions/index.js | 3 +++ .../structural-track-changes/structural-track-changes.test.js | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/index.js b/packages/super-editor/src/editors/v1/extensions/index.js index 5940506ddc..04762bc019 100644 --- a/packages/super-editor/src/editors/v1/extensions/index.js +++ b/packages/super-editor/src/editors/v1/extensions/index.js @@ -94,6 +94,7 @@ import { PermEnd, PermEndBlock } from './perm-end/index.js'; // Helpers import { trackChangesHelpers } from './track-changes/index.js'; import { Diffing } from './diffing/index.js'; +import { StructuralTrackChanges } from './structural-track-changes/index.js'; const getRichTextExtensions = () => { return [ @@ -239,6 +240,7 @@ const getStarterExtensions = () => { PassthroughInline, PassthroughBlock, Diffing, + StructuralTrackChanges, ]; }; @@ -302,6 +304,7 @@ export { getStarterExtensions, getRichTextExtensions, Diffing, + StructuralTrackChanges, AiMark, AiAnimationMark, AiLoaderNode, diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js index c675cddcb6..60b89077bb 100644 --- a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js @@ -58,8 +58,8 @@ describe('StructuralTrackChanges extension', () => { expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); }); - it('is NOT registered in default starter extensions (opt-in only)', () => { + it('is registered in default starter extensions', () => { const names = getStarterExtensions().map((e) => e.name); - expect(names).not.toContain('structuralTrackChanges'); + expect(names).toContain('structuralTrackChanges'); }); }); From e14d1e0f4c696e9e0b6fd8e1402c9279f4dc3083 Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 22:19:51 +0100 Subject: [PATCH 4/4] fix(track-changes): skip block-level walk for docs without tracked rows The comments-plugin apply() reducer walked the entire doc for block-level tracked rows on every tr.docChanged, even on docs that had no tracked rows. Combined with in-place mutation of pluginState.trackedChanges and the view.update decoration rebuild path, this created a stream of state references that downstream consumers reacted to, contributing to focus steal in side-by-side layouts where the editor sat next to an unrelated input control. Two structural changes: - Track a hasBlockChanges bit on plugin state, computed once at init and refreshed only when a walk runs. Gate the block walk on (hasBlockChanges OR inputType='acceptReject' meta) so typing in a doc with no tracked rows never triggers the walk. - Stop mutating pluginState.trackedChanges and pluginState.activeThreadId inside apply(); compute next-values locally and return them via the spread at the end. Plugin state must be treated as immutable across apply() per ProseMirror convention. Adds regression tests that lock the gate (hasBlockChanges stays false through typing, flips true once a tracked row appears). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../comment/blockTrackedChangeBubble.test.js | 23 ++++++++ .../v1/extensions/comment/comments-plugin.js | 54 +++++++++++++------ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js index 40363f61ee..3e9b6f69cb 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -51,6 +51,29 @@ describe('comments-plugin — block-level tracked-change registration', () => { expect(tracked['r1']).toBeUndefined(); }); + it('hasBlockChanges starts false on a doc without tracked rows and stays false through typing', () => { + installDoc([schema.nodes.paragraph.create(null, schema.text('hello'))]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(false); + + // Simulate a typing transaction; the gate should keep the block walk + // skipped so trackedChanges remains an empty stable reference. + const before = CommentsPluginKey.getState(editor.state).trackedChanges; + const tr = editor.state.tr.insertText('!', editor.state.doc.content.size - 1); + editor.view.dispatch(tr); + const after = CommentsPluginKey.getState(editor.state); + expect(after.hasBlockChanges).toBe(false); + expect(after.trackedChanges).toEqual(before); + }); + + it('hasBlockChanges flips to true once a tracked row appears', () => { + installDoc([schema.nodes.paragraph.create(null, schema.text('x'))]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(false); + + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); + installDoc([table]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(true); + }); + it('drops a block-level entry once its row is no longer tracked (resolved op)', () => { const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); installDoc([table]); diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index a0042638df..24b1f0b426 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js @@ -459,6 +459,7 @@ export const CommentsPlugin = Extension.create({ // initial doc (e.g. when a docx is loaded with pre-marked rows or // when EditorState is reconstructed via setState during tests). const trackedChanges = {}; + let hasBlockChanges = false; if (editorState?.doc) { const blockEntries = getBlockTrackedChanges(editorState); const seenOperations = new Set(); @@ -473,6 +474,7 @@ export const CommentsPlugin = Extension.create({ range: { from: entry.from, to: entry.to }, isBlockLevel: true, }; + hasBlockChanges = true; } } return { @@ -483,6 +485,7 @@ export const CommentsPlugin = Extension.create({ allCommentPositions: {}, allCommentIds: [], trackedChanges, + hasBlockChanges, }; }, @@ -522,24 +525,39 @@ export const CommentsPlugin = Extension.create({ }; } - // If this is a tracked change transaction, handle separately + // If this is a tracked change transaction, handle separately. + // Compute the next trackedChanges value without mutating prev state; + // ProseMirror plugin state must be treated as immutable across apply(). const trackedChangeMeta = tr.getMeta(TrackChangesBasePluginKey); - const currentTrackedChanges = pluginState.trackedChanges; + let nextTrackedChanges = pluginState.trackedChanges; + let nextHasBlockChanges = pluginState.hasBlockChanges; if (trackedChangeMeta) { - pluginState.trackedChanges = handleTrackedChangeTransaction( + nextTrackedChanges = handleTrackedChangeTransaction( trackedChangeMeta, - currentTrackedChanges, + pluginState.trackedChanges, newEditorState, editor, ); } - // Block-level tracked changes don't fire TrackChangesBasePluginKey meta - // (they're applied as PM node attrs via setNodeMarkup). On any doc - // change, refresh the block-level entries in trackedChanges. Rows that - // share an operationId collapse into a single entry so a multi-row - // delete surfaces as one bubble. - if (tr.docChanged || trackedChangeMeta) { + // Block-level tracked changes don't fire TrackChangesBasePluginKey + // meta (they're applied as PM node attrs via setNodeMarkup). When the + // doc changes we may need to refresh the block-level entries. But we + // only walk the doc when: + // - the previous state already had block changes (so they may have + // been resolved or shifted), or + // - this transaction is a structural change stamping new rows + // (applyHunks / accept-reject row commands set + // inputType='acceptReject'). + // Skipping the walk on every typing transaction in a doc with no + // block-level changes avoids re-creating trackedChanges references + // and triggering downstream view rebuilds that can re-sync DOM + // selection. + const inputTypeMeta = tr.getMeta('inputType'); + const isBlockStampingTr = inputTypeMeta === 'acceptReject'; + const shouldWalkBlock = + (tr.docChanged || trackedChangeMeta) && (pluginState.hasBlockChanges || isBlockStampingTr); + if (shouldWalkBlock) { const blockEntries = getBlockTrackedChanges(newEditorState); const blockTracked = {}; const seenOperations = new Set(); @@ -561,13 +579,15 @@ export const CommentsPlugin = Extension.create({ // must not leak back via the merge. Inline entries are owned by // handleTrackedChangeTransaction and are kept as-is. const previousInline = {}; - for (const [k, v] of Object.entries(pluginState.trackedChanges || {})) { + for (const [k, v] of Object.entries(nextTrackedChanges || {})) { if (!v?.isBlockLevel) previousInline[k] = v; } - pluginState.trackedChanges = { ...previousInline, ...blockTracked }; + nextTrackedChanges = { ...previousInline, ...blockTracked }; + nextHasBlockChanges = Object.keys(blockTracked).length > 0; } // Check for changes in the actively selected comment + let nextActiveThreadId = pluginState.activeThreadId; if (!tr.docChanged && tr.selectionSet) { const { selection } = tr; @@ -595,8 +615,7 @@ export const CommentsPlugin = Extension.create({ const isNonCollapsedClear = currentActiveThread == null && selection && selection.$from.pos !== selection.$to.pos; if (previousSelectionId !== currentActiveThread && !isNonCollapsedClear) { - // Update both the plugin state and the local variable - pluginState.activeThreadId = currentActiveThread; + nextActiveThreadId = currentActiveThread; const update = { type: comments_module_events.SELECTED, activeCommentId: currentActiveThread ? currentActiveThread : null, @@ -607,7 +626,12 @@ export const CommentsPlugin = Extension.create({ } } - return { ...pluginState }; + return { + ...pluginState, + trackedChanges: nextTrackedChanges, + hasBlockChanges: nextHasBlockChanges, + activeThreadId: nextActiveThreadId, + }; }, }, };