From 183aecf4139308c5b468b4d4914a8060f0392bfc Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 16:24:44 -0700 Subject: [PATCH 1/2] fix: surface hyperlink tracked changes in comments --- .../v2/exporter/commentsExporter.js | 2 + .../v2/exporter/commentsExporter.test.js | 21 ++ .../v2/importer/documentCommentsImporter.js | 3 + .../src/extensions/comment/comments-plugin.js | 42 +++- .../comment/comments-plugin.test.js | 110 ++++++++++ .../comment/tracked-change-display.js | 139 ++++++++++++ .../comment/tracked-change-display.test.js | 205 ++++++++++++++++++ .../src/extensions/track-changes/constants.js | 6 + .../track-changes-extension.test.js | 35 +++ .../trackChangesHelpers/addMarkStep.js | 7 +- .../trackChangesHelpers/removeMarkStep.js | 6 +- .../trackChangesHelpers.test.js | 20 ++ .../documentCommentsImporter.unit.test.js | 5 + packages/superdoc/src/SuperDoc.test.js | 39 ++++ packages/superdoc/src/SuperDoc.vue | 1 + .../CommentsLayer/CommentDialog.test.js | 35 +++ .../CommentsLayer/CommentDialog.vue | 10 +- .../components/CommentsLayer/use-comment.js | 3 + .../superdoc/src/stores/comments-store.js | 5 + .../src/stores/comments-store.test.js | 41 ++++ shared/common/comments-types.ts | 1 + .../sd-2250-hyperlink-change-display.spec.ts | 37 ++++ 22 files changed, 751 insertions(+), 22 deletions(-) create mode 100644 packages/super-editor/src/extensions/comment/tracked-change-display.js create mode 100644 packages/super-editor/src/extensions/comment/tracked-change-display.test.js create mode 100644 tests/behavior/tests/comments/sd-2250-hyperlink-change-display.spec.ts 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 a23c3de563..00b09d6bf5 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 @@ -46,6 +46,7 @@ export const getCommentDefinition = (comment, commentId, allComments, editor) => 'custom:trackedChange': comment.trackedChange, 'custom:trackedChangeText': comment.trackedChangeText || null, 'custom:trackedChangeType': comment.trackedChangeType, + 'custom:trackedChangeDisplayType': comment.trackedChangeDisplayType || null, 'custom:trackedDeletedText': comment.deletedText || null, }; @@ -138,6 +139,7 @@ export const updateCommentsXml = (commentDefs = [], commentsXml) => { 'custom:trackedChange': commentDef.attributes['custom:trackedChange'], 'custom:trackedChangeText': commentDef.attributes['custom:trackedChangeText'], 'custom:trackedChangeType': commentDef.attributes['custom:trackedChangeType'], + 'custom:trackedChangeDisplayType': commentDef.attributes['custom:trackedChangeDisplayType'], 'custom:trackedDeletedText': commentDef.attributes['custom:trackedDeletedText'], 'xmlns:custom': 'http://schemas.openxmlformats.org/wordprocessingml/2006/main', }; 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 eeb1a90575..72cce4898c 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 @@ -1,4 +1,5 @@ import { + getCommentDefinition, updateCommentsExtendedXml, updateCommentsIdsAndExtensible, updateCommentsXml, @@ -382,6 +383,26 @@ describe('prepareCommentsXmlFilesForExport', () => { }); }); +describe('getCommentDefinition', () => { + it('preserves tracked change display metadata for exported tracked-change comments', () => { + const definition = getCommentDefinition( + makeComment({ + trackedChange: true, + trackedChangeType: 'trackFormat', + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }), + '0', + [], + null, + ); + + expect(definition.attributes['custom:trackedChangeType']).toBe('trackFormat'); + expect(definition.attributes['custom:trackedChangeText']).toBe('https://example.com'); + expect(definition.attributes['custom:trackedChangeDisplayType']).toBe('hyperlinkAdded'); + }); +}); + // ============================================================================= // removeCommentsFilesFromConvertedXml // ============================================================================= 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 3f131cfc96..aa4b7431b5 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 @@ -43,6 +43,8 @@ export function importCommentData({ docx, editor, converter }) { const trackedChangeType = attributes['custom:trackedChangeType']; const trackedChangeText = attributes['custom:trackedChangeText'] !== 'null' ? attributes['custom:trackedChangeText'] : null; + const trackedChangeDisplayType = + attributes['custom:trackedChangeDisplayType'] !== 'null' ? attributes['custom:trackedChangeDisplayType'] : null; const trackedDeletedText = attributes['custom:trackedDeletedText'] !== 'null' ? attributes['custom:trackedDeletedText'] : null; @@ -79,6 +81,7 @@ export function importCommentData({ docx, editor, converter }) { trackedChange, trackedChangeText, trackedChangeType, + trackedChangeDisplayType, trackedDeletedText, isDone: false, origin: converter?.documentOrigin || 'word', diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index 393d46bb65..83bcf65319 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -8,6 +8,7 @@ import { resolveCommentById, translateFormatChangesToEnglish, } from './comments-helpers.js'; +import { resolveTrackedFormatDisplay } from './tracked-change-display.js'; // Example tracked-change keys, if needed import { comments_module_events } from '@superdoc/common'; @@ -990,6 +991,7 @@ const normalizeFormatAttrsForCommentText = (attrs = {}, nodes) => { const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsertion }) => { let trackedChangeText = ''; let deletionText = ''; + let trackedChangeDisplayType = null; // Extract deletion text first if (trackedChangeType === TrackDeleteMarkName || isDeletionInsertion) { @@ -1014,12 +1016,24 @@ const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsert // If this is a format change, let's get the string of what changes were made if (trackedChangeType === TrackFormatMarkName) { - trackedChangeText = translateFormatChangesToEnglish(normalizeFormatAttrsForCommentText(mark.attrs, nodes)); + const normalizedFormatAttrs = normalizeFormatAttrsForCommentText(mark.attrs, nodes); + const trackedFormatDisplay = resolveTrackedFormatDisplay({ + attrs: normalizedFormatAttrs, + nodes, + }); + + if (trackedFormatDisplay) { + trackedChangeText = trackedFormatDisplay.trackedChangeText; + trackedChangeDisplayType = trackedFormatDisplay.trackedChangeDisplayType; + } else { + trackedChangeText = translateFormatChangesToEnglish(normalizedFormatAttrs); + } } return { deletionText, trackedChangeText, + trackedChangeDisplayType, }; }; @@ -1032,18 +1046,24 @@ const createOrUpdateTrackedChangeComment = ({ documentId, trackedChangesForId, }) => { - const trackedMark = marks.insertedMark || marks.deletionMark || marks.formatMark; + const node = nodes[0]; + // Use pre-computed tracked changes when available (batch import path), + // otherwise scan the document (real-time edit path). + const fallbackTrackedMark = marks.insertedMark || marks.deletionMark || marks.formatMark; + if (!fallbackTrackedMark) { + return; + } + + const fallbackTrackedMarkId = fallbackTrackedMark.attrs?.id; + const trackedChangesWithId = trackedChangesForId || getTrackChanges(newEditorState, fallbackTrackedMarkId); + const liveFormatMark = trackedChangesWithId.find(({ mark }) => mark.type.name === TrackFormatMarkName)?.mark ?? null; + const trackedMark = marks.insertedMark || marks.deletionMark || liveFormatMark || marks.formatMark; const { type, attrs } = trackedMark; const { name: trackedChangeType } = type; const { author, authorEmail, authorImage, date, importedAuthor } = attrs; const id = attrs.id; - const node = nodes[0]; - // Use pre-computed tracked changes when available (batch import path), - // otherwise scan the document (real-time edit path). - const trackedChangesWithId = trackedChangesForId || getTrackChanges(newEditorState, id); - // Check metadata first - this should be set correctly by groupChanges() in createCommentForTrackChanges // for both newly created and imported tracked changes let isDeletionInsertion = !!(marks.insertedMark && marks.deletionMark); @@ -1059,7 +1079,7 @@ const createOrUpdateTrackedChangeComment = ({ // Collect nodes from the tracked changes found // We need to get the actual nodes at those positions - let nodesWithMark = []; + const nodesWithMark = []; trackedChangesWithId.forEach(({ from, to }) => { newEditorState.doc.nodesBetween(from, to, (node) => { // Only collect inline text nodes @@ -1094,8 +1114,6 @@ const createOrUpdateTrackedChangeComment = ({ ...(!hasInsertNode && nodes?.length ? nodes : []), ...(!hasDeleteNode && deletionNodes?.length ? deletionNodes : []), ]; - // safety net for identity dedupe - // work is done above nodesToUse = Array.from(new Set([...nodesWithMark, ...fallbackNodes])); } else { // For non-replacements, use nodes found in document or fall back to step nodes @@ -1106,8 +1124,7 @@ const createOrUpdateTrackedChangeComment = ({ return; } - const { deletionText, trackedChangeText } = getTrackedChangeText({ - state: newEditorState, + const { deletionText, trackedChangeText, trackedChangeDisplayType } = getTrackedChangeText({ nodes: nodesToUse, mark: trackedMark, trackedChangeType, @@ -1126,6 +1143,7 @@ const createOrUpdateTrackedChangeComment = ({ changeId: id, trackedChangeType: isDeletionInsertion ? 'both' : trackedChangeType, trackedChangeText, + trackedChangeDisplayType, deletedText: marks.deletionMark ? deletionText : null, author, authorEmail, 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 97fc239e07..bd17e305e7 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -80,6 +80,21 @@ const createCommentSchema = () => { toDOM: (mark) => [TrackFormatMarkName, mark.attrs], parseDOM: [{ tag: TrackFormatMarkName }], }, + underline: { + attrs: {}, + inclusive: false, + toDOM: () => ['underline', 0], + parseDOM: [{ tag: 'underline' }], + }, + link: { + attrs: { + href: { default: null }, + text: { default: null }, + }, + inclusive: false, + toDOM: (mark) => ['a', mark.attrs, 0], + parseDOM: [{ tag: 'a' }], + }, }; return new Schema({ nodes, marks }); @@ -1027,6 +1042,7 @@ describe('internal helper functions', () => { isDeletionInsertion: false, }); expect(formatResult.trackedChangeText).toBe('italic, removed bold'); + expect(formatResult.trackedChangeDisplayType).toBeNull(); const deltaFormatMark = schema.marks[TrackFormatMarkName].create({ id: 'format-2', @@ -1042,6 +1058,25 @@ describe('internal helper functions', () => { expect(deltaFormatResult.trackedChangeText).toContain('bold'); expect(deltaFormatResult.trackedChangeText).not.toContain('undefined'); + const hyperlinkFormatMark = schema.marks[TrackFormatMarkName].create({ + id: 'format-3', + before: [], + after: [ + { type: 'underline', attrs: {} }, + { type: 'link', attrs: { href: 'https://example.com', text: 'website' } }, + ], + }); + const hyperlinkFormatResult = getTrackedChangeText({ + nodes: [schema.text('website', [hyperlinkFormatMark, schema.marks.link.create({ href: 'https://example.com' })])], + mark: hyperlinkFormatMark, + trackedChangeType: TrackFormatMarkName, + isDeletionInsertion: false, + }); + expect(hyperlinkFormatResult).toMatchObject({ + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }); + const combinedResult = getTrackedChangeText({ nodes: [...insertionNodes, ...deletionNodes], mark: insertMark, @@ -1137,6 +1172,81 @@ describe('internal helper functions', () => { expect(emptyPayload).toBeUndefined(); }); + it('createOrUpdateTrackedChangeComment preserves hyperlink-specific display metadata for format changes', () => { + const schema = createCommentSchema(); + const formatMark = schema.marks[TrackFormatMarkName].create({ + id: 'format-link-1', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + before: [], + after: [ + { type: 'underline', attrs: {} }, + { type: 'link', attrs: { href: 'https://example.com', text: 'website' } }, + ], + }); + const nodes = [schema.text('website', [formatMark])]; + const state = EditorState.create({ + schema, + doc: schema.node('doc', null, [schema.node('paragraph', null, nodes)]), + }); + + const payload = createOrUpdateTrackedChangeComment({ + event: 'add', + marks: { insertedMark: null, deletionMark: null, formatMark }, + deletionNodes: [], + nodes, + newEditorState: state, + documentId: 'doc-1', + }); + + expect(payload).toMatchObject({ + trackedChangeType: TrackFormatMarkName, + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }); + }); + + it('createOrUpdateTrackedChangeComment prefers the live format mark when transaction meta is stale', () => { + const schema = createCommentSchema(); + const staleFormatMark = schema.marks[TrackFormatMarkName].create({ + id: 'format-link-2', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + before: [], + after: [{ type: 'underline', attrs: {} }], + }); + const liveFormatMark = schema.marks[TrackFormatMarkName].create({ + id: 'format-link-2', + author: 'Author', + authorEmail: 'author@example.com', + date: 'today', + before: [], + after: [{ type: 'underline', attrs: {} }], + }); + const nodes = [schema.text('website', [liveFormatMark, schema.marks.link.create({ href: 'https://example.com' })])]; + const state = EditorState.create({ + schema, + doc: schema.node('doc', null, [schema.node('paragraph', null, nodes)]), + }); + + const payload = createOrUpdateTrackedChangeComment({ + event: 'add', + marks: { insertedMark: null, deletionMark: null, formatMark: staleFormatMark }, + deletionNodes: [], + nodes, + newEditorState: state, + documentId: 'doc-1', + }); + + expect(payload).toMatchObject({ + trackedChangeType: TrackFormatMarkName, + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }); + }); + it('findRangeById returns ranges for comment and tracked marks', () => { const schema = createCommentSchema(); const commentMark = schema.marks[CommentMarkName].create({ commentId: 'comment-range' }); diff --git a/packages/super-editor/src/extensions/comment/tracked-change-display.js b/packages/super-editor/src/extensions/comment/tracked-change-display.js new file mode 100644 index 0000000000..dbdf02cf4a --- /dev/null +++ b/packages/super-editor/src/extensions/comment/tracked-change-display.js @@ -0,0 +1,139 @@ +/** + * Display token for tracked format changes that semantically represent + * hyperlink insertion rather than generic formatting. + */ +export const HyperlinkAddedDisplayType = 'hyperlinkAdded'; + +/** + * Display token for tracked format changes that modify an existing hyperlink + * (e.g. changing the href or re-saving the same link in suggesting mode). + */ +export const HyperlinkModifiedDisplayType = 'hyperlinkModified'; + +const getMarkSnapshots = (attrs = {}) => { + const before = Array.isArray(attrs.before) ? attrs.before : []; + const after = Array.isArray(attrs.after) ? attrs.after : []; + return { before, after }; +}; + +const findSnapshotByType = (snapshots, type) => { + return snapshots.find((snapshot) => snapshot?.type === type) ?? null; +}; + +const getNodeText = (nodes = []) => { + return nodes + .map((node) => node?.text || node?.textContent || '') + .join('') + .trim(); +}; + +const getHyperlinkTarget = (linkSnapshot) => { + const href = linkSnapshot?.attrs?.href; + if (typeof href === 'string' && href.trim().length > 0) { + return href.trim(); + } + + const anchor = linkSnapshot?.attrs?.anchor; + if (typeof anchor === 'string' && anchor.trim().length > 0) { + return `#${anchor.trim()}`; + } + + return null; +}; + +const getLiveLinkMark = (nodes = []) => { + for (const node of nodes) { + const linkMark = node?.marks?.find((mark) => mark?.type?.name === 'link'); + if (linkMark) { + return linkMark; + } + } + + return null; +}; + +const getHyperlinkLabel = ({ linkSnapshot, nodes }) => { + return ( + getHyperlinkTarget(linkSnapshot) || + (typeof linkSnapshot?.attrs?.text === 'string' && linkSnapshot.attrs.text.trim().length > 0 + ? linkSnapshot.attrs.text.trim() + : null) || + getNodeText(nodes) + ); +}; + +const getAddedMarkTypes = ({ before, after }) => { + const beforeTypes = new Set(before.map((snapshot) => snapshot?.type).filter(Boolean)); + const afterTypes = new Set(after.map((snapshot) => snapshot?.type).filter(Boolean)); + return [...afterTypes].filter((type) => !beforeTypes.has(type)); +}; + +const getRemovedMarkTypes = ({ before, after }) => { + const beforeTypes = new Set(before.map((snapshot) => snapshot?.type).filter(Boolean)); + const afterTypes = new Set(after.map((snapshot) => snapshot?.type).filter(Boolean)); + return [...beforeTypes].filter((type) => !afterTypes.has(type)); +}; + +const isUnderlineOnlyFormatDelta = ({ before, after }) => { + const addedTypes = getAddedMarkTypes({ before, after }); + const removedTypes = getRemovedMarkTypes({ before, after }); + return removedTypes.length === 0 && addedTypes.length === 1 && addedTypes[0] === 'underline'; +}; + +/** + * Detect tracked format changes that should render with hyperlink-specific copy. + * + * The tracked-change layer stores hyperlink application as a format change because + * it is implemented as mark mutations. For comment bubbles, however, "Format: + * underline" is misleading when the meaningful change is "a hyperlink was added." + * + * @param {Object} params + * @param {Object} [params.attrs={}] Tracked format mark attributes + * @param {Array} [params.nodes=[]] Live text nodes covered by the tracked change + * @returns {{trackedChangeDisplayType: string, trackedChangeText: string} | null} + */ +export const resolveTrackedFormatDisplay = ({ attrs = {}, nodes = [] }) => { + const { before, after } = getMarkSnapshots(attrs); + const beforeLink = findSnapshotByType(before, 'link'); + const afterLink = findSnapshotByType(after, 'link'); + const inferredLiveLink = + !beforeLink && !afterLink && isUnderlineOnlyFormatDelta({ before, after }) ? getLiveLinkMark(nodes) : null; + const addedLink = afterLink || inferredLiveLink; + + // Link exists in both before and after — either a no-op re-save or a real edit. + // Handle here instead of falling through to the generic translator, which would + // produce a spurious "Format: underline" comment. + if (beforeLink && addedLink) { + const beforeTarget = getHyperlinkTarget(beforeLink); + const afterTarget = getHyperlinkTarget(addedLink); + + // No-op: same link target — return empty text so the caller's + // `if (!deletionText && !trackedChangeText)` guard suppresses the comment. + if (beforeTarget && afterTarget && beforeTarget === afterTarget) { + return { trackedChangeDisplayType: null, trackedChangeText: '' }; + } + + const trackedChangeText = getHyperlinkLabel({ linkSnapshot: addedLink, nodes }); + if (!trackedChangeText) { + return null; + } + return { + trackedChangeDisplayType: HyperlinkModifiedDisplayType, + trackedChangeText, + }; + } + + if (!addedLink) { + return null; + } + + const trackedChangeText = getHyperlinkLabel({ linkSnapshot: addedLink, nodes }); + if (!trackedChangeText) { + return null; + } + + return { + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText, + }; +}; diff --git a/packages/super-editor/src/extensions/comment/tracked-change-display.test.js b/packages/super-editor/src/extensions/comment/tracked-change-display.test.js new file mode 100644 index 0000000000..9d10857f37 --- /dev/null +++ b/packages/super-editor/src/extensions/comment/tracked-change-display.test.js @@ -0,0 +1,205 @@ +import { describe, it, expect } from 'vitest'; +import { + resolveTrackedFormatDisplay, + HyperlinkAddedDisplayType, + HyperlinkModifiedDisplayType, +} from './tracked-change-display.js'; + +const makeNode = ({ text = '', marks = [] } = {}) => ({ + text, + textContent: text, + marks, +}); + +const makeLinkMark = (href = 'https://example.com') => ({ + type: { name: 'link' }, + attrs: { href }, +}); + +describe('resolveTrackedFormatDisplay', () => { + it('returns null when attrs have no link in after snapshots and no underline-only delta', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { before: [], after: [{ type: 'bold', attrs: {} }] }, + nodes: [], + }); + expect(result).toBeNull(); + }); + + it('detects hyperlink added when link snapshot is in after array', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [ + { type: 'underline', attrs: {} }, + { type: 'link', attrs: { href: 'https://example.com', text: 'click' } }, + ], + }, + nodes: [makeNode({ text: 'click' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: 'https://example.com', + }); + }); + + it('returns hyperlinkModified when link exists in both before and after (link edit)', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [{ type: 'link', attrs: { href: 'https://old.com' } }], + after: [{ type: 'link', attrs: { href: 'https://new.com' } }], + }, + nodes: [makeNode({ text: 'click' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkModifiedDisplayType, + trackedChangeText: 'https://new.com', + }); + }); + + it('suppresses no-op link re-saves by returning empty text', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [{ type: 'link', attrs: { href: 'https://example.com' } }], + after: [ + { type: 'link', attrs: { href: 'https://example.com' } }, + { type: 'underline', attrs: {} }, + ], + }, + nodes: [makeNode({ text: 'click' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: null, + trackedChangeText: '', + }); + }); + + it('returns null for link modification when no label can be resolved', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [{ type: 'link', attrs: { href: 'https://old.com' } }], + after: [{ type: 'link', attrs: {} }], + }, + nodes: [], + }); + expect(result).toBeNull(); + }); + + it('uses anchor attribute when href is absent', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'link', attrs: { anchor: 'heading-1' } }], + }, + nodes: [makeNode({ text: 'link text' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: '#heading-1', + }); + }); + + it('falls back to link text attr when href and anchor are empty', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'link', attrs: { href: '', text: 'display text' } }], + }, + nodes: [], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: 'display text', + }); + }); + + it('falls back to node text when link snapshot has no useful attributes', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'link', attrs: {} }], + }, + nodes: [makeNode({ text: 'node content' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: 'node content', + }); + }); + + it('returns null when no label can be resolved from any source', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'link', attrs: {} }], + }, + nodes: [], + }); + expect(result).toBeNull(); + }); + + it('infers hyperlink from live link mark when format delta is underline-only', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'underline', attrs: {} }], + }, + nodes: [makeNode({ text: 'website', marks: [makeLinkMark('https://inferred.com')] })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: 'https://inferred.com', + }); + }); + + it('does not infer hyperlink when format delta includes more than underline', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [ + { type: 'underline', attrs: {} }, + { type: 'bold', attrs: {} }, + ], + }, + nodes: [makeNode({ text: 'text', marks: [makeLinkMark()] })], + }); + expect(result).toBeNull(); + }); + + it('does not infer hyperlink when format delta removes marks alongside underline add', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [{ type: 'italic', attrs: {} }], + after: [{ type: 'underline', attrs: {} }], + }, + nodes: [makeNode({ text: 'text', marks: [makeLinkMark()] })], + }); + expect(result).toBeNull(); + }); + + it('handles missing attrs gracefully', () => { + expect(resolveTrackedFormatDisplay({ attrs: undefined, nodes: [] })).toBeNull(); + expect(resolveTrackedFormatDisplay({ attrs: {}, nodes: [] })).toBeNull(); + }); + + it('handles non-array before/after gracefully', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { before: 'invalid', after: null }, + nodes: [], + }); + expect(result).toBeNull(); + }); + + it('joins text from multiple nodes', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [], + after: [{ type: 'link', attrs: {} }], + }, + nodes: [makeNode({ text: 'hello ' }), makeNode({ text: 'world' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkAddedDisplayType, + trackedChangeText: 'hello world', + }); + }); +}); diff --git a/packages/super-editor/src/extensions/track-changes/constants.js b/packages/super-editor/src/extensions/track-changes/constants.js index 88a56dc69c..fb06b680e2 100644 --- a/packages/super-editor/src/extensions/track-changes/constants.js +++ b/packages/super-editor/src/extensions/track-changes/constants.js @@ -1,3 +1,9 @@ export const TrackInsertMarkName = 'trackInsert'; export const TrackDeleteMarkName = 'trackDelete'; export const TrackFormatMarkName = 'trackFormat'; + +/** + * Marks that should be represented as tracked formatting changes in suggesting mode. + * These are the marks that contribute visible formatting or inline semantics. + */ +export const TrackedFormatMarkNames = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight', 'link']; diff --git a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js index 457569b28c..22402fa40a 100644 --- a/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js +++ b/packages/super-editor/src/extensions/track-changes/track-changes-extension.test.js @@ -880,6 +880,41 @@ describe('TrackChanges extension commands', () => { } }); + it('interaction: setLink in suggesting mode emits hyperlink-specific tracked change messaging', () => { + const { editor: interactionEditor } = initTestEditor({ + mode: 'text', + content: '

