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
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