diff --git a/packages/super-editor/src/extensions/comment/comments-helpers.js b/packages/super-editor/src/extensions/comment/comments-helpers.js index 33d12a748f..55394880b2 100644 --- a/packages/super-editor/src/extensions/comment/comments-helpers.js +++ b/packages/super-editor/src/extensions/comment/comments-helpers.js @@ -706,6 +706,8 @@ export const translateFormatChangesToEnglish = (attrs = {}) => { const label = formatAttrName(attr); // Convert camelCase to lowercase words if (beforeValue === undefined || beforeValue === null) { textStyleChanges.push(`Set ${label} to ${afterValue}`); + } else if (afterValue === undefined || afterValue === null) { + textStyleChanges.push(`Removed ${label} (was ${beforeValue})`); } else { textStyleChanges.push(`Changed ${label} from ${beforeValue} to ${afterValue}`); } diff --git a/packages/super-editor/src/extensions/comment/comments-plugin.js b/packages/super-editor/src/extensions/comment/comments-plugin.js index d6b8262b18..a59d077ab0 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -782,6 +782,65 @@ const handleTrackedChangeTransaction = (trackedChangeMeta, trackedChanges, newEd return newTrackedChanges; }; +const normalizeFormatAttrsForCommentText = (attrs = {}, nodes) => { + const before = Array.isArray(attrs.before) ? attrs.before : []; + const after = Array.isArray(attrs.after) ? attrs.after : []; + const beforeTextStyle = before.find((mark) => mark?.type === 'textStyle'); + + if (!beforeTextStyle) { + return { + ...attrs, + before, + after, + }; + } + + const afterTextStyleIndex = after.findIndex((mark) => mark?.type === 'textStyle'); + const wasTextStyleRemoved = nodes.some((node) => { + const hasTextStyleMark = node.marks.find((mark) => mark.type.name === 'textStyle'); + return !hasTextStyleMark; + }); + + if (afterTextStyleIndex === -1) { + if (wasTextStyleRemoved) { + return { + ...attrs, + before, + after, + }; + } else { + return { + ...attrs, + before, + after: [ + ...after, + { + type: 'textStyle', + attrs: { + ...beforeTextStyle.attrs, + }, + }, + ], + }; + } + } + + const mergedAfter = [...after]; + mergedAfter[afterTextStyleIndex] = { + ...mergedAfter[afterTextStyleIndex], + attrs: { + ...(beforeTextStyle.attrs || {}), + ...(mergedAfter[afterTextStyleIndex].attrs || {}), + }, + }; + + return { + ...attrs, + before, + after: mergedAfter, + }; +}; + const getTrackedChangeText = ({ nodes, mark, trackedChangeType, isDeletionInsertion, marks }) => { let trackedChangeText = ''; let deletionText = ''; @@ -809,7 +868,7 @@ 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(mark.attrs); + trackedChangeText = translateFormatChangesToEnglish(normalizeFormatAttrsForCommentText(mark.attrs, nodes)); } return { 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 f22b5e6861..b1ecfdf2d7 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -907,6 +907,20 @@ describe('internal helper functions', () => { }); expect(formatResult.trackedChangeText).toContain('Added formatting'); + const deltaFormatMark = schema.marks[TrackFormatMarkName].create({ + id: 'format-2', + before: [{ type: 'textStyle', attrs: { color: '#111111', fontSize: '12px' } }], + after: [{ type: 'bold', attrs: {} }], + }); + const deltaFormatResult = getTrackedChangeText({ + nodes: [schema.text('Format', [deltaFormatMark])], + mark: deltaFormatMark, + trackedChangeType: TrackFormatMarkName, + isDeletionInsertion: false, + }); + expect(deltaFormatResult.trackedChangeText).toContain('Added formatting: bold'); + expect(deltaFormatResult.trackedChangeText).not.toContain('undefined'); + const combinedResult = getTrackedChangeText({ nodes: [...insertionNodes, ...deletionNodes], mark: insertMark, 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 37e774feda..384496c3b9 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -19,7 +19,7 @@ export const addMarkStep = ({ state, step, newTr, doc, user, date }) => { const meta = {}; doc.nodesBetween(step.from, step.to, (node, pos) => { - if (!node.isInline) { + if (!node.isInline || node.type.name === 'run') { return; } 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 c00dd6a99f..784e22c8b2 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -16,7 +16,7 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { const meta = {}; doc.nodesBetween(step.from, step.to, (node, pos) => { - if (!node.isInline) { + if (!node.isInline || node.type.name === 'run') { return true; } @@ -42,22 +42,32 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { before = [...formatChangeMark.attrs.before]; } else { after = [...formatChangeMark.attrs.after]; + let foundBefore = formatChangeMark.attrs.before.find((mark) => mark.type === step.mark.type.name); + if (foundBefore) { + before = [...formatChangeMark.attrs.before]; + } else { + before = [ + ...formatChangeMark.attrs.before, + { + type: step.mark.type.name, + attrs: { ...step.mark.attrs }, + }, + ]; + } + } + } else { + after = []; + let existingMark = node.marks.find((mark) => mark.type === step.mark.type); + if (existingMark) { before = [ - ...formatChangeMark.attrs.before, { type: step.mark.type.name, - attrs: { ...step.mark.attrs }, + attrs: { ...existingMark.attrs }, }, ]; + } else { + before = []; } - } else { - after = []; - before = [ - { - type: step.mark.type.name, - attrs: { ...step.mark.attrs }, - }, - ]; } if (after.length || before.length) {