diff --git a/packages/super-editor/src/extensions/heading/heading.test.js b/packages/super-editor/src/extensions/heading/heading.test.js index b69e941d93..4d68619516 100644 --- a/packages/super-editor/src/extensions/heading/heading.test.js +++ b/packages/super-editor/src/extensions/heading/heading.test.js @@ -56,13 +56,12 @@ describe('Heading Extension', () => { }); describe('toggleHeading', () => { - it('should return false for an empty selection', () => { + it('should apply heading with a cursor (empty) selection', () => { tr.setSelection(TextSelection.create(tr.doc, 1)); // Cursor selection const result = editor.commands.toggleHeading({ level: 1 }); - expect(result).toBe(false); - const styleId = editor.state.doc.content.content[0].attrs.paragraphProperties?.styleId ?? null; - expect(styleId).toBeNull(); + expect(result).toBe(true); + expect(editor.state.doc.content.content[0].attrs.paragraphProperties?.styleId).toBe('Heading1'); }); it('should toggle heading on for a paragraph', () => { diff --git a/packages/super-editor/src/extensions/linked-styles/linked-styles.js b/packages/super-editor/src/extensions/linked-styles/linked-styles.js index 837a93c42d..7138f7e131 100644 --- a/packages/super-editor/src/extensions/linked-styles/linked-styles.js +++ b/packages/super-editor/src/extensions/linked-styles/linked-styles.js @@ -61,17 +61,14 @@ export const LinkedStyles = Extension.create({ * @example * const style = editor.helpers.linkedStyles.getStyleById('Heading1'); * editor.commands.toggleLinkedStyle(style) - * @note If selection is empty, returns false + * @note Works with both cursor position and text selection * @note Removes style if already applied, applies it if not */ toggleLinkedStyle: (style) => (params) => { const { tr } = params; - if (tr.selection.empty) { - return false; - } let node = tr.doc.nodeAt(tr.selection.$from.pos); - if (node && node.type.name !== 'paragraph') { + if (!node || node.type.name !== 'paragraph') { node = findParentNodeClosestToPos(tr.selection.$from, (n) => { return n.type.name === 'paragraph'; })?.node; diff --git a/packages/super-editor/src/extensions/linked-styles/linked-styles.test.js b/packages/super-editor/src/extensions/linked-styles/linked-styles.test.js index 790bfb20be..84c5568324 100644 --- a/packages/super-editor/src/extensions/linked-styles/linked-styles.test.js +++ b/packages/super-editor/src/extensions/linked-styles/linked-styles.test.js @@ -135,13 +135,29 @@ describe('LinkedStyles Extension', () => { }); describe('toggleLinkedStyle', () => { - it('should return false for an empty selection', () => { + it('should apply style with a cursor (empty) selection', () => { setParagraphCursor(editor.view, 0); // Cursor selection at first paragraph const result = editor.commands.toggleLinkedStyle(headingStyle, 'paragraph'); - expect(result).toBe(false); + expect(result).toBe(true); const firstParagraph = findParagraphInfo(editor.state.doc, 0); - expect(getParagraphProps(firstParagraph.node).styleId).toBeUndefined(); + expect(getParagraphProps(firstParagraph.node).styleId).toBe('Heading1'); + }); + + it('should toggle off style with a cursor (empty) selection', () => { + // Apply style first + setParagraphCursor(editor.view, 0); + editor.commands.setLinkedStyle(headingStyle); + let firstParagraph = findParagraphInfo(editor.state.doc, 0); + expect(getParagraphProps(firstParagraph.node).styleId).toBe('Heading1'); + + // Toggle off with cursor + setParagraphCursor(editor.view, 0); + const result = editor.commands.toggleLinkedStyle(headingStyle, 'paragraph'); + + expect(result).toBe(true); + firstParagraph = findParagraphInfo(editor.state.doc, 0); + expect(getParagraphProps(firstParagraph.node).styleId).toBe(null); }); it('should apply style when no style is currently set', () => { diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js index 0eee67ccb8..52ab9724f5 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.js @@ -94,6 +94,23 @@ export const replaceAroundStep = ({ return; } + // Detect node-markup-change steps (setNodeMarkup and setBlockType both + // produce this same ReplaceAroundStep shape — they can't be distinguished + // at the step level). Used here to let paragraph style changes through in + // suggesting mode (e.g. Normal → Heading1 via setNodeMarkup). + // step.insert === 1 excludes lift() operations (insert === 0). + // Note: setBlockType is not triggered via UI in suggesting mode, but if + // it were, it would also bypass tracking. SD-2191 will add proper tracked + // change marks for these operations. + const isNodeMarkupChange = + step.structure && step.insert === 1 && step.gapFrom === step.from + 1 && step.gapTo === step.to - 1; + + if (isNodeMarkupChange) { + newTr.step(step); + map.appendMap(step.getMap()); + return; + } + const inputType = tr.getMeta('inputType'); const isBackspace = inputType === 'deleteContentBackward'; diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js index 5764beddea..9f4b501117 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/replaceAroundStep.test.js @@ -7,7 +7,7 @@ import { replaceAroundStep } from './replaceAroundStep.js'; import { TrackDeleteMarkName, TrackInsertMarkName } from '../constants.js'; import { TrackChangesBasePluginKey } from '../plugins/trackChangesBasePlugin.js'; import { initTestEditor } from '@tests/helpers/helpers.js'; -import { findTextPos } from './testUtils.js'; +import { findTextPos, findFirstParagraphRange } from './testUtils.js'; describe('replaceAroundStep handler', () => { let editor; @@ -53,16 +53,7 @@ describe('replaceAroundStep handler', () => { // We find the first paragraph and create a step that would "unwrap" it // by replacing the paragraph's opening and closing tokens while preserving // the content between them. - let paraStart = null; - let paraEnd = null; - doc.forEach((node, offset) => { - if (paraStart === null && node.type.name === 'paragraph') { - paraStart = offset; - paraEnd = offset + node.nodeSize; - } - }); - - if (paraStart === null) throw new Error('No paragraph found'); + const { paraStart, paraEnd } = findFirstParagraphRange(doc); // Build a transaction with a ReplaceAroundStep. // The step unwraps the paragraph: replaces the paragraph node but keeps its inline content. @@ -148,6 +139,132 @@ describe('replaceAroundStep handler', () => { return newTr; }; + describe('isNodeMarkupChange detection', () => { + it('allows setNodeMarkup-style steps through (structure=true, insert=1, gap=±1)', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create( + { paragraphProperties: { styleId: 'Normal' } }, + schema.nodes.run.create({}, [schema.text('Hello')]), + ), + ); + const state = createState(doc); + + const { paraStart, paraEnd } = findFirstParagraphRange(state.doc); + + const newParagraph = schema.nodes.paragraph.create({ paragraphProperties: { styleId: 'Heading1' } }); + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(newParagraph), 0, 0), + 1, + true, + ); + + const tr = state.tr; + tr.setMeta('inputType', 'insertParagraph'); // non-backspace — would normally be blocked + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // The step should be applied directly (not blocked) + expect(newTr.steps.length).toBe(1); + expect(newTr.steps[0]).toBe(step); + }); + + it('blocks lift-style steps (structure=true, insert=0, gap=±1)', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create({}, schema.nodes.run.create({}, [schema.text('Hello')])), + ); + const state = createState(doc); + + const { paraStart, paraEnd } = findFirstParagraphRange(state.doc); + + // lift-style step: insert=0, structure=true, gap=±1 + const step = new ReplaceAroundStep(paraStart, paraEnd, paraStart + 1, paraEnd - 1, Slice.empty, 0, true); + + const tr = state.tr; + tr.setMeta('inputType', 'insertParagraph'); + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // Should be blocked — not a node markup change + expect(newTr.steps.length).toBe(0); + }); + + it('appends step mapping after applying node markup change', () => { + const doc = schema.nodes.doc.create( + {}, + schema.nodes.paragraph.create( + { paragraphProperties: { styleId: 'Normal' } }, + schema.nodes.run.create({}, [schema.text('Hello')]), + ), + ); + const state = createState(doc); + + const { paraStart, paraEnd } = findFirstParagraphRange(state.doc); + + const newParagraph = schema.nodes.paragraph.create({ paragraphProperties: { styleId: 'Heading1' } }); + const step = new ReplaceAroundStep( + paraStart, + paraEnd, + paraStart + 1, + paraEnd - 1, + new Slice(Fragment.from(newParagraph), 0, 0), + 1, + true, + ); + + const tr = state.tr; + const newTr = state.tr; + const map = new Mapping(); + + replaceAroundStep({ + state, + tr, + step, + newTr, + map, + doc: state.doc, + user, + date, + originalStep: step, + originalStepIndex: 0, + }); + + // map should have been updated + expect(map.maps.length).toBe(1); + }); + }); + describe('non-backspace blocking', () => { it('blocks non-backspace ReplaceAroundStep (no steps added to newTr)', () => { const doc = schema.nodes.doc.create( diff --git a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js index 62d9928ba2..b17ee5b6ef 100644 --- a/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js +++ b/packages/super-editor/src/extensions/track-changes/trackChangesHelpers/testUtils.js @@ -17,3 +17,21 @@ export function findTextPos(docNode, exactText) { }); return found; } + +/** + * Find the start and end positions of the first paragraph node in a document. + * @param {import('prosemirror-model').Node} doc - Document node to search + * @returns {{ paraStart: number, paraEnd: number }} + */ +export function findFirstParagraphRange(doc) { + let paraStart = null; + let paraEnd = null; + doc.forEach((node, offset) => { + if (paraStart === null && node.type.name === 'paragraph') { + paraStart = offset; + paraEnd = offset + node.nodeSize; + } + }); + if (paraStart === null) throw new Error('No paragraph found'); + return { paraStart, paraEnd }; +} diff --git a/tests/behavior/tests/formatting/heading-style-suggesting-mode.spec.ts b/tests/behavior/tests/formatting/heading-style-suggesting-mode.spec.ts new file mode 100644 index 0000000000..37367bdf08 --- /dev/null +++ b/tests/behavior/tests/formatting/heading-style-suggesting-mode.spec.ts @@ -0,0 +1,92 @@ +import { test, expect } from '../../fixtures/superdoc.js'; +import type { Page } from '@playwright/test'; + +async function getFirstParagraphStyleId(page: Page): Promise { + return page.evaluate(() => { + const editor = (window as any).editor; + let result: string | null = null; + editor.state.doc.descendants((node: any) => { + if (node.type.name === 'paragraph') { + result = node.attrs?.paragraphProperties?.styleId ?? null; + return false; + } + return true; + }); + return result; + }); +} + +test.use({ config: { toolbar: 'full', comments: 'on', trackChanges: true } }); + +test.describe('SD-2182 heading style changes in suggesting mode', () => { + test('applying heading style via setStyleById works in suggesting mode', async ({ superdoc }) => { + // Type text in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Select all and apply Heading1 style + await superdoc.selectAll(); + await superdoc.page.evaluate(() => { + (window as any).editor.commands.setStyleById('Heading1'); + }); + await superdoc.waitForStable(); + + const styleId = await getFirstParagraphStyleId(superdoc.page); + expect(styleId).toBe('Heading1'); + }); + + test('toggling heading style with cursor works in suggesting mode', async ({ superdoc }) => { + // Type text in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + + // Switch to suggesting mode (cursor is at end of text — empty selection) + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Apply Heading1 via toggleLinkedStyle with cursor (no selection) + const result = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const style = editor.helpers.linkedStyles.getStyleById('Heading1'); + return editor.commands.toggleLinkedStyle(style); + }); + await superdoc.waitForStable(); + + expect(result).toBe(true); + + const styleId = await getFirstParagraphStyleId(superdoc.page); + expect(styleId).toBe('Heading1'); + }); + + test('toggling heading style off with cursor works in suggesting mode', async ({ superdoc }) => { + // Type text and apply heading in editing mode + await superdoc.type('Hello world'); + await superdoc.waitForStable(); + await superdoc.selectAll(); + await superdoc.page.evaluate(() => { + (window as any).editor.commands.setStyleById('Heading1'); + }); + await superdoc.waitForStable(); + + // Switch to suggesting mode + await superdoc.setDocumentMode('suggesting'); + await superdoc.waitForStable(); + + // Toggle off Heading1 with cursor (no selection) + const result = await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + const style = editor.helpers.linkedStyles.getStyleById('Heading1'); + return editor.commands.toggleLinkedStyle(style); + }); + await superdoc.waitForStable(); + + expect(result).toBe(true); + + const styleId = await getFirstParagraphStyleId(superdoc.page); + expect(styleId).toBeNull(); + }); +});