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 59f60805c8..df204da6b3 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -343,6 +343,12 @@ export type TrackedChangeMeta = { id: string; overlapParentId?: string; relationship?: 'parent' | 'child' | 'standalone'; + /** + * 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:…'`). @@ -890,6 +896,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/layout-bridge/src/cache.ts b/packages/layout-engine/layout-bridge/src/cache.ts index f2efb115ff..bba5273f5c 100644 --- a/packages/layout-engine/layout-bridge/src/cache.ts +++ b/packages/layout-engine/layout-bridge/src/cache.ts @@ -286,6 +286,21 @@ const hashRuns = (block: FlowBlock): string => { continue; } + // Row-level tracked change (block-level diff replay sets `trackedChange` + // on a tableRow's attrs). Without folding it into the measure-cache + // fingerprint, applyHunks-style transactions that only mutate the row's + // `trackChange` attr don't invalidate the cache, so the page is never + // re-measured and the visible cells never receive their decoration + // classes. Read from `row.attrs.trackedChange` — the canonical location + // written by the v1 layout-adapter from the OOXML-aligned + // `attrs.trackChange.type` shape. Mirror of the painter page-fingerprint + // fix in renderer.ts and the canonical-version fix in + // layout-resolved/versionSignature.ts. + const rowTC = row.attrs?.trackedChange; + if (rowTC) { + cellHashes.push(`rtc:${rowTC.kind ?? ''}:${rowTC.id ?? ''}:${rowTC.operationId ?? ''}`); + } + for (const cell of row.cells) { // Include cell-level attributes that affect rendering (borders, padding, etc.) // This ensures cache invalidation when cell formatting changes (e.g., remove borders). diff --git a/packages/layout-engine/layout-resolved/src/versionSignature.ts b/packages/layout-engine/layout-resolved/src/versionSignature.ts index 5e3f2ee76c..0109fcbe98 100644 --- a/packages/layout-engine/layout-resolved/src/versionSignature.ts +++ b/packages/layout-engine/layout-resolved/src/versionSignature.ts @@ -488,6 +488,20 @@ export const deriveBlockVersion = (block: FlowBlock): string => { for (const row of rows) { if (!row || !Array.isArray(row.cells)) continue; hash = hashNumber(hash, row.cells.length); + // Row-level tracked change (block-level diff replay sets `trackedChange` + // on a tableRow's attrs). Without folding it into the canonical block + // version, applyHunks-style transactions that only mutate the row's + // `trackChange` attr don't bump the version, so the resolved-layout + // pipeline reuses cached entries and the painter never sees the update. + // Read from `row.attrs.trackedChange` — the canonical location written + // by the v1 layout-adapter from the OOXML-aligned `attrs.trackChange.type` + // shape. Mirror of the fixes in renderer.ts and layout-bridge/cache.ts. + const rowTC = row.attrs?.trackedChange; + if (rowTC) { + hash = hashString(hash, rowTC.kind ?? ''); + hash = hashString(hash, rowTC.id ?? ''); + hash = hashString(hash, rowTC.operationId ?? ''); + } for (const cell of row.cells) { if (!cell) continue; const cellBlocks = cell.blocks ?? (cell.paragraph ? [cell.paragraph] : []); diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index b8eb27442e..4bb6831166 100644 --- a/packages/layout-engine/painters/dom/src/styles.test.ts +++ b/packages/layout-engine/painters/dom/src/styles.test.ts @@ -406,4 +406,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/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css index 7fd224b92c..cfa01e613c 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 @@ -306,6 +306,34 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html } /* Track changes - end */ +/* + * Block-level tracked changes (row granularity for v1) — contenteditable path. + * + * 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. + */ +.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 { + text-decoration: line-through; + text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); +} + +.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 */ + /* Collaboration cursors */ .sd-editor-scoped .ProseMirror > .ProseMirror-yjs-cursor:first-child { margin-top: 16px; diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts index c192c63b1d..05b9c8413f 100644 --- a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts +++ b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts @@ -798,12 +798,21 @@ 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 changes flow through `row.attrs.trackedChange`, populated + // above via `buildRowTrackedChangeMeta` from the OOXML-aligned + // `attrs.trackChange.type` shape. The legacy top-level `row.trackedChange` + // copy this block used to populate from the old `{ kind }` shape is gone now + // that applyHunks writes the OOXML format; the painter and cache layers all + // read `row.attrs.trackedChange`. + + return row; }; /** diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/input/PresentationInputBridge.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/input/PresentationInputBridge.ts index 41f54261d0..a4ba624f36 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/input/PresentationInputBridge.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/input/PresentationInputBridge.ts @@ -305,6 +305,17 @@ export class PresentationInputBridge { return null; } + // The candidate must be a SuperDoc editor: every SuperDoc editor's + // ProseMirror DOM sits inside a `.sd-editor-scoped` ancestor. Without + // this check the bridge redirects keystrokes from any unrelated + // ProseMirror editor on the page (Tiptap, Remirror, raw PM) because + // they share the same .ProseMirror[contenteditable="true"] signature. + // Class-based (not WeakSet) so story editors that the bridge has never + // explicitly owned still benefit from stale-target recovery. + if (!staleEditorTarget.closest('.sd-editor-scoped')) { + return null; + } + return { activeTarget, staleEditorTarget, diff --git a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationInputBridge.test.ts b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationInputBridge.test.ts index 4e9abb5450..a397d18ebf 100644 --- a/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationInputBridge.test.ts +++ b/packages/super-editor/src/editors/v1/core/presentation-editor/tests/PresentationInputBridge.test.ts @@ -264,11 +264,22 @@ describe('PresentationInputBridge - Context Menu Handling', () => { expect(forwardedEvents).toEqual(['beforeinput']); }); + // Wrap a stale ProseMirror editor in a `.sd-editor-scoped` ancestor so + // the bridge recognizes it as a SuperDoc editor (matches the production + // DOM where every SuperDoc PM editor sits inside .sd-editor-scoped). + const makeSuperDocStaleEditor = (): HTMLElement => { + const wrapper = document.createElement('div'); + wrapper.className = 'sd-editor-scoped'; + const editor = document.createElement('div'); + editor.className = 'ProseMirror'; + editor.setAttribute('contenteditable', 'true'); + wrapper.appendChild(editor); + document.body.appendChild(wrapper); + return editor; + }; + it('reroutes beforeinput from a stale hidden editor to the active target when window fallback is enabled', () => { - const staleBodyEditor = document.createElement('div'); - staleBodyEditor.className = 'ProseMirror'; - staleBodyEditor.setAttribute('contenteditable', 'true'); - document.body.appendChild(staleBodyEditor); + const staleBodyEditor = makeSuperDocStaleEditor(); const staleEvent = new InputEvent('beforeinput', { data: 'a', @@ -300,10 +311,7 @@ describe('PresentationInputBridge - Context Menu Handling', () => { }); it('reroutes non-text keyboard commands from a stale hidden editor to the active target', () => { - const staleBodyEditor = document.createElement('div'); - staleBodyEditor.className = 'ProseMirror'; - staleBodyEditor.setAttribute('contenteditable', 'true'); - document.body.appendChild(staleBodyEditor); + const staleBodyEditor = makeSuperDocStaleEditor(); const staleEvent = new KeyboardEvent('keydown', { key: 'Backspace', @@ -332,6 +340,34 @@ describe('PresentationInputBridge - Context Menu Handling', () => { expect(staleEvent.defaultPrevented).toBe(true); }); + it('does NOT reroute keystrokes from a foreign ProseMirror editor (Tiptap, Remirror, etc.)', () => { + // Reproduces the al-pmo regression: a sibling editor on the page using + // the same .ProseMirror[contenteditable="true"] signature must not have + // its keystrokes hijacked by SuperDoc's stale-editor recovery. + const foreignEditor = document.createElement('div'); + foreignEditor.className = 'tiptap ProseMirror'; + foreignEditor.setAttribute('contenteditable', 'true'); + document.body.appendChild(foreignEditor); + + const targetFocusSpy = vi.spyOn(targetDom, 'focus').mockImplementation(() => {}); + const targetDispatchSpy = vi.spyOn(targetDom, 'dispatchEvent'); + + bridge.destroy(); + bridge = new PresentationInputBridge(windowRoot, layoutSurface, getTargetDom, isEditable, undefined, { + useWindowFallback: true, + }); + bridge.bind(); + // Note: foreignEditor is NOT registered with the bridge. + + foreignEditor.dispatchEvent(new KeyboardEvent('keydown', { key: 'h', bubbles: true, cancelable: true })); + foreignEditor.dispatchEvent( + new InputEvent('beforeinput', { data: 'h', inputType: 'insertText', bubbles: true, cancelable: true }), + ); + + expect(targetFocusSpy).not.toHaveBeenCalled(); + expect(targetDispatchSpy).not.toHaveBeenCalled(); + }); + it('does not reroute keyboard input from a registered UI surface editor', () => { const commentEditor = document.createElement('div'); commentEditor.className = 'ProseMirror'; 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..3e9b6f69cb --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -0,0 +1,103 @@ +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(); + }); + + 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]); + 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 74df3e32a0..9031666c9e 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 @@ -18,6 +18,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]; @@ -480,8 +481,30 @@ 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 = {}; + let hasBlockChanges = false; + 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, + }; + hasBlockChanges = true; + } + } return { activeThreadId: null, externalColor: highlightColors.external ?? '#B1124B', @@ -489,7 +512,8 @@ export const CommentsPlugin = Extension.create({ decorations: DecorationSet.empty, allCommentPositions: {}, allCommentIds: [], - trackedChanges: {}, + trackedChanges, + hasBlockChanges, }; }, @@ -529,19 +553,69 @@ 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). 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(); + 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, + }; + } + // 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(nextTrackedChanges || {})) { + if (!v?.isBlockLevel) previousInline[k] = v; + } + 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; @@ -569,8 +643,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, @@ -581,7 +654,12 @@ export const CommentsPlugin = Extension.create({ } } - return { ...pluginState }; + return { + ...pluginState, + trackedChanges: nextTrackedChanges, + hasBlockChanges: nextHasBlockChanges, + activeThreadId: nextActiveThreadId, + }; }, }, }; @@ -723,6 +801,42 @@ 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). Accept both the legacy `{kind}` shape + // and the upstream OOXML-aligned `{type: 'rowInsert'|'rowDelete'}` + // shape. + const blockTrackChange = node?.attrs?.trackChange; + const blockKind = + blockTrackChange?.kind === 'insert' || blockTrackChange?.kind === 'delete' + ? blockTrackChange.kind + : blockTrackChange?.type === 'rowInsert' + ? 'insert' + : blockTrackChange?.type === 'rowDelete' + ? 'delete' + : null; + if (blockTrackChange && blockKind && 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/index.js b/packages/super-editor/src/editors/v1/extensions/index.js index dcbc408aaa..7755253560 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 { Diffing } from './diffing/index.js'; +import { StructuralTrackChanges } from './structural-track-changes/index.js'; const getRichTextExtensions = () => { return [ @@ -243,6 +244,7 @@ const getStarterExtensions = () => { PassthroughInline, PassthroughBlock, Diffing, + StructuralTrackChanges, ]; }; @@ -307,6 +309,7 @@ export { getStarterExtensions, getRichTextExtensions, Diffing, + StructuralTrackChanges, AiMark, AiAnimationMark, AiLoaderNode, 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..c56b420377 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js @@ -0,0 +1,149 @@ +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` in the + * OOXML-aligned `{ type: 'rowInsert' | 'rowDelete', id, operationId }` shape. + * The v1 layout-adapter propagates that to `row.attrs.trackedChange` on the + * FlowBlock, where the painter reads it and applies the row-cell decoration + * classes. 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..62646ee499 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js @@ -0,0 +1,92 @@ +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 registered in default starter extensions', () => { + const names = getStarterExtensions().map((e) => e.name); + expect(names).toContain('structuralTrackChanges'); + }); + + it('acceptAllTrackedChanges resolves block-level row tracked changes (table + shell gone)', () => { + // Consumers like al-pmo only call acceptAllTrackedChanges; without + // block-level handling, cell contents would clear but the table shell + // would remain. + const table = editor.state.doc.firstChild; + editor.commands.setStructuralDiff([{ kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }]); + editor.commands.acceptAllTrackedChanges(); + let hasTable = false; + editor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(false); + expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); + }); + + it('rejectAllTrackedChanges clears block-level trackChange attrs and keeps the table', () => { + const table = editor.state.doc.firstChild; + editor.commands.setStructuralDiff([{ kind: 'remove', changeId: 't1', basePos: 0, baseNodeSize: table.nodeSize }]); + editor.commands.rejectAllTrackedChanges(); + let hasTable = false; + editor.state.doc.descendants((n) => { + if (n.type.name === 'table') hasTable = true; + }); + expect(hasTable).toBe(true); + expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); + }); +}); 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..d3111e4167 --- /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: { type: 'rowDelete', 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: { type: 'rowInsert', 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..1b2697493c --- /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?.type).toBe('rowDelete'); + 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?.type).toBe('rowInsert'); + }); + }); + + 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..f31483438e --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js @@ -0,0 +1,102 @@ +/** + * Compute structural hunks describing whole-block add/removes between two docs. + * + * Walks both docs at depth 1, matches blocks by `identityKey` (default: a + * content fingerprint built from the block's normalized `textContent` and + * node type). For each base block absent from proposal: emit + * `{ kind: 'remove' }`. For each proposal block (of allowed type) absent from + * base: emit `{ kind: 'insert' }`. + * + * Why content fingerprint instead of sdBlockId: the docx importer assigns a + * fresh sdBlockId on every load — there's no standard OOXML attribute for + * tables to persist a stable id. So two independently-loaded copies of the + * same document have different ids; matching by id would flag every block + * as changed. Content fingerprinting matches identical blocks across + * imports. + * + * Limitations of the default fingerprint: + * - Two truly-identical blocks (e.g. empty placeholder tables) share a + * fingerprint; the algorithm treats them as one. Pass a custom + * `identityKey` if your domain has identical blocks that must be + * distinguished. + * - A block whose content changed (same structure, different text) is + * treated as a remove of the old + insert of the new — visually rendered + * as "old red + new green." Consumers wanting "table modified in-place" + * semantics should pass a structural fingerprint (e.g., row count). + * + * Consumers whose proposal is a true in-place edit (sdBlockIds preserved + * across base/proposal) can opt back into id-based matching: + * computeStructuralDiff(base, proposal, { + * identityKey: (n) => n.attrs?.sdBlockId ?? null, + * }) + * + * @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) => { + if (!node) return null; + const typeName = node.type?.name ?? 'node'; + const text = typeof node.textContent === 'string' ? node.textContent : ''; + return `${typeName}:${text.replace(/\s+/g, ' ').trim()}`; +}; + +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..7a2daa4fd3 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js @@ -0,0 +1,139 @@ +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)); + // Default cell text falls back to the id, so each table built here has a + // unique content fingerprint matching the matcher's default behavior. + const table = + (id, text = id) => + (schema) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text(text))); + const row = schema.nodes.tableRow.create(null, [cell]); + return schema.nodes.table.create(idAttr(id), [row]); + }; + const tableFingerprint = (text) => `table:${text}`; + + 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: tableFingerprint('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: tableFingerprint('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('matches tables across independent imports even when sdBlockIds differ', () => { + // Reproduces the bug: two docx files loaded independently get fresh + // sdBlockIds per import. The diff must still recognize identical tables. + const base = makeDoc([table('base-a', 'first'), table('base-b', 'second')]); + const proposal = makeDoc([table('prop-x', 'first'), table('prop-y', 'second')]); + expect(computeStructuralDiff(base, proposal)).toHaveLength(0); + }); + + it('with multiple unchanged tables and one modified, marks only the modified one', () => { + const base = makeDoc([ + table('base-a', 'unchanged-1'), + table('base-b', 'will-edit'), + table('base-c', 'unchanged-2'), + ]); + const proposal = makeDoc([ + table('prop-a', 'unchanged-1'), + table('prop-b', 'edited-content'), + table('prop-c', 'unchanged-2'), + ]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks).toHaveLength(2); + expect(hunks.some((h) => h.kind === 'remove' && h.changeId === tableFingerprint('will-edit'))).toBe(true); + expect(hunks.some((h) => h.kind === 'insert' && h.changeId === tableFingerprint('edited-content'))).toBe(true); + }); + + 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(tableFingerprint('t1')); + }); + + it('consumer can opt back into sdBlockId-based matching via identityKey override', () => { + // For consumers whose proposals are in-place edits (sdBlockIds preserved + // across base and proposal), id-based matching is sometimes preferable. + const sdBlockIdKey = (node) => node.attrs?.sdBlockId ?? null; + const base = makeDoc([table('t1', 'same')]); + const proposal = makeDoc([table('t2', 'same')]); // same content, different id + const hunks = computeStructuralDiff(base, proposal, { identityKey: sdBlockIdKey }); + // With id-based matching, these differ even though content is identical. + expect(hunks.filter((h) => h.kind === 'remove')).toHaveLength(1); + expect(hunks.filter((h) => h.kind === 'insert')).toHaveLength(1); + }); + + 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 6b410b8ee8..8f2217fed4 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 @@ -120,6 +121,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..fbc914ade2 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -0,0 +1,41 @@ +/** + * 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; + // Without an id, the entry can never be resolved by + // applyRowTrackedChangeResolution (which matches by id) and would just + // sit on the row as a no-op attr. getBlockTrackedChanges already + // filters these out; reject them at parse time so the doc never holds + // a half-formed trackChange shape (also keeps the runtime type + // consistent with the documented `id: string`). + const id = el.getAttribute('data-track-change-id'); + if (!id) return null; + return { + kind, + id, + operationId: el.getAttribute('data-track-change-operation') ?? undefined, + }; + }, + renderDOM: (attrs) => { + const tc = attrs?.trackChange; + 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 new file mode 100644 index 0000000000..1e1221e1d4 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js @@ -0,0 +1,61 @@ +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 kind, id, and operationId when present', () => { + expect( + blockTrackedChangeAttrSpec.trackChange.renderDOM({ + trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' }, + }), + ).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); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-extension.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-extension.test.js index 4b196c77c3..9640db2490 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-extension.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes-extension.test.js @@ -2026,5 +2026,91 @@ describe('TrackChanges extension commands', () => { authorImage: undefined, }); }); + + // Regression test for ALPMO-245: when text is inserted at a position + // where bare text can't live (e.g. at doc end past the last block), PM + // auto-wraps the text in the schema's required parents. The old + // implementation passed `insertedNode.nodeSize` as the mark `to`, which + // covered the wrapper open tokens instead of the trailing characters of + // the actual text — so the last char(s) escaped the trackInsert mark and + // survived reject as orphans. + it('marks the full inserted text when PM auto-wraps at end-of-doc insertion', () => { + const doc = createDoc('Hello'); + const state = createState(doc); + const insertAt = doc.content.size; // past the last block — triggers auto-wrap + + let nextState; + const result = commands.insertTrackedChange({ + from: insertAt, + to: insertAt, + text: 'World.', + })({ + state, + dispatch: (tr) => { + nextState = state.apply(tr); + }, + editor: { + options: { user: { name: 'Test', email: 'test@example.com' } }, + commands: { addCommentReply: vi.fn() }, + }, + }); + + expect(result).toBe(true); + expect(nextState).toBeDefined(); + // The full inserted text — including the trailing '.' — must carry + // trackInsert; no character may sit in an unmarked text node. + expect(getMarkedText(nextState.doc, TrackInsertMarkName)).toBe('World.'); + let unmarkedAppendedText = ''; + nextState.doc.descendants((node) => { + if (!node.isText) return; + if (node.text === 'Hello') return; // pre-existing base content + const hasTrackMark = node.marks.some((m) => m.type.name === TrackInsertMarkName); + if (!hasTrackMark) unmarkedAppendedText += node.text ?? ''; + }); + expect(unmarkedAppendedText).toBe(''); + }); + + it('rejectTrackedChangesBetween leaves no orphan chars after end-of-doc insertion', () => { + const doc = createDoc('Hello'); + const state = createState(doc); + const insertAt = doc.content.size; + + let nextState; + commands.insertTrackedChange({ + from: insertAt, + to: insertAt, + text: 'World.', + })({ + state, + dispatch: (tr) => { + nextState = state.apply(tr); + }, + editor: { + options: { user: { name: 'Test', email: 'test@example.com' } }, + commands: { addCommentReply: vi.fn() }, + }, + }); + + const afterInsert = nextState; + let afterReject; + commands.rejectTrackedChangesBetween( + 0, + afterInsert.doc.content.size, + )({ + state: afterInsert, + dispatch: (tr) => { + afterReject = afterInsert.apply(tr); + }, + editor: { + options: { user: { name: 'Test', email: 'test@example.com' } }, + commands: { addCommentReply: vi.fn() }, + }, + }); + + expect(afterReject).toBeDefined(); + // Doc must be restored to just 'Hello' — no orphan '.' or 'd' or 'l' + // surviving in a new wrapper paragraph the AI-style insertion created. + expect(afterReject.doc.textContent).toBe('Hello'); + }); }); }); 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 e839e0b236..50f6ff8a92 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 @@ -14,6 +14,8 @@ import { sliceFromText, } from './review-model/edit-intent.js'; import { decideTrackedChanges, buildDecisionBubbleEvents } from './review-model/decision-engine.js'; +import { getBlockTrackedChanges } from './trackChangesHelpers/getBlockTrackedChanges.js'; +import { applyRowTrackedChangeResolution } from './trackChangesHelpers/acceptRejectRowTrackedChange.js'; /** * @typedef {{ code: string, message: string, details?: unknown }} TrackChangesFailure @@ -331,7 +333,7 @@ export const TrackChanges = Extension.create({ acceptTrackedChangeById: (id) => - ({ state, dispatch, editor }) => { + ({ state, dispatch, editor, commands }) => { const reviewDecision = dispatchReviewDecision({ editor, state, @@ -339,12 +341,39 @@ export const TrackChanges = Extension.create({ decision: 'accept', target: { kind: 'id', id }, }); - return reviewDecision.applied; + if (reviewDecision.applied) return true; + + // Block-level fallback: the review-model decision engine only + // handles inline (mark-based) tracked changes; a row-level + // structural change carries its tracked-change id on the + // tableRow.attrs.trackChange object, not on an inline mark, so + // dispatchReviewDecision returns applied:false for it. + // Resolve it via the row-attr path (by row id, then 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: () => - ({ state, dispatch, editor }) => { + ({ state, dispatch, editor, commands }) => { const reviewDecision = dispatchReviewDecision({ editor, state, @@ -352,12 +381,19 @@ export const TrackChanges = Extension.create({ decision: 'accept', target: { kind: 'all' }, }); + // Block-level row tracked changes live on node attrs, not inline + // marks, so dispatchReviewDecision can't touch them. Resolve them + // alongside so a single accept-all surface handles both kinds. + const blockEntries = getBlockTrackedChanges(state); + if (blockEntries.length > 0 && commands.acceptAllStructuralChanges) { + commands.acceptAllStructuralChanges(); + } return reviewDecision.applied; }, rejectTrackedChangeById: (id) => - ({ state, dispatch, editor }) => { + ({ state, dispatch, editor, commands }) => { const reviewDecision = dispatchReviewDecision({ editor, state, @@ -365,7 +401,29 @@ export const TrackChanges = Extension.create({ decision: 'reject', target: { kind: 'id', id }, }); - return reviewDecision.applied; + if (reviewDecision.applied) return true; + + // Block-level fallback (see acceptTrackedChangeById for rationale). + 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; }, rejectTrackedChange: @@ -418,7 +476,7 @@ export const TrackChanges = Extension.create({ rejectAllTrackedChanges: () => - ({ state, dispatch, editor }) => { + ({ state, dispatch, editor, commands }) => { const reviewDecision = dispatchReviewDecision({ editor, state, @@ -426,6 +484,12 @@ export const TrackChanges = Extension.create({ decision: 'reject', target: { kind: 'all' }, }); + // Mirror acceptAllTrackedChanges — block-level row tracked changes + // live on node attrs and need their own resolver. + const blockEntries = getBlockTrackedChanges(state); + if (blockEntries.length > 0 && commands.rejectAllStructuralChanges) { + commands.rejectAllStructuralChanges(); + } return reviewDecision.applied; }, 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..b050a535af --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js @@ -0,0 +1,52 @@ +/** + * 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, + * }>} + */ +// Normalize the row's stored trackChange shape into the canonical `kind` +// value resolvers expect. The OOXML-aligned shape (upstream) uses +// `{ type: 'rowInsert' | 'rowDelete' }`; the legacy shape this extension +// originally wrote used `{ kind: 'insert' | 'delete' }`. Accept both so the +// resolver works across applyHunks-produced rows and any docs imported with +// the upstream-spec attribute names. +const resolveKind = (tc) => { + if (!tc || typeof tc !== 'object') return undefined; + if (tc.kind === 'insert' || tc.kind === 'delete') return tc.kind; + if (tc.type === 'rowInsert') return 'insert'; + if (tc.type === 'rowDelete') return 'delete'; + return undefined; +}; + +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; + const kind = resolveKind(tc); + if (tc && tc.id && kind) { + out.push({ + id: tc.id, + 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 5903771adf..890bfc516e 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 @@ -351,6 +351,33 @@ 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; + // Accept both the legacy `{ kind }` shape and the OOXML-aligned + // `{ type: 'rowInsert' | 'rowDelete' }` shape that applyHunks writes. + const tc = node?.attrs?.trackChange; + if (tc && (tc.kind || tc.type === 'rowInsert' || tc.type === 'rowDelete')) { + found = true; + return false; + } + return undefined; + }); + return found; +}; + /** * Process a transaction through the track-changes pipeline and return * a modified transaction with tracked-change marks applied (or the @@ -409,6 +436,18 @@ 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 — but still append to `map` so + // subsequent steps in the same transaction map through this step's + // changes (matches the pass-through pattern in replaceStep.js). + if (step instanceof ReplaceStep && sliceContainsPreMarkedBlockTrackedChange(step.slice)) { + newTr.step(step); + map.appendMap(step.getMap()); + 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 fcf98a5d4b..ffd9cb3603 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'; @@ -109,6 +110,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 0000000000..1904c3fd0f Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/diffing/diff_after_table_remove.docx differ 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 0000000000..6d38931b1c Binary files /dev/null and b/packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx differ diff --git a/packages/superdoc/src/index.js b/packages/superdoc/src/index.js index 151d06d2db..c528e60f80 100644 --- a/packages/superdoc/src/index.js +++ b/packages/superdoc/src/index.js @@ -33,6 +33,9 @@ import { AIWriter, ContextMenu, SlashMenu, + // Structural (block-level) tracked changes + StructuralTrackChanges, + computeStructuralDiff, } from '@superdoc/super-editor'; import { DOCX, PDF, HTML, getFileObject, compareVersions } from '@superdoc/common'; // @ts-expect-error Vite resolves DOCX asset URL imports; plain tsc does not. @@ -298,4 +301,8 @@ export { AIWriter, ContextMenu, SlashMenu, + + // Structural (block-level) tracked changes + StructuralTrackChanges, + computeStructuralDiff, }; diff --git a/tests/consumer-typecheck/snapshots/superdoc-super-editor.txt b/tests/consumer-typecheck/snapshots/superdoc-super-editor.txt index 43aa841de4..a0ed013280 100644 --- a/tests/consumer-typecheck/snapshots/superdoc-super-editor.txt +++ b/tests/consumer-typecheck/snapshots/superdoc-super-editor.txt @@ -135,6 +135,7 @@ SelectionSlice SelectorFn SlashMenu StoryLocator +StructuralTrackChanges Subscribable SuperConverter SuperDocEditorLike @@ -178,6 +179,7 @@ ViewportRect ViewportRectResult VirtualizationOptions assertNodeType +computeStructuralDiff createHeadlessToolbar createOrUpdateTrackedChangeComment createSuperDocUI