From d0d86b276ed1a6ad4e0c6973815b4914f4282d6c Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 12:12:36 +0100 Subject: [PATCH 01/15] feat(track-changes): block-level structural tracked changes for tables Adds a new StructuralTrackChanges extension that gives block-level (table) add/remove operations the same review pipeline as inline tracked changes - same data-track-change rendering, same accept/reject command surface, plus block-level entry registration in the comments-plugin. Consumer pattern: compute structural hunks via computeStructuralDiff (or construct them yourself), dispatch via editor.commands.setStructuralDiff (hunks). The extension stamps a trackChange PM attribute on each affected tableRow. Rendering is handled by the painter (reads row.trackedChange, stamps data-track-change* on cells). Accept/reject operates on PM attrs via getBlockTrackedChanges + applyRowTrackedChangeResolution. Pipeline pieces: - Shared blockTrackedChangeAttrSpec helper + TableRow.addAttributes spread - TableRow.trackedChange contract field - pm-adapter populates TableRow.trackedChange from PM attr - painter renderTableRow stamps data-track-change on cells - CSS rules for [data-track-change="insert" | "delete"] - getBlockTrackedChanges walks the doc for block-level entries - applyRowTrackedChangeResolution handles accept/reject by id - acceptTrackedChangeById / rejectTrackedChangeById extended to route inline marks, row attrs, or operationId - trackedTransaction passes through ReplaceSteps that already carry block-level metadata so inline marks don't double-track - comments-plugin walks block-level entries alongside inline marks Existing Diffing extension, compareDocuments, replayDifferences, and inline TrackChanges are untouched. The new extension is opt-in via editorExtensions: [StructuralTrackChanges] (not in starter extensions). Tests: ~30 new unit tests across the touched modules, plus an end-to-end test using a real .docx fixture pair that exercises compute -> set -> accept-all and compute -> set -> reject-all. Out of scope (separate follow-ups): - DOCX round-trip of block-level tracked changes - Block-level entries surfacing in the default SuperDoc bubble (requires mirroring inline's commentsUpdate event payload pipeline) - Cell-level / column-level tracking - A combined acceptAllChanges that batches inline + structural into one transaction (currently two sequential commands; two undo steps) Co-Authored-By: Claude Opus 4.7 (1M context) --- .../layout-engine/contracts/src/index.test.ts | 16 +- packages/layout-engine/contracts/src/index.ts | 12 ++ .../v1/assets/styles/elements/prosemirror.css | 33 ++++ .../core/layout-adapter/converters/table.ts | 34 +++- .../comment/blockTrackedChangeBubble.test.js | 53 +++++++ .../v1/extensions/comment/comments-plugin.js | 82 +++++++++- .../structural-track-changes/index.js | 2 + .../structural-track-changes-e2e.test.js | 69 ++++++++ .../structural-track-changes.js | 147 ++++++++++++++++++ .../structural-track-changes.test.js | 65 ++++++++ .../applyHunks.js | 86 ++++++++++ .../applyHunks.test.js | 86 ++++++++++ .../computeStructuralDiff.js | 77 +++++++++ .../computeStructuralDiff.test.js | 97 ++++++++++++ .../v1/extensions/table-row/table-row.js | 3 + .../track-changes/blockTrackedChangeAttr.js | 26 ++++ .../blockTrackedChangeAttr.test.js | 38 +++++ .../extensions/track-changes/track-changes.js | 76 ++++++++- .../acceptRejectRowTrackedChange.js | 70 +++++++++ .../acceptRejectRowTrackedChange.test.js | 101 ++++++++++++ .../blockTrackedChangePassthrough.test.js | 71 +++++++++ .../getBlockTrackedChanges.js | 37 +++++ .../getBlockTrackedChanges.test.js | 50 ++++++ .../trackChangesHelpers/trackedTransaction.js | 33 ++++ packages/super-editor/src/editors/v1/index.js | 4 + .../data/diffing/diff_after_table_remove.docx | Bin 0 -> 6757 bytes .../diffing/diff_before_table_remove.docx | Bin 0 -> 7235 bytes 27 files changed, 1358 insertions(+), 10 deletions(-) create mode 100644 packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/index.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes-e2e.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/applyHunks.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.js create mode 100644 packages/super-editor/src/editors/v1/extensions/structural-track-changes/structuralTrackChangesHelpers/computeStructuralDiff.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/acceptRejectRowTrackedChange.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/blockTrackedChangePassthrough.test.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js create mode 100644 packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js create mode 100644 packages/super-editor/src/editors/v1/tests/data/diffing/diff_after_table_remove.docx create mode 100644 packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx diff --git a/packages/layout-engine/contracts/src/index.test.ts b/packages/layout-engine/contracts/src/index.test.ts index 51286c7172..9496677ac9 100644 --- a/packages/layout-engine/contracts/src/index.test.ts +++ b/packages/layout-engine/contracts/src/index.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import { cloneColumnLayout, extractHeaderFooterSpace, normalizeColumnLayout, widthsEqual } from './index.js'; -import type { FlowBlock, Layout } from './index.js'; +import type { FlowBlock, Layout, TableRow, TrackedChangeMeta } from './index.js'; describe('contracts', () => { it('accepts a basic FlowBlock structure', () => { @@ -85,4 +85,18 @@ describe('contracts', () => { }); expect(normalizeColumnLayout({ count: 2, gap: 24 }, 624).widths).toEqual([300, 300]); }); + + it('TrackedChangeMeta accepts an optional operationId', () => { + const meta: TrackedChangeMeta = { kind: 'delete', id: 'r1', operationId: 'op-table-1' }; + expect(meta.operationId).toBe('op-table-1'); + }); + + it('TableRow accepts an optional trackedChange field carrying row-level metadata', () => { + const row: TableRow = { + id: 'row-1', + cells: [], + trackedChange: { kind: 'insert', id: 'r1', operationId: 'op-table-1' }, + }; + expect(row.trackedChange?.kind).toBe('insert'); + }); }); diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 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/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css b/packages/super-editor/src/editors/v1/assets/styles/elements/prosemirror.css index 7fd224b92c..10d25ecb4a 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,39 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html } /* Track changes - end */ +/* + * Block-level tracked changes (row granularity for v1). + * + * The `data-track-change` attribute is stamped in two places: + * 1. ProseMirror's contenteditable via the schema's renderDOM + * (hidden in layout-engine mode but kept consistent). + * 2. The DomPainter's per-cell
elements (no exists in layout + * mode; cells are absolutely positioned). Every cell of a tracked row + * carries the attribute → a continuous red/green strip across the row. + * + * Selectors are intentionally minimal so the rule applies wherever the painter + * places the cell — across re-renders and table-fragment boundaries. + */ +[data-track-change='delete'] { + background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)) !important; + outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47) !important; + outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px) !important; +} + +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] td, +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th, +.superdoc-table-fragment [data-track-change='delete'] { + text-decoration: line-through; + text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); +} + +[data-track-change='insert'] { + background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)) !important; + outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d) !important; + outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px) !important; +} +/* Block-level tracked changes - end */ + /* Collaboration cursors */ .sd-editor-scoped .ProseMirror > .ProseMirror-yjs-cursor:first-child { margin-top: 16px; diff --git a/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts b/packages/super-editor/src/editors/v1/core/layout-adapter/converters/table.ts index c192c63b1d..d89f5d5866 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,44 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { // The PM table-row extension has both cantSplit as a top-level attr AND within tableRowProperties // For layout engine, we only need to read from tableRowProperties.cantSplit - return { + const row: TableRow = { id: context.nextBlockId(`row-${rowIndex}`), cells, attrs, sourceAnchor: sourceAnchorFromNode(rowNode), }; + + // Row-level tracked change (block-level diff replay sets this on the PM node). + // See packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js + const trackChangeAttr = rowNode.attrs?.trackChange as + | { + kind?: 'insert' | 'delete'; + id?: string; + operationId?: string; + author?: string; + authorEmail?: string; + date?: string; + storyKey?: string; + } + | null + | undefined; + if ( + trackChangeAttr && + (trackChangeAttr.kind === 'insert' || trackChangeAttr.kind === 'delete') && + trackChangeAttr.id + ) { + row.trackedChange = { + kind: trackChangeAttr.kind, + id: trackChangeAttr.id, + operationId: trackChangeAttr.operationId, + author: trackChangeAttr.author, + authorEmail: trackChangeAttr.authorEmail, + date: trackChangeAttr.date, + storyKey: trackChangeAttr.storyKey, + }; + } + + return row; }; /** diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js new file mode 100644 index 0000000000..a453195e06 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -0,0 +1,53 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { EditorState } from 'prosemirror-state'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { CommentsPluginKey } from './comments-plugin.js'; + +describe('comments-plugin — block-level tracked-change registration', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

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

hi

after

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

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

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const makeDoc = (blocks) => + schema.nodes.doc.create( + null, + blocks.map((b) => b(schema)), + ); + const p = (id, text) => (schema) => schema.nodes.paragraph.create(idAttr(id), schema.text(text)); + const table = (id) => (schema) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + const row = schema.nodes.tableRow.create(null, [cell]); + return schema.nodes.table.create(idAttr(id), [row]); + }; + + it('emits a remove hunk for a base table absent from proposal', () => { + const base = makeDoc([p('p1', 'a'), table('t1'), p('p2', 'b')]); + const proposal = makeDoc([p('p1', 'a'), p('p2', 'b')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'remove', changeId: 't1' }); + }); + + it('emits an insert hunk for a proposal table absent from base', () => { + const base = makeDoc([p('p1', 'a')]); + const proposal = makeDoc([p('p1', 'a'), table('t1')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'insert', changeId: 't1' }); + }); + + it('emits no hunks when both sides have matching tables', () => { + const base = makeDoc([table('t1')]); + const proposal = makeDoc([table('t1')]); + expect(computeStructuralDiff(base, proposal)).toHaveLength(0); + }); + + it('anchor for insert is end of last shared base block', () => { + const base = makeDoc([p('p1', 'first'), p('p2', 'second')]); + const proposal = makeDoc([p('p1', 'first'), table('t-new'), p('p2', 'second')]); + const hunks = computeStructuralDiff(base, proposal); + const insert = hunks.find((h) => h.kind === 'insert'); + expect(insert).toBeDefined(); + expect(insert.anchorBasePos).toBe(base.child(0).nodeSize); + }); + + it('insert with no shared base block anchors at 0', () => { + const base = makeDoc([p('p1', 'only')]); + const proposal = makeDoc([table('t-new')]); + const hunks = computeStructuralDiff(base, proposal); + const insert = hunks.find((h) => h.kind === 'insert'); + expect(insert?.anchorBasePos).toBe(0); + }); + + it('blockTypes filter (default ["table"]) restricts insert events', () => { + const base = makeDoc([]); + const proposal = makeDoc([table('t1'), p('p1', 'extra')]); + const hunks = computeStructuralDiff(base, proposal); + expect(hunks.filter((h) => h.kind === 'insert')).toHaveLength(1); + expect(hunks.find((h) => h.kind === 'insert')?.changeId).toBe('t1'); + }); + + it('custom identityKey override', () => { + const fingerprint = (node) => `${node.type.name}:${node.textContent}`; + const noId = (text) => (schema) => schema.nodes.paragraph.create(null, schema.text(text)); + const base = makeDoc([noId('A'), noId('B')]); + const proposal = makeDoc([noId('A')]); + const hunks = computeStructuralDiff(base, proposal, { + identityKey: fingerprint, + blockTypes: ['paragraph'], + }); + expect(hunks).toHaveLength(1); + expect(hunks[0]).toMatchObject({ kind: 'remove', changeId: 'paragraph:B' }); + }); + + it('does not descend into matched tables (nested table inside matched outer is not double-counted)', () => { + const outerWithCellPara = (id) => (schema) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('inner'))); + const row = schema.nodes.tableRow.create(null, [cell]); + return schema.nodes.table.create(idAttr(id), [row]); + }; + const base = makeDoc([outerWithCellPara('t-outer')]); + const proposal = makeDoc([outerWithCellPara('t-outer')]); + expect(computeStructuralDiff(base, proposal)).toHaveLength(0); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js b/packages/super-editor/src/editors/v1/extensions/table-row/table-row.js index 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..a893c77bf2 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -0,0 +1,26 @@ +/** + * Shared ProseMirror addAttributes-compatible spec for block-level tracked + * changes. Apply to block-unit nodes (TableRow for v1; TableCell, Image, + * PageBreak in future) by spreading into addAttributes(). + * + * Value shape: + * { kind: 'insert' | 'delete', id: string, operationId?: string } | null + */ +export const blockTrackedChangeAttrSpec = { + trackChange: { + default: null, + parseDOM: (el) => { + const kind = el.getAttribute('data-track-change'); + if (kind !== 'insert' && kind !== 'delete') return null; + return { + kind, + id: el.getAttribute('data-track-change-id') ?? null, + operationId: el.getAttribute('data-track-change-operation') ?? undefined, + }; + }, + renderDOM: (attrs) => { + const tc = attrs?.trackChange; + return tc ? { 'data-track-change': tc.kind } : {}; + }, + }, +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js new file mode 100644 index 0000000000..629fa391b7 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js @@ -0,0 +1,38 @@ +import { describe, it, expect } from 'vitest'; +import { blockTrackedChangeAttrSpec } from './blockTrackedChangeAttr.js'; + +describe('blockTrackedChangeAttrSpec', () => { + it('declares a trackChange attribute with null default', () => { + expect(blockTrackedChangeAttrSpec.trackChange).toBeDefined(); + expect(blockTrackedChangeAttrSpec.trackChange.default).toBeNull(); + }); + + it('parseDOM reads data-track-change* into an attribute value', () => { + const el = document.createElement('tr'); + el.setAttribute('data-track-change', 'delete'); + el.setAttribute('data-track-change-id', 'row-abc'); + el.setAttribute('data-track-change-operation', 'op-xyz'); + const parsed = blockTrackedChangeAttrSpec.trackChange.parseDOM(el); + expect(parsed).toMatchObject({ kind: 'delete', id: 'row-abc', operationId: 'op-xyz' }); + }); + + it('parseDOM returns null when data-track-change is missing', () => { + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(document.createElement('tr'))).toBeNull(); + }); + + it('parseDOM rejects unknown kinds', () => { + const el = document.createElement('tr'); + el.setAttribute('data-track-change', 'foo'); + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toBeNull(); + }); + + it('renderDOM emits data-track-change when set; nothing when null', () => { + expect( + blockTrackedChangeAttrSpec.trackChange.renderDOM({ + trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' }, + }), + ).toEqual({ 'data-track-change': 'insert' }); + expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: null })).toEqual({}); + expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({})).toEqual({}); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js b/packages/super-editor/src/editors/v1/extensions/track-changes/track-changes.js index 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..99ccd18451 --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.js @@ -0,0 +1,37 @@ +/** + * Walk the doc collecting block-level tracked changes — PM nodes whose + * `trackChange` attribute is set. Mirror of `getTrackChanges` (mark-based). + * + * Returned entries: `{ id, kind, operationId?, nodeType, from, to, node }`. + * + * @param {{ doc: import('prosemirror-model').Node } | import('prosemirror-state').EditorState | null | undefined} stateOrDoc + * @returns {Array<{ + * id: string, + * kind: 'insert' | 'delete', + * operationId?: string, + * nodeType: string, + * from: number, + * to: number, + * node: import('prosemirror-model').Node, + * }>} + */ +export const getBlockTrackedChanges = (stateOrDoc) => { + const doc = stateOrDoc?.doc ?? stateOrDoc; + const out = []; + if (!doc || typeof doc.descendants !== 'function') return out; + doc.descendants((node, pos) => { + const tc = node?.attrs?.trackChange; + if (tc && tc.id && (tc.kind === 'insert' || tc.kind === 'delete')) { + out.push({ + id: tc.id, + kind: tc.kind, + operationId: tc.operationId, + nodeType: node.type.name, + from: pos, + to: pos + node.nodeSize, + node, + }); + } + }); + return out; +}; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js new file mode 100644 index 0000000000..a512a5fa6a --- /dev/null +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/getBlockTrackedChanges.test.js @@ -0,0 +1,50 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { initTestEditor } from '@tests/helpers/helpers.js'; +import { getBlockTrackedChanges } from './getBlockTrackedChanges.js'; + +describe('getBlockTrackedChanges', () => { + let editor, schema; + beforeEach(() => { + ({ editor } = initTestEditor({ mode: 'text', content: '

' })); + schema = editor.schema; + }); + afterEach(() => editor?.destroy()); + + const trackedRow = (kind, id, operationId) => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + return schema.nodes.tableRow.create({ trackChange: { kind, id, operationId } }, [cell]); + }; + const tableWith = (rows) => schema.nodes.table.create({ sdBlockId: 't1' }, rows); + + it('returns an entry per node with trackChange attr set', () => { + const doc = schema.nodes.doc.create(null, [ + tableWith([trackedRow('delete', 'r1', 'op1'), trackedRow('delete', 'r2', 'op1')]), + ]); + const items = getBlockTrackedChanges({ doc }); + expect(items).toHaveLength(2); + items.forEach((it) => { + expect(it.kind).toBe('delete'); + expect(it.operationId).toBe('op1'); + expect(it.nodeType).toBe('tableRow'); + expect(it.to).toBeGreaterThan(it.from); + }); + expect(items.map((it) => it.id)).toEqual(['r1', 'r2']); + }); + + it('returns [] when no node carries trackChange attr', () => { + expect(getBlockTrackedChanges({ doc: schema.nodes.doc.create(null, [schema.nodes.paragraph.create()]) })).toEqual( + [], + ); + }); + + it('skips defensively when kind is not insert/delete', () => { + const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); + const row = schema.nodes.tableRow.create({ trackChange: { kind: 'format', id: 'r1' } }, [cell]); + expect(getBlockTrackedChanges({ doc: schema.nodes.doc.create(null, [tableWith([row])]) })).toEqual([]); + }); + + it('returns [] for null/undefined input', () => { + expect(getBlockTrackedChanges(null)).toEqual([]); + expect(getBlockTrackedChanges(undefined)).toEqual([]); + }); +}); diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js b/packages/super-editor/src/editors/v1/extensions/track-changes/trackChangesHelpers/trackedTransaction.js index 5903771adf..3ca685216f 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,30 @@ const getPendingDeadKeyPlaceholder = ({ tr, newTr, user }) => { }; }; +/** + * Detects whether a ReplaceStep's slice already carries block-level + * `trackChange` metadata via PM node attributes (e.g. a table whose rows + * have `trackChange.insert` stamped by `applyHunks`). When true, the slice + * should be applied as-is — wrapping its inline content with `trackInsert` + * marks would double-track. + * + * @param {import('prosemirror-model').Slice} slice + * @returns {boolean} + */ +const sliceContainsPreMarkedBlockTrackedChange = (slice) => { + if (!slice || !slice.content || slice.content.size === 0) return false; + let found = false; + slice.content.descendants((node) => { + if (found) return false; + if (node?.attrs?.trackChange?.kind) { + found = true; + return false; + } + return undefined; + }); + return found; +}; + /** * Process a transaction through the track-changes pipeline and return * a modified transaction with tracked-change marks applied (or the @@ -409,6 +433,15 @@ export const trackedTransaction = ({ tr, state, user, replacements = 'paired' }) step = normalizeCompositionInsertStep({ step, doc, tr, user, pendingDeadKeyPlaceholder }); + // Block-level tracked-change replay path: the inserted slice already + // carries row-level tracked metadata via PM node attrs (see applyHunks). + // Wrapping the inner cell content with inline trackInsert marks would + // double-track. Apply such steps as-is. + if (step instanceof ReplaceStep && sliceContainsPreMarkedBlockTrackedChange(step.slice)) { + newTr.step(step); + return; + } + if (step instanceof ReplaceStep) { replaceStep({ state, diff --git a/packages/super-editor/src/editors/v1/index.js b/packages/super-editor/src/editors/v1/index.js index 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 0000000000000000000000000000000000000000..1904c3fd0fcebb5a6ee2054e069d1aca87395f49 GIT binary patch literal 6757 zcmaJ`1z1#Fw;sB?q!}9NloAA_1eB0Y$DxOAq!DQtVE{q8Lz*EZq@=sUkq+q+Q0ju; zfB(V7U$%CobxAPIcc{U=!un$Cnhbf?v%kSnl)?(a)#Os~sRd%eLx-k%7Ym<#> z6UW{-bj2?Sn$p2&c0|QrxIXF1d52q{{{|~9Z*}|FO_T{`Lq-s)a15hqVCw(e;ghi` zTPpU(H#lH~M?;9E;Y=i)QPHKWH9Bfd9Q6S@+4(`ZelDOcgjO?$%d?VP^Zcby_zFi6 zA1e#mR<0~=LZ(&f?73##yFmqYlZQ5F?!B)PKj3ncPG7_LWqIWA6dH~6bjP2Y8epyq zgU_ac?oXyJPHR;}-c%6y>Kvt7IY9M=U=kEMx*KM4G~KjRdW=-^NyL8ZA2>A_T!T3f zPG~mHv9f!`)@;vTL2D_Vx^t}3cvIM-N`BQrdic|;s4hD`vK&)p4F`fdiy`Ck$P@)k zA0*`3KZ~5=d&HGNbK`IDB0Ko!n9wlaV=^3*^q@| zHeLcFN-rb2AS1N%IgIkW**`-%IJr{YtS6U&Qj`eAq|VeNB^o#-&MWAG`a*=pJhMv2 zF1sf)!qJLclV~X_WP-+^ZqmFnOKQRjLQ)~;L)fEH~OdLy^j4B)Z$UAD(PRYqh1wA605CP0Ep|Xw!F7B(-ER77* zr%nv%GONBQ0@A;q`0{<2^-#NU=YC<266a0qsa(H7^lppKYxh%h){+arH#{?6`o$mQ z0X-Wpqk81iUD#V7T?RA)ES``UzbQj&(ZxU7#rKWPmX7IpabAhe$qQN|=?Zp~HEV^XLA6wHw~;B65+j6g&hla(VWJ5y^UEsW z`G(b}w?DmKJG|bdzdMa^S(69PkO2TI%>Q;8{|z`64)(6kUw|Oyzm8*qiv9dwfP?vE z9u_^Jgw`)J%7Ad4J_V4(#bF+Y0L?$w@tSWh&B-uuaw^kYE#UTMbnK45*XfnXOqyB+ z!&Q|TajT6|1laspv|fG*T$COc+^A zn!+Lp4w1(L`hk>%7hBwMEa1Tmrg?EoO4HUKfwW!gxeD=aVIhgQrFHUFEXmy1y^qf} z$wXQ9^^YL>r6E`aQKTUe!g?#5-+Z2pyIAwXzu`>V_prbOBICq(Z#;|FO6WMLGFp_ir?{(UnYKn`Y>f; z0bX5l(#P|O(-D-Lkdbrs*syVQkHE!>g3a;!cd6E&=oJkF2c9@)%XW0Z3TrHiM;D^+ z1DP)T`0xl@_kC5)6}+a=p2>9()!X&$J1^Uo*lBDFiGa-t-l)1G*5ZVck!U6QJz$xb z2s>+!?VamiAe8ws)vfLpgzEk)2od}ngj~HK=D#2ak+f?+$4C06-0Upi=#v^IlwU~P zt@ISQt77PZ#1;-DkFlG@hZ$e(bYcOq*wu64w*CRJzI61Tq`E~M79Qqiw3L8TQ zb1vv%3S3>TVjh0dKJ_W$C`AEXkM7C4SVra^@2yYH52_Y(%Q$CV6{Y5leo=-k3r;6! z1ZhI2qnL^%C{%MZUA4Fnq@DLybcs~N*l9pPlT+Ux@;Y+88B9|hNtQRt6D3xy3K8HD ztkqk@oQ)lmJ4vo{Y_QZwWGp_SG!6`l3Dih7<54m2=27!tWdZv_h_+#{6_KW4NM%6u z*C-e%iQT)f1~Z~KO~d`jp$O3vZ%iw~1jP)((+w^rWu$TJmzp_9#p^9oE{3Dd6ZSv} zZsu<5%(Jz4jh z5Cez*UW1g*`l3^rm$VdkuH^jX;=!)Luc=x`T*owX{jpl0doC>!84TUoM@{zm! z$!>PrnH3VH#+cA&f*0>=6a0P5=3Z4K2S)LwOVl9;>$nR>f=dG*6&r?USv;{X`q0&g z&T?aQOm)JsdXcyGRKyN-v#JJY11ZDnvU}6>zBIKL*e(`i<^uXzj?yjSXYrVtL2Cfi z@OetcUM1<0cnTA~HfJoO1O=|I4~x^&MXkLrI`vs{S}NxKX#~dRxIe8$pUkThk{f2= z-yrweCnWoGz5D1{*U0gJZC0v;A1WuYLE#FMY(A+67<#r^ixHX(#JI@F-Ru1k5# zM97R7*XS!_NXeO!DT=mey4uBk`4?GGp4F@jTl)3jjiH(2m;#7)T4Nm5tKGQ!5crXp z;`5u#!s|$4H_{U_VFt8dfp^k*ggtBtn28StJu`vLtf)lz9`v3(t_%(a zVcZnV)bn$<*rFdjt6f3jyefk<-*efn{u*lLK>yUgK~+3ARZ_XnRKHMs@`HFfJd^2$ zOP=>!1@D1uXZQZ{9xs)AVt^f9y4jiA zyWU~?ZqlIhya-`HM&^FN5+b{>udJgICil=gS36fo^ST z)Cam{Imz@ufa9mDCFja|uN%GlI>Ek&)s6({vDL&I_X5 zHpTrUX~d4^i>hMO=qSv&0`uB1?8ooBDu=Pz8#3|BFjj$Ltr@F9wgy{q~B1qiCh?WY7Qw2{vr-^dgBpcAz{8YI`>6)@kubK6XOVEL%su znxso-i5m0sjw1Wd$qWLy7yC}-nGd|VHDvfMB|5E=M0WMIxQ90-Yw;!Psj}Dll82$d z3yhl1AOur)U$sO<*9&OZ1M-C*9%}INH%`V%!@WEpF-oCZ>Gr@T^h(Oi!N@{qBn6&8-3e-UvqeIG$?)| z6{RBxzpV-L@nT+FN{tqa|IGbrrj;7A1hQx~_zBrJoGTIdewu=FtDKf~@Rk8KrtH9o zaAsnKEkH9zFxw`BF%-IUGTGJfN!#)5rq2;M$;Odl^pN)9;3U3Fl+2iODdv?WREdfW@u@i4-}u5xY~23d9%wLy_)s{@!Cb zgp+_nhQiD4@F4COLAK*%GXwRV0Gey_Db>0zGwEHSOXGy?X1uoi_tiMrX~)gGP78?6 z6=ClmXm=@FZSZ!%^)_WAZ)@&9cLg;6TaZni%^@zlzb7uw-JQ}nHJc7T!jRh*Fl=_Z zYMlN_c&ZvE*zgMq`_Xx*0TWQ|iSI=Oj2RzwJS1s9-0OHSYU(|`M3@6^rAKaNk29fC z*W^Puwp0mXg*vP!#pa=?EEb+X zij;WV(vvx#Z{%zDhfU4jSv>NJ=QxE2Rg~ipPHrAb&z4zx2OC<(kcaQrKvQk_(Y8hy zIYoP=t9vx>^<3bVuuw7z6s&b|cFiWsc^VLBIdvvT;l^tleGm)VU;%Dg5!@U5q8k<| zjB!}C$?r+y=p7L+RhRT2qv`Tj=%9d-?Zex^aNUOQpF@Xp`;Wh(_A64+!xH%Ie7NX~ zB#xmy5^DVcgf?SnKsWg{fW~q@(V(2N=gBG94A9wXb#Z3duuJF8`XH*;vpciMhG8>T zx62xRW2yctlCr)ii^V>)EeK1z#pao4q>=j=Se6mZDZCBlGNYBU`dTNq55v`5;HQ;5 zkNJcUrWX;_xBRj?p%&^^Y1J`K^h#Yk=nJg#*xl|?B}q>&_~ub8iD@`vWFfy`{Y7D+ zH@u`Bd5!zZ^R4tR*2Zl$%vgE5j{CRF?VneA3;KVl1Flx)cILdlKKXxPyS|FUya>%F zf*B76R|oT1oh?+ih?&ayQZ;I3`%pUT@WdJQ>_=nDKGbt70Z1JmUxbp)*oD3@3?!e) zyAYZchtQ6cSA3#NoF+yLI{6u zYFFk)wqI6rX{I}>JJ(JvtfuUP|9P8vQ@$)Q5$Z{$QHLGp^ZWYMcHi0r5-Gf7L7pPZ zjP2O@j1|6t1OX16JKm+WAjNlGvxAZGZPhq-!1Z17?m}H7$|mcj{7yU(H2sd5b{?N)QG(&X!}D4oH*y#d6NkiWEp|73MRL6usnn~Wm_6x-A8-0cgE17({6*A_JDf7L zn$NEzKhgU}TzelOB127+4>h}QuE#AuUH#0XEOcEwE~mdaJUKjekh(bz4MjhH(<(VO5lYUxVGZ>kN}0ipZ;3p61@d;`yPJ)kx~bC;SooD6~)2HCll%DO_15%;WO| zLl5;(L6Qr#DpPq`Z8Hmyvfk%t-9x>loBQB zDTW|TjWh8Ye3zNJ8RlZfknC5it9SzqF<(|+AqW-2H7uz@#ZQ?UFgPMcMRXNdJmIoq zVx36cDjmL|#BQ~7Eo#9dsSMxZEBO*omnOYaD~MQ~Mwd97#j_8cq9uY_qK6xu>h6W3vCT(Lb}nz>($mbFdq9hTKns7y@l0cRmgvFuV!7*!9Jyag?OM5vl2 z64c6D96&GPK|SEF#LkMUhT@SPJ(wCdj#N`(vE(-=u=hWn@-V=gnIWmFdW)9tm6VO7 zlk0QeeLZ}#yhCmYYPE=S@u6F*l21$r9d6g}koLVdwix(2{&9yi&>JnOsAI)g$Fom_ zg-+}_vPXfxvCQ>cHO%-1DdZ7JHqgtHybijs4Oc*CvjV8$crlZC{u=BZ5Gc993`Cm6fHJnXyIs# zXAf!LJ{HsaEV2~B%V3D&pS;Rs5$`+^(BrXT9R8@YU2cZkfq5G?( z9F2h{3>HT=Idr$3#4=gR*%cLnT! zIseX#|8)PIFWwRAzwHO!zuf;TZWr|%+x*k<_sqJZYkwOZ;lHNv oCwcqx5x*afyA=G}qRIc?gj82SyA3S>fOUJx-YUL$_wIiE5BH(JivR!s literal 0 HcmV?d00001 diff --git a/packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx b/packages/super-editor/src/editors/v1/tests/data/diffing/diff_before_table_remove.docx new file mode 100644 index 0000000000000000000000000000000000000000..6d38931b1c5d6b354cfc63ac717502c984e50426 GIT binary patch literal 7235 zcmaJ`1z1#Fw;p21p-W1-9J(6;X;8XLVt}E$r8|Y8QMyGML_$hZx@BdBGZ-?zm?uL~mbArV|0M(E@N8bZib^O!xV2vsjkxg%DYO4#Y? zfZO>JprmZe%-IXogw#Q4WkV4t(5=rq`5lM_fBF{2A;Bt%A>E{}tu?N0q>J*M$LxHX z%1wUi>a1RYH?$JVSMxa2(mq~?8zw}gskLD&N!CM7qD@UAm4f@~`#Z1-rArXgf+G;> z6eqD?YQ>=L9bZrM+>L3C%!|kxQTUrCoXAhll7`&Gs0tMEb#xohEV8)sQzHa3ia>6c zfm!rTa&8Djhe62bb*eu|ypG=0{bttD$2=vDjM?65_RiAY5^a9&rV49s1t~UbA3@O$ zFn$H7(F3f9y^3aJWD^Kn=t<)xQyC7j17O+h$agq*l`?tgLf^OTDc>gH87kODq<6if zL=eF+!)0lOQ&vDm33eDJ&VvI0(vbiF<^Nh@oPU+r+1$nDzQE4EDx9p4F!xa5cp>Gv zAPNLCiI_C0LV6uD&3J^~1e>yvGFoD+hBz}l-Z=ICc!IIT1)~9_ruCHR7ef%4T#O`Y zgg$B{F6#I$br|tQi(i&lP+FC;ac@2)F+UE1VS|xjdJNT+ARCu6;!9pK^XzI(+uYvl zNC!(oRh;GMkO?x~hDr0T9MK7AEUnhaSSBa*g$hZJ^@yn%5|o8k#AJwRY^=BAXM7AZ zTBPh99`z4cvq5jhGaiPHdSV)-RG}dJ!rXpD5=ZjUod9NfN<1e9n!cmgC(CJMcc=MN8-JgozA7KRaS$d>)37j z#RbRN(yfbMG!*$`O`HfDj>;MMNI9s~Pf1FO20kU2;04UkAkvQpEFCD5EsuQpkU24= zMXUI-go+|#;_K-!{gFn~?vvtPS@2!lndE?O%wDTcu-h3Dec2UY3&Yr#V(F65zjwnl zx>xG0Gh=I9&0~J>EDF%NE3CezH3>oV_dZ@h{oncPq4h3& z{rDkPl$QRRy0u_Sx~RF!>%^@$Skx5CC9JlF^u)%s?`&2sr}iqFFz1yF3tL!hZLKV< z`LL<9Xf_tuq**ZL(iYW5-b&jN*RB_r2iB9+xsFU3mgz6> zdi~QY*gklV;$b${Nf`3IfCm69QT}N*{u^*0_I57nFHLOBe@(|E1-tpb00-skJS=8H z7C0b2O1a=NeFnfwNI*FWp)&v6z-GR)JSR>G2FsD%EQ0#7I`_siH2cJ}Q>NC;>J;Uu zK{fj6oD6<<&CHMGRuCRx_LIC$%sX7OIyav;3*l*JP_gqV)OlWttuI|-kuJ+h7Hq|-j_kF^>U=jw-_aRzpXaZrc`m}x_ z4%gI)>cJW?qfMy5-~WOo{F~pM5=7WI`lv{Ua@@~B)v{2b9}|05kPbG ziX8*H?Z8*zLfUg0_(HN1=Ywtkfzyh0nXSqWH?Ns-QK+KZ!g>OC8XURMfIBQ3Wx>|U zeP{Rf7YJpWX1X@qgHXdif)LifLCD3^#{3rqEu`$(t?=WARv7QYt@~wQQx0V3nur_{9+@o9B=0>D*ul#qo-!?&8Z^~0vP5*GCwK9UjjeI?gt92DFYJ@jU| zd;^3XqaD2k-M>D$UWaUh^5IDkUg8hFAx{|EXu`^>=PGeS>AS7*4L6o zl^6fU8C$%s&-prVOsUFA4SOeg@fq`awnI3orqJ|xb6eFTUUyD$3qr@l63lBMlHT|tiP7hi7f87Z~;;aB1xMEHZogn6>u-4!Qux?QfBh=$r{#uAyGgZ zS58$`#aeUf?}1XX^0vOrg5Z^r9ja_`^~b&v{Fyb;v_M60G*b*xar%|!u&va9$`#S9 zHI-b})5!M~{P>h^dyGr)B0INa=cOx1;@%~S>OfCaYWb(dxKCNBunEgmngJgJN>f)g zDxB=%O+a9{tybA}oiuW<7$;A8|2z@fQ^C2cKyx3X{Z$3aIs*R8Oyom7|0UtJT8!|K z0dtU+_mlv?XdIq8nf^ZV4FGLTm!w06ufECHt~B|nx7z;Nrp&6fa%&NvS@4v+n>=pfzMJ?4ZuB>0kjy3ll{#-eJH0}|ME z`5@^?Q~W_na)`|w%7})QHREpT^w5TRaS%FeB+RORDxMXkNF}v?H6inztxQ7oSvdt^ zc)Y!G?W1YdpI;w&y=`QHIrp#`OGzlWdOhofnQU!wz*i(BK00S-Z$O)=t5{493u5;s zeSt~4U4U60EI@Qe1n-;%rAf>+Wv|Wke|ChWdp5--J{q-MJzuIz)AeaOR6Co)Z!(FF z)D08-Bb#XcC)}24Hd;@Oxls}$Gr<_T=jO&-*yU0q*IDy8onHCSO#+Gc9-YPvbYKHl z^DpthD9*w$0FRBS%TP?bfg_E!Ou~hYto5WR6o~;$nB)Zd`U8I{(v~ael?k*M2 zwBM=Od`%@}OAoka*z)L>Uj*y%b$|U?c!Eu~wtSs2kT2QaSC;S7FELpHnLH{4l3x-r zd3|Q%`HF&*B^uR~(p$VLEx7GTBXay&L;sM}Q|MCV)a=@?Y7b%L0`*Qh<_nC5Fxn+{ z^XCdv*4o0S2jHKA0?#gy5-!`VNGo|TzNU2Wm8M%2M~xK-YN2~{Fu86|6eZ4_CyekCl^TVs zH#a9mw+yM(J{O-88k-Lv@P?~|3tClzCzkh3=A z?ITIGV!fs{Ww6N0%eRIE zo0xK(kDWbV%g#Ad;(D$MLAh1Ut)fjPHcCK5w1iU8D~a3CMNoALLky79)COsFNi!P3 z_>2r~IlbO8W(n{rNzRJF8C)+-STD&yQ_D;QICh@2GDnE}1eT$n1+X(#=u`7CaV)#j zo^GY~9YFM1l?8AT^*@I^cZN8@Hnwe<7r_zya0pU`uDKh`NIlPJA0vDR5dr~tngjf7 zGRwit+g8UV0e;SWQqPKH3_zLV8)O=4!>w=fzQMUNG*?<%Z6h%E$#l1CM{CybwC=*R z;?S)6wZ_@rI2F@}`ufq3$MJ6z&#L598a%-#UL1Go-6)2vvDaYCTfd7sRMxM}KQX0k zw1I3nJXshxXJc$H3(!x}Jpf?3`0ba}YC^6~tdsM_DBq&Gur^<*_cJEW!HM_B_8{j!uh>rwvR~>8C%(T#7G$%Ai$F63RHA(N=Gc@FZo55ekBha5} z`K6#Q-6gGw0e(z(y+2#A78r-Vky4-0``_Y^Z2o@cDhNB+)WnA=^jky zjI6)YAm>w|{@6t@>F(QL;sSpGlB}#~<>vDth@Sj#ENk=hLajyJyQQs1$E6veyT~^+ zh7M!s9UBur5^0Pl22n27?EL6G=(~eP$910jxs0bC0Ew;BjJ@YiC1h+b3p47O)yAz3 z_&uI0iSfaAWM|+UhNAZOq2Q+|xaZuMC98YP40|oIN(vCbuN0Z~biTs7j+^+AOZ3)_ zf#-5?{WQSW%||(_-0+@bs7w+?uAQNC=6k@zM8ud5i7vh!q#TU-UUh7JWJ^x3-Yzwy zqbM2w(9G1c%&=|=)|hulIkYYNfN!5+RfP^}A-c&GNF19Ql<&y$hqovc3ge z?s~rCr+GMk7uj@;_5ij$PLdX^Kmb6O;h(^k>|b=t$jRKsneBJTWqr6)o}dKnWXBG< z-{6POPFIgp$VX%2-BsZQj+(*iiFW(B94co97K4Y3`S37PzZ(FgR0!~ zvwNMeWxFRu>d-{X7FJ2ad()vJ{1QMK=@}T-4i*`AQToiOXe7Rbig?fNVnKAFWSQxA zVO1|H{mS%2Q2CEJpDn`mC&NbOsgS3hiA-m8ft3~L*pr(_Vzck9yn^&BVhJM-YU4AZ z9Kh`nYA}DFSWU0$quwh}867b-XVH2$xO+BD(nA+F$FVC(6qKl;|4tx$gN|y`66?{} zSFP|U9^|9yO%4w-2d~IP(S{VhtmbRrL+auIWjjariDAC)RsVgvjDG)*zq0l#Q!&Fr zm>ujOq$NCuuwEgh0e@`h7?8?UY8{}mQiwAsDd+Ly%*+_j)n<8hZqc~+*o~eqy3eC0 zy97$PnXlDtg|xB!;TxQs4nG~_Aig~iRj?KMf%ZOlx98t53&4s)JSOJ57t%i#{R-1GAP{-*cs`Y-Lr#nRl?obA^q$FHue zqhLSJOZEwC#+}l|-h5Vb8__j#rfR-iiImnZ?6Flu@{Drs(=j<8(z#WCxXzC+!w6<< z!(QqI5Y7}_anB0ckdIVUetMkDlrfa2)PPxz#VdDndw?u=d7RW4p5seRyY4ve#5%?N z%p~rL8*^@IPy9||KtgqSrYE{5-&QHS_We7*i+1zoLJ3?P#2;1qowi{0CptB@TkV|5 zM4l2R9=t2m9cYErmA(O3{`OtFUgh;BGO68!k&cKq_A$~EG?wwzwNpi3yg{udbmPFd z2m`jnVl^{uzhODQwP$W=Pp?wzSmIYtg5U(y2~D)0c#PCTXM3ur+2-kR)D$dvKE66* z+C+o*t9UyaVXTcSc=LU`U@*Z?WaCr#&Z*bJTeaf>z|2XTss-HqZbRk}tO%(;EULMQ zr(x>d3eRl}@ETe)?x+R6=mst7w>3b)gE3yf#mwaesT(2Eg?)M8!O$TX{o=M@G6_PE z6(AwK+5&VT8Rq1qGwYh=p=kylPI1FQ81)9;XNqbLII(n`f7nEzL}%6HmWC$8f%K)ezM00w;lyj}H55jo-hr|EVj>^!21bLBWzPM0 zl?^%_v(;KIZlgX?_(tA(9WO+M8KoVm_T1f$TYS3tSwLLuvUE~Gad-6N=)_+1?j$S> z=_0gEl3X+!=k+GNz*8Y%_)ZXfuX1LL3ARb8noUsNG%k;YDm zkP{Dg|IDZ7t{ldNccoNqBqgC?3^9?@{v4xqq`ll*%n_R!+Z>=_sp{lT-5l9j5Gacr z>^N}}0i#dXGf`DJ7p%o}o@tn&Eu{|0^`>9L_|h2rbDy)wSKNuDQF~A1+Q&f#* zyKp@0RWn?8ok~nv`47QmA$2f!H?aa^=lbu@*^>Obp;1?tPLiK&l5MJWC|$*4VC_~R zJghBm*%^@Es-~}|7c}tu}6gZ<6&W zndd6AoAjvAo1Si{r>_pkFdsG9wXzK&uPxVT*w^jtl$$4Mw?Yk7fxkAJvtmbIsb4So z+LmbTIJBbStSP|bp4Lu)6A=%^8(=T$f2?Fw(8YDLW5;#cdHKlLlD=p>chu=tl4bXD zBw;k%1KQVbqbs~>D%EATNs&!O8buEB;yc+rE(v~al=U2O&8V$bBkiV_}Q zTd=mT2*L9jIO~;dNYu{$*c*#iWlBrQ0b^{IU)4Qq+=R&w$-_mToO>chc;slnj2ih7 z-{_?~GL`x5;7{n{9-IgZL5J*^9nX$d3>RkKw@>dMy{{)yn3&q9nzdPuV-#&huI5|z z;(}a8DjFwa)k{GR{*N2}Zg1qNv-HZGT#Bb+nU;dH#eC1UjCUU|EG3?ci(RGB=1$f`@Nr$ zx~GG=v%y2Nse>s!|3yXG$f@mBR+E7e#a2X5Q`&OL+Ugj}L+;knU}tBFW4yQ;ud(Bm z%r*v_&tq6T1s6TFY^d^La&Qx!n9)!KNl2%XJoH$l-E*I+wlfkTsGDFhey=Ipa>@dr z+hJ-)V5odd!97K4?%9&I+~$H>0(#zV%^RqxA@@LX3L3!=+DioZvyF-~No8pWRc z6eBRC2XQdKu(RpdIT6tQ%)1=IMyZG3m$pU&NpzaHoW` z`>=)o+g9&K{N?}qxx%0BzpI1)IlSY2u)o~@LkRxU|95TSK|}o8Ebk}WU&H@fQT)^S zcdq=Ou|v`R*ZFs5{HOcxeDQ%$|83Lvq5K;AztHPHpY%H)d*GLU8}5Bvzp>3f9e-b0 z4|MHsyS{h)6~dq7?axR2J~J`i$_l{y)B*sg_m{-Ip1(u<@aum7ku;+z literal 0 HcmV?d00001 From ff9db8d064609f84f09ef4249f1cdaf8132ff8a6 Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 13:05:05 +0100 Subject: [PATCH 02/15] fix(track-changes): address PR review on block-level structural tracked changes - Move painter-targeting CSS into BLOCK_TRACK_CHANGE_STYLES in the painter's styles module so document visuals live behind the painter boundary; keep contenteditable-side rules scoped under .sd-editor-scoped .ProseMirror tr. - Drop stale isBlockLevel entries from previous state before merging in freshly-computed block entries, so accept/reject clears the bubble. - renderDOM now emits data-track-change-id and data-track-change-operation alongside data-track-change so HTML round-trips preserve enough state for getBlockTrackedChanges and acceptTrackedChangeById to resolve the row. Adds regression tests: stale-entry pruning in comments-plugin apply(), renderDOM/parseDOM round-trip in blockTrackedChangeAttr, and a styles.ts guard that fails if a bare [data-track-change=...] selector regresses. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../painters/dom/src/styles.test.ts | 15 ++++++++ .../v1/assets/styles/elements/prosemirror.css | 35 ++++++++----------- .../comment/blockTrackedChangeBubble.test.js | 27 ++++++++++++++ .../v1/extensions/comment/comments-plugin.js | 14 +++++--- .../track-changes/blockTrackedChangeAttr.js | 9 ++++- .../blockTrackedChangeAttr.test.js | 27 ++++++++++++-- 6 files changed, 100 insertions(+), 27 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/styles.test.ts b/packages/layout-engine/painters/dom/src/styles.test.ts index 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 10d25ecb4a..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 @@ -307,35 +307,30 @@ https://github.com/ProseMirror/prosemirror-tables/blob/master/demo/index.html /* Track changes - end */ /* - * Block-level tracked changes (row granularity for v1). + * Block-level tracked changes (row granularity for v1) — contenteditable path. * - * The `data-track-change` attribute is stamped in two places: - * 1. ProseMirror's contenteditable via the schema's renderDOM - * (hidden in layout-engine mode but kept consistent). - * 2. The DomPainter's per-cell
elements (no exists in layout - * mode; cells are absolutely positioned). Every cell of a tracked row - * carries the attribute → a continuous red/green strip across the row. - * - * Selectors are intentionally minimal so the rule applies wherever the painter - * places the cell — across re-renders and table-fragment boundaries. + * The data-track-change attribute is stamped on PM's via the schema's + * renderDOM. The painter-side stamping (per-cell
in layout mode) is + * styled by BLOCK_TRACK_CHANGE_STYLES in + * packages/layout-engine/painters/dom/src/styles.ts to keep document visuals + * inside the painter's stylesheet boundary. */ -[data-track-change='delete'] { - background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)) !important; - outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47) !important; - outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px) !important; +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] { + background-color: var(--sd-block-tracked-deleted-bg, rgba(203, 14, 71, 0.18)); + outline: var(--sd-block-tracked-deleted-outline-width, 2px) dashed var(--sd-block-tracked-deleted-outline, #cb0e47); + outline-offset: var(--sd-block-tracked-deleted-outline-offset, -1px); } .sd-editor-scoped .ProseMirror tr[data-track-change='delete'] td, -.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th, -.superdoc-table-fragment [data-track-change='delete'] { +.sd-editor-scoped .ProseMirror tr[data-track-change='delete'] th { text-decoration: line-through; text-decoration-color: var(--sd-block-tracked-deleted-outline, #cb0e47); } -[data-track-change='insert'] { - background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)) !important; - outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d) !important; - outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px) !important; +.sd-editor-scoped .ProseMirror tr[data-track-change='insert'] { + background-color: var(--sd-block-tracked-inserted-bg, rgba(57, 156, 114, 0.18)); + outline: var(--sd-block-tracked-inserted-outline-width, 2px) dashed var(--sd-block-tracked-inserted-outline, #00853d); + outline-offset: var(--sd-block-tracked-inserted-outline-offset, -1px); } /* Block-level tracked changes - end */ diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js index a453195e06..40363f61ee 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -50,4 +50,31 @@ describe('comments-plugin — block-level tracked-change registration', () => { expect(tracked['OP1']).toBeUndefined(); expect(tracked['r1']).toBeUndefined(); }); + + it('drops a block-level entry once its row is no longer tracked (resolved op)', () => { + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); + installDoc([table]); + expect(CommentsPluginKey.getState(editor.state).trackedChanges['OP1']).toBeDefined(); + + // Walk the doc to find the tracked row, then clear its trackChange attr + // through a transaction. This exercises the apply() reducer's stale-entry + // pruning rather than recomputing state from init(). + let rowPos = null; + let rowNode = null; + editor.state.doc.descendants((node, pos) => { + if (node.type.name === 'tableRow' && node.attrs.trackChange) { + rowPos = pos; + rowNode = node; + return false; + } + return true; + }); + expect(rowPos).not.toBeNull(); + + const tr = editor.state.tr.setNodeMarkup(rowPos, undefined, { ...rowNode.attrs, trackChange: null }); + editor.view.dispatch(tr); + + const pluginState = CommentsPluginKey.getState(editor.state); + expect(pluginState.trackedChanges['OP1']).toBeUndefined(); + }); }); diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index c8548aae33..54dbafa669 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 @@ -583,10 +583,16 @@ export const CommentsPlugin = Extension.create({ isBlockLevel: true, }; } - // Merge: keep existing inline entries, add block-level entries. - // Block-level keys (operationId / row-id) shouldn't collide with - // inline ones (mark-id), but if they ever do, inline wins. - pluginState.trackedChanges = { ...blockTracked, ...pluginState.trackedChanges }; + // Drop stale block-level entries from the previous state first. + // getBlockTrackedChanges is the source of truth for block-level + // tracking, so anything no longer in the doc (accepted/rejected) + // must not leak back via the merge. Inline entries are owned by + // handleTrackedChangeTransaction and are kept as-is. + const previousInline = {}; + for (const [k, v] of Object.entries(pluginState.trackedChanges || {})) { + if (!v?.isBlockLevel) previousInline[k] = v; + } + pluginState.trackedChanges = { ...previousInline, ...blockTracked }; } // Check for changes in the actively selected comment diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js index a893c77bf2..5106987f92 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -20,7 +20,14 @@ export const blockTrackedChangeAttrSpec = { }, renderDOM: (attrs) => { const tc = attrs?.trackChange; - return tc ? { 'data-track-change': tc.kind } : {}; + if (!tc) return {}; + const out = { 'data-track-change': tc.kind }; + // Emit id + operationId so HTML round-trips (clipboard, getHTML/setContent, + // collaboration patches) preserve enough to resolve the change via + // getBlockTrackedChanges and accept/reject it. + if (tc.id) out['data-track-change-id'] = tc.id; + if (tc.operationId) out['data-track-change-operation'] = tc.operationId; + return out; }, }, }; diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js index 629fa391b7..1e1221e1d4 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.test.js @@ -26,13 +26,36 @@ describe('blockTrackedChangeAttrSpec', () => { expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toBeNull(); }); - it('renderDOM emits data-track-change when set; nothing when null', () => { + it('renderDOM emits kind, id, and operationId when present', () => { expect( blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: { kind: 'insert', id: 'r1', operationId: 'op1' }, }), - ).toEqual({ 'data-track-change': 'insert' }); + ).toEqual({ + 'data-track-change': 'insert', + 'data-track-change-id': 'r1', + 'data-track-change-operation': 'op1', + }); + }); + + it('renderDOM omits optional id / operationId when missing', () => { + expect( + blockTrackedChangeAttrSpec.trackChange.renderDOM({ + trackChange: { kind: 'delete' }, + }), + ).toEqual({ 'data-track-change': 'delete' }); + }); + + it('renderDOM emits nothing when trackChange is null or absent', () => { expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: null })).toEqual({}); expect(blockTrackedChangeAttrSpec.trackChange.renderDOM({})).toEqual({}); }); + + it('renderDOM and parseDOM round-trip preserve id and operationId', () => { + const original = { kind: 'delete', id: 'row-42', operationId: 'op-99' }; + const rendered = blockTrackedChangeAttrSpec.trackChange.renderDOM({ trackChange: original }); + const el = document.createElement('tr'); + for (const [k, v] of Object.entries(rendered)) el.setAttribute(k, v); + expect(blockTrackedChangeAttrSpec.trackChange.parseDOM(el)).toEqual(original); + }); }); From b63975cd2a32ff8434a5c5a5874d756f58dd10ef Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 14:25:20 +0100 Subject: [PATCH 03/15] feat(track-changes): register StructuralTrackChanges in getStarterExtensions Consumers (Superdoc Vue host, @superdoc-dev/react wrapper) take their extension list from getStarterExtensions(); without this entry the new block-level extension was exported but never loaded into the editor. Updates the dedicated test that previously asserted opt-in-only behavior. Co-Authored-By: Claude Opus 4.7 (1M context) --- packages/super-editor/src/editors/v1/extensions/index.js | 3 +++ .../structural-track-changes/structural-track-changes.test.js | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/index.js b/packages/super-editor/src/editors/v1/extensions/index.js index 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/structural-track-changes.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js index c675cddcb6..60b89077bb 100644 --- a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js @@ -58,8 +58,8 @@ describe('StructuralTrackChanges extension', () => { expect(getBlockTrackedChanges(editor.state)).toHaveLength(0); }); - it('is NOT registered in default starter extensions (opt-in only)', () => { + it('is registered in default starter extensions', () => { const names = getStarterExtensions().map((e) => e.name); - expect(names).not.toContain('structuralTrackChanges'); + expect(names).toContain('structuralTrackChanges'); }); }); From dad620035a10bad222d108bbdf8432d94be55c4f Mon Sep 17 00:00:00 2001 From: Shri H Date: Sat, 16 May 2026 22:19:51 +0100 Subject: [PATCH 04/15] fix(track-changes): skip block-level walk for docs without tracked rows The comments-plugin apply() reducer walked the entire doc for block-level tracked rows on every tr.docChanged, even on docs that had no tracked rows. Combined with in-place mutation of pluginState.trackedChanges and the view.update decoration rebuild path, this created a stream of state references that downstream consumers reacted to, contributing to focus steal in side-by-side layouts where the editor sat next to an unrelated input control. Two structural changes: - Track a hasBlockChanges bit on plugin state, computed once at init and refreshed only when a walk runs. Gate the block walk on (hasBlockChanges OR inputType='acceptReject' meta) so typing in a doc with no tracked rows never triggers the walk. - Stop mutating pluginState.trackedChanges and pluginState.activeThreadId inside apply(); compute next-values locally and return them via the spread at the end. Plugin state must be treated as immutable across apply() per ProseMirror convention. Adds regression tests that lock the gate (hasBlockChanges stays false through typing, flips true once a tracked row appears). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../comment/blockTrackedChangeBubble.test.js | 23 ++++++++ .../v1/extensions/comment/comments-plugin.js | 54 +++++++++++++------ 2 files changed, 62 insertions(+), 15 deletions(-) diff --git a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js index 40363f61ee..3e9b6f69cb 100644 --- a/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js +++ b/packages/super-editor/src/editors/v1/extensions/comment/blockTrackedChangeBubble.test.js @@ -51,6 +51,29 @@ describe('comments-plugin — block-level tracked-change registration', () => { expect(tracked['r1']).toBeUndefined(); }); + it('hasBlockChanges starts false on a doc without tracked rows and stays false through typing', () => { + installDoc([schema.nodes.paragraph.create(null, schema.text('hello'))]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(false); + + // Simulate a typing transaction; the gate should keep the block walk + // skipped so trackedChanges remains an empty stable reference. + const before = CommentsPluginKey.getState(editor.state).trackedChanges; + const tr = editor.state.tr.insertText('!', editor.state.doc.content.size - 1); + editor.view.dispatch(tr); + const after = CommentsPluginKey.getState(editor.state); + expect(after.hasBlockChanges).toBe(false); + expect(after.trackedChanges).toEqual(before); + }); + + it('hasBlockChanges flips to true once a tracked row appears', () => { + installDoc([schema.nodes.paragraph.create(null, schema.text('x'))]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(false); + + const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); + installDoc([table]); + expect(CommentsPluginKey.getState(editor.state).hasBlockChanges).toBe(true); + }); + it('drops a block-level entry once its row is no longer tracked (resolved op)', () => { const table = schema.nodes.table.create({ sdBlockId: 't1' }, [makeRow('delete', 'r1', 'OP1')]); installDoc([table]); diff --git a/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index 54dbafa669..a8ec23279b 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 @@ -487,6 +487,7 @@ export const CommentsPlugin = Extension.create({ // initial doc (e.g. when a docx is loaded with pre-marked rows or // when EditorState is reconstructed via setState during tests). const trackedChanges = {}; + let hasBlockChanges = false; if (editorState?.doc) { const blockEntries = getBlockTrackedChanges(editorState); const seenOperations = new Set(); @@ -501,6 +502,7 @@ export const CommentsPlugin = Extension.create({ range: { from: entry.from, to: entry.to }, isBlockLevel: true, }; + hasBlockChanges = true; } } return { @@ -511,6 +513,7 @@ export const CommentsPlugin = Extension.create({ allCommentPositions: {}, allCommentIds: [], trackedChanges, + hasBlockChanges, }; }, @@ -550,24 +553,39 @@ export const CommentsPlugin = Extension.create({ }; } - // If this is a tracked change transaction, handle separately + // If this is a tracked change transaction, handle separately. + // Compute the next trackedChanges value without mutating prev state; + // ProseMirror plugin state must be treated as immutable across apply(). const trackedChangeMeta = tr.getMeta(TrackChangesBasePluginKey); - const currentTrackedChanges = pluginState.trackedChanges; + let nextTrackedChanges = pluginState.trackedChanges; + let nextHasBlockChanges = pluginState.hasBlockChanges; if (trackedChangeMeta) { - pluginState.trackedChanges = handleTrackedChangeTransaction( + nextTrackedChanges = handleTrackedChangeTransaction( trackedChangeMeta, - currentTrackedChanges, + pluginState.trackedChanges, newEditorState, editor, ); } - // Block-level tracked changes don't fire TrackChangesBasePluginKey meta - // (they're applied as PM node attrs via setNodeMarkup). On any doc - // change, refresh the block-level entries in trackedChanges. Rows that - // share an operationId collapse into a single entry so a multi-row - // delete surfaces as one bubble. - if (tr.docChanged || trackedChangeMeta) { + // Block-level tracked changes don't fire TrackChangesBasePluginKey + // meta (they're applied as PM node attrs via setNodeMarkup). When the + // doc changes we may need to refresh the block-level entries. But we + // only walk the doc when: + // - the previous state already had block changes (so they may have + // been resolved or shifted), or + // - this transaction is a structural change stamping new rows + // (applyHunks / accept-reject row commands set + // inputType='acceptReject'). + // Skipping the walk on every typing transaction in a doc with no + // block-level changes avoids re-creating trackedChanges references + // and triggering downstream view rebuilds that can re-sync DOM + // selection. + const inputTypeMeta = tr.getMeta('inputType'); + const isBlockStampingTr = inputTypeMeta === 'acceptReject'; + const shouldWalkBlock = + (tr.docChanged || trackedChangeMeta) && (pluginState.hasBlockChanges || isBlockStampingTr); + if (shouldWalkBlock) { const blockEntries = getBlockTrackedChanges(newEditorState); const blockTracked = {}; const seenOperations = new Set(); @@ -589,13 +607,15 @@ export const CommentsPlugin = Extension.create({ // must not leak back via the merge. Inline entries are owned by // handleTrackedChangeTransaction and are kept as-is. const previousInline = {}; - for (const [k, v] of Object.entries(pluginState.trackedChanges || {})) { + for (const [k, v] of Object.entries(nextTrackedChanges || {})) { if (!v?.isBlockLevel) previousInline[k] = v; } - pluginState.trackedChanges = { ...previousInline, ...blockTracked }; + nextTrackedChanges = { ...previousInline, ...blockTracked }; + nextHasBlockChanges = Object.keys(blockTracked).length > 0; } // Check for changes in the actively selected comment + let nextActiveThreadId = pluginState.activeThreadId; if (!tr.docChanged && tr.selectionSet) { const { selection } = tr; @@ -623,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, @@ -635,7 +654,12 @@ export const CommentsPlugin = Extension.create({ } } - return { ...pluginState }; + return { + ...pluginState, + trackedChanges: nextTrackedChanges, + hasBlockChanges: nextHasBlockChanges, + activeThreadId: nextActiveThreadId, + }; }, }, }; From e94d426970620ac62faa8db3493cd431fdea7559 Mon Sep 17 00:00:00 2001 From: Shri H Date: Mon, 18 May 2026 09:34:21 +0100 Subject: [PATCH 05/15] fix(presentation-editor): scope stale-target redirect to SuperDoc-owned editors Track which DOMs the bridge has owned and only redirect keystrokes to a stale ProseMirror[contenteditable=true] candidate if it is in that set. Without this guard, an unrelated PM-based editor (Tiptap, Remirror) mounted alongside SuperDoc would receive redirected input. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../input/PresentationInputBridge.ts | 21 ++++++++++ .../tests/PresentationInputBridge.test.ts | 42 +++++++++++++++++++ 2 files changed, 63 insertions(+) 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..85a5aed783 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 @@ -28,6 +28,15 @@ export class PresentationInputBridge { #onTargetChanged?: (target: HTMLElement | null) => void; #listeners: Array<{ type: string; handler: EventListener; target: EventTarget; useCapture: boolean }>; #currentTarget: HTMLElement | null = null; + /** + * DOMs that this bridge has ever owned as a target (main editor, footnote, + * header, footer, etc.). Used by #resolveStaleEditorOrigin to ignore + * `.ProseMirror[contenteditable="true"]` elements that belong to *another* + * editor on the page (e.g. a Tiptap chat composer next to SuperDoc). + * Stale-recovery should only redirect when the candidate is a SuperDoc + * editor this bridge knows about. + */ + #ownedEditorDoms: WeakSet = new WeakSet(); #destroyed = false; #useWindowFallback: boolean; @@ -143,6 +152,7 @@ export class PresentationInputBridge { } } this.#currentTarget = nextTarget; + if (nextTarget) this.#ownedEditorDoms.add(nextTarget); this.#onTargetChanged?.(nextTarget ?? null); } @@ -177,6 +187,7 @@ export class PresentationInputBridge { } this.#currentTarget = target; + this.#ownedEditorDoms.add(target); try { const canceled = !target.dispatchEvent(synthetic) || synthetic.defaultPrevented; if (canceled) { @@ -195,6 +206,7 @@ export class PresentationInputBridge { if (!target) return null; const isConnected = (target as { isConnected?: boolean }).isConnected; if (isConnected === false) return null; + this.#ownedEditorDoms.add(target); return target; } @@ -305,6 +317,15 @@ export class PresentationInputBridge { return null; } + // The candidate must be a SuperDoc editor this bridge has owned at some + // point (main editor, footnote, header, footer). Without this check the + // bridge redirects keystrokes from any unrelated ProseMirror editor on + // the page (Tiptap, Remirror, raw PM) because they all carry the same + // .ProseMirror[contenteditable="true"] signature. + if (!this.#ownedEditorDoms.has(staleEditorTarget)) { + 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..f4320ebab6 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,6 +264,18 @@ describe('PresentationInputBridge - Context Menu Handling', () => { expect(forwardedEvents).toEqual(['beforeinput']); }); + // Simulate a "footnote/header was once active, now the main editor is + // active and the previous editor still has native focus" flow. Returning + // the stale editor briefly via getTargetDom causes the bridge to register + // it in its owned-editors set; switching back makes it a stale candidate. + const registerAsPreviouslyOwned = (staleEditor: HTMLElement) => { + (getTargetDom as unknown as { mockReturnValueOnce: (value: HTMLElement) => void }).mockReturnValueOnce( + staleEditor, + ); + bridge.notifyTargetChanged(); + bridge.notifyTargetChanged(); + }; + 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'; @@ -285,6 +297,7 @@ describe('PresentationInputBridge - Context Menu Handling', () => { useWindowFallback: true, }); bridge.bind(); + registerAsPreviouslyOwned(staleBodyEditor); staleBodyEditor.dispatchEvent(staleEvent); @@ -319,6 +332,7 @@ describe('PresentationInputBridge - Context Menu Handling', () => { useWindowFallback: true, }); bridge.bind(); + registerAsPreviouslyOwned(staleBodyEditor); staleBodyEditor.dispatchEvent(staleEvent); @@ -332,6 +346,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'; From 1f65eb4d8529e316f457ceab4635a0c05ab671b5 Mon Sep 17 00:00:00 2001 From: Shri H Date: Mon, 18 May 2026 10:41:21 +0100 Subject: [PATCH 06/15] feat(track-changes): handle block-level changes in accept/rejectAll and export structural API acceptAllTrackedChanges and rejectAllTrackedChanges now also dispatch acceptAllStructuralChanges / rejectAllStructuralChanges when block-level row entries exist, so the single accept-all/reject-all surface resolves both inline marks and table-row tracked changes. Without this, a table-delete operation would leave the row, cell, and table shell in place after accept-all. Also re-export StructuralTrackChanges and computeStructuralDiff from the superdoc package entry so SDK consumers can wire up structural diffs without importing from super-editor directly. git commit -F /tmp/cmsg2.txt --- .../structural-track-changes.test.js | 27 +++++++++++++++++++ packages/superdoc/src/index.js | 7 +++++ 2 files changed, 34 insertions(+) diff --git a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js index 60b89077bb..62646ee499 100644 --- a/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js +++ b/packages/super-editor/src/editors/v1/extensions/structural-track-changes/structural-track-changes.test.js @@ -62,4 +62,31 @@ describe('StructuralTrackChanges extension', () => { 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/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, }; From 954ee84cc73300e0673480a7b5b7950d32bacc94 Mon Sep 17 00:00:00 2001 From: Shri H Date: Mon, 18 May 2026 17:25:55 +0100 Subject: [PATCH 07/15] fix(presentation-editor): scope stale-target check via .sd-editor-scoped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WeakSet-based ownership check broke header/footer/footnote/endnote accept/reject + undo (6 behavior tests in story-surface-tracked-change-decide): story editors weren't registered in the bridge's owned set when sidebar operations were the first interaction, so the bridge dropped the stale-target redirect that those flows depend on. Switch the check to a class-based ancestor lookup (`closest('.sd-editor-scoped')`). Every SuperDoc editor — including header/footer/footnote/endnote — is rendered inside a `.sd-editor-scoped` container, so they're recognized without needing prior bridge activation; foreign PM editors (Tiptap, Remirror, raw PM) are not, so the focus-steal fix is preserved. Bridge unit tests updated to wrap stale editors in a `.sd-editor-scoped` ancestor (matches production DOM); foreign-editor regression test unchanged (its `.tiptap.ProseMirror` element has no SuperDoc ancestor and is correctly ignored). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../input/PresentationInputBridge.ts | 26 +++++--------- .../tests/PresentationInputBridge.test.ts | 34 ++++++++----------- 2 files changed, 22 insertions(+), 38 deletions(-) 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 85a5aed783..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 @@ -28,15 +28,6 @@ export class PresentationInputBridge { #onTargetChanged?: (target: HTMLElement | null) => void; #listeners: Array<{ type: string; handler: EventListener; target: EventTarget; useCapture: boolean }>; #currentTarget: HTMLElement | null = null; - /** - * DOMs that this bridge has ever owned as a target (main editor, footnote, - * header, footer, etc.). Used by #resolveStaleEditorOrigin to ignore - * `.ProseMirror[contenteditable="true"]` elements that belong to *another* - * editor on the page (e.g. a Tiptap chat composer next to SuperDoc). - * Stale-recovery should only redirect when the candidate is a SuperDoc - * editor this bridge knows about. - */ - #ownedEditorDoms: WeakSet = new WeakSet(); #destroyed = false; #useWindowFallback: boolean; @@ -152,7 +143,6 @@ export class PresentationInputBridge { } } this.#currentTarget = nextTarget; - if (nextTarget) this.#ownedEditorDoms.add(nextTarget); this.#onTargetChanged?.(nextTarget ?? null); } @@ -187,7 +177,6 @@ export class PresentationInputBridge { } this.#currentTarget = target; - this.#ownedEditorDoms.add(target); try { const canceled = !target.dispatchEvent(synthetic) || synthetic.defaultPrevented; if (canceled) { @@ -206,7 +195,6 @@ export class PresentationInputBridge { if (!target) return null; const isConnected = (target as { isConnected?: boolean }).isConnected; if (isConnected === false) return null; - this.#ownedEditorDoms.add(target); return target; } @@ -317,12 +305,14 @@ export class PresentationInputBridge { return null; } - // The candidate must be a SuperDoc editor this bridge has owned at some - // point (main editor, footnote, header, footer). Without this check the - // bridge redirects keystrokes from any unrelated ProseMirror editor on - // the page (Tiptap, Remirror, raw PM) because they all carry the same - // .ProseMirror[contenteditable="true"] signature. - if (!this.#ownedEditorDoms.has(staleEditorTarget)) { + // 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; } 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 f4320ebab6..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,23 +264,22 @@ describe('PresentationInputBridge - Context Menu Handling', () => { expect(forwardedEvents).toEqual(['beforeinput']); }); - // Simulate a "footnote/header was once active, now the main editor is - // active and the previous editor still has native focus" flow. Returning - // the stale editor briefly via getTargetDom causes the bridge to register - // it in its owned-editors set; switching back makes it a stale candidate. - const registerAsPreviouslyOwned = (staleEditor: HTMLElement) => { - (getTargetDom as unknown as { mockReturnValueOnce: (value: HTMLElement) => void }).mockReturnValueOnce( - staleEditor, - ); - bridge.notifyTargetChanged(); - bridge.notifyTargetChanged(); + // 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', @@ -297,7 +296,6 @@ describe('PresentationInputBridge - Context Menu Handling', () => { useWindowFallback: true, }); bridge.bind(); - registerAsPreviouslyOwned(staleBodyEditor); staleBodyEditor.dispatchEvent(staleEvent); @@ -313,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,7 +327,6 @@ describe('PresentationInputBridge - Context Menu Handling', () => { useWindowFallback: true, }); bridge.bind(); - registerAsPreviouslyOwned(staleBodyEditor); staleBodyEditor.dispatchEvent(staleEvent); From 2738c4a7131fe9b397fd79ee4333f2682b947918 Mon Sep 17 00:00:00 2001 From: Shri H Date: Mon, 18 May 2026 17:29:03 +0100 Subject: [PATCH 08/15] chore(types): refresh SD-3176 snapshot for StructuralTrackChanges export Adds StructuralTrackChanges + computeStructuralDiff to the SD-3176 legacy-exports snapshot (intentional growth on superdoc/super-editor). The named export is what consumers will import via: import { StructuralTrackChanges, computeStructuralDiff } from "superdoc"; Co-Authored-By: Claude Opus 4.7 (1M context) --- tests/consumer-typecheck/snapshots/superdoc-super-editor.txt | 2 ++ 1 file changed, 2 insertions(+) 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 From ca99505646f5ad6c9c5face2dbacadfbc9a042ad Mon Sep 17 00:00:00 2001 From: Shri H Date: Thu, 28 May 2026 00:42:52 +0100 Subject: [PATCH 09/15] fix(track-changes): mark full inserted range when PM auto-wraps text When insertTrackedChange is called with from === to at a position where bare text can't sit (e.g. at doc end past the last block), PM auto-wraps the inserted text in the schema's required parents (paragraph + run). The previous mark range used insertedNode.nodeSize -- the bare text length -- which then covered the wrapper's open tokens instead of the trailing characters of the actual text. Those trailing chars escaped the trackInsert mark and survived accept/reject as orphan text nodes (ALPMO-245). Capture the doc size delta around the insert step and use it as the mark range. addMark only attaches to text nodes inside the range, so marking wrapper boundaries is a no-op there but pulls in the trailing chars that PM's auto-wrap pushed out of the bare-nodeSize window. Co-Authored-By: Claude Opus 4.7 --- .../track-changes-extension.test.js | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) 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'); + }); }); }); From fff5326beddddfa238562f0379d6de311e93ba0e Mon Sep 17 00:00:00 2001 From: Shri H Date: Thu, 28 May 2026 00:49:05 +0100 Subject: [PATCH 10/15] fix(layout): invalidate row caches on tableRow.trackChange attr changes Block-level diff replay sets `trackedChange` on a tableRow via the StructuralTrackChanges extension. The three caches that drive painted output -- the painter's per-page fingerprint in renderer.ts, the measure cache in layout-bridge, and the canonical block version in layout-resolved -- previously hashed only the cells, missing the row-level attribute. Result: applyHunks-style transactions that only mutated `row.trackChange` reused stale cache entries, the page never re-measured or repainted, and visible cells never received their `data-track-change` attribute (so the row stayed unmarked until a full scroll forced a remeasure). Mirror the row.trackedChange fingerprint into all three caches so the same transaction invalidates each one. --- .../layout-engine/layout-bridge/src/cache.ts | 12 + .../layout-resolved/src/versionSignature.ts | 11 + .../painters/dom/src/renderer.ts | 735 ++++++++++++++++++ 3 files changed, 758 insertions(+) diff --git a/packages/layout-engine/layout-bridge/src/cache.ts b/packages/layout-engine/layout-bridge/src/cache.ts index f2efb115ff..b86c585af1 100644 --- a/packages/layout-engine/layout-bridge/src/cache.ts +++ b/packages/layout-engine/layout-bridge/src/cache.ts @@ -286,6 +286,18 @@ const hashRuns = (block: FlowBlock): string => { continue; } + // Row-level tracked change (block-level diff replay sets `trackedChange` + // on a tableRow). Without this, applyHunks-style transactions that only + // mutate the row's `trackChange` attr don't invalidate the measure + // cache, so the page is never re-measured and the visible cells never + // receive their `data-track-change` attribute. Mirror of the painter + // page-fingerprint fix in renderer.ts. + if (row.trackedChange) { + cellHashes.push( + `rtc:${row.trackedChange.kind ?? ''}:${row.trackedChange.id ?? ''}:${row.trackedChange.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..861e685b08 100644 --- a/packages/layout-engine/layout-resolved/src/versionSignature.ts +++ b/packages/layout-engine/layout-resolved/src/versionSignature.ts @@ -488,6 +488,17 @@ 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). Without this, applyHunks-style transactions that only + // mutate the row's `trackChange` attr don't bump the block version, so + // the resolved-layout pipeline reuses cached entries and the painter + // never receives row data with `trackedChange` populated. Mirror of the + // fixes in renderer.ts and layout-bridge/cache.ts. + if (row.trackedChange) { + hash = hashString(hash, row.trackedChange.kind ?? ''); + hash = hashString(hash, row.trackedChange.id ?? ''); + hash = hashString(hash, row.trackedChange.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/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index e5ef3ea5ca..6ad6264dd4 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -4250,3 +4250,738 @@ const isNonBodyStoryBlockId = (blockId: string | undefined): boolean => blockId.startsWith('endnote-') || blockId.startsWith('__sd_semantic_footnote-') || blockId.startsWith('__sd_semantic_endnote-')); +const getSdtMetadataId = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + if ('id' in metadata && metadata.id != null) { + return String(metadata.id); + } + return ''; +}; + +const getSdtMetadataLockMode = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + return metadata.type === 'structuredContent' ? (metadata.lockMode ?? '') : ''; +}; + +const getSdtMetadataVersion = (metadata: SdtMetadata | null | undefined): string => { + if (!metadata) return ''; + return [metadata.type, getSdtMetadataLockMode(metadata), getSdtMetadataId(metadata)].join(':'); +}; + +/** + * Type guard to validate list marker attributes structure. + * + * @param attrs - The paragraph attributes to validate + * @returns True if the attrs contain valid list marker properties + */ +const hasListMarkerProperties = ( + attrs: unknown, +): attrs is { + numberingProperties: { numId?: number | string; ilvl?: number }; + wordLayout?: { marker?: { markerText?: string } }; +} => { + if (!attrs || typeof attrs !== 'object') return false; + const obj = attrs as Record; + + if (!obj.numberingProperties || typeof obj.numberingProperties !== 'object') return false; + const numProps = obj.numberingProperties as Record; + + // Validate numId is number or string if present + if ('numId' in numProps) { + const numId = numProps.numId; + if (typeof numId !== 'number' && typeof numId !== 'string') return false; + } + + // Validate ilvl is number if present + if ('ilvl' in numProps) { + const ilvl = numProps.ilvl; + if (typeof ilvl !== 'number') return false; + } + + // Validate wordLayout structure if present + if ('wordLayout' in obj && obj.wordLayout !== undefined) { + if (typeof obj.wordLayout !== 'object' || obj.wordLayout === null) return false; + const wordLayout = obj.wordLayout as Record; + + if ('marker' in wordLayout && wordLayout.marker !== undefined) { + if (typeof wordLayout.marker !== 'object' || wordLayout.marker === null) return false; + const marker = wordLayout.marker as Record; + + if ('markerText' in marker && marker.markerText !== undefined) { + if (typeof marker.markerText !== 'string') return false; + } + } + } + + return true; +}; + +/** + * Derives a version string for a flow block based on its content and styling properties. + * + * This version string is used for cache invalidation - when any visual property of the block + * changes, the version string changes, triggering a DOM rebuild instead of reusing cached elements. + * + * The version includes all properties that affect visual rendering: + * - Text content + * - Font properties (family, size, bold, italic) + * - Text decorations (underline style/color, strike, highlight) + * - Spacing (letterSpacing) + * - Position markers (pmStart, pmEnd) + * - Special tokens (page numbers, etc.) + * - List marker properties (numId, ilvl, markerText) - for list indent changes + * - Paragraph attributes (alignment, spacing, indent, borders, shading, direction, tabs) + * - Table cell content and paragraph formatting within cells + * + * For table blocks, a deep hash is computed across all rows and cells, including: + * - Cell block content (paragraph runs, text, formatting) + * - Paragraph-level attributes in cells (alignment, spacing, line height, indent, borders, shading) + * - Run-level formatting (color, highlight, bold, italic, fontSize, fontFamily, underline, strike) + * + * This ensures toolbar commands that modify paragraph or run formatting within tables + * trigger proper DOM updates. + * + * @param block - The flow block to generate a version string for + * @returns A pipe-delimited string representing all visual properties of the block. + * Changes to any included property will change the version string. + */ +const deriveBlockVersion = (block: FlowBlock): string => { + if (block.kind === 'paragraph') { + // Include list marker info in version to detect indent/marker changes + const markerVersion = hasListMarkerProperties(block.attrs) + ? `marker:${block.attrs.numberingProperties.numId ?? ''}:${block.attrs.numberingProperties.ilvl ?? 0}:${block.attrs.wordLayout?.marker?.markerText ?? ''}` + : ''; + + const runsVersion = block.runs + .map((run) => { + // Handle ImageRun + if (run.kind === 'image') { + const imgRun = run as ImageRun; + return [ + 'img', + imgRun.src, + imgRun.width, + imgRun.height, + imgRun.alt ?? '', + imgRun.title ?? '', + imgRun.clipPath ?? '', + imgRun.distTop ?? '', + imgRun.distBottom ?? '', + imgRun.distLeft ?? '', + imgRun.distRight ?? '', + readClipPathValue((imgRun as { clipPath?: unknown }).clipPath), + // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection + ].join(','); + } + + // Handle LineBreakRun + if (run.kind === 'lineBreak') { + // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection + return 'linebreak'; + } + + // Handle TabRun + if (run.kind === 'tab') { + // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection + return [run.text ?? '', 'tab'].join(','); + } + + // Handle FieldAnnotationRun + if (run.kind === 'fieldAnnotation') { + const size = run.size ? `${run.size.width ?? ''}x${run.size.height ?? ''}` : ''; + const highlighted = run.highlighted !== false ? 1 : 0; + return [ + 'field', + run.variant ?? '', + run.displayLabel ?? '', + run.fieldColor ?? '', + run.borderColor ?? '', + highlighted, + run.hidden ? 1 : 0, + run.visibility ?? '', + run.imageSrc ?? '', + run.linkUrl ?? '', + run.rawHtml ?? '', + size, + run.fontFamily ?? '', + run.fontSize ?? '', + run.textColor ?? '', + run.textHighlight ?? '', + run.bold ? 1 : 0, + run.italic ? 1 : 0, + run.underline ? 1 : 0, + run.fieldId ?? '', + run.fieldType ?? '', + ].join(','); + } + + // Handle TextRun (kind is 'text' or undefined) + const textRun = run as TextRun; + const trackedChangeVersion = textRun.trackedChange + ? [ + textRun.trackedChange.kind ?? '', + textRun.trackedChange.id ?? '', + textRun.trackedChange.storyKey ?? '', + textRun.trackedChange.author ?? '', + textRun.trackedChange.authorEmail ?? '', + textRun.trackedChange.authorImage ?? '', + textRun.trackedChange.date ?? '', + textRun.trackedChange.before ? JSON.stringify(textRun.trackedChange.before) : '', + textRun.trackedChange.after ? JSON.stringify(textRun.trackedChange.after) : '', + ].join(':') + : ''; + return [ + textRun.text ?? '', + textRun.fontFamily, + textRun.fontSize, + textRun.bold ? 1 : 0, + textRun.italic ? 1 : 0, + textRun.color ?? '', + // Text decorations - ensures DOM updates when decoration properties change. + textRun.underline?.style ?? '', + textRun.underline?.color ?? '', + textRun.strike ? 1 : 0, + textRun.highlight ?? '', + textRun.letterSpacing != null ? textRun.letterSpacing : '', + textRun.vertAlign ?? '', + textRun.baselineShift != null ? textRun.baselineShift : '', + // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection + textRun.token ?? '', + // Tracked changes - force re-render when any rendered tracked-change metadata changes. + trackedChangeVersion, + // Comment annotations - force re-render when comments are enabled/disabled + textRun.comments?.length ?? 0, + ].join(','); + }) + .join('|'); + + // Include paragraph-level attributes that affect rendering (alignment, spacing, indent, etc.) + // This ensures DOM updates when toolbar commands like "align center" change these properties. + const attrs = block.attrs as ParagraphAttrs | undefined; + + const paragraphAttrsVersion = attrs + ? [ + attrs.alignment ?? '', + attrs.spacing?.before ?? '', + attrs.spacing?.after ?? '', + attrs.spacing?.line ?? '', + attrs.spacing?.lineRule ?? '', + attrs.indent?.left ?? '', + attrs.indent?.right ?? '', + attrs.indent?.firstLine ?? '', + attrs.indent?.hanging ?? '', + attrs.borders ? hashParagraphBorders(attrs.borders) : '', + attrs.shading?.fill ?? '', + attrs.shading?.color ?? '', + getParagraphInlineDirection(attrs) ?? '', + attrs.tabs?.length ? JSON.stringify(attrs.tabs) : '', + ].join(':') + : ''; + + // Include SDT metadata so lock-mode (and other SDT property) changes invalidate the cache. + const sdtAttrs = (block.attrs as ParagraphAttrs | undefined)?.sdt; + const sdtVersion = getSdtMetadataVersion(sdtAttrs); + + // Combine marker version, runs version, paragraph attrs version, and SDT version + const parts = [markerVersion, runsVersion, paragraphAttrsVersion, sdtVersion].filter(Boolean); + return parts.join('|'); + } + + if (block.kind === 'list') { + return block.items.map((item) => `${item.id}:${item.marker.text}:${deriveBlockVersion(item.paragraph)}`).join('|'); + } + + if (block.kind === 'image') { + const imgSdt = (block as ImageBlock).attrs?.sdt; + const imgSdtVersion = getSdtMetadataVersion(imgSdt); + return [ + block.src ?? '', + block.width ?? '', + block.height ?? '', + block.alt ?? '', + block.title ?? '', + resolveBlockClipPath(block), + imgSdtVersion, + ].join('|'); + } + + if (block.kind === 'drawing') { + if (block.drawingKind === 'image') { + // Type narrowing: block is ImageDrawing (not ImageBlock) + const imageLike = block as ImageDrawing; + return [ + 'drawing:image', + imageLike.src ?? '', + imageLike.width ?? '', + imageLike.height ?? '', + imageLike.alt ?? '', + resolveBlockClipPath(imageLike), + ].join('|'); + } + if (block.drawingKind === 'vectorShape') { + const vector = block as VectorShapeDrawing; + return [ + 'drawing:vector', + vector.shapeKind ?? '', + vector.fillColor ?? '', + vector.strokeColor ?? '', + vector.strokeWidth ?? '', + vector.geometry.width, + vector.geometry.height, + vector.geometry.rotation ?? 0, + vector.geometry.flipH ? 1 : 0, + vector.geometry.flipV ? 1 : 0, + ].join('|'); + } + if (block.drawingKind === 'shapeGroup') { + const group = block as ShapeGroupDrawing; + const childSignature = group.shapes + .map((child) => `${child.shapeType}:${JSON.stringify(child.attrs ?? {})}`) + .join(';'); + return [ + 'drawing:group', + group.geometry.width, + group.geometry.height, + group.groupTransform ? JSON.stringify(group.groupTransform) : '', + childSignature, + ].join('|'); + } + if (block.drawingKind === 'chart') { + return [ + 'drawing:chart', + block.chartData?.chartType ?? '', + block.chartData?.series?.length ?? 0, + block.geometry.width, + block.geometry.height, + block.chartRelId ?? '', + ].join('|'); + } + // Exhaustiveness check: if a new drawingKind is added, TypeScript will error here + const _exhaustive: never = block; + return `drawing:unknown:${(block as DrawingBlock).id}`; + } + + if (block.kind === 'table') { + const tableBlock = block as TableBlock; + /** + * Local hash function for strings using FNV-1a algorithm. + * Used to create a robust hash across all table rows/cells so deep edits invalidate version. + * + * @param seed - Initial hash value + * @param value - String value to hash + * @returns Updated hash value + */ + const hashString = (seed: number, value: string): number => { + let hash = seed >>> 0; + for (let i = 0; i < value.length; i++) { + hash ^= value.charCodeAt(i); + hash = Math.imul(hash, 16777619); // FNV-style mix + } + return hash >>> 0; + }; + + /** + * Local hash function for numbers. + * Handles undefined/null values safely by treating them as 0. + * + * @param seed - Initial hash value + * @param value - Number value to hash (or undefined/null) + * @returns Updated hash value + */ + const hashNumber = (seed: number, value: number | undefined | null): number => { + const n = Number.isFinite(value) ? (value as number) : 0; + let hash = seed ^ n; + hash = Math.imul(hash, 16777619); + hash ^= hash >>> 13; + return hash >>> 0; + }; + + let hash = 2166136261; + hash = hashString(hash, block.id); + hash = hashNumber(hash, tableBlock.rows.length); + hash = (tableBlock.columnWidths ?? []).reduce((acc, width) => hashNumber(acc, Math.round(width * 1000)), hash); + + // Defensive guards: ensure rows array exists and iterate safely + const rows = tableBlock.rows ?? []; + 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). Without this, applyHunks-style transactions that only mutate + // the row's `trackChange` attr don't invalidate the page cache, so the + // visible cells never receive their `data-track-change` attribute and the + // row stays unmarked. + if (row.trackedChange) { + hash = hashString(hash, row.trackedChange.kind ?? ''); + hash = hashString(hash, row.trackedChange.id ?? ''); + hash = hashString(hash, row.trackedChange.operationId ?? ''); + } + for (const cell of row.cells) { + if (!cell) continue; + const cellBlocks = cell.blocks ?? (cell.paragraph ? [cell.paragraph] : []); + hash = hashNumber(hash, cellBlocks.length); + // Include cell attributes that affect rendering (rowSpan, colSpan, borders, etc.) + hash = hashNumber(hash, cell.rowSpan ?? 1); + hash = hashNumber(hash, cell.colSpan ?? 1); + + // Include cell-level attributes (borders, padding, background) that affect rendering + // This ensures cache invalidation when cell formatting changes (e.g., remove borders). + if (cell.attrs) { + const cellAttrs = cell.attrs as TableCellAttrs; + if (cellAttrs.borders) { + hash = hashString(hash, hashCellBorders(cellAttrs.borders)); + } + if (cellAttrs.padding) { + const p = cellAttrs.padding; + hash = hashNumber(hash, p.top ?? 0); + hash = hashNumber(hash, p.right ?? 0); + hash = hashNumber(hash, p.bottom ?? 0); + hash = hashNumber(hash, p.left ?? 0); + } + if (cellAttrs.verticalAlign) { + hash = hashString(hash, cellAttrs.verticalAlign); + } + if (cellAttrs.background) { + hash = hashString(hash, cellAttrs.background); + } + } + + for (const cellBlock of cellBlocks) { + hash = hashString(hash, cellBlock?.kind ?? 'unknown'); + if (cellBlock?.kind === 'paragraph') { + const paragraphBlock = cellBlock as ParagraphBlock; + const runs = paragraphBlock.runs ?? []; + hash = hashNumber(hash, runs.length); + + // Include paragraph-level attributes that affect rendering + // (alignment, spacing, indent, etc.) - fixes toolbar commands not updating tables + const attrs = paragraphBlock.attrs as ParagraphAttrs | undefined; + + if (attrs) { + hash = hashString(hash, attrs.alignment ?? ''); + hash = hashNumber(hash, attrs.spacing?.before ?? 0); + hash = hashNumber(hash, attrs.spacing?.after ?? 0); + hash = hashNumber(hash, attrs.spacing?.line ?? 0); + hash = hashString(hash, attrs.spacing?.lineRule ?? ''); + hash = hashNumber(hash, attrs.indent?.left ?? 0); + hash = hashNumber(hash, attrs.indent?.right ?? 0); + hash = hashNumber(hash, attrs.indent?.firstLine ?? 0); + hash = hashNumber(hash, attrs.indent?.hanging ?? 0); + hash = hashString(hash, attrs.shading?.fill ?? ''); + hash = hashString(hash, attrs.shading?.color ?? ''); + hash = hashString(hash, getParagraphInlineDirection(attrs) ?? ''); + if (attrs.borders) { + hash = hashString(hash, hashParagraphBorders(attrs.borders)); + } + } + + for (const run of runs) { + // Only text runs have .text property; ImageRun does not + if ('text' in run && typeof run.text === 'string') { + hash = hashString(hash, run.text); + } + hash = hashNumber(hash, run.pmStart ?? -1); + hash = hashNumber(hash, run.pmEnd ?? -1); + + // Include run formatting properties that affect rendering + // (color, highlight, bold, italic, etc.) - fixes toolbar commands not updating tables + hash = hashString(hash, getRunStringProp(run, 'color')); + hash = hashString(hash, getRunStringProp(run, 'highlight')); + hash = hashString(hash, getRunBooleanProp(run, 'bold') ? '1' : ''); + hash = hashString(hash, getRunBooleanProp(run, 'italic') ? '1' : ''); + hash = hashNumber(hash, getRunNumberProp(run, 'fontSize')); + hash = hashString(hash, getRunStringProp(run, 'fontFamily')); + hash = hashString(hash, getRunUnderlineStyle(run)); + hash = hashString(hash, getRunUnderlineColor(run)); + hash = hashString(hash, getRunBooleanProp(run, 'strike') ? '1' : ''); + hash = hashString(hash, getRunStringProp(run, 'vertAlign')); + hash = hashNumber(hash, getRunNumberProp(run, 'baselineShift')); + } + } else if (cellBlock?.kind) { + // Non-paragraph cell blocks participate in the parent table version + // through their own block-level signatures. layout-bridge/cache.ts + // mirrors this policy so repaint and remeasure stay aligned for + // nested tables, images, drawings, and other embedded cell content. + hash = hashString(hash, deriveBlockVersion(cellBlock as FlowBlock)); + } + } + } + } + + // Include table-level attributes (borders, etc.) that affect rendering + // This ensures cache invalidation when table formatting changes (e.g., remove borders). + if (tableBlock.attrs) { + const tblAttrs = tableBlock.attrs as TableAttrs; + if (tblAttrs.borders) { + hash = hashString(hash, hashTableBorders(tblAttrs.borders)); + } + if (tblAttrs.borderCollapse) { + hash = hashString(hash, tblAttrs.borderCollapse); + } + if (tblAttrs.cellSpacing !== undefined) { + const cs = tblAttrs.cellSpacing; + if (typeof cs === 'number') { + hash = hashNumber(hash, cs); + } else { + // Stable key: value and type only (avoid JSON.stringify key-order variance) + const v = (cs as { value?: number; type?: string }).value ?? 0; + const t = (cs as { value?: number; type?: string }).type ?? 'px'; + hash = hashString(hash, `cs:${v}:${t}`); + } + } + // Include SDT metadata so lock-mode changes invalidate the cache. + if (tblAttrs.sdt) { + hash = hashString(hash, tblAttrs.sdt.type); + hash = hashString(hash, getSdtMetadataLockMode(tblAttrs.sdt)); + hash = hashString(hash, getSdtMetadataId(tblAttrs.sdt)); + } + } + + return [block.id, tableBlock.rows.length, hash.toString(16)].join('|'); + } + + return block.id; +}; + +const DEFAULT_SUPERSCRIPT_RAISE_RATIO = 0.33; +const DEFAULT_SUBSCRIPT_LOWER_RATIO = 0.14; + +const hasVerticalPositioning = (run: TextRun): boolean => + normalizeBaselineShift(run.baselineShift) != null || run.vertAlign === 'superscript' || run.vertAlign === 'subscript'; + +const applyRunVerticalPositioning = (element: HTMLElement, run: TextRun): void => { + // Vertically shifted runs should use a tight inline box. If they inherit the + // parent line's full line-height, the glyph remains visually low inside an + // oversized inline box even when the superscript/subscript offset is correct. + if (hasVerticalPositioning(run)) { + element.style.lineHeight = '1'; + } + + const explicitBaselineShift = normalizeBaselineShift(run.baselineShift); + if (explicitBaselineShift != null) { + element.style.verticalAlign = `${explicitBaselineShift}pt`; + return; + } + + if (run.vertAlign === 'superscript') { + const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); + element.style.verticalAlign = `${baseFontSize * DEFAULT_SUPERSCRIPT_RAISE_RATIO}px`; + return; + } + + if (run.vertAlign === 'subscript') { + const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); + element.style.verticalAlign = `${-(baseFontSize * DEFAULT_SUBSCRIPT_LOWER_RATIO)}px`; + return; + } + + if (run.vertAlign === 'baseline') { + element.style.verticalAlign = 'baseline'; + } +}; + +/** + * Applies run styling properties to a DOM element. + * + * @param element - The HTML element to style + * @param run - The run object containing styling information + * @param _isLink - Whether this run is part of a hyperlink. Note: This parameter + * is kept for API compatibility but no longer affects behavior - + * inline colors are now applied to all runs (including links) to + * ensure OOXML hyperlink character styles appear correctly. + */ +const applyRunStyles = (element: HTMLElement, run: Run, _isLink = false): void => { + if ( + run.kind === 'tab' || + run.kind === 'image' || + run.kind === 'lineBreak' || + run.kind === 'break' || + run.kind === 'fieldAnnotation' || + run.kind === 'math' + ) { + // Tab, image, lineBreak, break, and fieldAnnotation runs don't have text styling properties + return; + } + + element.style.fontFamily = run.fontFamily; + element.style.fontSize = `${run.fontSize}px`; + if (run.bold) element.style.fontWeight = 'bold'; + if (run.italic) element.style.fontStyle = 'italic'; + + // Apply inline color even for links so OOXML hyperlink styles appear when CSS is absent + if (run.color) element.style.color = run.color; + + if (run.letterSpacing != null) { + element.style.letterSpacing = `${run.letterSpacing}px`; + } + if (run.highlight) { + element.style.backgroundColor = run.highlight; + } + if (run.textTransform) { + element.style.textTransform = run.textTransform; + } + + // Apply text decorations from the run. Even for links, inline decorations should reflect + // the document styling (tests assert underline presence on anchors). + const decorations: string[] = []; + if (run.underline) { + decorations.push('underline'); + const u = run.underline; + element.style.textDecorationStyle = u.style && u.style !== 'single' ? u.style : 'solid'; + if (u.color) { + element.style.textDecorationColor = u.color; + } + } + if (run.strike) { + decorations.push('line-through'); + } + if (decorations.length > 0) { + element.style.textDecorationLine = decorations.join(' '); + } + + applyRunVerticalPositioning(element, run); +}; + +const CLIP_PATH_PREFIXES = ['inset(', 'polygon(', 'circle(', 'ellipse(', 'path(', 'rect(']; + +const readClipPathValue = (value: unknown): string => { + if (typeof value !== 'string') return ''; + const normalized = value.trim(); + if (normalized.length === 0) return ''; + const lower = normalized.toLowerCase(); + if (!CLIP_PATH_PREFIXES.some((prefix) => lower.startsWith(prefix))) return ''; + return normalized; +}; + +const resolveClipPathFromAttrs = (attrs: unknown): string => { + if (!attrs || typeof attrs !== 'object') return ''; + const record = attrs as Record; + return readClipPathValue(record.clipPath); +}; + +const resolveBlockClipPath = (block: unknown): string => { + if (!block || typeof block !== 'object') return ''; + const record = block as Record; + return readClipPathValue(record.clipPath) || resolveClipPathFromAttrs(record.attrs); +}; + +/** + * Applies data-* attributes from a text run to a DOM element. + * Validates attribute names and safely sets them on the element. + * Invalid or unsafe attributes are skipped with development-mode logging. + * + * @param element - The HTML element to apply attributes to + * @param dataAttrs - Record of data-* attribute key-value pairs from the text run + * + * @example + * ```typescript + * const span = document.createElement('span'); + * applyRunDataAttributes(span, { 'data-id': '123', 'data-name': 'test' }); + * // span now has: + * ``` + */ +export const applyRunDataAttributes = (element: HTMLElement, dataAttrs?: Record): void => { + if (!dataAttrs) return; + Object.entries(dataAttrs).forEach(([key, value]) => { + if (typeof key !== 'string' || !key.toLowerCase().startsWith('data-')) return; + if (typeof value !== 'string') return; + try { + element.setAttribute(key, value); + } catch (error) { + if (process.env.NODE_ENV === 'development') { + console.warn(`[DomPainter] Failed to set data attribute "${key}":`, error); + } + } + }); +}; + +const applyParagraphBlockStyles = (element: HTMLElement, attrs?: ParagraphAttrs): void => { + if (!attrs) return; + if (attrs.styleId) { + element.setAttribute('styleid', attrs.styleId); + } + applyRtlStyles(element, attrs); + if ((attrs as Record).dropCap) { + element.classList.add('sd-editor-dropcap'); + } + const indent = attrs.indent; + if (indent) { + // Only apply positive indents as padding. + // Negative indents are handled by fragment positioning in the layout engine. + if (indent.left && indent.left > 0) { + element.style.paddingLeft = `${indent.left}px`; + } + if (indent.right && indent.right > 0) { + element.style.paddingRight = `${indent.right}px`; + } + // Skip textIndent when left indent is negative - fragment positioning handles the indent, + // and per-line paddingLeft handles the hanging indent for body lines. + const hasNegativeLeftIndent = indent.left != null && indent.left < 0; + if (!hasNegativeLeftIndent) { + const textIndent = (indent.firstLine ?? 0) - (indent.hanging ?? 0); + if (textIndent) { + element.style.textIndent = `${textIndent}px`; + } + } + } +}; + +// getParagraphBorderBox, createParagraphDecorationLayers, applyParagraphBorderStyles, +// setBorderSideStyle, applyParagraphShadingStyles — moved to features/paragraph-borders/ + +const stripListIndent = (attrs?: ParagraphAttrs): ParagraphAttrs | undefined => { + if (!attrs?.indent || attrs.indent.left == null) { + return attrs; + } + const nextIndent = { ...attrs.indent }; + delete nextIndent.left; + + return { + ...attrs, + indent: Object.keys(nextIndent).length > 0 ? nextIndent : undefined, + }; +}; + +// applyParagraphShadingStyles — moved to features/paragraph-borders/border-layer.ts + +const applyStyles = (el: HTMLElement, styles: Partial): void => { + Object.entries(styles).forEach(([key, value]) => { + if (value != null && value !== '' && key in el.style) { + (el.style as unknown as Record)[key] = String(value); + } + }); +}; + +const resolveRunText = (run: Run, context: FragmentRenderContext): string => { + const runToken = 'token' in run ? run.token : undefined; + + if (run.kind === 'tab') { + return run.text; + } + if (run.kind === 'image') { + // Image runs don't have text content + return ''; + } + if (run.kind === 'lineBreak') { + // Line break runs don't render text - the measurer creates new lines for them + return ''; + } + if (run.kind === 'break') { + // Break runs don't render text - the measurer creates new lines for them + return ''; + } + if (!('text' in run)) { + // Safety check - if run doesn't have text property, return empty string + return ''; + } + if (!runToken) { + return run.text ?? ''; + } + if (runToken === 'pageNumber') { + return context.pageNumberText ?? String(context.pageNumber); + } + if (runToken === 'totalPageCount') { + return context.totalPages ? String(context.totalPages) : (run.text ?? ''); + } + return run.text ?? ''; +}; From 24e6120660615a7ab9771bb9c6c83d42b822803b Mon Sep 17 00:00:00 2001 From: Shri H Date: Thu, 28 May 2026 00:50:45 +0100 Subject: [PATCH 11/15] fix(structural-track-changes): default identity key to content fingerprint computeStructuralDiff previously matched top-level blocks by sdBlockId. That works in-process but breaks across imports: the docx importer assigns a fresh sdBlockId on every load (there's no standard OOXML attribute for tables to carry a stable id), so the AI-review flow -- where the proposal is a separately-imported docx -- saw every block as new and flagged the whole document as changed. Switch the default identityKey to a normalized content fingerprint (`${typeName}:${normalizedText}`) so identical blocks match across imports. Document the limitations in the JSDoc: two truly-identical blocks share a fingerprint and the algorithm treats them as one (acceptable for current AI-review flow; consumers needing precise duplicate handling can pass a custom `identityKey`). Consumers whose proposal preserves sdBlockIds can opt back in via the option. --- .../computeStructuralDiff.js | 37 ++++++++++-- .../computeStructuralDiff.test.js | 58 ++++++++++++++++--- 2 files changed, 81 insertions(+), 14 deletions(-) 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 index 9da9c19846..f31483438e 100644 --- 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 @@ -1,14 +1,34 @@ /** * Compute structural hunks describing whole-block add/removes between two docs. * - * Walks both docs at depth 1, matches blocks by `identityKey` (default: - * `sdBlockId`). For each base block absent from proposal: emit + * 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' }`. * - * Identity matching is intentionally simple. AI integrations whose proposal - * docx has different `sdBlockId`s across imports can provide a structural - * fingerprint via `opts.identityKey`. + * 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 @@ -23,7 +43,12 @@ * }>} */ const DEFAULT_BLOCK_TYPES = Object.freeze(['table']); -const defaultIdentityKey = (node) => node?.attrs?.sdBlockId ?? null; +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; 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 index ce5057d6a3..7a2daa4fd3 100644 --- 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 @@ -18,18 +18,23 @@ describe('computeStructuralDiff', () => { blocks.map((b) => b(schema)), ); const p = (id, text) => (schema) => schema.nodes.paragraph.create(idAttr(id), schema.text(text)); - const table = (id) => (schema) => { - const cell = schema.nodes.tableCell.create(null, schema.nodes.paragraph.create(null, schema.text('x'))); - const row = schema.nodes.tableRow.create(null, [cell]); - return schema.nodes.table.create(idAttr(id), [row]); - }; + // 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: 't1' }); + expect(hunks[0]).toMatchObject({ kind: 'remove', changeId: tableFingerprint('t1') }); }); it('emits an insert hunk for a proposal table absent from base', () => { @@ -37,7 +42,7 @@ describe('computeStructuralDiff', () => { const proposal = makeDoc([p('p1', 'a'), table('t1')]); const hunks = computeStructuralDiff(base, proposal); expect(hunks).toHaveLength(1); - expect(hunks[0]).toMatchObject({ kind: 'insert', changeId: 't1' }); + expect(hunks[0]).toMatchObject({ kind: 'insert', changeId: tableFingerprint('t1') }); }); it('emits no hunks when both sides have matching tables', () => { @@ -46,6 +51,31 @@ describe('computeStructuralDiff', () => { 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')]); @@ -68,7 +98,19 @@ describe('computeStructuralDiff', () => { const proposal = makeDoc([table('t1'), p('p1', 'extra')]); const hunks = computeStructuralDiff(base, proposal); expect(hunks.filter((h) => h.kind === 'insert')).toHaveLength(1); - expect(hunks.find((h) => h.kind === 'insert')?.changeId).toBe('t1'); + 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', () => { From 4f0680498c4e72d80e2bd70dece91c42e86039e0 Mon Sep 17 00:00:00 2001 From: Shri H Date: Fri, 5 Jun 2026 12:33:54 +0100 Subject: [PATCH 12/15] fix(track-changes): append step map for block-level pre-marked shortcut The block-level tracked-change shortcut in trackedTransaction applied the original step as-is via `newTr.step(step)` but didn't call `map.appendMap(step.getMap())`. In a multi-step transaction where this shortcut fires before another step, the next iteration's `originalStep.map(map)` then mapped through stale positions -- the later step landed in the wrong place or got silently dropped when the mapping returned null. Matches the existing pass-through pattern in replaceStep.js where applying a step as-is is always paired with `map.appendMap`. The StructuralTrackChanges applyHunks path typically dispatches one step per transaction, so the bug rarely hit today, but it becomes a real correctness issue the moment two structural hunks share a transaction or any other tracked change rides alongside a structural one. --- .../track-changes/trackChangesHelpers/trackedTransaction.js | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) 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 3ca685216f..99de914185 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 @@ -436,9 +436,12 @@ export const trackedTransaction = ({ tr, state, user, replacements = 'paired' }) // 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. + // 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; } From fce23c2b0f57ad24b218eb527c53b74810f1beb7 Mon Sep 17 00:00:00 2001 From: Shri H Date: Fri, 5 Jun 2026 12:38:54 +0100 Subject: [PATCH 13/15] fix(track-changes): reject id-less trackChange attrs at parse time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `blockTrackedChangeAttrSpec.parseDOM` accepted nodes with `data-track-change` set but `data-track-change-id` missing, producing a `trackChange: { kind, id: null, operationId }` shape on the PM node. That shape: - can never be resolved by `applyRowTrackedChangeResolution`, which matches by id and would never find a null - is already filtered out by `getBlockTrackedChanges` (so the entry is unactionable, just lingering as a no-op attr on the row) - violates the documented `id: string` runtime type - is asymmetric with `renderDOM`, which only emits `data-track-change-id` when `tc.id` is truthy — a faithful HTML round-trip always carries the id Reject the missing-id case at parse time so the doc never holds a half-formed trackChange shape. --- .../extensions/track-changes/blockTrackedChangeAttr.js | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js index 5106987f92..fbc914ade2 100644 --- a/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js +++ b/packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js @@ -12,9 +12,17 @@ export const blockTrackedChangeAttrSpec = { 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: el.getAttribute('data-track-change-id') ?? null, + id, operationId: el.getAttribute('data-track-change-operation') ?? undefined, }; }, From 5c7432ce65a4fa52c79bd02f1ab6dd4f04507b66 Mon Sep 17 00:00:00 2001 From: Shri H Date: Wed, 10 Jun 2026 16:26:35 +0100 Subject: [PATCH 14/15] fix(track-changes): align block-level producer with upstream OOXML format Upstream main introduced an OOXML-aligned shape for row tracked changes (`attrs.trackChange.type === 'rowInsert' | 'rowDelete'`) and a `buildRowTrackedChangeMeta` reader on the v1 layout-adapter that writes the resolved metadata to `row.attrs.trackedChange`. Several of our block-level pieces still spoke the older `{ kind }` shape and wrote to the top-level `row.trackedChange`, so after the rebase onto main the producer/consumer pair disagreed and painting silently no-oped: - `applyHunks` stamped `{ kind, id, operationId }` on the PM tableRow. The new reader (which keys on `type`) ignored it, so `row.attrs.trackedChange` was never populated. Switched to `{ type: 'rowInsert' | 'rowDelete', id, operationId, ... }`. Tests updated. - `comments-plugin` block-walk and `getBlockTrackedChanges` accepted only `{ kind }`. They now accept both shapes and emit the same normalized `kind` downstream. - `trackedTransaction.sliceContainsPreMarkedBlockTrackedChange` looked for `kind`; broadened to also detect `type === 'rowInsert' | 'rowDelete'` so paste/replay slices skip double-tracking under either shape. - The legacy fallback path in `table.ts` (v1 layout-adapter) that populated the top-level `row.trackedChange` from the old shape is removed; `buildRowTrackedChangeMeta` is now the single producer. - The painter page-fingerprint, layout-bridge measure-cache, and layout-resolved canonical block version now hash `row.attrs.trackedChange` instead of the (now-unpopulated) top-level field, so applyHunks-style transactions invalidate the right caches. - `structural-track-changes` JSDoc refreshed to describe the new flow. Net effect: a block-level diff replay now reliably paints the red/green row decoration via the helper-based path (`applyRowTrackedChangeToCell`) that upstream extracted. --- .../layout-engine/layout-bridge/src/cache.ts | 21 ++++++----- .../layout-resolved/src/versionSignature.ts | 21 ++++++----- .../painters/dom/src/renderer.ts | 21 ++++++----- .../core/layout-adapter/converters/table.ts | 35 ++++--------------- .../v1/extensions/comment/comments-plugin.js | 18 ++++++---- .../structural-track-changes.js | 8 +++-- .../applyHunks.js | 4 +-- .../applyHunks.test.js | 4 +-- .../getBlockTrackedChanges.js | 19 ++++++++-- .../trackChangesHelpers/trackedTransaction.js | 5 ++- 10 files changed, 84 insertions(+), 72 deletions(-) diff --git a/packages/layout-engine/layout-bridge/src/cache.ts b/packages/layout-engine/layout-bridge/src/cache.ts index b86c585af1..bba5273f5c 100644 --- a/packages/layout-engine/layout-bridge/src/cache.ts +++ b/packages/layout-engine/layout-bridge/src/cache.ts @@ -287,15 +287,18 @@ const hashRuns = (block: FlowBlock): string => { } // Row-level tracked change (block-level diff replay sets `trackedChange` - // on a tableRow). Without this, applyHunks-style transactions that only - // mutate the row's `trackChange` attr don't invalidate the measure - // cache, so the page is never re-measured and the visible cells never - // receive their `data-track-change` attribute. Mirror of the painter - // page-fingerprint fix in renderer.ts. - if (row.trackedChange) { - cellHashes.push( - `rtc:${row.trackedChange.kind ?? ''}:${row.trackedChange.id ?? ''}:${row.trackedChange.operationId ?? ''}`, - ); + // 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) { diff --git a/packages/layout-engine/layout-resolved/src/versionSignature.ts b/packages/layout-engine/layout-resolved/src/versionSignature.ts index 861e685b08..0109fcbe98 100644 --- a/packages/layout-engine/layout-resolved/src/versionSignature.ts +++ b/packages/layout-engine/layout-resolved/src/versionSignature.ts @@ -489,15 +489,18 @@ export const deriveBlockVersion = (block: FlowBlock): string => { 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). Without this, applyHunks-style transactions that only - // mutate the row's `trackChange` attr don't bump the block version, so - // the resolved-layout pipeline reuses cached entries and the painter - // never receives row data with `trackedChange` populated. Mirror of the - // fixes in renderer.ts and layout-bridge/cache.ts. - if (row.trackedChange) { - hash = hashString(hash, row.trackedChange.kind ?? ''); - hash = hashString(hash, row.trackedChange.id ?? ''); - hash = hashString(hash, row.trackedChange.operationId ?? ''); + // 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; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 6ad6264dd4..704d7d8e81 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -4606,15 +4606,18 @@ 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). Without this, applyHunks-style transactions that only mutate - // the row's `trackChange` attr don't invalidate the page cache, so the - // visible cells never receive their `data-track-change` attribute and the - // row stays unmarked. - if (row.trackedChange) { - hash = hashString(hash, row.trackedChange.kind ?? ''); - hash = hashString(hash, row.trackedChange.id ?? ''); - hash = hashString(hash, row.trackedChange.operationId ?? ''); + // Row-level tracked change (block-level diff replay sets `trackedChange` + // on a tableRow's attrs). Without folding it into the page fingerprint, + // applyHunks-style transactions that only mutate the row's `trackChange` + // attr don't invalidate the page cache, so the visible cells never receive + // their decoration classes and the row stays unmarked. Read from + // `row.attrs.trackedChange` — the canonical location written by the v1 + // layout-adapter from the OOXML-aligned `attrs.trackChange.type` shape. + 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; 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 d89f5d5866..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 @@ -805,35 +805,12 @@ const parseTableRow = (args: ParseTableRowArgs): TableRow | null => { sourceAnchor: sourceAnchorFromNode(rowNode), }; - // Row-level tracked change (block-level diff replay sets this on the PM node). - // See packages/super-editor/src/editors/v1/extensions/track-changes/blockTrackedChangeAttr.js - const trackChangeAttr = rowNode.attrs?.trackChange as - | { - kind?: 'insert' | 'delete'; - id?: string; - operationId?: string; - author?: string; - authorEmail?: string; - date?: string; - storyKey?: string; - } - | null - | undefined; - if ( - trackChangeAttr && - (trackChangeAttr.kind === 'insert' || trackChangeAttr.kind === 'delete') && - trackChangeAttr.id - ) { - row.trackedChange = { - kind: trackChangeAttr.kind, - id: trackChangeAttr.id, - operationId: trackChangeAttr.operationId, - author: trackChangeAttr.author, - authorEmail: trackChangeAttr.authorEmail, - date: trackChangeAttr.date, - storyKey: trackChangeAttr.storyKey, - }; - } + // 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/extensions/comment/comments-plugin.js b/packages/super-editor/src/editors/v1/extensions/comment/comments-plugin.js index a8ec23279b..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 @@ -805,13 +805,19 @@ export const CommentsPlugin = Extension.create({ // 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). + // operation, not per row). Accept both the legacy `{kind}` shape + // and the upstream OOXML-aligned `{type: 'rowInsert'|'rowDelete'}` + // shape. const blockTrackChange = node?.attrs?.trackChange; - if ( - blockTrackChange && - (blockTrackChange.kind === 'insert' || blockTrackChange.kind === 'delete') && - blockTrackChange.id - ) { + 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; 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 index c6c13c02f4..c56b420377 100644 --- 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 @@ -9,9 +9,11 @@ import { applyRowTrackedChangeResolution } from '../track-changes/trackChangesHe * * Pattern: consumer computes `StructuralHunk[]` (e.g. via `computeStructuralDiff`) * and dispatches via `editor.commands.setStructuralDiff(hunks)`. The extension - * stamps a `trackChange` PM attribute on each affected `tableRow`. Rendering is - * handled by the painter (reads `data-track-change` data attrs from row.trackedChange). - * The review bubble appears via the comments-plugin's block-level walk. + * 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 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 index 24d6c4e8e0..d3111e4167 100644 --- 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 @@ -44,7 +44,7 @@ export const applyHunks = ({ tr, state, hunks }) => { if (row.type.name === 'tableRow') { tr.setNodeMarkup(rowPos, null, { ...row.attrs, - trackChange: { kind: 'delete', id: uuidv4(), operationId }, + trackChange: { type: 'rowDelete', id: uuidv4(), operationId }, }); } rowPos += row.nodeSize; @@ -67,7 +67,7 @@ export const applyHunks = ({ tr, state, hunks }) => { } trackedRows.push( row.type.create( - { ...row.attrs, trackChange: { kind: 'insert', id: uuidv4(), operationId } }, + { ...row.attrs, trackChange: { type: 'rowInsert', id: uuidv4(), operationId } }, row.content, row.marks, ), 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 index cb205af91a..1b2697493c 100644 --- 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 @@ -37,7 +37,7 @@ describe('applyHunks', () => { expect(next.childCount).toBe(3); const opIds = new Set(); next.forEach((row) => { - expect(row.attrs.trackChange?.kind).toBe('delete'); + expect(row.attrs.trackChange?.type).toBe('rowDelete'); opIds.add(row.attrs.trackChange?.operationId); }); expect(opIds.size).toBe(1); @@ -59,7 +59,7 @@ describe('applyHunks', () => { const insertedTable = nextDoc.child(nextDoc.childCount - 1); expect(insertedTable.type.name).toBe('table'); insertedTable.forEach((row) => { - expect(row.attrs.trackChange?.kind).toBe('insert'); + expect(row.attrs.trackChange?.type).toBe('rowInsert'); }); }); 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 index 99ccd18451..b050a535af 100644 --- 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 @@ -15,16 +15,31 @@ * 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; - if (tc && tc.id && (tc.kind === 'insert' || tc.kind === 'delete')) { + const kind = resolveKind(tc); + if (tc && tc.id && kind) { out.push({ id: tc.id, - kind: tc.kind, + kind, operationId: tc.operationId, nodeType: node.type.name, from: pos, 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 99de914185..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 @@ -366,7 +366,10 @@ const sliceContainsPreMarkedBlockTrackedChange = (slice) => { let found = false; slice.content.descendants((node) => { if (found) return false; - if (node?.attrs?.trackChange?.kind) { + // 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; } From a3bb8b352972427472fbd4f5c141d0b4a6330dbb Mon Sep 17 00:00:00 2001 From: Shri H Date: Wed, 10 Jun 2026 23:29:53 +0100 Subject: [PATCH 15/15] fix(layout): remove orphan deriveBlockVersion duplicate from painter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit fff5326b ("invalidate row caches on tableRow.trackChange attr changes") accidentally added a 738-line block to the bottom of the painter's renderer.ts — a duplicate `deriveBlockVersion` implementation plus supporting helpers (`getSdtMetadataId`, `hasListMarkerProperties`, `hasVerticalPositioning`, etc.) that referenced names (`SdtMetadata`, `TableAttrs`, `TextRun`, `ParagraphAttrs`, `getRunBooleanProp`, `hashTableBorders`, `normalizeBaselineShift`, `applyRtlStyles`, …) which were never imported into renderer.ts. The block was never called from outside (`grep` confirms no external `deriveBlockVersion` reference in the painter package), and the canonical `deriveBlockVersion` lives in `layout-resolved/src/versionSignature.ts` which already has the row-attrs.trackedChange hash from the audit commit. So the only effect of the orphan block was to break `tsc -b` and the upstream CI build. Truncate renderer.ts back to its upstream-main shape (last meaningful line: `isNonBodyStoryBlockId`). The row-level cache invalidation continues to work via `versionSignature.ts` and `layout-bridge/cache.ts` (which read `row.attrs.trackedChange` correctly). --- .../painters/dom/src/renderer.ts | 738 ------------------ 1 file changed, 738 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 704d7d8e81..e5ef3ea5ca 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -4250,741 +4250,3 @@ const isNonBodyStoryBlockId = (blockId: string | undefined): boolean => blockId.startsWith('endnote-') || blockId.startsWith('__sd_semantic_footnote-') || blockId.startsWith('__sd_semantic_endnote-')); -const getSdtMetadataId = (metadata: SdtMetadata | null | undefined): string => { - if (!metadata) return ''; - if ('id' in metadata && metadata.id != null) { - return String(metadata.id); - } - return ''; -}; - -const getSdtMetadataLockMode = (metadata: SdtMetadata | null | undefined): string => { - if (!metadata) return ''; - return metadata.type === 'structuredContent' ? (metadata.lockMode ?? '') : ''; -}; - -const getSdtMetadataVersion = (metadata: SdtMetadata | null | undefined): string => { - if (!metadata) return ''; - return [metadata.type, getSdtMetadataLockMode(metadata), getSdtMetadataId(metadata)].join(':'); -}; - -/** - * Type guard to validate list marker attributes structure. - * - * @param attrs - The paragraph attributes to validate - * @returns True if the attrs contain valid list marker properties - */ -const hasListMarkerProperties = ( - attrs: unknown, -): attrs is { - numberingProperties: { numId?: number | string; ilvl?: number }; - wordLayout?: { marker?: { markerText?: string } }; -} => { - if (!attrs || typeof attrs !== 'object') return false; - const obj = attrs as Record; - - if (!obj.numberingProperties || typeof obj.numberingProperties !== 'object') return false; - const numProps = obj.numberingProperties as Record; - - // Validate numId is number or string if present - if ('numId' in numProps) { - const numId = numProps.numId; - if (typeof numId !== 'number' && typeof numId !== 'string') return false; - } - - // Validate ilvl is number if present - if ('ilvl' in numProps) { - const ilvl = numProps.ilvl; - if (typeof ilvl !== 'number') return false; - } - - // Validate wordLayout structure if present - if ('wordLayout' in obj && obj.wordLayout !== undefined) { - if (typeof obj.wordLayout !== 'object' || obj.wordLayout === null) return false; - const wordLayout = obj.wordLayout as Record; - - if ('marker' in wordLayout && wordLayout.marker !== undefined) { - if (typeof wordLayout.marker !== 'object' || wordLayout.marker === null) return false; - const marker = wordLayout.marker as Record; - - if ('markerText' in marker && marker.markerText !== undefined) { - if (typeof marker.markerText !== 'string') return false; - } - } - } - - return true; -}; - -/** - * Derives a version string for a flow block based on its content and styling properties. - * - * This version string is used for cache invalidation - when any visual property of the block - * changes, the version string changes, triggering a DOM rebuild instead of reusing cached elements. - * - * The version includes all properties that affect visual rendering: - * - Text content - * - Font properties (family, size, bold, italic) - * - Text decorations (underline style/color, strike, highlight) - * - Spacing (letterSpacing) - * - Position markers (pmStart, pmEnd) - * - Special tokens (page numbers, etc.) - * - List marker properties (numId, ilvl, markerText) - for list indent changes - * - Paragraph attributes (alignment, spacing, indent, borders, shading, direction, tabs) - * - Table cell content and paragraph formatting within cells - * - * For table blocks, a deep hash is computed across all rows and cells, including: - * - Cell block content (paragraph runs, text, formatting) - * - Paragraph-level attributes in cells (alignment, spacing, line height, indent, borders, shading) - * - Run-level formatting (color, highlight, bold, italic, fontSize, fontFamily, underline, strike) - * - * This ensures toolbar commands that modify paragraph or run formatting within tables - * trigger proper DOM updates. - * - * @param block - The flow block to generate a version string for - * @returns A pipe-delimited string representing all visual properties of the block. - * Changes to any included property will change the version string. - */ -const deriveBlockVersion = (block: FlowBlock): string => { - if (block.kind === 'paragraph') { - // Include list marker info in version to detect indent/marker changes - const markerVersion = hasListMarkerProperties(block.attrs) - ? `marker:${block.attrs.numberingProperties.numId ?? ''}:${block.attrs.numberingProperties.ilvl ?? 0}:${block.attrs.wordLayout?.marker?.markerText ?? ''}` - : ''; - - const runsVersion = block.runs - .map((run) => { - // Handle ImageRun - if (run.kind === 'image') { - const imgRun = run as ImageRun; - return [ - 'img', - imgRun.src, - imgRun.width, - imgRun.height, - imgRun.alt ?? '', - imgRun.title ?? '', - imgRun.clipPath ?? '', - imgRun.distTop ?? '', - imgRun.distBottom ?? '', - imgRun.distLeft ?? '', - imgRun.distRight ?? '', - readClipPathValue((imgRun as { clipPath?: unknown }).clipPath), - // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection - ].join(','); - } - - // Handle LineBreakRun - if (run.kind === 'lineBreak') { - // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection - return 'linebreak'; - } - - // Handle TabRun - if (run.kind === 'tab') { - // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection - return [run.text ?? '', 'tab'].join(','); - } - - // Handle FieldAnnotationRun - if (run.kind === 'fieldAnnotation') { - const size = run.size ? `${run.size.width ?? ''}x${run.size.height ?? ''}` : ''; - const highlighted = run.highlighted !== false ? 1 : 0; - return [ - 'field', - run.variant ?? '', - run.displayLabel ?? '', - run.fieldColor ?? '', - run.borderColor ?? '', - highlighted, - run.hidden ? 1 : 0, - run.visibility ?? '', - run.imageSrc ?? '', - run.linkUrl ?? '', - run.rawHtml ?? '', - size, - run.fontFamily ?? '', - run.fontSize ?? '', - run.textColor ?? '', - run.textHighlight ?? '', - run.bold ? 1 : 0, - run.italic ? 1 : 0, - run.underline ? 1 : 0, - run.fieldId ?? '', - run.fieldType ?? '', - ].join(','); - } - - // Handle TextRun (kind is 'text' or undefined) - const textRun = run as TextRun; - const trackedChangeVersion = textRun.trackedChange - ? [ - textRun.trackedChange.kind ?? '', - textRun.trackedChange.id ?? '', - textRun.trackedChange.storyKey ?? '', - textRun.trackedChange.author ?? '', - textRun.trackedChange.authorEmail ?? '', - textRun.trackedChange.authorImage ?? '', - textRun.trackedChange.date ?? '', - textRun.trackedChange.before ? JSON.stringify(textRun.trackedChange.before) : '', - textRun.trackedChange.after ? JSON.stringify(textRun.trackedChange.after) : '', - ].join(':') - : ''; - return [ - textRun.text ?? '', - textRun.fontFamily, - textRun.fontSize, - textRun.bold ? 1 : 0, - textRun.italic ? 1 : 0, - textRun.color ?? '', - // Text decorations - ensures DOM updates when decoration properties change. - textRun.underline?.style ?? '', - textRun.underline?.color ?? '', - textRun.strike ? 1 : 0, - textRun.highlight ?? '', - textRun.letterSpacing != null ? textRun.letterSpacing : '', - textRun.vertAlign ?? '', - textRun.baselineShift != null ? textRun.baselineShift : '', - // Note: pmStart/pmEnd intentionally excluded to prevent O(n) change detection - textRun.token ?? '', - // Tracked changes - force re-render when any rendered tracked-change metadata changes. - trackedChangeVersion, - // Comment annotations - force re-render when comments are enabled/disabled - textRun.comments?.length ?? 0, - ].join(','); - }) - .join('|'); - - // Include paragraph-level attributes that affect rendering (alignment, spacing, indent, etc.) - // This ensures DOM updates when toolbar commands like "align center" change these properties. - const attrs = block.attrs as ParagraphAttrs | undefined; - - const paragraphAttrsVersion = attrs - ? [ - attrs.alignment ?? '', - attrs.spacing?.before ?? '', - attrs.spacing?.after ?? '', - attrs.spacing?.line ?? '', - attrs.spacing?.lineRule ?? '', - attrs.indent?.left ?? '', - attrs.indent?.right ?? '', - attrs.indent?.firstLine ?? '', - attrs.indent?.hanging ?? '', - attrs.borders ? hashParagraphBorders(attrs.borders) : '', - attrs.shading?.fill ?? '', - attrs.shading?.color ?? '', - getParagraphInlineDirection(attrs) ?? '', - attrs.tabs?.length ? JSON.stringify(attrs.tabs) : '', - ].join(':') - : ''; - - // Include SDT metadata so lock-mode (and other SDT property) changes invalidate the cache. - const sdtAttrs = (block.attrs as ParagraphAttrs | undefined)?.sdt; - const sdtVersion = getSdtMetadataVersion(sdtAttrs); - - // Combine marker version, runs version, paragraph attrs version, and SDT version - const parts = [markerVersion, runsVersion, paragraphAttrsVersion, sdtVersion].filter(Boolean); - return parts.join('|'); - } - - if (block.kind === 'list') { - return block.items.map((item) => `${item.id}:${item.marker.text}:${deriveBlockVersion(item.paragraph)}`).join('|'); - } - - if (block.kind === 'image') { - const imgSdt = (block as ImageBlock).attrs?.sdt; - const imgSdtVersion = getSdtMetadataVersion(imgSdt); - return [ - block.src ?? '', - block.width ?? '', - block.height ?? '', - block.alt ?? '', - block.title ?? '', - resolveBlockClipPath(block), - imgSdtVersion, - ].join('|'); - } - - if (block.kind === 'drawing') { - if (block.drawingKind === 'image') { - // Type narrowing: block is ImageDrawing (not ImageBlock) - const imageLike = block as ImageDrawing; - return [ - 'drawing:image', - imageLike.src ?? '', - imageLike.width ?? '', - imageLike.height ?? '', - imageLike.alt ?? '', - resolveBlockClipPath(imageLike), - ].join('|'); - } - if (block.drawingKind === 'vectorShape') { - const vector = block as VectorShapeDrawing; - return [ - 'drawing:vector', - vector.shapeKind ?? '', - vector.fillColor ?? '', - vector.strokeColor ?? '', - vector.strokeWidth ?? '', - vector.geometry.width, - vector.geometry.height, - vector.geometry.rotation ?? 0, - vector.geometry.flipH ? 1 : 0, - vector.geometry.flipV ? 1 : 0, - ].join('|'); - } - if (block.drawingKind === 'shapeGroup') { - const group = block as ShapeGroupDrawing; - const childSignature = group.shapes - .map((child) => `${child.shapeType}:${JSON.stringify(child.attrs ?? {})}`) - .join(';'); - return [ - 'drawing:group', - group.geometry.width, - group.geometry.height, - group.groupTransform ? JSON.stringify(group.groupTransform) : '', - childSignature, - ].join('|'); - } - if (block.drawingKind === 'chart') { - return [ - 'drawing:chart', - block.chartData?.chartType ?? '', - block.chartData?.series?.length ?? 0, - block.geometry.width, - block.geometry.height, - block.chartRelId ?? '', - ].join('|'); - } - // Exhaustiveness check: if a new drawingKind is added, TypeScript will error here - const _exhaustive: never = block; - return `drawing:unknown:${(block as DrawingBlock).id}`; - } - - if (block.kind === 'table') { - const tableBlock = block as TableBlock; - /** - * Local hash function for strings using FNV-1a algorithm. - * Used to create a robust hash across all table rows/cells so deep edits invalidate version. - * - * @param seed - Initial hash value - * @param value - String value to hash - * @returns Updated hash value - */ - const hashString = (seed: number, value: string): number => { - let hash = seed >>> 0; - for (let i = 0; i < value.length; i++) { - hash ^= value.charCodeAt(i); - hash = Math.imul(hash, 16777619); // FNV-style mix - } - return hash >>> 0; - }; - - /** - * Local hash function for numbers. - * Handles undefined/null values safely by treating them as 0. - * - * @param seed - Initial hash value - * @param value - Number value to hash (or undefined/null) - * @returns Updated hash value - */ - const hashNumber = (seed: number, value: number | undefined | null): number => { - const n = Number.isFinite(value) ? (value as number) : 0; - let hash = seed ^ n; - hash = Math.imul(hash, 16777619); - hash ^= hash >>> 13; - return hash >>> 0; - }; - - let hash = 2166136261; - hash = hashString(hash, block.id); - hash = hashNumber(hash, tableBlock.rows.length); - hash = (tableBlock.columnWidths ?? []).reduce((acc, width) => hashNumber(acc, Math.round(width * 1000)), hash); - - // Defensive guards: ensure rows array exists and iterate safely - const rows = tableBlock.rows ?? []; - 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 page fingerprint, - // applyHunks-style transactions that only mutate the row's `trackChange` - // attr don't invalidate the page cache, so the visible cells never receive - // their decoration classes and the row stays unmarked. Read from - // `row.attrs.trackedChange` — the canonical location written by the v1 - // layout-adapter from the OOXML-aligned `attrs.trackChange.type` shape. - 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] : []); - hash = hashNumber(hash, cellBlocks.length); - // Include cell attributes that affect rendering (rowSpan, colSpan, borders, etc.) - hash = hashNumber(hash, cell.rowSpan ?? 1); - hash = hashNumber(hash, cell.colSpan ?? 1); - - // Include cell-level attributes (borders, padding, background) that affect rendering - // This ensures cache invalidation when cell formatting changes (e.g., remove borders). - if (cell.attrs) { - const cellAttrs = cell.attrs as TableCellAttrs; - if (cellAttrs.borders) { - hash = hashString(hash, hashCellBorders(cellAttrs.borders)); - } - if (cellAttrs.padding) { - const p = cellAttrs.padding; - hash = hashNumber(hash, p.top ?? 0); - hash = hashNumber(hash, p.right ?? 0); - hash = hashNumber(hash, p.bottom ?? 0); - hash = hashNumber(hash, p.left ?? 0); - } - if (cellAttrs.verticalAlign) { - hash = hashString(hash, cellAttrs.verticalAlign); - } - if (cellAttrs.background) { - hash = hashString(hash, cellAttrs.background); - } - } - - for (const cellBlock of cellBlocks) { - hash = hashString(hash, cellBlock?.kind ?? 'unknown'); - if (cellBlock?.kind === 'paragraph') { - const paragraphBlock = cellBlock as ParagraphBlock; - const runs = paragraphBlock.runs ?? []; - hash = hashNumber(hash, runs.length); - - // Include paragraph-level attributes that affect rendering - // (alignment, spacing, indent, etc.) - fixes toolbar commands not updating tables - const attrs = paragraphBlock.attrs as ParagraphAttrs | undefined; - - if (attrs) { - hash = hashString(hash, attrs.alignment ?? ''); - hash = hashNumber(hash, attrs.spacing?.before ?? 0); - hash = hashNumber(hash, attrs.spacing?.after ?? 0); - hash = hashNumber(hash, attrs.spacing?.line ?? 0); - hash = hashString(hash, attrs.spacing?.lineRule ?? ''); - hash = hashNumber(hash, attrs.indent?.left ?? 0); - hash = hashNumber(hash, attrs.indent?.right ?? 0); - hash = hashNumber(hash, attrs.indent?.firstLine ?? 0); - hash = hashNumber(hash, attrs.indent?.hanging ?? 0); - hash = hashString(hash, attrs.shading?.fill ?? ''); - hash = hashString(hash, attrs.shading?.color ?? ''); - hash = hashString(hash, getParagraphInlineDirection(attrs) ?? ''); - if (attrs.borders) { - hash = hashString(hash, hashParagraphBorders(attrs.borders)); - } - } - - for (const run of runs) { - // Only text runs have .text property; ImageRun does not - if ('text' in run && typeof run.text === 'string') { - hash = hashString(hash, run.text); - } - hash = hashNumber(hash, run.pmStart ?? -1); - hash = hashNumber(hash, run.pmEnd ?? -1); - - // Include run formatting properties that affect rendering - // (color, highlight, bold, italic, etc.) - fixes toolbar commands not updating tables - hash = hashString(hash, getRunStringProp(run, 'color')); - hash = hashString(hash, getRunStringProp(run, 'highlight')); - hash = hashString(hash, getRunBooleanProp(run, 'bold') ? '1' : ''); - hash = hashString(hash, getRunBooleanProp(run, 'italic') ? '1' : ''); - hash = hashNumber(hash, getRunNumberProp(run, 'fontSize')); - hash = hashString(hash, getRunStringProp(run, 'fontFamily')); - hash = hashString(hash, getRunUnderlineStyle(run)); - hash = hashString(hash, getRunUnderlineColor(run)); - hash = hashString(hash, getRunBooleanProp(run, 'strike') ? '1' : ''); - hash = hashString(hash, getRunStringProp(run, 'vertAlign')); - hash = hashNumber(hash, getRunNumberProp(run, 'baselineShift')); - } - } else if (cellBlock?.kind) { - // Non-paragraph cell blocks participate in the parent table version - // through their own block-level signatures. layout-bridge/cache.ts - // mirrors this policy so repaint and remeasure stay aligned for - // nested tables, images, drawings, and other embedded cell content. - hash = hashString(hash, deriveBlockVersion(cellBlock as FlowBlock)); - } - } - } - } - - // Include table-level attributes (borders, etc.) that affect rendering - // This ensures cache invalidation when table formatting changes (e.g., remove borders). - if (tableBlock.attrs) { - const tblAttrs = tableBlock.attrs as TableAttrs; - if (tblAttrs.borders) { - hash = hashString(hash, hashTableBorders(tblAttrs.borders)); - } - if (tblAttrs.borderCollapse) { - hash = hashString(hash, tblAttrs.borderCollapse); - } - if (tblAttrs.cellSpacing !== undefined) { - const cs = tblAttrs.cellSpacing; - if (typeof cs === 'number') { - hash = hashNumber(hash, cs); - } else { - // Stable key: value and type only (avoid JSON.stringify key-order variance) - const v = (cs as { value?: number; type?: string }).value ?? 0; - const t = (cs as { value?: number; type?: string }).type ?? 'px'; - hash = hashString(hash, `cs:${v}:${t}`); - } - } - // Include SDT metadata so lock-mode changes invalidate the cache. - if (tblAttrs.sdt) { - hash = hashString(hash, tblAttrs.sdt.type); - hash = hashString(hash, getSdtMetadataLockMode(tblAttrs.sdt)); - hash = hashString(hash, getSdtMetadataId(tblAttrs.sdt)); - } - } - - return [block.id, tableBlock.rows.length, hash.toString(16)].join('|'); - } - - return block.id; -}; - -const DEFAULT_SUPERSCRIPT_RAISE_RATIO = 0.33; -const DEFAULT_SUBSCRIPT_LOWER_RATIO = 0.14; - -const hasVerticalPositioning = (run: TextRun): boolean => - normalizeBaselineShift(run.baselineShift) != null || run.vertAlign === 'superscript' || run.vertAlign === 'subscript'; - -const applyRunVerticalPositioning = (element: HTMLElement, run: TextRun): void => { - // Vertically shifted runs should use a tight inline box. If they inherit the - // parent line's full line-height, the glyph remains visually low inside an - // oversized inline box even when the superscript/subscript offset is correct. - if (hasVerticalPositioning(run)) { - element.style.lineHeight = '1'; - } - - const explicitBaselineShift = normalizeBaselineShift(run.baselineShift); - if (explicitBaselineShift != null) { - element.style.verticalAlign = `${explicitBaselineShift}pt`; - return; - } - - if (run.vertAlign === 'superscript') { - const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); - element.style.verticalAlign = `${baseFontSize * DEFAULT_SUPERSCRIPT_RAISE_RATIO}px`; - return; - } - - if (run.vertAlign === 'subscript') { - const baseFontSize = resolveBaseFontSizeForVerticalText(run.fontSize, run); - element.style.verticalAlign = `${-(baseFontSize * DEFAULT_SUBSCRIPT_LOWER_RATIO)}px`; - return; - } - - if (run.vertAlign === 'baseline') { - element.style.verticalAlign = 'baseline'; - } -}; - -/** - * Applies run styling properties to a DOM element. - * - * @param element - The HTML element to style - * @param run - The run object containing styling information - * @param _isLink - Whether this run is part of a hyperlink. Note: This parameter - * is kept for API compatibility but no longer affects behavior - - * inline colors are now applied to all runs (including links) to - * ensure OOXML hyperlink character styles appear correctly. - */ -const applyRunStyles = (element: HTMLElement, run: Run, _isLink = false): void => { - if ( - run.kind === 'tab' || - run.kind === 'image' || - run.kind === 'lineBreak' || - run.kind === 'break' || - run.kind === 'fieldAnnotation' || - run.kind === 'math' - ) { - // Tab, image, lineBreak, break, and fieldAnnotation runs don't have text styling properties - return; - } - - element.style.fontFamily = run.fontFamily; - element.style.fontSize = `${run.fontSize}px`; - if (run.bold) element.style.fontWeight = 'bold'; - if (run.italic) element.style.fontStyle = 'italic'; - - // Apply inline color even for links so OOXML hyperlink styles appear when CSS is absent - if (run.color) element.style.color = run.color; - - if (run.letterSpacing != null) { - element.style.letterSpacing = `${run.letterSpacing}px`; - } - if (run.highlight) { - element.style.backgroundColor = run.highlight; - } - if (run.textTransform) { - element.style.textTransform = run.textTransform; - } - - // Apply text decorations from the run. Even for links, inline decorations should reflect - // the document styling (tests assert underline presence on anchors). - const decorations: string[] = []; - if (run.underline) { - decorations.push('underline'); - const u = run.underline; - element.style.textDecorationStyle = u.style && u.style !== 'single' ? u.style : 'solid'; - if (u.color) { - element.style.textDecorationColor = u.color; - } - } - if (run.strike) { - decorations.push('line-through'); - } - if (decorations.length > 0) { - element.style.textDecorationLine = decorations.join(' '); - } - - applyRunVerticalPositioning(element, run); -}; - -const CLIP_PATH_PREFIXES = ['inset(', 'polygon(', 'circle(', 'ellipse(', 'path(', 'rect(']; - -const readClipPathValue = (value: unknown): string => { - if (typeof value !== 'string') return ''; - const normalized = value.trim(); - if (normalized.length === 0) return ''; - const lower = normalized.toLowerCase(); - if (!CLIP_PATH_PREFIXES.some((prefix) => lower.startsWith(prefix))) return ''; - return normalized; -}; - -const resolveClipPathFromAttrs = (attrs: unknown): string => { - if (!attrs || typeof attrs !== 'object') return ''; - const record = attrs as Record; - return readClipPathValue(record.clipPath); -}; - -const resolveBlockClipPath = (block: unknown): string => { - if (!block || typeof block !== 'object') return ''; - const record = block as Record; - return readClipPathValue(record.clipPath) || resolveClipPathFromAttrs(record.attrs); -}; - -/** - * Applies data-* attributes from a text run to a DOM element. - * Validates attribute names and safely sets them on the element. - * Invalid or unsafe attributes are skipped with development-mode logging. - * - * @param element - The HTML element to apply attributes to - * @param dataAttrs - Record of data-* attribute key-value pairs from the text run - * - * @example - * ```typescript - * const span = document.createElement('span'); - * applyRunDataAttributes(span, { 'data-id': '123', 'data-name': 'test' }); - * // span now has: - * ``` - */ -export const applyRunDataAttributes = (element: HTMLElement, dataAttrs?: Record): void => { - if (!dataAttrs) return; - Object.entries(dataAttrs).forEach(([key, value]) => { - if (typeof key !== 'string' || !key.toLowerCase().startsWith('data-')) return; - if (typeof value !== 'string') return; - try { - element.setAttribute(key, value); - } catch (error) { - if (process.env.NODE_ENV === 'development') { - console.warn(`[DomPainter] Failed to set data attribute "${key}":`, error); - } - } - }); -}; - -const applyParagraphBlockStyles = (element: HTMLElement, attrs?: ParagraphAttrs): void => { - if (!attrs) return; - if (attrs.styleId) { - element.setAttribute('styleid', attrs.styleId); - } - applyRtlStyles(element, attrs); - if ((attrs as Record).dropCap) { - element.classList.add('sd-editor-dropcap'); - } - const indent = attrs.indent; - if (indent) { - // Only apply positive indents as padding. - // Negative indents are handled by fragment positioning in the layout engine. - if (indent.left && indent.left > 0) { - element.style.paddingLeft = `${indent.left}px`; - } - if (indent.right && indent.right > 0) { - element.style.paddingRight = `${indent.right}px`; - } - // Skip textIndent when left indent is negative - fragment positioning handles the indent, - // and per-line paddingLeft handles the hanging indent for body lines. - const hasNegativeLeftIndent = indent.left != null && indent.left < 0; - if (!hasNegativeLeftIndent) { - const textIndent = (indent.firstLine ?? 0) - (indent.hanging ?? 0); - if (textIndent) { - element.style.textIndent = `${textIndent}px`; - } - } - } -}; - -// getParagraphBorderBox, createParagraphDecorationLayers, applyParagraphBorderStyles, -// setBorderSideStyle, applyParagraphShadingStyles — moved to features/paragraph-borders/ - -const stripListIndent = (attrs?: ParagraphAttrs): ParagraphAttrs | undefined => { - if (!attrs?.indent || attrs.indent.left == null) { - return attrs; - } - const nextIndent = { ...attrs.indent }; - delete nextIndent.left; - - return { - ...attrs, - indent: Object.keys(nextIndent).length > 0 ? nextIndent : undefined, - }; -}; - -// applyParagraphShadingStyles — moved to features/paragraph-borders/border-layer.ts - -const applyStyles = (el: HTMLElement, styles: Partial): void => { - Object.entries(styles).forEach(([key, value]) => { - if (value != null && value !== '' && key in el.style) { - (el.style as unknown as Record)[key] = String(value); - } - }); -}; - -const resolveRunText = (run: Run, context: FragmentRenderContext): string => { - const runToken = 'token' in run ? run.token : undefined; - - if (run.kind === 'tab') { - return run.text; - } - if (run.kind === 'image') { - // Image runs don't have text content - return ''; - } - if (run.kind === 'lineBreak') { - // Line break runs don't render text - the measurer creates new lines for them - return ''; - } - if (run.kind === 'break') { - // Break runs don't render text - the measurer creates new lines for them - return ''; - } - if (!('text' in run)) { - // Safety check - if run doesn't have text property, return empty string - return ''; - } - if (!runToken) { - return run.text ?? ''; - } - if (runToken === 'pageNumber') { - return context.pageNumberText ?? String(context.pageNumber); - } - if (runToken === 'totalPageCount') { - return context.totalPages ? String(context.totalPages) : (run.text ?? ''); - } - return run.text ?? ''; -};