From 7f1c79df845cabdbc1c97776e707089a21763c05 Mon Sep 17 00:00:00 2001 From: Nick Bernal Date: Sat, 28 Feb 2026 12:41:19 -0800 Subject: [PATCH] fix: splitting run with header adds empty row --- .../src/core/commands/splitBlock.js | 67 +++++++++--- .../src/core/commands/splitBlock.test.js | 94 ++++++++++++++++ .../importMarkdown.integration.test.js | 17 +++ .../helpers/markdown/mdastToProseMirror.ts | 36 ++++++- .../src/extensions/run/commands/split-run.js | 43 +++++++- .../extensions/run/commands/split-run.test.js | 40 +++++++ tests/behavior/harness/main.ts | 38 ++++++- .../enter-page-boundary-line-count.spec.ts | 102 ++++++++++++++++++ .../fixtures/enter-page-boundary.md | 47 ++++++++ 9 files changed, 462 insertions(+), 22 deletions(-) create mode 100644 tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts create mode 100644 tests/behavior/tests/basic-commands/fixtures/enter-page-boundary.md diff --git a/packages/super-editor/src/core/commands/splitBlock.js b/packages/super-editor/src/core/commands/splitBlock.js index e58c0b43fa..40bfee781e 100644 --- a/packages/super-editor/src/core/commands/splitBlock.js +++ b/packages/super-editor/src/core/commands/splitBlock.js @@ -3,6 +3,23 @@ import { canSplit } from 'prosemirror-transform'; import { defaultBlockAt } from '../helpers/defaultBlockAt.js'; import { Attribute } from '../Attribute.js'; +const isHeadingStyleId = (styleId) => typeof styleId === 'string' && /^heading\s*[1-6]$/i.test(styleId.trim()); + +const clearHeadingStyleId = (attrs) => { + if (!attrs || typeof attrs !== 'object') return attrs; + const paragraphProperties = attrs.paragraphProperties; + const styleId = paragraphProperties?.styleId; + if (!isHeadingStyleId(styleId)) return attrs; + + const nextParagraphProperties = { ...paragraphProperties }; + delete nextParagraphProperties.styleId; + + return { + ...attrs, + paragraphProperties: nextParagraphProperties, + }; +}; + const ensureMarks = (state, splittableMarks) => { const marks = state.storedMarks || (state.selection.$to.parentOffset && state.selection.$from.marks()); if (marks) { @@ -62,11 +79,18 @@ export const splitBlock = if (can) { tr.split(tr.mapping.map($from.pos), 1, types); - if (deflt && !atEnd && !$from.parentOffset && $from.parent.type !== deflt) { + if (deflt && !atEnd && !$from.parentOffset) { const first = tr.mapping.map($from.before()); const $first = tr.doc.resolve(first); - if ($from.node(-1).canReplaceWith($first.index(), $first.index() + 1, deflt)) { - tr.setNodeMarkup(tr.mapping.map($from.before()), deflt); + const shouldChangeType = $from.parent.type !== deflt; + const normalizedAttrs = clearHeadingStyleId($from.parent.attrs); + const shouldNormalizeAttrs = normalizedAttrs !== $from.parent.attrs; + + if ( + $from.node(-1).canReplaceWith($first.index(), $first.index() + 1, deflt) && + (shouldChangeType || shouldNormalizeAttrs) + ) { + tr.setNodeMarkup(first, deflt, normalizedAttrs); } } } @@ -79,19 +103,34 @@ export const splitBlock = }; function deleteAttributes(attrs, attrsToRemove) { - const newAttrs = { ...attrs }; - attrsToRemove.forEach((attrName) => { + let nextAttrs = { ...attrs }; + for (const attrName of attrsToRemove) { const parts = attrName.split('.'); if (parts.length === 1) { - delete newAttrs[attrName]; - } else { - let current = newAttrs; - for (let i = 0; i < parts.length - 1; i++) { - if (current[parts[i]] == null) return; - current = current[parts[i]]; + delete nextAttrs[attrName]; + continue; + } + + let source = nextAttrs; + for (let i = 0; i < parts.length - 1; i += 1) { + if (source == null || typeof source !== 'object') { + source = null; + break; } - delete current[parts[parts.length - 1]]; + source = source[parts[i]]; } - }); - return newAttrs; + + if (source == null || typeof source !== 'object') continue; + + let target = nextAttrs; + for (let i = 0; i < parts.length - 1; i += 1) { + const key = parts[i]; + const value = target[key]; + target[key] = { ...value }; + target = target[key]; + } + + delete target[parts[parts.length - 1]]; + } + return nextAttrs; } diff --git a/packages/super-editor/src/core/commands/splitBlock.test.js b/packages/super-editor/src/core/commands/splitBlock.test.js index b9c4e3388c..b332353409 100644 --- a/packages/super-editor/src/core/commands/splitBlock.test.js +++ b/packages/super-editor/src/core/commands/splitBlock.test.js @@ -233,5 +233,99 @@ describe('splitBlock', () => { // ensureMarks should NOT be called expect(mockTr.ensureMarks).not.toHaveBeenCalled(); }); + + it('clears heading style on leading block when splitting at start of heading paragraph', () => { + const canReplaceWith = vi.fn(() => true); + const paragraphType = { name: 'paragraph', isTextblock: true, hasRequiredAttrs: vi.fn(() => false) }; + const parentNode = { + contentMatchAt: vi.fn(() => ({ + edgeCount: 1, + edge: vi.fn(() => ({ type: paragraphType })), + })), + canReplaceWith, + }; + const headingAttrs = { paragraphProperties: { styleId: 'Heading2' } }; + const $from = createMockResolvedPos({ + depth: 1, + parent: { + isBlock: true, + content: { size: 10 }, + type: { name: 'paragraph' }, + inlineContent: true, + attrs: headingAttrs, + }, + parentOffset: 0, + node: vi.fn((depth) => { + if (depth === -1) return parentNode; + return { type: { name: 'paragraph' }, attrs: headingAttrs }; + }), + }); + const $to = createMockResolvedPos({ + pos: 5, + parent: { isBlock: true, content: { size: 10 }, type: { name: 'paragraph' }, inlineContent: true }, + parentOffset: 0, + }); + + mockTr.selection = { $from, $to }; + mockState.selection = mockTr.selection; + mockTr.doc = { + resolve: vi.fn(() => ({ index: vi.fn(() => 0) })), + }; + + const command = splitBlock(); + command({ tr: mockTr, state: mockState, dispatch: () => {}, editor: mockEditor }); + + expect(mockTr.setNodeMarkup).toHaveBeenCalled(); + const attrs = mockTr.setNodeMarkup.mock.calls[0][2]; + expect(attrs.paragraphProperties?.styleId).toBeUndefined(); + }); + + it('does not mutate source attrs when removing nested override attributes', () => { + const paragraphType = { name: 'paragraph', isTextblock: true, hasRequiredAttrs: vi.fn(() => false) }; + const parentNode = { + contentMatchAt: vi.fn(() => ({ + edgeCount: 1, + edge: vi.fn(() => ({ type: paragraphType })), + })), + }; + const sourceAttrs = { + paragraphProperties: { styleId: 'Heading2', keep: true }, + preserve: true, + }; + const $from = createMockResolvedPos({ + depth: 1, + parent: { + isBlock: true, + content: { size: 10 }, + type: { name: 'paragraph' }, + inlineContent: true, + attrs: sourceAttrs, + }, + parentOffset: 0, + node: vi.fn((depth) => { + if (depth === -1) return parentNode; + return { type: { name: 'paragraph' }, attrs: sourceAttrs }; + }), + }); + const $to = createMockResolvedPos({ + pos: 5, + parent: { isBlock: true, content: { size: 10 }, type: { name: 'paragraph' }, inlineContent: true }, + parentOffset: 10, + }); + + mockTr.selection = { $from, $to }; + mockState.selection = mockTr.selection; + mockTr.doc = { + resolve: vi.fn(() => ({ index: vi.fn(() => 0) })), + }; + + const command = splitBlock({ attrsToRemoveOverride: ['paragraphProperties.styleId'] }); + command({ tr: mockTr, state: mockState, dispatch: () => {}, editor: mockEditor }); + + const splitTypes = mockTr.split.mock.calls[0][2]; + expect(splitTypes?.[0]?.attrs?.paragraphProperties?.styleId).toBeUndefined(); + expect(splitTypes?.[0]?.attrs?.paragraphProperties?.keep).toBe(true); + expect(sourceAttrs.paragraphProperties.styleId).toBe('Heading2'); + }); }); }); diff --git a/packages/super-editor/src/core/helpers/importMarkdown.integration.test.js b/packages/super-editor/src/core/helpers/importMarkdown.integration.test.js index 54f7f0ce3c..adbbab8a26 100644 --- a/packages/super-editor/src/core/helpers/importMarkdown.integration.test.js +++ b/packages/super-editor/src/core/helpers/importMarkdown.integration.test.js @@ -52,6 +52,23 @@ function paragraphByText(paragraphs, expectedText) { } describe('markdown to DOCX integration', () => { + it('retains blank lines between root blocks as empty paragraphs', () => { + const markdown = `First paragraph. + + +Second paragraph. + + + +Third paragraph.`; + + const doc = createDocFromMarkdown(markdown, editor); + const paragraphs = collectTopLevelParagraphs(doc); + const texts = paragraphs.map((node) => node.textContent); + + expect(texts).toEqual(['First paragraph.', '', '', 'Second paragraph.', '', '', '', 'Third paragraph.']); + }); + it('converts complete markdown document with headings and lists', () => { const markdown = `# Main Title diff --git a/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts b/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts index 1883c3624c..7447cb3c9d 100644 --- a/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts +++ b/packages/super-editor/src/core/helpers/markdown/mdastToProseMirror.ts @@ -44,7 +44,7 @@ import type { MdastConversionContext, MarkdownDiagnostic } from './types.js'; * suitable for constructing a full doc or a fragment. */ export function convertMdastToBlocks(root: Root, ctx: MdastConversionContext): JsonNode[] { - return flatMapChildren(root, ctx); + return flatMapRootChildrenPreserveBlankLines(root, ctx); } // --------------------------------------------------------------------------- @@ -68,6 +68,26 @@ interface JsonMark { // Block-level converters // --------------------------------------------------------------------------- +function flatMapRootChildrenPreserveBlankLines(root: Root, ctx: MdastConversionContext): JsonNode[] { + const children = root.children ?? []; + const blocks: JsonNode[] = []; + + for (let i = 0; i < children.length; i += 1) { + const child = children[i]; + blocks.push(...convertBlockNode(child, ctx)); + + const next = children[i + 1]; + if (!next) continue; + + const blankLines = countBlankLinesBetweenSiblings(child, next); + for (let j = 0; j < blankLines; j += 1) { + blocks.push(makeParagraph([])); + } + } + + return blocks; +} + function flatMapChildren(parent: MdastNode & { children?: MdastNode[] }, ctx: MdastConversionContext): JsonNode[] { if (!parent.children) return []; const blocks: JsonNode[] = []; @@ -77,6 +97,20 @@ function flatMapChildren(parent: MdastNode & { children?: MdastNode[] }, ctx: Md return blocks; } +function countBlankLinesBetweenSiblings(previous: MdastNode, next: MdastNode): number { + const previousEndLine = previous.position?.end?.line; + const nextStartLine = next.position?.start?.line; + + if (typeof previousEndLine !== 'number' || typeof nextStartLine !== 'number') { + return 0; + } + + // mdast line numbers are 1-based and inclusive: + // previous ends on line A, next starts on line B. + // Lines strictly between them are explicit blank lines in the source. + return Math.max(0, nextStartLine - previousEndLine - 1); +} + function convertBlockNode(node: MdastNode, ctx: MdastConversionContext): JsonNode[] { switch (node.type) { case 'paragraph': diff --git a/packages/super-editor/src/extensions/run/commands/split-run.js b/packages/super-editor/src/extensions/run/commands/split-run.js index 54845346da..e9dfa24dd1 100644 --- a/packages/super-editor/src/extensions/run/commands/split-run.js +++ b/packages/super-editor/src/extensions/run/commands/split-run.js @@ -5,6 +5,25 @@ import { defaultBlockAt } from '@core/helpers/defaultBlockAt.js'; import { resolveRunProperties, encodeMarksFromRPr } from '@core/super-converter/styles.js'; import { extractTableInfo } from '../calculateInlineRunPropertiesPlugin.js'; +function isHeadingStyleId(styleId) { + return typeof styleId === 'string' && /^heading\s*[1-6]$/i.test(styleId.trim()); +} + +function clearHeadingStyleId(attrs) { + if (!attrs || typeof attrs !== 'object') return attrs; + const paragraphProperties = attrs.paragraphProperties; + const styleId = paragraphProperties?.styleId; + if (!isHeadingStyleId(styleId)) return attrs; + + const nextParagraphProperties = { ...paragraphProperties }; + delete nextParagraphProperties.styleId; + + return { + ...attrs, + paragraphProperties: nextParagraphProperties, + }; +} + /** * Splits a run node at the current selection into two paragraphs. * @returns {import('@core/commands/types').Command} @@ -99,11 +118,21 @@ export function splitBlockPatch(state, dispatch, editor) { } if (!can) return false; tr.split(splitPos, types.length, types); - if (!atEnd && atStart && $from.node(splitDepth).type != deflt) { - let first = tr.mapping.map($from.before(splitDepth)), - $first = tr.doc.resolve(first); - if (deflt && $from.node(splitDepth - 1).canReplaceWith($first.index(), $first.index() + 1, deflt)) - tr.setNodeMarkup(tr.mapping.map($from.before(splitDepth)), deflt); + if (!atEnd && atStart) { + const first = tr.mapping.map($from.before(splitDepth)); + const $first = tr.doc.resolve(first); + const sourceNode = $from.node(splitDepth); + const shouldChangeType = sourceNode.type != deflt; + const normalizedAttrs = clearHeadingStyleId(sourceNode.attrs); + const shouldNormalizeAttrs = normalizedAttrs !== sourceNode.attrs; + + if ( + deflt && + $from.node(splitDepth - 1).canReplaceWith($first.index(), $first.index() + 1, deflt) && + (shouldChangeType || shouldNormalizeAttrs) + ) { + tr.setNodeMarkup(first, deflt, normalizedAttrs); + } } applyStyleMarks(state, tr, editor, paragraphAttrs, tableInfo); @@ -195,6 +224,10 @@ function applyStyleMarks(state, tr, editor, paragraphAttrs, tableInfo) { } } +/** + * Splits the current run node into two sibling runs at the cursor position. + * @returns {import('@core/commands/types').Command} + */ export const splitRunAtCursor = () => (props) => { let { state, dispatch, tr } = props; const sel = state.selection; diff --git a/packages/super-editor/src/extensions/run/commands/split-run.test.js b/packages/super-editor/src/extensions/run/commands/split-run.test.js index f1da316f8c..a790bafa71 100644 --- a/packages/super-editor/src/extensions/run/commands/split-run.test.js +++ b/packages/super-editor/src/extensions/run/commands/split-run.test.js @@ -170,6 +170,15 @@ describe('splitRunToParagraph command', () => { expect(handled).toBe(false); }); + + it('returns false for splitRunAtCursor when cursor is not in a run node', () => { + loadDoc(PLAIN_PARAGRAPH_DOC); + updateSelection(1); + + const handled = editor.commands.splitRunAtCursor(); + + expect(handled).toBe(false); + }); }); describe('splitRunToParagraph with style marks', () => { @@ -312,6 +321,37 @@ describe('splitRunToParagraph with style marks', () => { expect(paragraphTexts).toEqual(['Heading', ' Text']); }); + it('clears heading style on the leading empty paragraph when splitting at heading start', () => { + const mockConverter = { + convertedXml: {}, + numbering: {}, + translatedNumbering: {}, + translatedLinkedStyles: {}, + documentGuid: 'test-guid-123', + promoteToGuid: vi.fn(), + }; + + editor.converter = mockConverter; + loadDoc(STYLED_PARAGRAPH_DOC); + + const start = findTextPos('Heading Text'); + expect(start).not.toBeNull(); + updateSelection(start ?? 0); + + const handled = editor.commands.splitRunToParagraph(); + expect(handled).toBe(true); + + const paragraphs = []; + editor.view.state.doc.descendants((node) => { + if (node.type.name === 'paragraph') paragraphs.push(node); + }); + + expect(paragraphs).toHaveLength(2); + expect(paragraphs[0].textContent).toBe(''); + expect(paragraphs[0].attrs?.paragraphProperties?.styleId).toBeUndefined(); + expect(paragraphs[1].attrs?.paragraphProperties?.styleId).toBe('Heading1'); + }); + it('handles missing converter gracefully during split', () => { const mockConverter = { convertedXml: {}, diff --git a/tests/behavior/harness/main.ts b/tests/behavior/harness/main.ts index ed3dc70e55..3a0b7d289c 100644 --- a/tests/behavior/harness/main.ts +++ b/tests/behavior/harness/main.ts @@ -4,12 +4,18 @@ import { SuperDoc } from 'superdoc'; type SuperDocConfig = ConstructorParameters[0]; type SuperDocInstance = InstanceType; type SuperDocReadyPayload = Parameters>[0]; +type OverrideType = 'markdown' | 'html' | 'text'; +type ContentOverrideInput = { + contentOverride?: string; + overrideType?: OverrideType; +}; type HarnessWindow = Window & typeof globalThis & { superdocReady?: boolean; superdoc?: SuperDocInstance; editor?: unknown; + behaviorHarnessInit?: (input?: ContentOverrideInput) => void; }; const harnessWindow = window as HarnessWindow; @@ -21,6 +27,8 @@ const showSelection = params.get('showSelection') === '1'; const toolbar = params.get('toolbar'); const comments = params.get('comments'); const trackChanges = params.get('trackChanges') === '1'; +const contentOverride = params.get('contentOverride') ?? undefined; +const overrideType = (params.get('overrideType') as OverrideType | null) ?? undefined; if (!showCaret) { document.documentElement.style.setProperty('caret-color', 'transparent', 'important'); @@ -28,7 +36,27 @@ if (!showCaret) { let instance: SuperDocInstance | null = null; -function init(file?: File) { +function applyContentOverride(config: SuperDocConfig, input?: ContentOverrideInput) { + if (!input?.contentOverride || !input?.overrideType) return; + + if (input.overrideType === 'markdown') { + config.markdown = input.contentOverride; + return; + } + + if (input.overrideType === 'html') { + config.html = input.contentOverride; + return; + } + + // SuperDoc config does not expose a plain-text bootstrap field directly. + // Use markdown as a lossless text carrier for behavior harness purposes. + if (input.overrideType === 'text') { + config.markdown = input.contentOverride; + } +} + +function init(file?: File, content?: ContentOverrideInput) { if (instance) { instance.destroy(); instance = null; @@ -52,6 +80,8 @@ function init(file?: File) { if (file) { config.document = file; + } else { + applyContentOverride(config, content); } // Toolbar — pass selector string, not DOM element @@ -94,4 +124,8 @@ fileInput.addEventListener('change', (e) => { if (file) init(file); }); -init(); +harnessWindow.behaviorHarnessInit = (input?: ContentOverrideInput) => { + init(undefined, input); +}; + +init(undefined, { contentOverride, overrideType }); diff --git a/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts b/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts new file mode 100644 index 0000000000..ea08a43889 --- /dev/null +++ b/tests/behavior/tests/basic-commands/enter-page-boundary-line-count.spec.ts @@ -0,0 +1,102 @@ +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { test } from '../../fixtures/superdoc.js'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const MARKDOWN_PATH = path.resolve(__dirname, 'fixtures/enter-page-boundary.md'); + +test('enter at heading boundary does not insert extra blank line above heading on next page', async ({ superdoc }) => { + const markdown = fs.readFileSync(MARKDOWN_PATH, 'utf8'); + + await superdoc.page.evaluate((contentOverride) => { + const init = (window as any).behaviorHarnessInit; + if (typeof init !== 'function') { + throw new Error('behaviorHarnessInit is unavailable in behavior harness.'); + } + + init({ + contentOverride, + overrideType: 'markdown', + }); + }, markdown); + + await superdoc.page.waitForFunction(() => (window as any).superdocReady === true, null, { timeout: 30_000 }); + await superdoc.waitForStable(); + + const headingText = 'heading 2 before break'; + const headingStartPos = await superdoc.page.evaluate((needle) => { + const editor = (window as any).editor; + const state = editor?.state; + if (!state?.doc) return null; + + let foundPos: number | null = null; + state.doc.descendants((node: any, pos: number) => { + if (foundPos !== null) return false; + if (node?.isText && typeof node.text === 'string') { + const idx = node.text.indexOf(needle); + if (idx >= 0) { + foundPos = pos + idx; + return false; + } + } + return true; + }); + + return foundPos; + }, headingText); + + if (headingStartPos == null) { + throw new Error(`Unable to find heading text "${headingText}" in document.`); + } + + await superdoc.page.evaluate(() => { + const editor = (window as any).editor; + editor?.commands?.focus?.(); + }); + await superdoc.setTextSelection(headingStartPos); + await superdoc.press('Enter'); + await superdoc.waitForStable(); + + const pageCheck = await superdoc.page.evaluate((needle) => { + const normalize = (value: string | null | undefined) => (value ?? '').replace(/\s+/g, ' ').trim(); + const lines = Array.from(document.querySelectorAll('.superdoc-line')) as HTMLElement[]; + const headingLine = lines.find((line) => normalize(line.textContent).toLowerCase() === needle.toLowerCase()); + if (!headingLine) { + return { ok: false, reason: 'heading line not found in rendered DOM' }; + } + + const page = headingLine.closest('.superdoc-page'); + if (!page) { + return { ok: false, reason: 'heading line is not inside a .superdoc-page element' }; + } + + const bodyLines = Array.from(page.querySelectorAll('.superdoc-line')).filter((line) => { + const element = line as HTMLElement; + return !element.closest('.superdoc-page-header') && !element.closest('.superdoc-page-footer'); + }) as HTMLElement[]; + + const headingLineIndex = bodyLines.findIndex((line) => line === headingLine); + if (headingLineIndex < 0) { + return { ok: false, reason: 'heading line not found among page body lines' }; + } + + const previousLineText = headingLineIndex > 0 ? normalize(bodyLines[headingLineIndex - 1].textContent) : null; + const firstLineText = bodyLines[0] ? normalize(bodyLines[0].textContent) : null; + + return { + ok: headingLineIndex === 0, + headingLineIndex, + previousLineText, + firstLineText, + }; + }, headingText); + + if (!pageCheck.ok) { + throw new Error( + `Expected heading to be first body line on its page. Got index=${String(pageCheck.headingLineIndex)}, prev="${String( + pageCheck.previousLineText, + )}", first="${String(pageCheck.firstLineText)}"${pageCheck.reason ? `, reason=${pageCheck.reason}` : ''}`, + ); + } +}); diff --git a/tests/behavior/tests/basic-commands/fixtures/enter-page-boundary.md b/tests/behavior/tests/basic-commands/fixtures/enter-page-boundary.md new file mode 100644 index 0000000000..30ad0bef3b --- /dev/null +++ b/tests/behavior/tests/basic-commands/fixtures/enter-page-boundary.md @@ -0,0 +1,47 @@ +# Heading 1 + + + + + + + + + + + + + + + +some content + + + + + + + + + + + + + + + +some content + + + + + + + + +## heading 2 before break + +More content directly after heading 2 + + +