Visit website

', + user: { name: 'Track Tester', email: 'track@example.com' }, + }); + + try { + interactionEditor.setDocumentMode('suggesting'); + + const websiteRange = getSubstringRange(interactionEditor.state.doc, 'website'); + interactionEditor.view.dispatch( + interactionEditor.state.tr.setSelection( + TextSelection.create(interactionEditor.state.doc, websiteRange.from, websiteRange.to), + ), + ); + + const emitSpy = vi.spyOn(interactionEditor, 'emit'); + interactionEditor.commands.setLink({ href: 'https://example.com' }); + + const trackedChangePayload = emitSpy.mock.calls.find( + ([eventName, payload]) => + eventName === 'commentsUpdate' && payload?.type === 'trackedChange' && payload?.event === 'add', + )?.[1]; + + expect(trackedChangePayload).toMatchObject({ + trackedChangeType: TrackFormatMarkName, + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }); + } finally { + interactionEditor.destroy(); + } + }); + it('undo/redo restores partially accepted insertion splits', () => { const { editor: interactionEditor } = initTestEditor({ mode: 'text', diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js index b2f115f08f..64c93d4e51 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -1,4 +1,4 @@ -import { TrackDeleteMarkName, TrackFormatMarkName } from '../constants.js'; +import { TrackDeleteMarkName, TrackFormatMarkName, TrackedFormatMarkNames } from '../constants.js'; import { v4 as uuidv4 } from 'uuid'; import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; import { CommentsPluginKey } from '../../comment/comments-plugin.js'; @@ -49,10 +49,7 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { const wid = existingChangeMark ? existingChangeMark.attrs.id : (sharedWid ?? (sharedWid = uuidv4())); newTr.addMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), step.mark); - const allowedMarks = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight']; - - // ![TrackDeleteMarkName].includes(step.mark.type.name) - if (allowedMarks.includes(step.mark.type.name) && !hasMatchingMark(liveMarks, step.mark)) { + if (TrackedFormatMarkNames.includes(step.mark.type.name) && !hasMatchingMark(liveMarks, step.mark)) { const formatChangeMark = liveMarks.find((mark) => mark.type.name === TrackFormatMarkName); let after = []; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js index 017c7070eb..a56e4c413d 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -1,5 +1,5 @@ import { v4 as uuidv4 } from 'uuid'; -import { TrackDeleteMarkName, TrackFormatMarkName } from '../constants.js'; +import { TrackDeleteMarkName, TrackFormatMarkName, TrackedFormatMarkNames } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; import { CommentsPluginKey } from '../../comment/comments-plugin.js'; import { hasMatchingMark, markSnapshotMatchesStepMark, upsertMarkSnapshotByType } from './markSnapshotHelpers.js'; @@ -36,9 +36,7 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { }); newTr.removeMark(Math.max(step.from, pos), Math.min(step.to, pos + node.nodeSize), step.mark); - const allowedMarks = ['bold', 'italic', 'strike', 'underline', 'textStyle', 'highlight']; - - if (allowedMarks.includes(step.mark.type.name) && hasMatchingMark(liveMarksBeforeRemove, step.mark)) { + if (TrackedFormatMarkNames.includes(step.mark.type.name) && hasMatchingMark(liveMarksBeforeRemove, step.mark)) { const formatChangeMark = liveMarksBeforeRemove.find((mark) => mark.type.name === TrackFormatMarkName); let after = []; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js index 8ea87ee294..8e9f247ac7 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/trackChangesHelpers.test.js @@ -399,6 +399,26 @@ describe('trackChangesHelpers', () => { expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'highlight', attrs: { color: '#E4668C' } }]); }); + it('addMarkStep tracks hyperlink marks so comment summaries can identify hyperlink changes', () => { + const state = createState(createDocWithText('website')); + const linkMark = schema.marks.link.create({ href: 'https://example.com', text: 'website' }); + const step = new AddMarkStep(1, 8, linkMark); + const newTr = state.tr; + + addMarkStep({ + state, + step, + newTr, + doc: state.doc, + user, + date, + }); + + const meta = newTr.getMeta(TrackChangesBasePluginKey); + expect(meta?.formatMark?.type.name).toBe(TrackFormatMarkName); + expect(meta?.formatMark?.attrs?.after).toEqual([{ type: 'link', attrs: linkMark.attrs }]); + }); + it('addMarkStep does not include unrelated marks in before (SD-2077)', () => { const highlight = schema.marks.highlight.create({ color: '#FFFF00' }); const doc = createDocWithText('Hello', [highlight]); 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 937acae8cc..f9e5541484 100644 --- a/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js +++ b/packages/super-editor/src/tests/import/documentCommentsImporter.unit.test.js @@ -40,6 +40,7 @@ const buildDocx = ({ comments = [], extended = [], documentRanges = [] } = {}) = 'custom:trackedChange': comment.trackedChange, 'custom:trackedChangeText': comment.trackedChangeText, 'custom:trackedChangeType': comment.trackedChangeType, + 'custom:trackedChangeDisplayType': comment.trackedChangeDisplayType, 'custom:trackedDeletedText': comment.trackedDeletedText, }, elements: comment.elements ?? [{ fakeParaId: comment.paraId ?? `para-${comment.id}` }], @@ -204,6 +205,7 @@ describe('importCommentData metadata parsing', () => { trackedChange: 'true', trackedChangeText: 'Added text', trackedChangeType: 'insert', + trackedChangeDisplayType: 'hyperlinkAdded', trackedDeletedText: 'Removed text', }, ], @@ -216,6 +218,7 @@ describe('importCommentData metadata parsing', () => { expect(comment.trackedChange).toBe(true); expect(comment.trackedChangeText).toBe('Added text'); expect(comment.trackedChangeType).toBe('insert'); + expect(comment.trackedChangeDisplayType).toBe('hyperlinkAdded'); expect(comment.trackedDeletedText).toBe('Removed text'); }); @@ -227,6 +230,7 @@ describe('importCommentData metadata parsing', () => { trackedChange: 'false', trackedChangeText: 'null', trackedChangeType: undefined, + trackedChangeDisplayType: 'null', trackedDeletedText: 'null', }, ], @@ -236,6 +240,7 @@ describe('importCommentData metadata parsing', () => { expect(comment.trackedChange).toBe(false); expect(comment.trackedChangeText).toBeNull(); expect(comment.trackedChangeType).toBeUndefined(); + expect(comment.trackedChangeDisplayType).toBeNull(); expect(comment.trackedDeletedText).toBeNull(); }); diff --git a/packages/superdoc/src/SuperDoc.test.js b/packages/superdoc/src/SuperDoc.test.js index 663020dec0..809ff69bf4 100644 --- a/packages/superdoc/src/SuperDoc.test.js +++ b/packages/superdoc/src/SuperDoc.test.js @@ -691,6 +691,45 @@ describe('SuperDoc.vue', () => { expect(existingComment.threadingParentCommentId).toBe('thread-parent-new'); }); + it('updates replayed tracked-change display metadata for existing comments', async () => { + const superdocStub = createSuperdocStub(); + const wrapper = await mountComponent(superdocStub); + await nextTick(); + const { default: useComment } = await import('./components/CommentsLayer/use-comment.js'); + + const options = wrapper.findComponent(SuperEditorStub).props('options'); + const existingComment = useComment({ + commentId: 'tracked-change-1', + importedId: 'imp-tracked-change-1', + fileId: 'doc-1', + trackedChange: true, + trackedChangeType: 'trackFormat', + trackedChangeText: 'underline', + trackedChangeDisplayType: null, + creatorEmail: 'ada@example.com', + creatorName: 'Ada', + }); + commentsStoreStub.commentsList.value = [existingComment]; + commentsStoreStub.addComment.mockClear(); + superdocStub.activeEditor = { options: { documentId: 'doc-1' } }; + + options.onCommentsUpdate({ + type: 'update', + comment: { + commentId: 'tracked-change-1', + importedId: 'imp-tracked-change-1', + trackedChange: true, + trackedChangeType: 'trackFormat', + trackedChangeText: 'https://example.com', + trackedChangeDisplayType: 'hyperlinkAdded', + }, + }); + + expect(commentsStoreStub.addComment).not.toHaveBeenCalled(); + expect(existingComment.trackedChangeText).toBe('https://example.com'); + expect(existingComment.trackedChangeDisplayType).toBe('hyperlinkAdded'); + }); + it('maps replayed isDone updates to resolved fields when explicit resolved metadata is missing', async () => { const superdocStub = createSuperdocStub(); const wrapper = await mountComponent(superdocStub); diff --git a/packages/superdoc/src/SuperDoc.vue b/packages/superdoc/src/SuperDoc.vue index 293b52efc5..9348fb39c4 100644 --- a/packages/superdoc/src/SuperDoc.vue +++ b/packages/superdoc/src/SuperDoc.vue @@ -731,6 +731,7 @@ const REPLAY_MUTABLE_COMMENT_FIELDS = new Set([ 'trackedChange', 'trackedChangeType', 'trackedChangeText', + 'trackedChangeDisplayType', 'deletedText', 'resolvedTime', 'resolvedByEmail', diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js index 3f1b8d79bd..e99341738e 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.test.js @@ -118,6 +118,7 @@ const mountDialog = async ({ baseCommentOverrides = {}, extraComments = [], prop importedId: null, trackedChangeType: null, trackedChangeText: null, + trackedChangeDisplayType: null, deletedText: null, selection: { getValues: () => ({ selectionBounds: { top: 110, bottom: 130, left: 15, right: 30 } }), @@ -298,6 +299,40 @@ describe('CommentDialog.vue', () => { expect(superdocStub.focus).toHaveBeenCalledTimes(2); }); + it('renders hyperlink additions without a format label', async () => { + const { wrapper } = await mountDialog({ + baseCommentOverrides: { + trackedChange: true, + trackedChangeType: 'trackFormat', + trackedChangeDisplayType: 'hyperlinkAdded', + trackedChangeText: 'https://example.com', + }, + }); + + const trackedChange = wrapper.find('.tracked-change'); + expect(trackedChange.text()).toContain('Added hyperlink'); + expect(trackedChange.text()).toContain('https://example.com'); + expect(trackedChange.text()).not.toContain('Format:'); + expect(trackedChange.text()).not.toContain('underline'); + }); + + it('renders hyperlink modifications without a format label', async () => { + const { wrapper } = await mountDialog({ + baseCommentOverrides: { + trackedChange: true, + trackedChangeType: 'trackFormat', + trackedChangeDisplayType: 'hyperlinkModified', + trackedChangeText: 'https://new.com', + }, + }); + + const trackedChange = wrapper.find('.tracked-change'); + expect(trackedChange.text()).toContain('Changed hyperlink to'); + expect(trackedChange.text()).toContain('https://new.com'); + expect(trackedChange.text()).not.toContain('Format:'); + expect(trackedChange.text()).not.toContain('underline'); + }); + it('calls custom accept handler instead of default behavior when configured', async () => { const customAcceptHandler = vi.fn(); diff --git a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue index 53276798fe..432cee85e4 100644 --- a/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue +++ b/packages/superdoc/src/components/CommentsLayer/CommentDialog.vue @@ -616,7 +616,15 @@ watch(editingCommentId, (commentId) => { :class="{ 'is-truncated': shouldTruncate && index === 0 }" :ref="index === 0 ? (el) => (parentBodyRef = el) : undefined" > -
+
+ Added hyperlink + "{{ comment.trackedChangeText }}" +
+
+ Changed hyperlink to + "{{ comment.trackedChangeText }}" +
+
Format: {{ comment.trackedChangeText }}
diff --git a/packages/superdoc/src/components/CommentsLayer/use-comment.js b/packages/superdoc/src/components/CommentsLayer/use-comment.js index 4a33a6396d..b67955fa27 100644 --- a/packages/superdoc/src/components/CommentsLayer/use-comment.js +++ b/packages/superdoc/src/components/CommentsLayer/use-comment.js @@ -57,6 +57,7 @@ export default function useComment(params) { const trackedChange = ref(params.trackedChange); const trackedChangeType = ref(params.trackedChangeType || null); const trackedChangeText = ref(params.trackedChangeText || null); + const trackedChangeDisplayType = ref(params.trackedChangeDisplayType || null); const deletedText = ref(params.deletedText || null); const resolvedTime = ref(params.resolvedTime || null); @@ -251,6 +252,7 @@ export default function useComment(params) { trackedChange: trackedChange.value, trackedChangeText: trackedChangeText.value, trackedChangeType: trackedChangeType.value, + trackedChangeDisplayType: trackedChangeDisplayType.value, deletedText: deletedText.value, resolvedTime: resolvedTime.value, resolvedByEmail: resolvedByEmail.value, @@ -286,6 +288,7 @@ export default function useComment(params) { deletedText, trackedChangeType, trackedChangeText, + trackedChangeDisplayType, resolvedTime, resolvedByEmail, resolvedByName, diff --git a/packages/superdoc/src/stores/comments-store.js b/packages/superdoc/src/stores/comments-store.js index a6ebfcfe40..f668a9cffe 100644 --- a/packages/superdoc/src/stores/comments-store.js +++ b/packages/superdoc/src/stores/comments-store.js @@ -474,6 +474,7 @@ export const useCommentsStore = defineStore('comments', () => { changeId, trackedChangeText, trackedChangeType, + trackedChangeDisplayType, deletedText, authorEmail, authorImage, @@ -491,6 +492,7 @@ export const useCommentsStore = defineStore('comments', () => { trackedChange: true, trackedChangeText, trackedChangeType, + trackedChangeDisplayType, deletedText, createdTime: date, creatorName: authorName, @@ -536,6 +538,7 @@ export const useCommentsStore = defineStore('comments', () => { // clear fields explicitly when the updated payload no longer includes them. existing.trackedChangeText = trackedChangeText ?? null; existing.trackedChangeType = trackedChangeType ?? null; + existing.trackedChangeDisplayType = trackedChangeDisplayType ?? null; existing.deletedText = deletedText ?? null; const emitData = { @@ -558,6 +561,7 @@ export const useCommentsStore = defineStore('comments', () => { // clear fields explicitly when the updated payload no longer includes them. existingTrackedChange.trackedChangeText = trackedChangeText ?? null; existingTrackedChange.trackedChangeType = trackedChangeType ?? null; + existingTrackedChange.trackedChangeDisplayType = trackedChangeDisplayType ?? null; existingTrackedChange.deletedText = deletedText ?? null; const emitData = { @@ -961,6 +965,7 @@ export const useCommentsStore = defineStore('comments', () => { trackedChange: comment.trackedChange || false, trackedChangeText: comment.trackedChangeText, trackedChangeType: comment.trackedChangeType, + trackedChangeDisplayType: comment.trackedChangeDisplayType, deletedText: comment.trackedDeletedText, // Preserve origin metadata for export origin: comment.origin || 'word', // Default to 'word' for backward compatibility diff --git a/packages/superdoc/src/stores/comments-store.test.js b/packages/superdoc/src/stores/comments-store.test.js index e109135cd2..8547668029 100644 --- a/packages/superdoc/src/stores/comments-store.test.js +++ b/packages/superdoc/src/stores/comments-store.test.js @@ -346,6 +346,45 @@ describe('comments-store', () => { expect(existingComment.resolvedByName).toBeNull(); }); + it('preserves hyperlink-specific tracked-change display metadata on updates', () => { + const superdoc = { + emit: vi.fn(), + }; + + const existingComment = { + commentId: 'change-link-1', + trackedChangeText: 'underline', + trackedChangeType: 'trackFormat', + trackedChangeDisplayType: null, + deletedText: null, + getValues: vi.fn(() => ({ commentId: 'change-link-1' })), + }; + + store.commentsList = [existingComment]; + + store.handleTrackedChangeUpdate({ + superdoc, + params: { + event: 'update', + changeId: 'change-link-1', + trackedChangeText: 'https://example.com', + trackedChangeType: 'trackFormat', + trackedChangeDisplayType: 'hyperlinkAdded', + deletedText: null, + authorEmail: 'user@example.com', + author: 'User', + date: 123, + importedAuthor: null, + documentId: 'doc-1', + coords: {}, + }, + }); + + expect(existingComment.trackedChangeText).toBe('https://example.com'); + expect(existingComment.trackedChangeType).toBe('trackFormat'); + expect(existingComment.trackedChangeDisplayType).toBe('hyperlinkAdded'); + }); + it('clears stale tracked-change metadata when an update removes one side of a replacement', () => { const superdoc = { emit: vi.fn(), @@ -536,6 +575,7 @@ describe('comments-store', () => { changeId: 'change-add-1', trackedChangeText: 'Inserted text', trackedChangeType: 'trackInsert', + trackedChangeDisplayType: 'hyperlinkAdded', authorEmail: 'user@example.com', author: 'User', date: Date.now(), @@ -547,6 +587,7 @@ describe('comments-store', () => { expect(store.commentsList).toHaveLength(1); expect(store.commentsList[0].selection.source).toBe('super-editor'); + expect(store.commentsList[0].trackedChangeDisplayType).toBe('hyperlinkAdded'); }); it('clears stale tracked-change positions when editor sends empty positions', async () => { diff --git a/shared/common/comments-types.ts b/shared/common/comments-types.ts index 71b936c683..1a6b037ad9 100644 --- a/shared/common/comments-types.ts +++ b/shared/common/comments-types.ts @@ -22,6 +22,7 @@ export type Comment = { trackedChange: boolean; trackedChangeText: string | null; trackedChangeType: 'trackInsert' | 'trackDelete' | 'both' | 'trackFormat'; + trackedChangeDisplayType?: 'hyperlinkAdded' | 'hyperlinkModified' | null; deletedText: string | null; resolvedTime: number | null; resolvedByEmail: string | null; diff --git a/tests/behavior/tests/comments/sd-2250-hyperlink-change-display.spec.ts b/tests/behavior/tests/comments/sd-2250-hyperlink-change-display.spec.ts new file mode 100644 index 0000000000..d57d9d68f9 --- /dev/null +++ b/tests/behavior/tests/comments/sd-2250-hyperlink-change-display.spec.ts @@ -0,0 +1,37 @@ +import { test, expect } from '../../fixtures/superdoc.js'; + +test.use({ config: { comments: 'on', trackChanges: true } }); + +test('SD-2250 hyperlink tracked-change bubble describes a hyperlink instead of underline formatting', async ({ + superdoc, +}) => { + await superdoc.type('Visit website'); + await superdoc.waitForStable(); + + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + const websiteStart = await superdoc.findTextPos('website'); + await superdoc.setTextSelection(websiteStart, websiteStart + 'website'.length); + await superdoc.waitForStable(); + + await superdoc.executeCommand('setLink', { href: 'https://example.com' }); + await superdoc.waitForStable(); + + await superdoc.assertTrackedChangeExists('format'); + await superdoc.assertTextHasMarks('website', ['link']); + await superdoc.assertTextMarkAttrs('website', 'link', { href: 'https://example.com' }); + + const hyperlinkDialog = superdoc.page.locator('.comment-placeholder .comments-dialog', { + has: superdoc.page.locator('.change-type', { hasText: 'Added hyperlink' }), + }); + await expect(hyperlinkDialog).toBeVisible({ timeout: 10_000 }); + await expect(hyperlinkDialog.locator('.tracked-change-text', { hasText: 'https://example.com' })).toBeVisible(); + + await expect( + superdoc.page.locator('.comment-placeholder .comments-dialog .change-type', { hasText: 'Format:' }), + ).toHaveCount(0); + await expect( + superdoc.page.locator('.comment-placeholder .comments-dialog .tracked-change-text', { hasText: 'underline' }), + ).toHaveCount(0); +}); From a431084b0501783e7a13961e337e07b13cb2d8a6 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Thu, 19 Mar 2026 16:49:54 -0700 Subject: [PATCH 2/2] fix: show hyperlink-specific copy in tracked change comments instead of underline --- .../comment/tracked-change-display.js | 23 ++++++++++++++----- .../comment/tracked-change-display.test.js | 17 ++++++++++++++ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/super-editor/src/extensions/comment/tracked-change-display.js b/packages/super-editor/src/extensions/comment/tracked-change-display.js index dbdf02cf4a..caae2ebdb8 100644 --- a/packages/super-editor/src/extensions/comment/tracked-change-display.js +++ b/packages/super-editor/src/extensions/comment/tracked-change-display.js @@ -80,6 +80,21 @@ const isUnderlineOnlyFormatDelta = ({ before, after }) => { return removedTypes.length === 0 && addedTypes.length === 1 && addedTypes[0] === 'underline'; }; +const snapshotAttrsEqual = (a, b) => { + if (a === b) return true; + if (!a || !b) return false; + const keysA = Object.keys(a); + const keysB = Object.keys(b); + if (keysA.length !== keysB.length) return false; + return keysA.every((key) => { + const valA = a[key]; + const valB = b[key]; + // Treat null/undefined as equivalent (attrs often mix the two) + if (valA == null && valB == null) return true; + return valA === valB; + }); +}; + /** * Detect tracked format changes that should render with hyperlink-specific copy. * @@ -104,12 +119,8 @@ export const resolveTrackedFormatDisplay = ({ attrs = {}, nodes = [] }) => { // Handle here instead of falling through to the generic translator, which would // produce a spurious "Format: underline" comment. if (beforeLink && addedLink) { - const beforeTarget = getHyperlinkTarget(beforeLink); - const afterTarget = getHyperlinkTarget(addedLink); - - // No-op: same link target — return empty text so the caller's - // `if (!deletionText && !trackedChangeText)` guard suppresses the comment. - if (beforeTarget && afterTarget && beforeTarget === afterTarget) { + // True no-op: every link attr is identical — suppress the comment. + if (snapshotAttrsEqual(beforeLink.attrs, addedLink.attrs)) { return { trackedChangeDisplayType: null, trackedChangeText: '' }; } diff --git a/packages/super-editor/src/extensions/comment/tracked-change-display.test.js b/packages/super-editor/src/extensions/comment/tracked-change-display.test.js index 9d10857f37..daf43e4e71 100644 --- a/packages/super-editor/src/extensions/comment/tracked-change-display.test.js +++ b/packages/super-editor/src/extensions/comment/tracked-change-display.test.js @@ -73,6 +73,23 @@ describe('resolveTrackedFormatDisplay', () => { }); }); + it('treats same-target link text edits as a modification, not a no-op', () => { + const result = resolveTrackedFormatDisplay({ + attrs: { + before: [{ type: 'link', attrs: { href: 'https://example.com', text: 'old label' } }], + after: [ + { type: 'link', attrs: { href: 'https://example.com', text: 'new label' } }, + { type: 'underline', attrs: {} }, + ], + }, + nodes: [makeNode({ text: 'new label' })], + }); + expect(result).toEqual({ + trackedChangeDisplayType: HyperlinkModifiedDisplayType, + trackedChangeText: 'https://example.com', + }); + }); + it('returns null for link modification when no label can be resolved', () => { const result = resolveTrackedFormatDisplay({ attrs: {