From 058e57ee117973de4b74ea9dfa0d9760903aff16 Mon Sep 17 00:00:00 2001 From: cam Date: Wed, 19 Nov 2025 21:33:54 -0800 Subject: [PATCH] Fix comment export to merge fragmented marks per commentId --- .../extensions/comment/comments-helpers.js | 108 ++++++++---------- .../src/extensions/comment/comments.test.js | 30 +++++ 2 files changed, 79 insertions(+), 59 deletions(-) diff --git a/packages/super-editor/src/extensions/comment/comments-helpers.js b/packages/super-editor/src/extensions/comment/comments-helpers.js index 2804092732..189484f06b 100644 --- a/packages/super-editor/src/extensions/comment/comments-helpers.js +++ b/packages/super-editor/src/extensions/comment/comments-helpers.js @@ -56,75 +56,65 @@ export const getCommentPositionsById = (commentId, doc) => { * @returns {void} */ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { - // Collect all pending insertions in an array - const startNodes = []; - const endNodes = []; - const seen = new Set(); + // Collect a min..max range for every commentId (and children), then insert a + // single start/end pair per id. This prevents fragmented highlights when marks + // span multiple adjacent nodes. + const ranges = new Map(); + + const addRange = (id, from, to, attrs = {}) => { + if (!id || id === 'pending') return; + const existing = ranges.get(id); + if (existing) { + existing.from = Math.min(existing.from, from); + existing.to = Math.max(existing.to, to); + existing.attrs = { ...existing.attrs, ...attrs }; + } else { + ranges.set(id, { from, to, attrs: { ...attrs } }); + } + }; doc.descendants((node, pos) => { const commentMarks = node.marks?.filter((mark) => mark.type.name === CommentMarkName); commentMarks.forEach((commentMark) => { - if (commentMark) { - const { attrs = {} } = commentMark; - const { commentId } = attrs; - - if (commentId === 'pending') return; - if (seen.has(commentId)) return; - seen.add(commentId); - - const commentStartNodeAttrs = getPreparedComment(commentMark.attrs); - const startNode = schema.nodes.commentRangeStart.create(commentStartNodeAttrs); - startNodes.push({ - pos, - node: startNode, - }); - - const endNode = schema.nodes.commentRangeEnd.create(commentStartNodeAttrs); - endNodes.push({ - pos: pos + node.nodeSize, - node: endNode, - }); - - const parentId = commentId; - if (parentId) { - const childComments = comments - .filter((c) => c.parentCommentId === parentId) - .sort((a, b) => a.createdTime - b.createdTime); - - childComments.forEach((c) => { - const childMark = getPreparedComment(c); - const childStartNode = schema.nodes.commentRangeStart.create(childMark); - seen.add(c.commentId); - startNodes.push({ - pos: pos, - node: childStartNode, - }); - - const childEndNode = schema.nodes.commentRangeEnd.create(childMark); - endNodes.push({ - pos: pos + node.nodeSize, - node: childEndNode, - }); - }); - } - } - }); - }); + if (!commentMark) return; + const { attrs = {} } = commentMark; + const { commentId } = attrs; + const from = pos; + const to = pos + node.nodeSize; - startNodes.forEach((n) => { - const { pos, node } = n; - const mappedPos = tr.mapping.map(pos); + addRange(commentId, from, to, attrs); - tr.insert(mappedPos, node); - }); + // Also include child comments so they inherit the same span + const childComments = comments + .filter((c) => c.parentCommentId === commentId) + .sort((a, b) => a.createdTime - b.createdTime); - endNodes.forEach((n) => { - const { pos, node } = n; - const mappedPos = tr.mapping.map(pos); + childComments.forEach((c) => { + addRange(c.commentId, from, to, c); + }); + }); + }); - tr.insert(mappedPos, node); + const insertions = []; + ranges.forEach((range, id) => { + const attrs = getPreparedComment(range.attrs); + // Preserve internal flag if present + if (range.attrs.internal !== undefined) { + attrs.internal = range.attrs.internal; + } + const startNode = schema.nodes.commentRangeStart.create(attrs); + const endNode = schema.nodes.commentRangeEnd.create(attrs); + insertions.push({ pos: range.from, node: startNode }); + insertions.push({ pos: range.to, node: endNode }); }); + insertions + .sort((a, b) => a.pos - b.pos) + .forEach(({ pos, node }) => { + const mappedPos = tr.mapping.map(pos); + tr.insert(mappedPos, node); + }); + return tr; }; diff --git a/packages/super-editor/src/extensions/comment/comments.test.js b/packages/super-editor/src/extensions/comment/comments.test.js index f87787e888..a6e9ca7281 100644 --- a/packages/super-editor/src/extensions/comment/comments.test.js +++ b/packages/super-editor/src/extensions/comment/comments.test.js @@ -154,6 +154,36 @@ describe('comment helpers', () => { expect(insertedEnds).toEqual(['root', 'child-0', 'child-1']); }); + it('merges fragmented marks for the same comment into one range on export', () => { + const schema = createCommentSchema(); + const mark = schema.marks[CommentMarkName].create({ commentId: 'frag', internal: true }); + const paragraph = schema.nodes.paragraph.create(null, [ + schema.text('Plaintiffs ', [mark]), + schema.text('allege', [mark]), + schema.text('s', [mark]), + ]); + const doc = schema.nodes.doc.create(null, [paragraph]); + const state = EditorState.create({ schema, doc }); + const tr = state.tr; + + prepareCommentsForExport(state.doc, tr, schema, []); + const applied = state.apply(tr); + + const starts = []; + const ends = []; + applied.doc.descendants((node, pos) => { + if (node.type.name === 'commentRangeStart') starts.push(pos); + if (node.type.name === 'commentRangeEnd') ends.push(pos); + }); + + expect(starts).toHaveLength(1); + expect(ends).toHaveLength(1); + // The range should span the full "Plaintiffs alleges" from the first char. + expect(starts[0]).toBe(1); + const totalTextLength = 'Plaintiffs alleges'.length; + expect(ends[0]).toBe(1 + totalTextLength + 1); // +1 because positions are exclusive + }); + it('prepares comments for import by converting nodes into marks', () => { const schema = createCommentSchema(); const doc = schema.nodes.doc.create(null, [