diff --git a/packages/document-api/src/contract/command-catalog.ts b/packages/document-api/src/contract/command-catalog.ts index 86adc33d2..1aaef1e24 100644 --- a/packages/document-api/src/contract/command-catalog.ts +++ b/packages/document-api/src/contract/command-catalog.ts @@ -1,5 +1,5 @@ import type { CommandCatalog, CommandStaticMetadata } from './types.js'; -import { OPERATION_IDS, projectFromDefinitions } from './operation-definitions.js'; +import { OPERATION_IDS, projectFromDefinitions } from './operation-definitions.js'; export const COMMAND_CATALOG: CommandCatalog = projectFromDefinitions((_id, entry) => entry.metadata); diff --git a/packages/super-editor/src/extensions/comment/comments-helpers.js b/packages/super-editor/src/extensions/comment/comments-helpers.js index 4bd1e1452..659d62b69 100644 --- a/packages/super-editor/src/extensions/comment/comments-helpers.js +++ b/packages/super-editor/src/extensions/comment/comments-helpers.js @@ -736,6 +736,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 57c9bfab6..47ba154eb 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.js @@ -898,6 +898,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 }) => { let trackedChangeText = ''; let deletionText = ''; @@ -925,7 +984,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 7388b2cbf..aeed33fbb 100644 --- a/packages/super-editor/src/extensions/comment/comments-plugin.test.js +++ b/packages/super-editor/src/extensions/comment/comments-plugin.test.js @@ -950,6 +950,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 2449f1141..b8df514ac 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/addMarkStep.js @@ -20,7 +20,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 6b0796027..1a90a694a 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/removeMarkStep.js @@ -18,7 +18,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; } @@ -62,12 +62,17 @@ export const removeMarkStep = ({ state, step, newTr, doc, user, date }) => { } } else { after = []; - before = [ - { - type: step.mark.type.name, - attrs: { ...step.mark.attrs }, - }, - ]; + let existingMark = node.marks.find((mark) => mark.type === step.mark.type); + if (existingMark) { + before = [ + { + type: step.mark.type.name, + attrs: { ...existingMark.attrs }, + }, + ]; + } else { + before = []; + } } if (after.length || before.length) {