diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 6d555985f..b2b2513dc 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -2685,7 +2685,7 @@ describe('DomPainter', () => { expect(span.dataset.trackChangeAuthorEmail).toBe('reviewer@example.com'); }); - it('keeps comment metadata but skips highlight styles for tracked-change comments', () => { + it('applies background highlight for comments on tracked-change text', () => { const trackedCommentBlock: FlowBlock = { kind: 'paragraph', id: 'tracked-comment-block', @@ -2709,12 +2709,14 @@ describe('DomPainter', () => { ); const painter = createDomPainter({ blocks: [trackedCommentBlock], measures: [paragraphMeasure] }); + painter.setActiveComment('comment-1'); painter.paint(paragraphLayout, mount); const span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; expect(span).toBeTruthy(); expect(span.dataset.commentIds).toBe('comment-1'); - expect(span.style.backgroundColor).toBe(''); + // Comments on tracked change text should have normal background-color highlight + expect(span.style.backgroundColor).not.toBe(''); }); it('applies comment highlight styles for non-tracked-change comments', () => { @@ -2745,6 +2747,117 @@ describe('DomPainter', () => { expect(span.style.backgroundColor).not.toBe(''); }); + it('highlights only the active comment range when setActiveComment is called', () => { + // Single run with comment-A + const commentBlock: FlowBlock = { + kind: 'paragraph', + id: 'active-comment-block', + runs: [ + { + text: 'Commented text', + fontFamily: 'Arial', + fontSize: 16, + comments: [{ commentId: 'comment-A', internal: false }], + }, + ], + }; + + const { paragraphMeasure, paragraphLayout } = buildSingleParagraphData( + commentBlock.id, + commentBlock.runs[0].text.length, + ); + + const painter = createDomPainter({ blocks: [commentBlock], measures: [paragraphMeasure] }); + + // Initially (no active comment), should be highlighted + painter.paint(paragraphLayout, mount); + let span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; + expect(span.style.backgroundColor).not.toBe(''); + + // Select comment-A: should still be highlighted + painter.setActiveComment('comment-A'); + painter.paint(paragraphLayout, mount); + span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; + expect(span.style.backgroundColor).not.toBe(''); + + // Select a different comment (comment-B): should NOT be highlighted + painter.setActiveComment('comment-B'); + painter.paint(paragraphLayout, mount); + span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; + expect(span.style.backgroundColor).toBe(''); // Not highlighted because active comment doesn't match + }); + + it('shows nested comment indicators when outer comment is selected', () => { + // One run with two comments (outer and nested) + const nestedCommentBlock: FlowBlock = { + kind: 'paragraph', + id: 'nested-comment-block', + runs: [ + { + text: 'Nested area', + fontFamily: 'Arial', + fontSize: 16, + comments: [ + { commentId: 'outer-comment', internal: false }, + { commentId: 'inner-comment', internal: false }, + ], + }, + ], + }; + + const { paragraphMeasure, paragraphLayout } = buildSingleParagraphData( + nestedCommentBlock.id, + nestedCommentBlock.runs[0].text.length, + ); + + const painter = createDomPainter({ blocks: [nestedCommentBlock], measures: [paragraphMeasure] }); + + // Select outer comment + painter.setActiveComment('outer-comment'); + painter.paint(paragraphLayout, mount); + + const span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; + expect(span).toBeTruthy(); + expect(span.style.backgroundColor).not.toBe(''); + // Should have box-shadow indicating nested comment + expect(span.style.boxShadow).not.toBe(''); + }); + + it('clears active comment highlighting when setActiveComment(null) is called', () => { + const commentBlock: FlowBlock = { + kind: 'paragraph', + id: 'clear-comment-block', + runs: [ + { + text: 'Some text', + fontFamily: 'Arial', + fontSize: 16, + comments: [{ commentId: 'comment-X', internal: false }], + }, + ], + }; + + const { paragraphMeasure, paragraphLayout } = buildSingleParagraphData( + commentBlock.id, + commentBlock.runs[0].text.length, + ); + + const painter = createDomPainter({ blocks: [commentBlock], measures: [paragraphMeasure] }); + + // First select a comment + painter.setActiveComment('comment-X'); + painter.paint(paragraphLayout, mount); + + // Then deselect + painter.setActiveComment(null); + painter.paint(paragraphLayout, mount); + + const span = mount.querySelector('.superdoc-comment-highlight') as HTMLElement; + expect(span).toBeTruthy(); + // Should still have background (default highlighting) + expect(span.style.backgroundColor).not.toBe(''); + }); + it('respects trackedChangesMode modifiers for insertions', () => { const finalBlock: FlowBlock = { kind: 'paragraph', diff --git a/packages/layout-engine/painters/dom/src/index.ts b/packages/layout-engine/painters/dom/src/index.ts index cc1de409a..5f05c74c5 100644 --- a/packages/layout-engine/painters/dom/src/index.ts +++ b/packages/layout-engine/painters/dom/src/index.ts @@ -116,6 +116,8 @@ export const createDomPainter = ( ): PainterDOM & { setProviders?: (header?: PageDecorationProvider, footer?: PageDecorationProvider) => void; setVirtualizationPins?: (pageIndices: number[] | null | undefined) => void; + setActiveComment?: (commentId: string | null) => void; + getActiveComment?: () => string | null; } => { const painter = new DomPainter(options.blocks, options.measures, { pageStyles: options.pageStyles, @@ -148,5 +150,11 @@ export const createDomPainter = ( setVirtualizationPins(pageIndices: number[] | null | undefined) { painter.setVirtualizationPins(pageIndices); }, + setActiveComment(commentId: string | null) { + painter.setActiveComment(commentId); + }, + getActiveComment() { + return painter.getActiveComment(); + }, }; }; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 3efcf57f9..013b44e3a 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -382,7 +382,8 @@ const DEFAULT_PAGE_HEIGHT_PX = 1056; const DEFAULT_VIRTUALIZED_PAGE_GAP = 72; const COMMENT_EXTERNAL_COLOR = '#B1124B'; const COMMENT_INTERNAL_COLOR = '#078383'; -const COMMENT_INACTIVE_ALPHA = '22'; +const COMMENT_INACTIVE_ALPHA = '40'; // ~25% for inactive +const COMMENT_ACTIVE_ALPHA = '66'; // ~40% for active/selected type LinkRenderData = { href?: string; @@ -820,6 +821,8 @@ export class DomPainter { private onScrollHandler: ((e: Event) => void) | null = null; private onWindowScrollHandler: ((e: Event) => void) | null = null; private onResizeHandler: ((e: Event) => void) | null = null; + /** The currently active/selected comment ID for highlighting */ + private activeCommentId: string | null = null; constructor(blocks: FlowBlock[], measures: Measure[], options: PainterOptions = {}) { this.options = options; @@ -872,6 +875,37 @@ export class DomPainter { } } + /** + * Sets the active comment ID for highlighting. + * When set, only the active comment's range is highlighted. + * When null, all comments show depth-based highlighting. + */ + public setActiveComment(commentId: string | null): void { + if (this.activeCommentId !== commentId) { + this.activeCommentId = commentId; + // Force re-render of all pages by incrementing layout version + // This bypasses the virtualization cache check + this.layoutVersion += 1; + // Clear page states to force full re-render (activeCommentId affects run rendering) + // For virtualized mode: remove existing page elements before clearing state + // to prevent duplicate pages in the DOM + for (const state of this.pageIndexToState.values()) { + state.element.remove(); + } + this.pageIndexToState.clear(); + this.virtualMountedKey = ''; + // For non-virtualized mode: + this.pageStates = []; + } + } + + /** + * Gets the currently active comment ID. + */ + public getActiveComment(): string | null { + return this.activeCommentId; + } + /** * Updates the painter's block and measure data. * @@ -3722,11 +3756,18 @@ export class DomPainter { const textRun = run as TextRun; const commentAnnotations = textRun.comments; const hasAnyComment = !!commentAnnotations?.length; - const hasHighlightableComment = !!commentAnnotations?.some((c) => !c.trackedChange); - const commentColor = getCommentHighlight(textRun); - - if (commentColor && !textRun.highlight && hasHighlightableComment) { - (elem as HTMLElement).style.backgroundColor = commentColor; + const commentHighlight = getCommentHighlight(textRun, this.activeCommentId); + + if (commentHighlight.color && !textRun.highlight && hasAnyComment) { + (elem as HTMLElement).style.backgroundColor = commentHighlight.color; + // Add thin visual indicator for nested comments when outer comment is selected + // Use box-shadow instead of border to avoid affecting text layout + if (commentHighlight.hasNestedComments && commentHighlight.baseColor) { + const borderColor = `${commentHighlight.baseColor}99`; // Semi-transparent for subtlety + (elem as HTMLElement).style.boxShadow = `inset 1px 0 0 ${borderColor}, inset -1px 0 0 ${borderColor}`; + } else { + (elem as HTMLElement).style.boxShadow = ''; + } } // We still need to preserve the comment ids if (hasAnyComment) { @@ -5735,12 +5776,47 @@ const applyRunStyles = (element: HTMLElement, run: Run, _isLink = false): void = } }; -const getCommentHighlight = (run: TextRun): string | undefined => { +interface CommentHighlightResult { + color?: string; + baseColor?: string; + hasNestedComments?: boolean; +} + +const getCommentHighlight = (run: TextRun, activeCommentId: string | null): CommentHighlightResult => { const comments = run.comments; - if (!comments || comments.length === 0) return undefined; + if (!comments || comments.length === 0) return {}; + + // Helper to match comment by ID or importedId + const matchesId = (c: { commentId: string; importedId?: string }, id: string) => + c.commentId === id || c.importedId === id; + + // When a comment is selected, only highlight that comment's range + if (activeCommentId != null) { + const activeComment = comments.find((c) => + matchesId(c as { commentId: string; importedId?: string }, activeCommentId), + ); + if (activeComment) { + const base = activeComment.internal ? COMMENT_INTERNAL_COLOR : COMMENT_EXTERNAL_COLOR; + // Check if there are OTHER comments besides the active one (nested comments) + const nestedComments = comments.filter( + (c) => !matchesId(c as { commentId: string; importedId?: string }, activeCommentId), + ); + return { + color: `${base}${COMMENT_ACTIVE_ALPHA}`, + baseColor: base, + hasNestedComments: nestedComments.length > 0, + }; + } + // This run doesn't contain the active comment - still show light highlight for its own comments + const primary = comments[0]; + const base = primary.internal ? COMMENT_INTERNAL_COLOR : COMMENT_EXTERNAL_COLOR; + return { color: `${base}${COMMENT_INACTIVE_ALPHA}` }; + } + + // No active comment - show uniform light highlight (like Word/Google Docs) const primary = comments[0]; const base = primary.internal ? COMMENT_INTERNAL_COLOR : COMMENT_EXTERNAL_COLOR; - return `${base}${COMMENT_INACTIVE_ALPHA}`; + return { color: `${base}${COMMENT_INACTIVE_ALPHA}` }; }; /** diff --git a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts index 8b88df9e1..98e7bdfcc 100644 --- a/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts +++ b/packages/super-editor/src/core/presentation-editor/PresentationEditor.ts @@ -2277,6 +2277,27 @@ export class PresentationEditor extends EventEmitter { event: 'remoteHeaderFooterChanged', handler: handleRemoteHeaderFooterChanged as (...args: unknown[]) => void, }); + + // Listen for comment selection changes to update Layout Engine highlighting + const handleCommentsUpdate = (payload: { activeCommentId?: string | null }) => { + if (this.#domPainter?.setActiveComment) { + // Only update active comment when the field is explicitly present in the payload. + // This prevents unrelated events (like tracked change updates) from clearing + // the active comment selection unexpectedly. + if ('activeCommentId' in payload) { + const activeId = payload.activeCommentId ?? null; + this.#domPainter.setActiveComment(activeId); + // Mark as needing re-render to apply the new active comment highlighting + this.#pendingDocChange = true; + this.#scheduleRerender(); + } + } + }; + this.#editor.on('commentsUpdate', handleCommentsUpdate); + this.#editorListeners.push({ + event: 'commentsUpdate', + handler: handleCommentsUpdate as (...args: unknown[]) => void, + }); } /** diff --git a/packages/super-editor/src/core/super-converter/SuperConverter.js b/packages/super-editor/src/core/super-converter/SuperConverter.js index 6863be933..dbda828d3 100644 --- a/packages/super-editor/src/core/super-converter/SuperConverter.js +++ b/packages/super-editor/src/core/super-converter/SuperConverter.js @@ -961,7 +961,9 @@ class SuperConverter { exportJsonOnly = false, fieldsHighlightColor, ) { - const commentsWithParaIds = comments.map((c) => prepareCommentParaIds(c)); + // Filter out synthetic tracked change comments - they shouldn't be exported to comments.xml + const exportableComments = comments.filter((c) => !c.trackedChange); + const commentsWithParaIds = exportableComments.map((c) => prepareCommentParaIds(c)); const commentDefinitions = commentsWithParaIds.map((c, index) => getCommentDefinition(c, index, commentsWithParaIds, editor), ); @@ -969,7 +971,7 @@ class SuperConverter { const { result, params } = this.exportToXmlJson({ data: jsonData, editorSchema, - comments, + comments: exportableComments, commentDefinitions, commentsExportType, isFinalDoc, diff --git a/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.js b/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.js index 06a56757b..8758b491a 100644 --- a/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.js +++ b/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.js @@ -170,13 +170,22 @@ export const updateCommentsExtendedXml = (comments = [], commentsExtendedXml, th } const exportStrategy = typeof threadingProfile === 'string' ? threadingProfile : 'word'; const profile = typeof threadingProfile === 'string' ? null : threadingProfile; + const hasThreadedComments = comments.some((comment) => comment.threadingParentCommentId || comment.parentCommentId); + + // Always generate commentsExtended.xml when exporting comments (unless Google Docs style) + // This ensures that comments without threading relationships are explicitly marked as + // top-level comments, preventing range-based parenting on re-import from incorrectly + // creating threading relationships based on nested ranges. const shouldGenerateCommentsExtended = profile ? profile.defaultStyle === 'commentsExtended' || profile.mixed || comments.some((comment) => resolveThreadingStyle(comment, profile) === 'commentsExtended') - : exportStrategy === 'word' || comments.some((c) => c.originalXmlStructure?.hasCommentsExtended); + : exportStrategy !== 'google-docs'; // Generate for 'word' and 'unknown' strategies + + // If any threaded comments exist, always include commentsExtended.xml so Word can retain threads. + const shouldIncludeForThreads = hasThreadedComments; - if (!shouldGenerateCommentsExtended && exportStrategy === 'google-docs') { + if (!shouldGenerateCommentsExtended && !shouldIncludeForThreads) { return null; } @@ -195,7 +204,7 @@ export const updateCommentsExtendedXml = (comments = [], commentsExtendedXml, th // because Word doesn't recognize tracked changes as comment parents. const parentId = comment.threadingParentCommentId || comment.parentCommentId; const threadingStyle = resolveThreadingStyle(comment, profile); - if (parentId && threadingStyle === 'commentsExtended') { + if (parentId && (threadingStyle === 'commentsExtended' || shouldIncludeForThreads)) { const parentComment = comments.find((c) => c.commentId === parentId); const allowTrackedParent = profile?.defaultStyle === 'commentsExtended'; if (parentComment && (allowTrackedParent || !parentComment.trackedChange)) { @@ -276,7 +285,11 @@ export const updateDocumentRels = () => { export const generateConvertedXmlWithCommentFiles = (convertedXml, fileSet = null) => { const newXml = carbonCopy(convertedXml); newXml['word/comments.xml'] = COMMENTS_XML_DEFINITIONS.COMMENTS_XML_DEF; - const includeExtended = fileSet ? fileSet.hasCommentsExtended : true; + // Always include commentsExtended.xml - it's needed to explicitly mark comments as + // top-level (no threading) and prevent range-based parenting on re-import. + // The updateCommentsExtendedXml function will decide whether to actually include it + // based on export strategy (e.g., skip for Google Docs style). + const includeExtended = true; const includeExtensible = fileSet ? fileSet.hasCommentsExtensible : true; const includeIds = fileSet ? fileSet.hasCommentsIds : true; diff --git a/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.test.js b/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.test.js index 10495c57b..3d3fba6b2 100644 --- a/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.test.js +++ b/packages/super-editor/src/core/super-converter/v2/exporter/commentsExporter.test.js @@ -103,4 +103,44 @@ describe('updateCommentsExtendedXml', () => { expect(childEntry.attributes['w15:paraIdParent']).toBe('PARENT-PARA'); }); + + it('sets paraIdParent for range-based threads to preserve Word threading', () => { + const comments = [ + { + commentId: 'parent-comment', + commentParaId: 'PARENT-PARA', + resolvedTime: null, + threadingMethod: 'range-based', + originalXmlStructure: { hasCommentsExtended: false }, + }, + { + commentId: 'child-comment', + commentParaId: 'CHILD-PARA', + parentCommentId: 'parent-comment', + resolvedTime: null, + threadingMethod: 'range-based', + originalXmlStructure: { hasCommentsExtended: false }, + }, + ]; + + const commentsExtendedXml = { + elements: [{ elements: [] }], + }; + + const profile = { + defaultStyle: 'range-based', + mixed: false, + fileSet: { + hasCommentsExtended: false, + hasCommentsExtensible: false, + hasCommentsIds: false, + }, + }; + + const result = updateCommentsExtendedXml(comments, commentsExtendedXml, profile); + const entries = result.elements[0].elements; + const childEntry = entries.find((entry) => entry.attributes['w15:paraId'] === 'CHILD-PARA'); + + expect(childEntry.attributes['w15:paraIdParent']).toBe('PARENT-PARA'); + }); }); diff --git a/packages/super-editor/src/core/super-converter/v2/importer/documentCommentsImporter.js b/packages/super-editor/src/core/super-converter/v2/importer/documentCommentsImporter.js index b00b5652b..506a4327b 100644 --- a/packages/super-editor/src/core/super-converter/v2/importer/documentCommentsImporter.js +++ b/packages/super-editor/src/core/super-converter/v2/importer/documentCommentsImporter.js @@ -166,6 +166,11 @@ const generateCommentsWithExtendedData = ({ docx, comments, converter, threading const trackedChangeParent = trackedChangeParentMap.get(comment.importedId); const isInsideTrackedChange = trackedChangeParent?.isTrackedChangeParent; + // Track whether comment has an entry in commentsExtended.xml + // If it has an entry but no paraIdParent, it's explicitly a top-level comment + // and we should NOT use range-based parenting as a fallback + const hasExtendedEntry = !!extendedDef; + if (extendedDef) { const details = getExtendedDetails(extendedDef); isDone = details.isDone ?? false; @@ -177,22 +182,34 @@ const generateCommentsWithExtendedData = ({ docx, comments, converter, threading c.elements?.some((el) => el.attrs?.['w14:paraId'] === details.paraIdParent), ); const rangeParent = rangeParentMap.get(comment.commentId); - if (parentComment?.trackedChange && rangeParent) { - threadingParentCommentId = rangeParent; + if (parentComment?.trackedChange) { + // Parent is a tracked change - use range parent if available, otherwise leave parentCommentId undefined + // (TC association is tracked separately via trackedChangeParentId, not parentCommentId) + if (rangeParent) { + threadingParentCommentId = rangeParent; + parentCommentId = threadingParentCommentId; + } + // If no rangeParent, we intentionally leave parentCommentId undefined + // so the comment appears as a separate bubble from the TC } else { + // Parent is a real comment (not a TC) - use it for threading threadingParentCommentId = parentComment?.commentId; - } - if (!isInsideTrackedChange) { parentCommentId = threadingParentCommentId; } } } - if (isInsideTrackedChange) { - parentCommentId = trackedChangeParent.trackedChangeId; - } - - if (!parentCommentId && rangeParentMap.has(comment.commentId)) { + // Track the tracked change association but don't use it as parentCommentId + // This keeps comments and tracked changes as separate bubbles in the UI + // while preserving the relationship for export and visual purposes + const trackedChangeParentId = isInsideTrackedChange ? trackedChangeParent.trackedChangeId : undefined; + + // Only use range-based parenting as fallback when: + // 1. parentCommentId is not set from commentsExtended.xml, AND + // 2. The comment has NO entry in commentsExtended.xml at all + // If a comment has an entry in commentsExtended.xml but no paraIdParent, + // it's explicitly a top-level comment - don't override with range-based parenting + if (!parentCommentId && !hasExtendedEntry && rangeParentMap.has(comment.commentId)) { parentCommentId = rangeParentMap.get(comment.commentId); if (threadingProfile?.defaultStyle === 'commentsExtended') { threadingStyleOverride = 'range-based'; @@ -205,6 +222,7 @@ const generateCommentsWithExtendedData = ({ docx, comments, converter, threading parentCommentId, threadingStyleOverride, threadingParentCommentId, + trackedChangeParentId, }; }); }; @@ -594,23 +612,24 @@ const findCommentsWithSharedStartPosition = (comments, rangePositions) => { const applyParentRelationships = (comments, parentMap, trackedChangeParentMap = new Map()) => { return comments.map((comment) => { const trackedChangeParent = trackedChangeParentMap.get(comment.importedId); - if (trackedChangeParent && trackedChangeParent.isTrackedChangeParent) { - return { - ...comment, - parentCommentId: trackedChangeParent.trackedChangeId, - }; - } + const updatedComment = + trackedChangeParent && trackedChangeParent.isTrackedChangeParent + ? { + ...comment, + trackedChangeParentId: trackedChangeParent.trackedChangeId, + } + : comment; const parentImportedId = parentMap.get(comment.importedId); if (parentImportedId) { const parentComment = comments.find((c) => c.importedId === parentImportedId); if (parentComment) { return { - ...comment, + ...updatedComment, parentCommentId: parentComment.commentId, }; } } - return comment; + return updatedComment; }); }; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.js index e3b859990..60ead7a37 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.js @@ -67,24 +67,12 @@ const decode = (params) => { return commentSchema; } - const trackedMark = node.marks?.find((mark) => mark.type === 'trackInsert' || mark.type === 'trackDelete'); - if (trackedMark) { - const wrapperName = trackedMark.type === 'trackDelete' ? 'w:del' : 'w:ins'; - const markAttrs = trackedMark.attrs || {}; - const date = markAttrs.date || new Date(Date.now()).toISOString().replace(/\.\d{3}Z$/, 'Z'); - const wrapperAttributes = { - ...(markAttrs.id ? { 'w:id': String(markAttrs.id) } : {}), - ...(markAttrs.author ? { 'w:author': markAttrs.author } : {}), - ...(markAttrs.authorEmail ? { 'w:authorEmail': markAttrs.authorEmail } : {}), - 'w:date': date, - }; - - return { - name: wrapperName, - attributes: wrapperAttributes, - elements: Array.isArray(commentSchema) ? commentSchema : [commentSchema], - }; - } + // Note: Comment range nodes may have trackInsert/trackDelete marks attached + // from prepareCommentsForExport(), but we should NOT wrap them in their own + // / elements. The ECMA-376 spec allows comment markers inside + // tracked change elements, so they should be output as bare markers and will + // naturally sit inside or around the tracked change wrapper for the text content. + // See SD-1519 for details. if (!parentComment?.trackedChange) { return commentSchema; diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.test.js index 01c56e4c6..7255c5624 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/commentRange/comment-range-translator.test.js @@ -126,7 +126,10 @@ describe('w:commentRangeStart and w:commentRangeEnd', () => { }); describe('decode:range-based tracked change wrappers', () => { - test('wraps range markers with w:ins when trackInsert mark is present', () => { + // SD-1519: Comment markers with tracked change marks should NOT be wrapped in their own + // / elements. The ECMA-376 spec allows comment markers inside tracked changes, + // so they should be output as bare nodes and naturally sit inside/around the text's TC wrapper. + test('does NOT wrap range markers when trackInsert mark is present', () => { const result = commentRangeStartTranslator.decode({ node: { type: 'commentRangeStart', @@ -148,19 +151,14 @@ describe('w:commentRangeStart and w:commentRangeEnd', () => { commentsExportType: 'external', }); + // Should return bare comment marker, not wrapped in w:ins expect(result).toStrictEqual({ - name: 'w:ins', - attributes: { - 'w:id': 'tc-1', - 'w:author': 'Author A', - 'w:authorEmail': 'author@example.com', - 'w:date': '2025-01-01T00:00:00Z', - }, - elements: [{ attributes: { 'w:id': '0' }, name: 'w:commentRangeStart' }], + name: 'w:commentRangeStart', + attributes: { 'w:id': '0' }, }); }); - test('wraps range markers with w:del when trackDelete mark is present', () => { + test('does NOT wrap range markers when trackDelete mark is present', () => { const result = commentRangeEndTranslator.decode({ node: { type: 'commentRangeEnd', @@ -182,19 +180,14 @@ describe('w:commentRangeStart and w:commentRangeEnd', () => { commentsExportType: 'external', }); - expect(result.name).toBe('w:del'); - expect(result.attributes).toStrictEqual({ - 'w:id': 'tc-2', - 'w:author': 'Author B', - 'w:authorEmail': 'authorb@example.com', - 'w:date': '2025-01-02T00:00:00Z', - }); - expect(result.elements[0]).toStrictEqual({ attributes: { 'w:id': '0' }, name: 'w:commentRangeEnd' }); - expect(result.elements[1].name).toBe('w:r'); - expect(result.elements[1].elements[0]).toStrictEqual({ - attributes: { 'w:id': '0' }, - name: 'w:commentReference', - }); + // Should return bare comment markers, not wrapped in w:del + expect(result).toStrictEqual([ + { name: 'w:commentRangeEnd', attributes: { 'w:id': '0' } }, + { + name: 'w:r', + elements: [{ name: 'w:commentReference', attributes: { 'w:id': '0' } }], + }, + ]); }); test('wraps replace threading with w:ins for start and w:del for end', () => { diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/p/helpers/translate-paragraph-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/p/helpers/translate-paragraph-node.js index dfe58e313..7132c7b43 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/p/helpers/translate-paragraph-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/p/helpers/translate-paragraph-node.js @@ -1,6 +1,82 @@ import { translateChildNodes } from '@converter/v2/exporter/helpers/index.js'; import { generateParagraphProperties } from './generate-paragraph-properties.js'; +/** + * Merge consecutive tracked change elements (w:ins/w:del) with the same ID. + * Comment range markers between tracked changes with the same ID are included + * inside the merged wrapper, matching Word's OOXML structure. + * + * See SD-1519 for details on the ECMA-376 spec compliance. + * + * @param {Array} elements The translated paragraph elements + * @returns {Array} Elements with consecutive tracked changes merged + */ +function mergeConsecutiveTrackedChanges(elements) { + if (!Array.isArray(elements) || elements.length === 0) return elements; + + const result = []; + let i = 0; + + while (i < elements.length) { + const current = elements[i]; + + // Check if this is a tracked change wrapper (w:ins or w:del) + if (current?.name === 'w:ins' || current?.name === 'w:del') { + const tcId = current.attributes?.['w:id']; + const tcName = current.name; + + // Collect consecutive elements that belong to this tracked change + const mergedElements = [...(current.elements || [])]; + let j = i + 1; + + while (j < elements.length) { + const next = elements[j]; + + // Include comment markers - they can sit inside tracked changes per ECMA-376 + if (next?.name === 'w:commentRangeStart' || next?.name === 'w:commentRangeEnd') { + mergedElements.push(next); + j++; + continue; + } + + // Include comment references (w:r containing w:commentReference) + if (next?.name === 'w:r') { + const hasOnlyCommentRef = next.elements?.length === 1 && next.elements[0]?.name === 'w:commentReference'; + if (hasOnlyCommentRef) { + mergedElements.push(next); + j++; + continue; + } + } + + // Merge with next tracked change if same type and ID + if (next?.name === tcName && next.attributes?.['w:id'] === tcId) { + mergedElements.push(...(next.elements || [])); + j++; + continue; + } + + // Stop merging when we hit a different element + break; + } + + // Create the merged wrapper + result.push({ + name: tcName, + attributes: { ...current.attributes }, + elements: mergedElements, + }); + + i = j; + } else { + result.push(current); + i++; + } + } + + return result; +} + /** * Translate a paragraph node * @@ -8,7 +84,10 @@ import { generateParagraphProperties } from './generate-paragraph-properties.js' * @returns {XmlReadyNode} JSON of the XML-ready paragraph node */ export function translateParagraphNode(params) { - const elements = translateChildNodes(params); + let elements = translateChildNodes(params); + + // Merge consecutive tracked changes with the same ID, including comment markers between them + elements = mergeConsecutiveTrackedChanges(elements); // Replace current paragraph with content of html annotation const htmlAnnotationChild = elements.find((element) => element.name === 'htmlAnnotation'); diff --git a/packages/super-editor/src/extensions/comment/comment-import-helpers.js b/packages/super-editor/src/extensions/comment/comment-import-helpers.js index d7db5f1ea..d451b936d 100644 --- a/packages/super-editor/src/extensions/comment/comment-import-helpers.js +++ b/packages/super-editor/src/extensions/comment/comment-import-helpers.js @@ -7,11 +7,13 @@ export const resolveCommentMeta = ({ converter, importedId }) => { const resolvedCommentId = matchingImportedComment?.commentId ?? (importedId ? String(importedId) : uuidv4()); const internal = matchingImportedComment?.internal ?? matchingImportedComment?.isInternal ?? false; const parentCommentId = matchingImportedComment?.parentCommentId; + const trackedChangeParentId = matchingImportedComment?.trackedChangeParentId; const trackedChangeIds = converter?.trackedChangeIdMap ? new Set(Array.from(converter.trackedChangeIdMap.values()).map((id) => String(id))) : null; - const isTrackedChangeParent = - parentCommentId && trackedChangeIds ? trackedChangeIds.has(String(parentCommentId)) : false; + // Check both parentCommentId and trackedChangeParentId for TC association + const tcParentId = trackedChangeParentId || parentCommentId; + const isTrackedChangeParent = tcParentId && trackedChangeIds ? trackedChangeIds.has(String(tcParentId)) : false; return { resolvedCommentId, diff --git a/packages/super-editor/src/extensions/comment/comments-helpers.js b/packages/super-editor/src/extensions/comment/comments-helpers.js index 3e8827182..33d12a748 100644 --- a/packages/super-editor/src/extensions/comment/comments-helpers.js +++ b/packages/super-editor/src/extensions/comment/comments-helpers.js @@ -208,7 +208,7 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { }); const getThreadingParentId = (comment) => { - if (!comment) return comment?.parentCommentId; + if (!comment) return undefined; const usesRangeThreading = comment.threadingStyleOverride === 'range-based' || comment.threadingMethod === 'range-based' || @@ -278,6 +278,8 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { comment, parentCommentId, trackedChangeId, + actualStart: start, + actualEnd: end, }); return; } @@ -336,7 +338,7 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { }); if (trackedChangeSpanById.size > 0) { - trackedChangeCommentMeta.forEach(({ comment, parentCommentId, trackedChangeId }) => { + trackedChangeCommentMeta.forEach(({ comment, parentCommentId, trackedChangeId, actualStart, actualEnd }) => { if (!comment || !trackedChangeSpanById.has(trackedChangeId)) return; const span = trackedChangeSpanById.get(trackedChangeId); @@ -359,9 +361,13 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { ? [trackedMarks.insertMark] : undefined; + // Use actual comment range if available, fall back to full TC span + const startPos = actualStart ?? span.startPos; + const endPos = actualEnd ?? span.endPos; + const childStartNode = schema.nodes.commentRangeStart.create(childMark, null, startMarks); startNodes.push({ - pos: span.startPos, + pos: startPos, node: childStartNode, commentId: comment.commentId, parentCommentId, @@ -369,7 +375,7 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { const childEndNode = schema.nodes.commentRangeEnd.create(childMark, null, endMarks); endNodes.push({ - pos: span.endPos, + pos: endPos, node: childEndNode, commentId: comment.commentId, parentCommentId, @@ -384,8 +390,9 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { seen.add(c.commentId); const childRange = commentRanges.get(c.commentId); - const childStart = childRange?.start ?? span.startPos; - const childEnd = childRange?.end ?? span.endPos; + // Use child's own range, fall back to parent's actual range, then TC span + const childStart = childRange?.start ?? actualStart ?? span.startPos; + const childEnd = childRange?.end ?? actualEnd ?? span.endPos; const childStartMarks = childRange ? undefined : startMarks; const childEndMarks = childRange ? undefined : endMarks; @@ -412,14 +419,19 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { }); }); + // Handle comments that are on tracked change text (identified by trackedChangeParentId) comments - .filter((comment) => trackedChangeSpanById.has(comment.parentCommentId) && !comment.trackedChange) + .filter((comment) => { + const tcParentId = comment.trackedChangeParentId || comment.parentCommentId; + return trackedChangeSpanById.has(tcParentId) && !comment.trackedChange; + }) .sort((a, b) => a.createdTime - b.createdTime) .forEach((comment) => { if (seen.has(comment.commentId)) return; seen.add(comment.commentId); - const span = trackedChangeSpanById.get(comment.parentCommentId); + const tcParentId = comment.trackedChangeParentId || comment.parentCommentId; + const span = trackedChangeSpanById.get(tcParentId); if (!span) return; const childMark = getPreparedComment({ @@ -428,7 +440,7 @@ export const prepareCommentsForExport = (doc, tr, schema, comments = []) => { }); const parentCommentId = getThreadingParentId(comment); - const trackedMarks = trackedChangeMarksById.get(comment.parentCommentId) || {}; + const trackedMarks = trackedChangeMarksById.get(tcParentId) || {}; const startMarks = trackedMarks.insertMark ? [trackedMarks.insertMark] : trackedMarks.deleteMark diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index 024b7e6fd..5e2ac3e5f 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -335,10 +335,23 @@ export const CommentsPlugin = Extension.create({ if (type === 'setActiveComment') { shouldUpdate = true; - pluginState.activeThreadId = meta.activeThreadId; // Update the outer scope variable + const previousActiveThreadId = pluginState.activeThreadId; + const newActiveThreadId = meta.activeThreadId; + + // Emit commentsUpdate event when active comment changes (e.g., from comment bubble click) + // Defer emission to after transaction completes to avoid dispatching during apply() + if (previousActiveThreadId !== newActiveThreadId) { + const update = { + type: comments_module_events.SELECTED, + activeCommentId: newActiveThreadId ? newActiveThreadId : null, + }; + setTimeout(() => editor.emit('commentsUpdate', update), 0); + } + + pluginState.activeThreadId = newActiveThreadId; return { ...pluginState, - activeThreadId: meta.activeThreadId, + activeThreadId: newActiveThreadId, changedActiveThread: true, }; } @@ -614,67 +627,67 @@ const getActiveCommentId = (doc, selection) => { const nodeAtPos = doc.nodeAt($from.pos); if (!nodeAtPos) return; - // If we have a tracked change, we can return it right away + // Check for tracked change mark (we'll use this as fallback if no comment found) const trackedChangeMark = findTrackedMark({ doc, from: $from.pos, to: $to.pos, }); - if (trackedChangeMark) { - return trackedChangeMark.mark.attrs.id; - } - - // Otherwise, we need to check for comment nodes - const overlaps = []; - let found = false; + // Check for comment nodes first - comments take precedence over tracked changes + // This ensures that when cursor is on text that has both TC and comment, the comment is selected + // Collect all comment marks at the cursor position along with their ranges + const commentRanges = new Map(); // commentId -> { start, end } - // Look for commentRangeStart nodes before the current position - // There could be overlapping comments so we need to track all of them + // First pass: find all comment ranges in the document doc.descendants((node, pos) => { - if (found) return; - - // node goes from `pos` to `end = pos + node.nodeSize` - const end = pos + node.nodeSize; - - // If $from.pos is outside this node’s range, skip it - if ($from.pos < pos || $from.pos >= end) { - return; - } - - // Now we know $from.pos is within this node’s start/end const { marks = [] } = node; - const commentMark = marks.find((mark) => mark.type.name === CommentMarkName); - if (commentMark) { - overlaps.push({ - node, - pos, - size: node.nodeSize, - }); - } + const commentMarks = marks.filter((mark) => mark.type.name === CommentMarkName); + + commentMarks.forEach((mark) => { + const commentId = mark.attrs.commentId || mark.attrs.importedId; + if (!commentId) return; + + const existing = commentRanges.get(commentId); + const end = pos + node.nodeSize; + + if (!existing) { + commentRanges.set(commentId, { start: pos, end }); + } else { + // Extend the range if this node extends it + commentRanges.set(commentId, { + start: Math.min(existing.start, pos), + end: Math.max(existing.end, end), + }); + } + }); + }); - // If we've passed the position, we can stop - if (pos > $from.pos) { - found = true; + // Find which comments contain the cursor position + const containingComments = []; + commentRanges.forEach((range, commentId) => { + if ($from.pos >= range.start && $from.pos < range.end) { + containingComments.push({ + commentId, + start: range.start, + end: range.end, + size: range.end - range.start, + }); } }); - // Get the closest commentRangeStart node to the current position - let closest = null; - let closestCommentRangeStart = null; - overlaps.forEach(({ pos, node }) => { - if (!closest) closest = $from.pos - pos; - - const diff = $from.pos - pos; - if (diff >= 0 && diff <= closest) { - closestCommentRangeStart = node; - closest = diff; + if (containingComments.length === 0) { + // No comments found, fall back to tracked change if present + if (trackedChangeMark) { + return trackedChangeMark.mark.attrs.id; } - }); + return null; + } - const { marks: closestMarks = [] } = closestCommentRangeStart || {}; - const closestCommentMark = closestMarks.find((mark) => mark.type.name === CommentMarkName); - return closestCommentMark?.attrs?.commentId || closestCommentMark?.attrs?.importedId; + // Return the innermost comment (smallest range) + // For nested comments, the inner one has the smallest size + containingComments.sort((a, b) => a.size - b.size); + return containingComments[0].commentId; }; const findTrackedMark = ({ diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.test.js b/packages/super-editor/src/extensions/comment/comments-plugin.test.js index 7afa04ffa..f22b5e686 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -1013,3 +1013,124 @@ describe('internal helper functions', () => { expect(result).toBeUndefined(); }); }); + +describe('getActiveCommentId - nested comments and TC precedence', () => { + it('returns innermost comment when cursor is in nested range', () => { + // Doc: "Hello [outer: world [inner: !]]" + const schema = createCommentSchema(); + const outerMark = schema.marks[CommentMarkName].create({ commentId: 'outer-comment' }); + const innerMark = schema.marks[CommentMarkName].create({ commentId: 'inner-comment' }); + const paragraph = schema.node('paragraph', null, [ + schema.text('Hello '), + schema.text('world ', [outerMark]), + schema.text('!', [outerMark, innerMark]), + ]); + const doc = schema.node('doc', null, [paragraph]); + + // Position 13 is on "!" (0=doc, 1=paragraph start, then "Hello " is 6 chars, "world " is 6 chars = pos 13) + const selection = TextSelection.create(doc, 13); + expect(getActiveCommentId(doc, selection)).toBe('inner-comment'); + }); + + it('returns outer comment when cursor is outside inner range', () => { + // Doc: "Hello [outer: world [inner: !]]" + const schema = createCommentSchema(); + const outerMark = schema.marks[CommentMarkName].create({ commentId: 'outer-comment' }); + const innerMark = schema.marks[CommentMarkName].create({ commentId: 'inner-comment' }); + const paragraph = schema.node('paragraph', null, [ + schema.text('Hello '), + schema.text('world ', [outerMark]), + schema.text('!', [outerMark, innerMark]), + ]); + const doc = schema.node('doc', null, [paragraph]); + + // Position 8 is on "world" (outside inner range) + const selection = TextSelection.create(doc, 8); + expect(getActiveCommentId(doc, selection)).toBe('outer-comment'); + }); + + it('returns comment ID when both comment and TC exist at cursor position', () => { + // Doc: text has both TC and comment marks - comment should take precedence + const schema = createCommentSchema(); + const tcMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-1' }); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'comment-1' }); + const paragraph = schema.node('paragraph', null, [schema.text('lorem ipsum', [tcMark, commentMark])]); + const doc = schema.node('doc', null, [paragraph]); + + const selection = TextSelection.create(doc, 3); + expect(getActiveCommentId(doc, selection)).toBe('comment-1'); // NOT 'tc-1' + }); + + it('returns TC ID when only TC exists at cursor position', () => { + const schema = createCommentSchema(); + const tcMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-only' }); + const paragraph = schema.node('paragraph', null, [schema.text('TC only text', [tcMark])]); + const doc = schema.node('doc', null, [paragraph]); + + const selection = TextSelection.create(doc, 3); + expect(getActiveCommentId(doc, selection)).toBe('tc-only'); + }); + + it('returns comment ID on overlapping text, TC ID on TC-only text', () => { + // Doc: "[TC: Hello [comment: world]]" + const schema = createCommentSchema(); + const tcMark = schema.marks[TrackInsertMarkName].create({ id: 'tc-2' }); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'comment-2' }); + const paragraph = schema.node('paragraph', null, [ + schema.text('Hello ', [tcMark]), + schema.text('world', [tcMark, commentMark]), + ]); + const doc = schema.node('doc', null, [paragraph]); + + // Cursor on "Hello" (TC only) - position 3 + expect(getActiveCommentId(doc, TextSelection.create(doc, 3))).toBe('tc-2'); + // Cursor on "world" (both TC and comment) - position 8 + expect(getActiveCommentId(doc, TextSelection.create(doc, 8))).toBe('comment-2'); + }); + + it('handles three levels of nested comments', () => { + const schema = createCommentSchema(); + const outerMark = schema.marks[CommentMarkName].create({ commentId: 'outer' }); + const middleMark = schema.marks[CommentMarkName].create({ commentId: 'middle' }); + const innerMark = schema.marks[CommentMarkName].create({ commentId: 'inner' }); + const paragraph = schema.node('paragraph', null, [ + schema.text('Outer ', [outerMark]), + schema.text('Middle ', [outerMark, middleMark]), + schema.text('Inner', [outerMark, middleMark, innerMark]), + ]); + const doc = schema.node('doc', null, [paragraph]); + + // Position on "Inner" text (pos 14) - should return innermost + expect(getActiveCommentId(doc, TextSelection.create(doc, 14))).toBe('inner'); + // Position on "Middle" text (pos 8) - should return middle + expect(getActiveCommentId(doc, TextSelection.create(doc, 8))).toBe('middle'); + // Position on "Outer" text (pos 3) - should return outer + expect(getActiveCommentId(doc, TextSelection.create(doc, 3))).toBe('outer'); + }); + + it('returns null when cursor is outside all comments', () => { + const schema = createCommentSchema(); + const commentMark = schema.marks[CommentMarkName].create({ commentId: 'c-1' }); + const paragraph = schema.node('paragraph', null, [ + schema.text('No comment. '), + schema.text('Has comment.', [commentMark]), + ]); + const doc = schema.node('doc', null, [paragraph]); + + // Position 5 is on "No comment." (no marks) + expect(getActiveCommentId(doc, TextSelection.create(doc, 5))).toBeNull(); + }); + + it('correctly identifies adjacent non-overlapping comments', () => { + const schema = createCommentSchema(); + const markA = schema.marks[CommentMarkName].create({ commentId: 'a' }); + const markB = schema.marks[CommentMarkName].create({ commentId: 'b' }); + const paragraph = schema.node('paragraph', null, [schema.text('Hello', [markA]), schema.text('World', [markB])]); + const doc = schema.node('doc', null, [paragraph]); + + // Position 3 is on "Hello" (mark A) + expect(getActiveCommentId(doc, TextSelection.create(doc, 3))).toBe('a'); + // Position 8 is on "World" (mark B) + expect(getActiveCommentId(doc, TextSelection.create(doc, 8))).toBe('b'); + }); +}); diff --git a/packages/super-editor/src/extensions/comment/comments.test.js b/packages/super-editor/src/extensions/comment/comments.test.js index 47309e89f..9282fde88 100644 --- a/packages/super-editor/src/extensions/comment/comments.test.js +++ b/packages/super-editor/src/extensions/comment/comments.test.js @@ -343,12 +343,19 @@ describe('comment helpers', () => { externalColor: '#abcdef', }); + // Active comment gets brighter highlight (27% = 44 hex) const color = getHighlightColor({ activeThreadId: 'thread-1', threadId: 'thread-1', isInternal: false, editor }); expect(color).toBe('#abcdef44'); + // Other comments get lighter highlight (13% = 22 hex) when another is active const external = getHighlightColor({ activeThreadId: 'thread-2', threadId: 'thread-1', isInternal: false, editor }); expect(external).toBe('#abcdef22'); + // No active comment - shows lighter highlight (13% = 22 hex) + const inactive = getHighlightColor({ activeThreadId: null, threadId: 'thread-3', isInternal: false, editor }); + expect(inactive).toBe('#abcdef22'); + + // Internal comment when not in internal mode is hidden const hidden = getHighlightColor({ activeThreadId: null, threadId: 'thread-3', isInternal: true, editor }); expect(hidden).toBe('transparent'); }); diff --git a/packages/super-editor/src/tests/export/commentsRoundTrip.test.js b/packages/super-editor/src/tests/export/commentsRoundTrip.test.js index 8109a2ed8..32c3f4432 100644 --- a/packages/super-editor/src/tests/export/commentsRoundTrip.test.js +++ b/packages/super-editor/src/tests/export/commentsRoundTrip.test.js @@ -468,4 +468,52 @@ describe('Nested comments export', () => { editor.destroy(); } }); + + it('exports commentsExtended.xml and preserves comments as separate (not threaded) after round-trip', async () => { + // Regression test for SD-1519: Nested comments should remain separate bubbles, + // not get threaded together based on range nesting. + const { editor } = initTestEditor({ content: docx, media, mediaFiles, fonts }); + + try { + const originalComments = editor.converter.comments; + expect(originalComments.length).toBeGreaterThanOrEqual(2); + + // Verify original comments don't have parentCommentId (they're independent) + const originalNonThreaded = originalComments.filter((c) => !c.parentCommentId); + expect(originalNonThreaded.length).toBeGreaterThanOrEqual(2); + + const commentsForExport = originalComments.map((comment) => ({ + ...comment, + commentJSON: comment.textJson, + })); + + await editor.exportDocx({ + comments: commentsForExport, + commentsType: 'external', + }); + + const exportedXml = editor.converter.convertedXml; + + // Verify commentsExtended.xml is generated (key for preserving non-threaded status) + const commentsExtendedXml = exportedXml['word/commentsExtended.xml']; + expect(commentsExtendedXml).toBeDefined(); + const extendedElements = commentsExtendedXml?.elements?.[0]?.elements ?? []; + expect(extendedElements.length).toBe(originalComments.length); + + // Verify NO comments have w15:paraIdParent (they should be independent) + const hasParaIdParent = extendedElements.some((el) => el.attributes?.['w15:paraIdParent']); + expect(hasParaIdParent).toBe(false); + + // Re-import and verify comments remain independent (no parentCommentId from range-based threading) + const reimportedComments = importCommentData({ docx: carbonCopy(exportedXml) }) ?? []; + expect(reimportedComments.length).toBe(originalComments.length); + + // Key assertion: After round-trip, comments should NOT have parentCommentId + // set based on range nesting (they should remain separate bubbles) + const reimportedNonThreaded = reimportedComments.filter((c) => !c.parentCommentId); + expect(reimportedNonThreaded.length).toBe(originalNonThreaded.length); + } finally { + editor.destroy(); + } + }); }); diff --git a/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js b/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js index 5857777cd..417366998 100644 --- a/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js +++ b/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js @@ -324,7 +324,11 @@ describe('importCommentData extended metadata', () => { const comments = importCommentData({ docx }); const child = comments.find((comment) => comment.commentId === 'child-comment'); - expect(child.parentCommentId).toBe('tc-1'); + // With the new separation of TC and comments: + // - trackedChangeParentId tracks the TC association + // - parentCommentId tracks the threading relationship (from range nesting) + expect(child.trackedChangeParentId).toBe('tc-1'); + expect(child.parentCommentId).toBe('parent-comment'); expect(child.threadingParentCommentId).toBe('parent-comment'); }); }); @@ -833,8 +837,8 @@ describe('Google Docs tracked change comment threading', () => { expect(comments).toHaveLength(1); const comment = comments[0]; - // The parent should be the tracked change ID ('0') - expect(comment.parentCommentId).toBe('0'); + // The tracked change ID ('0') should be in trackedChangeParentId (not parentCommentId) + expect(comment.trackedChangeParentId).toBe('0'); }); it('detects comment inside tracked change insertion as child of tracked change', () => { @@ -865,7 +869,7 @@ describe('Google Docs tracked change comment threading', () => { expect(comments).toHaveLength(1); const comment = comments[0]; - expect(comment.parentCommentId).toBe('2'); + expect(comment.trackedChangeParentId).toBe('2'); }); it('detects multiple comments inside same tracked change', () => { @@ -903,9 +907,50 @@ describe('Google Docs tracked change comment threading', () => { const firstComment = comments.find((c) => c.commentId === 'first-comment'); const secondComment = comments.find((c) => c.commentId === 'second-comment'); - // Both should have the tracked change as parent - expect(firstComment.parentCommentId).toBe('1'); - expect(secondComment.parentCommentId).toBe('1'); + // Both should have the tracked change as trackedChangeParentId (not parentCommentId) + expect(firstComment.trackedChangeParentId).toBe('1'); + expect(secondComment.trackedChangeParentId).toBe('1'); + }); + + it('threads nested range replies inside a tracked change', () => { + const docx = buildDocx({ + comments: [ + { id: 0, internalId: 'root-comment', author: 'Author A', date: '2024-01-01T10:00:00Z' }, + { id: 1, internalId: 'reply-comment', author: 'Author B', date: '2024-01-01T10:05:00Z' }, + ], + documentRanges: [ + { + name: 'w:p', + elements: [ + { + name: 'w:ins', + attributes: { 'w:id': '55', 'w:author': 'Author A', 'w:date': '2024-01-01T09:00:00Z' }, + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '0' } }, + { name: 'w:commentRangeStart', attributes: { 'w:id': '1' } }, + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'Inserted text' }] }], + }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '0' } }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '1' } }, + ], + }, + ], + }, + ], + }); + + const comments = importCommentData({ docx }); + expect(comments).toHaveLength(2); + + const root = comments.find((c) => c.commentId === 'root-comment'); + const reply = comments.find((c) => c.commentId === 'reply-comment'); + + expect(root.trackedChangeParentId).toBe('55'); + expect(root.parentCommentId).toBeUndefined(); + expect(reply.trackedChangeParentId).toBe('55'); + expect(reply.parentCommentId).toBe('root-comment'); }); it('detects comments inside replacement tracked change (ins + del)', () => { @@ -953,9 +998,10 @@ describe('Google Docs tracked change comment threading', () => { const comment1 = comments.find((c) => c.commentId === 'replacement-comment-1'); const comment2 = comments.find((c) => c.commentId === 'replacement-comment-2'); - // Both should have the tracked change (ins) as parent since their range starts in the ins element - expect(comment1.parentCommentId).toBe('3'); - expect(comment2.parentCommentId).toBe('3'); + // Both should have the tracked change (ins) as trackedChangeParentId since their range starts in the ins element + // (parentCommentId is reserved for actual comment replies, not TC associations) + expect(comment1.trackedChangeParentId).toBe('3'); + expect(comment2.trackedChangeParentId).toBe('3'); }); it('does not affect comments outside tracked changes', () => { @@ -1004,12 +1050,106 @@ describe('Google Docs tracked change comment threading', () => { // Regular comment should have no parent expect(regularComment.parentCommentId).toBeUndefined(); - // TC comment should have the tracked change as parent - expect(tcComment.parentCommentId).toBe('1'); + // TC comment should have the tracked change as trackedChangeParentId (not parentCommentId) + expect(tcComment.trackedChangeParentId).toBe('1'); }); }); -describe('Word tracked change comment threading (with commentsExtended.xml)', () => { +describe('Word comment threading (with commentsExtended.xml)', () => { + it('treats nested ranges as separate comments when both have commentsExtended.xml entries without paraIdParent', () => { + // This tests the case where Word has nested ranges (like a comment on "world" inside + // a larger comment on "Hello world") but commentsExtended.xml explicitly defines + // them as separate top-level comments (no paraIdParent). + // Range-based nesting should NOT override the explicit commentsExtended.xml structure. + const docx = buildDocx({ + comments: [ + { id: 0, internalId: 'outer-comment', paraId: 'para-outer' }, + { id: 1, internalId: 'nested-comment', paraId: 'para-nested' }, + ], + documentRanges: [ + { + name: 'w:p', + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '0' } }, + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'Hello ' }] }], + }, + { name: 'w:commentRangeStart', attributes: { 'w:id': '1' } }, + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'world' }] }], + }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '1' } }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '0' } }, + ], + }, + ], + extended: [ + // Both comments have entries in commentsExtended.xml WITHOUT paraIdParent + // meaning they are explicitly defined as separate top-level comments + { paraId: 'para-outer', done: '0' }, + { paraId: 'para-nested', done: '0' }, + ], + }); + + const comments = importCommentData({ docx }); + expect(comments).toHaveLength(2); + + const outerComment = comments.find((c) => c.commentId === 'outer-comment'); + const nestedComment = comments.find((c) => c.commentId === 'nested-comment'); + + // Both should be top-level comments (no parentCommentId) + // because commentsExtended.xml explicitly defines them without paraIdParent + expect(outerComment.parentCommentId).toBeUndefined(); + expect(nestedComment.parentCommentId).toBeUndefined(); + }); + + it('still respects explicit paraIdParent threading in commentsExtended.xml for nested ranges', () => { + // When commentsExtended.xml explicitly defines a parent relationship via paraIdParent, + // that should be respected even with nested ranges + const docx = buildDocx({ + comments: [ + { id: 0, internalId: 'parent-comment', paraId: 'para-parent' }, + { id: 1, internalId: 'reply-comment', paraId: 'para-reply' }, + ], + documentRanges: [ + { + name: 'w:p', + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '0' } }, + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'Some text' }] }], + }, + { name: 'w:commentRangeStart', attributes: { 'w:id': '1' } }, + { + name: 'w:r', + elements: [{ name: 'w:t', elements: [{ type: 'text', text: ' with reply' }] }], + }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '1' } }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '0' } }, + ], + }, + ], + extended: [ + { paraId: 'para-parent', done: '0' }, + // Explicit parent relationship via paraIdParent + { paraId: 'para-reply', done: '0', parent: 'para-parent' }, + ], + }); + + const comments = importCommentData({ docx }); + expect(comments).toHaveLength(2); + + const parentComment = comments.find((c) => c.commentId === 'parent-comment'); + const replyComment = comments.find((c) => c.commentId === 'reply-comment'); + + expect(parentComment.parentCommentId).toBeUndefined(); + // Reply should have parent from commentsExtended.xml + expect(replyComment.parentCommentId).toBe('parent-comment'); + }); + it('detects comment inside tracked change insertion as child of tracked change even when commentsExtended.xml exists', () => { const docx = buildDocx({ comments: [{ id: 7, internalId: 'comment-on-insertion', author: 'Missy Fox', date: '2024-01-01T10:00:00Z' }], @@ -1039,7 +1179,9 @@ describe('Word tracked change comment threading (with commentsExtended.xml)', () expect(comments).toHaveLength(1); const comment = comments[0]; - expect(comment.parentCommentId).toBe('2'); + // Comment is on TC text, so trackedChangeParentId should be set instead of parentCommentId + expect(comment.trackedChangeParentId).toBe('2'); + expect(comment.parentCommentId).toBeUndefined(); }); it('detects root comment of a thread as child of tracked change, and replies as child of root', () => { @@ -1081,10 +1223,12 @@ describe('Word tracked change comment threading (with commentsExtended.xml)', () const root = comments.find((c) => c.commentId === 'root-comment'); const reply = comments.find((c) => c.commentId === 'reply-comment'); - // Root should point to tracked change - expect(root.parentCommentId).toBe('99'); - // Reply should point to tracked change (flattened) - expect(reply.parentCommentId).toBe('99'); + // Root should have trackedChangeParentId pointing to tracked change (not parentCommentId) + expect(root.trackedChangeParentId).toBe('99'); + expect(root.parentCommentId).toBeUndefined(); + // Reply should have trackedChangeParentId pointing to tracked change, but parentCommentId pointing to root + expect(reply.trackedChangeParentId).toBe('99'); + expect(reply.parentCommentId).toBe('root-comment'); }); it('detects comments that end before a deletion in the same paragraph as children of the deletion', () => { @@ -1130,8 +1274,92 @@ describe('Word tracked change comment threading (with commentsExtended.xml)', () const comment1 = comments.find((c) => c.commentId === 'comment-on-delete'); const comment2 = comments.find((c) => c.commentId === 'thread-on-delete'); - // Both comments should be associated with the deletion (flattened, not threaded) - expect(comment1.parentCommentId).toBe('9'); - expect(comment2.parentCommentId).toBe('9'); + // Both comments should have trackedChangeParentId pointing to the deletion + // Comment1 is the root, so parentCommentId is undefined + expect(comment1.trackedChangeParentId).toBe('9'); + expect(comment1.parentCommentId).toBeUndefined(); + // Comment2 is a reply to comment1 (via commentsExtended parent relationship) + expect(comment2.trackedChangeParentId).toBe('9'); + expect(comment2.parentCommentId).toBe('comment-on-delete'); + }); + + it('imports comment on TC text with trackedChangeParentId but no parentCommentId', () => { + // Regression test: A single comment placed entirely on tracked change text + // should have trackedChangeParentId set (for bubble association) but + // should NOT have parentCommentId (since it's not a reply to another comment) + const docx = buildDocx({ + comments: [{ id: 0, internalId: 'comment-on-tc', paraId: 'para-0' }], + documentRanges: [ + { + name: 'w:p', + elements: [ + { + name: 'w:ins', + attributes: { 'w:id': 'tc-1', 'w:author': 'Author', 'w:date': '2024-01-01T00:00:00Z' }, + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '0' } }, + { name: 'w:r', elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'TC text' }] }] }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '0' } }, + ], + }, + ], + }, + ], + extended: [{ paraId: 'para-0', done: '0' }], + }); + + const comments = importCommentData({ docx }); + expect(comments).toHaveLength(1); + expect(comments[0].trackedChangeParentId).toBe('tc-1'); + expect(comments[0].parentCommentId).toBeUndefined(); + }); + + it('separates trackedChangeParentId from parentCommentId for threaded comment in TC', () => { + // Regression test: When a comment thread exists on tracked change text, + // the child comment should have: + // - trackedChangeParentId: pointing to the TC (for bubble association) + // - parentCommentId: pointing to the parent comment (for threading) + const docx = buildDocx({ + comments: [ + { id: 1, internalId: 'parent-comment', paraId: 'para-1' }, + { id: 2, internalId: 'child-comment', paraId: 'para-2' }, + ], + documentRanges: [ + { + name: 'w:p', + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '1' } }, + { + name: 'w:ins', + attributes: { 'w:id': 'tc-1', 'w:author': 'Author', 'w:date': '2024-01-01T00:00:00Z' }, + elements: [ + { name: 'w:commentRangeStart', attributes: { 'w:id': '2' } }, + { name: 'w:r', elements: [{ name: 'w:t', elements: [{ type: 'text', text: 'TC' }] }] }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '2' } }, + ], + }, + { name: 'w:commentRangeEnd', attributes: { 'w:id': '1' } }, + ], + }, + ], + extended: [ + { paraId: 'para-1', done: '0' }, + { paraId: 'para-2', done: '0', parent: 'para-1' }, + ], + }); + + const comments = importCommentData({ docx }); + expect(comments).toHaveLength(2); + + const parent = comments.find((c) => c.commentId === 'parent-comment'); + const child = comments.find((c) => c.commentId === 'child-comment'); + + // Parent comment spans outside TC, so it should NOT have trackedChangeParentId + expect(parent.trackedChangeParentId).toBeUndefined(); + expect(parent.parentCommentId).toBeUndefined(); + + // Child comment is inside TC and is a reply to parent + expect(child.trackedChangeParentId).toBe('tc-1'); + expect(child.parentCommentId).toBe('parent-comment'); }); }); diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js index 25facaf4b..d96e85529 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js @@ -405,4 +405,65 @@ describe('CommentDialog.vue', () => { // Third should be child-2 (created at time 2000) expect(headers[2].props('comment').commentId).toBe('child-2'); }); + + it('threads range-based comments under tracked change parent', async () => { + const rangeBasedRoot = reactive({ + uid: 'uid-range-root', + commentId: 'range-root', + parentCommentId: null, + trackedChangeParentId: 'tc-parent', + threadingMethod: 'range-based', + email: 'root@example.com', + commentText: '

Root comment

', + createdTime: 1000, + fileId: 'doc-1', + fileType: 'DOCX', + setActive: vi.fn(), + setText: vi.fn(), + setIsInternal: vi.fn(), + resolveComment: vi.fn(), + trackedChange: false, + selection: { + getValues: () => ({ selectionBounds: { top: 120, bottom: 150, left: 20, right: 40 } }), + selectionBounds: { top: 120, bottom: 150, left: 20, right: 40 }, + }, + }); + + const replyToRoot = reactive({ + uid: 'uid-range-reply', + commentId: 'range-reply', + parentCommentId: 'range-root', + email: 'reply@example.com', + commentText: '

Reply comment

', + createdTime: 1500, + fileId: 'doc-1', + fileType: 'DOCX', + setActive: vi.fn(), + setText: vi.fn(), + setIsInternal: vi.fn(), + resolveComment: vi.fn(), + trackedChange: false, + selection: { + getValues: () => ({ selectionBounds: { top: 120, bottom: 150, left: 20, right: 40 } }), + selectionBounds: { top: 120, bottom: 150, left: 20, right: 40 }, + }, + }); + + const { wrapper } = await mountDialog({ + baseCommentOverrides: { + commentId: 'tc-parent', + trackedChange: true, + trackedChangeType: 'trackInsert', + trackedChangeText: 'Added', + createdTime: 500, + }, + extraComments: [replyToRoot, rangeBasedRoot], + }); + + const headers = wrapper.findAllComponents(CommentHeaderStub); + expect(headers).toHaveLength(3); + expect(headers[0].props('comment').commentId).toBe('tc-parent'); + expect(headers[1].props('comment').commentId).toBe('range-root'); + expect(headers[2].props('comment').commentId).toBe('range-reply'); + }); }); diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index a0a83f09a..d0b5c47d3 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -81,21 +81,63 @@ const showInputSection = computed(() => { ); }); +const isRangeThreadedComment = (comment) => { + if (!comment) return false; + return ( + comment.threadingStyleOverride === 'range-based' || + comment.threadingMethod === 'range-based' || + comment.originalXmlStructure?.hasCommentsExtended === false + ); +}; + +const collectTrackedChangeThread = (parentComment, allComments) => { + const trackedChangeId = parentComment.commentId; + const threadIds = new Set([trackedChangeId]); + const queue = []; + + allComments.forEach((comment) => { + if (comment.commentId === trackedChangeId) return; + const isDirectChild = comment.parentCommentId === trackedChangeId; + const isRangeBasedTrackedChangeComment = + comment.trackedChangeParentId === trackedChangeId && isRangeThreadedComment(comment); + + if (isDirectChild || isRangeBasedTrackedChangeComment) { + threadIds.add(comment.commentId); + queue.push(comment.commentId); + } + }); + + for (let i = 0; i < queue.length; i += 1) { + const parentId = queue[i]; + allComments.forEach((comment) => { + if (comment.parentCommentId === parentId && !threadIds.has(comment.commentId)) { + threadIds.add(comment.commentId); + queue.push(comment.commentId); + } + }); + } + + return allComments.filter((comment) => threadIds.has(comment.commentId)); +}; + const comments = computed(() => { const parentComment = props.comment; - return commentsStore.commentsList - .filter((c) => { - const isThreadedComment = c.parentCommentId === parentComment.commentId; - const isThisComment = c.commentId === props.comment.commentId; - return isThreadedComment || isThisComment; - }) - .sort((a, b) => { - // Parent comment (the one passed as prop) should always be first - if (a.commentId === props.comment.commentId) return -1; - if (b.commentId === props.comment.commentId) return 1; - // Sort remaining comments (children) by creation time - return a.createdTime - b.createdTime; - }); + const allComments = commentsStore.commentsList; + const threadComments = parentComment.trackedChange + ? collectTrackedChangeThread(parentComment, allComments) + : allComments.filter((comment) => { + const isThreadedComment = comment.parentCommentId === parentComment.commentId; + const isThisComment = comment.commentId === parentComment.commentId; + return isThreadedComment || isThisComment; + }); + + return threadComments.sort((a, b) => { + // Parent comment (the one passed as prop) should always be first + if (a.commentId === parentComment.commentId) return -1; + if (b.commentId === parentComment.commentId) return 1; + // Sort remaining comments (children) by creation time + return a.createdTime - b.createdTime; + }); }); const isInternalDropdownDisabled = computed(() => { diff --git a/packages/superdoc/src/components/CommentsLayer/use-comment.js b/packages/superdoc/src/components/CommentsLayer/use-comment.js index b4c411f2e..10c80472f 100644 --- a/packages/superdoc/src/components/CommentsLayer/use-comment.js +++ b/packages/superdoc/src/components/CommentsLayer/use-comment.js @@ -16,6 +16,7 @@ export default function useComment(params) { const commentId = params.commentId || uuidv4(); const importedId = params.importedId; const parentCommentId = params.parentCommentId; + const trackedChangeParentId = params.trackedChangeParentId; const fileId = params.fileId; const fileType = params.fileType; const createdAtVersionNumber = params.createdAtVersionNumber; @@ -231,6 +232,7 @@ export default function useComment(params) { commentId, importedId, parentCommentId, + trackedChangeParentId, fileId, fileType, mentions: mentions.value.map((u) => { @@ -266,6 +268,7 @@ export default function useComment(params) { commentId, importedId, parentCommentId, + trackedChangeParentId, fileId, fileType, mentions, diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index 64c1e176d..934e2e62c 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -93,6 +93,22 @@ export const useCommentsStore = defineStore('comments', () => { return getComment(comment.parentCommentId); }; + const isRangeThreadedComment = (comment) => { + if (!comment) return false; + return ( + comment.threadingStyleOverride === 'range-based' || + comment.threadingMethod === 'range-based' || + comment.originalXmlStructure?.hasCommentsExtended === false + ); + }; + + const shouldThreadWithTrackedChange = (comment) => { + if (!comment?.trackedChangeParentId) return false; + if (!isRangeThreadedComment(comment)) return false; + const trackedChange = getComment(comment.trackedChangeParentId); + return Boolean(trackedChange?.trackedChange); + }; + /** * Extract the position lookup key from a comment or comment ID. * Prefers importedId for imported comments since editor marks retain the original ID. @@ -187,7 +203,8 @@ export const useCommentsStore = defineStore('comments', () => { if (!isViewingMode.value) return true; const parent = getThreadParent(comment); if (!parent && comment?.parentCommentId) return false; - const isTrackedChange = Boolean(parent?.trackedChange); + // Check both parent's trackedChange flag and comment's trackedChangeParentId + const isTrackedChange = Boolean(parent?.trackedChange) || Boolean(comment?.trackedChangeParentId); return isTrackedChange ? viewingVisibility.trackChangesVisible : viewingVisibility.commentsVisible; }; @@ -389,22 +406,24 @@ export const useCommentsStore = defineStore('comments', () => { commentsList.value.forEach((comment) => { if (!isThreadVisible(comment)) return; + const trackedChangeParentId = shouldThreadWithTrackedChange(comment) ? comment.trackedChangeParentId : null; + const parentId = comment.parentCommentId || trackedChangeParentId; // Track resolved comments if (comment.resolvedTime) { resolvedComments.push(comment); } // Track parent comments - else if (!comment.parentCommentId && !comment.resolvedTime) { + else if (!parentId && !comment.resolvedTime) { parentComments.push({ ...comment }); } // Track child comments (threaded comments) - else if (comment.parentCommentId) { - if (!childCommentMap.has(comment.parentCommentId)) { - childCommentMap.set(comment.parentCommentId, []); + else if (parentId) { + if (!childCommentMap.has(parentId)) { + childCommentMap.set(parentId, []); } - childCommentMap.get(comment.parentCommentId).push(comment); + childCommentMap.get(parentId).push(comment); } }); @@ -616,6 +635,7 @@ export const useCommentsStore = defineStore('comments', () => { commentId: comment.commentId, isInternal: false, parentCommentId: comment.parentCommentId, + trackedChangeParentId: comment.trackedChangeParentId, creatorName, createdTime: comment.createdTime, creatorEmail: comment.creatorEmail, @@ -770,13 +790,27 @@ export const useCommentsStore = defineStore('comments', () => { const normalizeCommentForEditor = (node) => { if (!node || typeof node !== 'object') return node; + const stripTextStyleAttrs = (attrs) => { + if (!attrs) return attrs; + const rest = { ...attrs }; + delete rest.fontSize; + delete rest.fontFamily; + delete rest.eastAsiaFontFamily; + return Object.keys(rest).length ? rest : undefined; + }; + + const normalizeMark = (mark) => { + if (!mark) return mark; + const typeName = typeof mark.type === 'string' ? mark.type : mark.type?.name; + const attrs = mark?.attrs ? { ...mark.attrs } : undefined; + if (typeName === 'textStyle' && attrs) { + return { ...mark, attrs: stripTextStyleAttrs(attrs) }; + } + return { ...mark, attrs }; + }; + const cloneMarks = (marks) => - Array.isArray(marks) - ? marks.filter(Boolean).map((mark) => ({ - ...mark, - attrs: mark?.attrs ? { ...mark.attrs } : undefined, - })) - : undefined; + Array.isArray(marks) ? marks.filter(Boolean).map((mark) => normalizeMark(mark)) : undefined; const cloneAttrs = (attrs) => (attrs && typeof attrs === 'object' ? { ...attrs } : undefined);