Skip to content

fix: comment export to merge fragmented marks per commentId #1288

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
financialvice wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: comment export to merge fragmented marks per commentId #1288

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 49 additions & 59 deletions packages/super-editor/src/extensions/comment/comments-helpers.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
30 changes: 30 additions & 0 deletions packages/super-editor/src/extensions/comment/comments.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, [
Expand Down