From 383cfe4c46cacf4e74d4187d29860177a1bd2872 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Vitor=20Verona=20Biazibetti?= Date: Tue, 17 Feb 2026 18:51:36 -0300 Subject: [PATCH] feat: support column line separators - Extracts 'w:sep' tag according to OOXML spec - Renders one separator for each page column ('w:col' / 'w:num') in DOM painter Closes #2067 --- packages/layout-engine/contracts/src/index.ts | 8 ++ .../layout-bridge/src/incrementalLayout.ts | 25 +++- .../layout-engine/src/index.test.ts | 38 ++++++ .../layout-engine/layout-engine/src/index.ts | 32 ++++- .../layout-engine/src/paginator.ts | 6 +- .../layout-engine/src/section-breaks.d.ts | 2 + .../layout-engine/src/section-breaks.ts | 21 ++- .../layout-engine/src/section-props.test.ts | 32 +++++ .../layout-engine/src/section-props.ts | 10 +- .../painters/dom/src/renderer.ts | 34 +++++ .../pm-adapter/src/index.test.ts | 8 +- .../pm-adapter/src/sections/analysis.test.ts | 30 ++++- .../src/sections/extraction.test.ts | 124 ++++++++++++++++++ .../pm-adapter/src/sections/extraction.ts | 17 ++- .../pm-adapter/src/sections/index.ts | 2 +- .../pm-adapter/src/sections/types.ts | 4 +- .../layout/LayoutOptionParsing.ts | 12 +- 17 files changed, 365 insertions(+), 40 deletions(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index f344c8c285..f6820573eb 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -821,6 +821,7 @@ export type SectionBreakBlock = { count: number; gap: number; equalWidth?: boolean; + withSeparator?: boolean; }; /** * Vertical alignment of content within the section's pages. @@ -1295,6 +1296,7 @@ export type FlowBlock = export type ColumnLayout = { count: number; gap: number; + withSeparator?: boolean; }; /** A measured line within a block, output by the measurer. */ @@ -1499,6 +1501,12 @@ export type Page = { * Sections are 0-indexed, matching the sectionIndex in SectionMetadata. */ sectionIndex?: number; + /** + * Column layout configuration for this page. + * Used by the renderer to draw column separator lines when + * `withSeparator` is set to true. + */ + columns?: ColumnLayout; }; /** A paragraph fragment positioned on a page. */ diff --git a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts index 82433951d4..e3769ae1f8 100644 --- a/packages/layout-engine/layout-bridge/src/incrementalLayout.ts +++ b/packages/layout-engine/layout-bridge/src/incrementalLayout.ts @@ -130,21 +130,23 @@ const normalizeColumnsForFootnotes = (input: ColumnLayout | undefined, contentWi const gap = Math.max(0, input?.gap ?? 0); const totalGap = gap * (count - 1); const width = (contentWidth - totalGap) / count; + const withSeparator = input?.withSeparator ?? false; if (!Number.isFinite(width) || width <= COLUMN_EPSILON) { return { count: 1, gap: 0, width: Math.max(0, contentWidth), + withSeparator, }; } - return { count, gap, width }; + return { count, gap, width, withSeparator }; }; const resolveSectionColumnsByIndex = (options: LayoutOptions, blocks?: FlowBlock[]): Map => { const result = new Map(); - let activeColumns: ColumnLayout = options.columns ?? { count: 1, gap: 0 }; + let activeColumns: ColumnLayout = options.columns ?? { count: 1, gap: 0, withSeparator: false }; if (blocks && blocks.length > 0) { for (const block of blocks) { @@ -153,7 +155,11 @@ const resolveSectionColumnsByIndex = (options: LayoutOptions, blocks?: FlowBlock const sectionIndex = typeof sectionIndexRaw === 'number' && Number.isFinite(sectionIndexRaw) ? sectionIndexRaw : result.size; if (block.columns) { - activeColumns = { count: block.columns.count, gap: block.columns.gap }; + activeColumns = { + count: block.columns.count, + gap: block.columns.gap, + withSeparator: block.columns.withSeparator, + }; } result.set(sectionIndex, { ...activeColumns }); } @@ -183,7 +189,8 @@ const resolvePageColumns = (layout: Layout, options: LayoutOptions, blocks?: Flo ); const contentWidth = pageSize.w - (marginLeft + marginRight); const sectionIndex = page.sectionIndex ?? 0; - const columnsConfig = sectionColumns.get(sectionIndex) ?? options.columns ?? { count: 1, gap: 0 }; + const columnsConfig = sectionColumns.get(sectionIndex) ?? + options.columns ?? { count: 1, gap: 0, withSeparator: false }; const normalized = normalizeColumnsForFootnotes(columnsConfig, contentWidth); result.set(pageIndex, { ...normalized, left: marginLeft, contentWidth }); } @@ -256,7 +263,7 @@ const resolveFootnoteMeasurementWidth = (options: LayoutOptions, blocks?: FlowBl left: normalizeMargin(options.margins?.left, DEFAULT_MARGINS.left), }; let width = pageSize.w - (margins.left + margins.right); - let activeColumns: ColumnLayout = options.columns ?? { count: 1, gap: 0 }; + let activeColumns: ColumnLayout = options.columns ?? { count: 1, gap: 0, withSeparator: false }; let activePageSize = pageSize; let activeMargins = { ...margins }; @@ -277,7 +284,11 @@ const resolveFootnoteMeasurementWidth = (options: LayoutOptions, blocks?: FlowBl left: normalizeMargin(block.margins?.left, activeMargins.left), }; if (block.columns) { - activeColumns = { count: block.columns.count, gap: block.columns.gap }; + activeColumns = { + count: block.columns.count, + gap: block.columns.gap, + withSeparator: block.columns.withSeparator, + }; } const w = resolveColumnWidth(); if (w > 0 && w < width) width = w; @@ -1452,7 +1463,7 @@ export async function incrementalLayout( ); const pageContentWidth = pageSize.w - (marginLeft + marginRight); const fallbackColumns = normalizeColumnsForFootnotes( - options.columns ?? { count: 1, gap: 0 }, + options.columns ?? { count: 1, gap: 0, withSeparator: false }, pageContentWidth, ); const columns = pageColumns.get(pageIndex) ?? { diff --git a/packages/layout-engine/layout-engine/src/index.test.ts b/packages/layout-engine/layout-engine/src/index.test.ts index 2ac733e793..1aca335810 100644 --- a/packages/layout-engine/layout-engine/src/index.test.ts +++ b/packages/layout-engine/layout-engine/src/index.test.ts @@ -212,6 +212,44 @@ describe('layoutDocument', () => { expect(layout.columns).toMatchObject({ count: 2, gap: 20 }); }); + it('sets "page.columns" with separator when column separator is enabled', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: true }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toEqual({ count: 2, gap: 20, withSeparator: true }); + expect(layout.columns).toMatchObject({ count: 2, gap: 20, withSeparator: true }); + }); + + it('does not set "page.columns" on single column layout', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + }; + const layout = layoutDocument([block], [makeMeasure([350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toBeUndefined(); + expect(layout.columns).toBeUndefined(); + }); + + it('sets "page.columns" without separator when column separator is not enabled', () => { + const options: LayoutOptions = { + pageSize: { w: 600, h: 800 }, + margins: { top: 40, right: 40, bottom: 40, left: 40 }, + columns: { count: 2, gap: 20, withSeparator: false }, + }; + const layout = layoutDocument([block], [makeMeasure([350, 350, 350])], options); + + expect(layout.pages).toHaveLength(1); + expect(layout.pages[0].columns).toEqual({ count: 2, gap: 20, withSeparator: false }); + expect(layout.columns).toEqual({ count: 2, gap: 20, withSeparator: false }); + }); + it('applies spacing before and after paragraphs', () => { const spacingBlock: FlowBlock = { kind: 'paragraph', diff --git a/packages/layout-engine/layout-engine/src/index.ts b/packages/layout-engine/layout-engine/src/index.ts index 614b71ff3c..4e065c3b01 100644 --- a/packages/layout-engine/layout-engine/src/index.ts +++ b/packages/layout-engine/layout-engine/src/index.ts @@ -725,7 +725,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Track active and pending columns let activeColumns = options.columns ?? { count: 1, gap: 0 }; - let pendingColumns: { count: number; gap: number } | null = null; + let pendingColumns: { count: number; gap: number; withSeparator?: boolean } | null = null; // Track active and pending orientation let activeOrientation: 'portrait' | 'landscape' | null = null; @@ -822,11 +822,15 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // Update columns - if section has columns, use them; if undefined, reset to single column. // In OOXML, absence of means single column (default). if (block.columns) { - next.activeColumns = { count: block.columns.count, gap: block.columns.gap }; + next.activeColumns = { + count: block.columns.count, + gap: block.columns.gap, + withSeparator: block.columns.withSeparator, + }; next.pendingColumns = null; } else { // No columns specified = reset to single column (OOXML default) - next.activeColumns = { count: 1, gap: 0 }; + next.activeColumns = { count: 1, gap: 0, withSeparator: false }; next.pendingColumns = null; } // Schedule section refs for first section (will be applied on first page creation) @@ -932,7 +936,9 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options } // Helper to get column config: use block.columns if defined, otherwise reset to single column (OOXML default) const getColumnConfig = () => - block.columns ? { count: block.columns.count, gap: block.columns.gap } : { count: 1, gap: 0 }; + block.columns + ? { count: block.columns.count, gap: block.columns.gap, withSeparator: block.columns.withSeparator } + : { count: 1, gap: 0, withSeparator: false }; if (block.attrs?.requirePageBoundary) { next.pendingColumns = getColumnConfig(); @@ -972,6 +978,11 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options if (activeOrientation) { page.orientation = activeOrientation; } + + if (activeColumns.count > 1) { + page.columns = { count: activeColumns.count, gap: activeColumns.gap, withSeparator: activeColumns.withSeparator }; + } + // Set vertical alignment from active section state if (activeVAlign && activeVAlign !== 'top') { page.vAlign = activeVAlign; @@ -1307,7 +1318,10 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options const advanceColumn = paginator.advanceColumn; // Start a new mid-page region with different column configuration - const startMidPageRegion = (state: PageState, newColumns: { count: number; gap: number }): void => { + const startMidPageRegion = ( + state: PageState, + newColumns: { count: number; gap: number; withSeparator?: boolean }, + ): void => { // Record the boundary at current Y position const boundary: ConstraintBoundary = { y: state.cursorY, @@ -2288,7 +2302,10 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options // after processing sections. Page/region-specific column changes are encoded // implicitly via fragment positions. Consumers should not assume this is // a static document-wide value. - columns: activeColumns.count > 1 ? { count: activeColumns.count, gap: activeColumns.gap } : undefined, + columns: + activeColumns.count > 1 + ? { count: activeColumns.count, gap: activeColumns.gap, withSeparator: activeColumns.withSeparator } + : undefined, }; } @@ -2498,12 +2515,14 @@ function normalizeColumns(input: ColumnLayout | undefined, contentWidth: number) const gap = Math.max(0, input?.gap ?? 0); const totalGap = gap * (count - 1); const width = (contentWidth - totalGap) / count; + const withSeparator = input?.withSeparator ?? false; if (width <= COLUMN_EPSILON) { return { count: 1, gap: 0, width: contentWidth, + withSeparator, }; } @@ -2511,6 +2530,7 @@ function normalizeColumns(input: ColumnLayout | undefined, contentWidth: number) count, gap, width, + withSeparator, }; } diff --git a/packages/layout-engine/layout-engine/src/paginator.ts b/packages/layout-engine/layout-engine/src/paginator.ts index 6c15b4bdb8..5419ef65fa 100644 --- a/packages/layout-engine/layout-engine/src/paginator.ts +++ b/packages/layout-engine/layout-engine/src/paginator.ts @@ -4,7 +4,7 @@ export type NormalizedColumns = ColumnLayout & { width: number }; export type ConstraintBoundary = { y: number; - columns: { count: number; gap: number }; + columns: { count: number; gap: number; withSeparator?: boolean }; }; export type PageState = { @@ -27,7 +27,7 @@ export type PaginatorOptions = { getActiveFooterDistance(): number; getActivePageSize(): { w: number; h: number }; getDefaultPageSize(): { w: number; h: number }; - getActiveColumns(): { count: number; gap: number }; + getActiveColumns(): { count: number; gap: number; withSeparator?: boolean }; getCurrentColumns(): NormalizedColumns; createPage(number: number, pageMargins: PageMargins, pageSizeOverride?: { w: number; h: number }): Page; onNewPage?: (state: PageState) => void; @@ -37,7 +37,7 @@ export function createPaginator(opts: PaginatorOptions) { const states: PageState[] = []; const pages: Page[] = []; - const getActiveColumnsForState = (state: PageState): { count: number; gap: number } => { + const getActiveColumnsForState = (state: PageState): { count: number; gap: number; withSeparator?: boolean } => { if (state.activeConstraintIndex >= 0 && state.constraintBoundaries[state.activeConstraintIndex]) { return state.constraintBoundaries[state.activeConstraintIndex].columns; } diff --git a/packages/layout-engine/layout-engine/src/section-breaks.d.ts b/packages/layout-engine/layout-engine/src/section-breaks.d.ts index a4fec8ae80..5a28a08f4c 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.d.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.d.ts @@ -23,10 +23,12 @@ export type SectionState = { activeColumns: { count: number; gap: number; + withSeparator?: boolean; }; pendingColumns: { count: number; gap: number; + withSeparator?: boolean; } | null; activeOrientation: 'portrait' | 'landscape' | null; pendingOrientation: 'portrait' | 'landscape' | null; diff --git a/packages/layout-engine/layout-engine/src/section-breaks.ts b/packages/layout-engine/layout-engine/src/section-breaks.ts index ae462b8336..16b523ae66 100644 --- a/packages/layout-engine/layout-engine/src/section-breaks.ts +++ b/packages/layout-engine/layout-engine/src/section-breaks.ts @@ -15,8 +15,8 @@ export type SectionState = { pendingFooterDistance: number | null; activePageSize: { w: number; h: number }; pendingPageSize: { w: number; h: number } | null; - activeColumns: { count: number; gap: number }; - pendingColumns: { count: number; gap: number } | null; + activeColumns: { count: number; gap: number; withSeparator?: boolean }; + pendingColumns: { count: number; gap: number; withSeparator?: boolean } | null; activeOrientation: 'portrait' | 'landscape' | null; pendingOrientation: 'portrait' | 'landscape' | null; hasAnyPages: boolean; @@ -29,7 +29,10 @@ export type BreakDecision = { }; /** Default single-column configuration per OOXML spec (absence of w:cols element) */ -const SINGLE_COLUMN_DEFAULT: Readonly<{ count: number; gap: number }> = { count: 1, gap: 0 }; +const SINGLE_COLUMN_DEFAULT: Readonly<{ count: number; gap: number; withSeparator?: boolean }> = { + count: 1, + gap: 0, +}; /** * Get the column configuration for a section break. @@ -37,10 +40,16 @@ const SINGLE_COLUMN_DEFAULT: Readonly<{ count: number; gap: number }> = { count: * Per OOXML spec, absence of element means single column layout. * * @param blockColumns - The columns property from the section break block (may be undefined) - * @returns Column configuration with count and gap + * @returns Column configuration with count, gap, and separator presence */ -function getColumnConfig(blockColumns: { count: number; gap: number } | undefined): { count: number; gap: number } { - return blockColumns ? { count: blockColumns.count, gap: blockColumns.gap } : { ...SINGLE_COLUMN_DEFAULT }; +function getColumnConfig(blockColumns: { count: number; gap: number; withSeparator?: boolean } | undefined): { + count: number; + gap: number; + withSeparator?: boolean; +} { + return blockColumns + ? { count: blockColumns.count, gap: blockColumns.gap, withSeparator: blockColumns.withSeparator } + : { ...SINGLE_COLUMN_DEFAULT }; } /** diff --git a/packages/layout-engine/layout-engine/src/section-props.test.ts b/packages/layout-engine/layout-engine/src/section-props.test.ts index 61d486149e..8d3e02b173 100644 --- a/packages/layout-engine/layout-engine/src/section-props.test.ts +++ b/packages/layout-engine/layout-engine/src/section-props.test.ts @@ -61,4 +61,36 @@ describe('computeNextSectionPropsAtBreak', () => { expect(snapshot?.columns).toEqual({ count: 2, gap: 48 }); expect(snapshot?.columns).not.toBe(sourceColumns); }); + + it('propagates withSeparator flag through section property snapshots', () => { + const sourceColumns = { count: 2, gap: 48, withSeparator: true }; + const blocks: FlowBlock[] = [sectionBreak({ id: 'sb-0', columns: sourceColumns })]; + const map = computeNextSectionPropsAtBreak(blocks); + const snapshot = map.get(0); + + expect(snapshot?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + expect(snapshot?.columns).not.toBe(sourceColumns); + }); + + it('omits withSeparator from the snapshot when not set on source block', () => { + const sourceColumns = { count: 2, gap: 48 }; + const blocks: FlowBlock[] = [sectionBreak({ id: 'sb-0', columns: sourceColumns })]; + const map = computeNextSectionPropsAtBreak(blocks); + const snapshot = map.get(0); + + expect(snapshot?.columns).toEqual({ count: 2, gap: 48 }); + expect(snapshot?.columns?.withSeparator).toBeUndefined(); + }); + + it('propagates withSeparator from the next section in lookahead', () => { + const blocks: FlowBlock[] = [ + sectionBreak({ id: 'sb-0', columns: { count: 1, gap: 0 } }), + { kind: 'paragraph', id: 'p-1', runs: [] } as FlowBlock, + sectionBreak({ id: 'sb-2', columns: { count: 2, gap: 48, withSeparator: true } }), + ]; + const map = computeNextSectionPropsAtBreak(blocks); + + expect(map.get(0)?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + expect(map.get(2)?.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + }); }); diff --git a/packages/layout-engine/layout-engine/src/section-props.ts b/packages/layout-engine/layout-engine/src/section-props.ts index 6c8c6c84c3..837fd4da7a 100644 --- a/packages/layout-engine/layout-engine/src/section-props.ts +++ b/packages/layout-engine/layout-engine/src/section-props.ts @@ -16,7 +16,7 @@ import type { FlowBlock, SectionVerticalAlign } from '@superdoc/contracts'; export type SectionProps = { margins?: { header?: number; footer?: number; top?: number; right?: number; bottom?: number; left?: number }; pageSize?: { w: number; h: number }; - columns?: { count: number; gap: number }; + columns?: { count: number; gap: number; withSeparator?: boolean }; orientation?: 'portrait' | 'landscape'; vAlign?: SectionVerticalAlign; }; @@ -59,7 +59,7 @@ const _snapshotSectionProps = (block: FlowBlock): SectionProps | null => { } if (block.columns) { hasProps = true; - props.columns = { count: block.columns.count, gap: block.columns.gap }; + props.columns = { count: block.columns.count, gap: block.columns.gap, withSeparator: block.columns.withSeparator }; } if (block.orientation) { hasProps = true; @@ -135,7 +135,11 @@ export function computeNextSectionPropsAtBreak(blocks: FlowBlock[]): Map { expect(multiColumnBreak).toBeDefined(); expect((multiColumnBreak as FlowBlock).attrs?.requirePageBoundary).toBeUndefined(); // Gap is in pixels (0.5in = 48px @96DPI) - expect((multiColumnBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48 }); + expect((multiColumnBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48, withSeparator: false }); }); it('interprets missing w:num in w:cols as a single-column layout change', () => { @@ -1028,7 +1028,7 @@ describe('toFlowBlocks', () => { const allBreaks = getSectionBreaks(blocks, { includeFirst: true }); const tailBreak = allBreaks.find((b) => b.attrs?.sectionIndex === 0); expect(tailBreak).toBeDefined(); - expect((tailBreak as never).columns).toEqual({ count: 1, gap: 48 }); + expect((tailBreak as never).columns).toEqual({ count: 1, gap: 48, withSeparator: false }); }); describe('Regression tests for section property bug fixes', () => { @@ -1076,7 +1076,7 @@ describe('toFlowBlocks', () => { expect(firstBreak).toBeDefined(); expect(secondBreak).toBeDefined(); // Both have w:space="720" which means single column - expect((firstBreak as FlowBlock).columns).toEqual({ count: 1, gap: 48 }); + expect((firstBreak as FlowBlock).columns).toEqual({ count: 1, gap: 48, withSeparator: false }); expect((secondBreak as FlowBlock).type).toBe('continuous'); // Second sectPr }); @@ -1116,7 +1116,7 @@ describe('toFlowBlocks', () => { // Should emit the section break despite paragraph having content expect(contentBreak).toBeDefined(); - expect((contentBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48 }); + expect((contentBreak as FlowBlock).columns).toEqual({ count: 2, gap: 48, withSeparator: false }); }); it('detects column changes from single to multi to single column', () => { diff --git a/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts b/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts index 0abe5dda82..64a5b58485 100644 --- a/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts +++ b/packages/layout-engine/pm-adapter/src/sections/analysis.test.ts @@ -756,7 +756,7 @@ describe('analysis', () => { footerPx: 50, pageSizePx: { w: 12240, h: 15840 }, orientation: 'landscape', - columnsPx: { count: 2, gap: 100 }, + columnsPx: { count: 2, gap: 100, withSeparator: false }, headerRefs: { default: 'header1' }, footerRefs: { default: 'footer1' }, numbering: { format: 'decimal', start: 1 }, @@ -770,7 +770,7 @@ describe('analysis', () => { margins: { header: 100, footer: 50 }, pageSize: { w: 12240, h: 15840 }, orientation: 'landscape', - columns: { count: 2, gap: 100 }, + columns: { count: 2, gap: 100, withSeparator: false }, headerRefs: { default: 'header1' }, footerRefs: { default: 'footer1' }, numbering: { format: 'decimal', start: 1 }, @@ -960,6 +960,32 @@ describe('analysis', () => { expect(result!.numbering).toEqual({ format: 'decimal', start: 5 }); }); + it('should have column separator flag set to true when present in extracted data', () => { + const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; + + vi.mocked(extractionModule.extractSectionData).mockReturnValue({ + titlePg: false, + columnsPx: { count: 2, gap: 48, withSeparator: true }, + }); + + const result = createFinalSectionFromBodySectPr(bodySectPr, 0, 10, 0); + + expect(result!.columns).toEqual({ count: 2, gap: 48, withSeparator: true }); + }); + + it('should have column separator flag set to false when present as "false" in extracted data', () => { + const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; + + vi.mocked(extractionModule.extractSectionData).mockReturnValue({ + titlePg: false, + columnsPx: { count: 2, gap: 48, withSeparator: false }, + }); + + const result = createFinalSectionFromBodySectPr(bodySectPr, 0, 10, 0); + + expect(result!.columns).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + it('should respect body section type from extracted data', () => { const bodySectPr: SectPrElement = { type: 'element', name: 'w:sectPr' }; diff --git a/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts b/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts index 9dee115ce5..d2e7a02935 100644 --- a/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts +++ b/packages/layout-engine/pm-adapter/src/sections/extraction.test.ts @@ -250,6 +250,7 @@ describe('extraction', () => { expect(result?.columnsPx).toEqual({ count: 2, gap: 48, // 720 twips = 0.5 inches = 48 pixels + withSeparator: false, }); }); @@ -341,6 +342,129 @@ describe('extraction', () => { }); }); + // ==================== extractSectionData - column separator (w:sep) tests ==================== + describe('extractSectionData - column separator', () => { + it('should include separator when w:sep="1"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': '1' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result).not.toBeNull(); + expect(result?.columnsPx).toEqual({ + count: 2, + gap: 48, + withSeparator: true, + }); + }); + + it('should include separator when w:sep="true"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': 'true' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx?.withSeparator).toBe(true); + }); + + it('should include separator when w:sep="on"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': 'on' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx?.withSeparator).toBe(true); + }); + + it('should not include separator when w:sep is absent', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + + it('should not include separator when w:sep="0"', () => { + const para: PMNode = { + type: 'paragraph', + attrs: { + paragraphProperties: { + sectPr: { + type: 'element', + name: 'w:sectPr', + elements: [ + { + name: 'w:cols', + attributes: { 'w:num': '2', 'w:space': '720', 'w:sep': '0' }, + }, + ], + }, + }, + }, + }; + const result = extractSectionData(para); + + expect(result?.columnsPx).toEqual({ count: 2, gap: 48, withSeparator: false }); + }); + }); + // ==================== parseColumnGap Tests ==================== describe('parseColumnGap', () => { it('should return default 0.5 inches when gapTwips is undefined', () => { diff --git a/packages/layout-engine/pm-adapter/src/sections/extraction.ts b/packages/layout-engine/pm-adapter/src/sections/extraction.ts index a956c6583e..2466ce3844 100644 --- a/packages/layout-engine/pm-adapter/src/sections/extraction.ts +++ b/packages/layout-engine/pm-adapter/src/sections/extraction.ts @@ -42,6 +42,15 @@ export function parseColumnGap(gapTwips: string | number | undefined): number { return Number.isFinite(gap) ? gap / TWIPS_PER_INCH : DEFAULT_COLUMN_GAP_INCHES; } +/** + * Parse presence of column separator from w:sep attribute (can be '1', 'true' or 'on'). + * @param rawValue - Raw value from w:sep attribute + * @returns Presence of column separator + */ +export function parseColumnSeparator(rawValue: string | number | undefined): boolean { + return rawValue === '1' || rawValue === 'true' || rawValue === 'on' || rawValue === 1; +} + type SectionType = 'continuous' | 'nextPage' | 'evenPage' | 'oddPage'; type Orientation = 'portrait' | 'landscape'; type HeaderRefType = Partial>; @@ -208,16 +217,20 @@ function extractPageNumbering(elements: SectionElement[]): /** * Extract columns from element. */ -function extractColumns(elements: SectionElement[]): { count: number; gap: number } | undefined { +function extractColumns( + elements: SectionElement[], +): { count: number; gap: number; withSeparator?: boolean } | undefined { const cols = elements.find((el) => el?.name === 'w:cols'); if (!cols?.attributes) return undefined; const count = parseColumnCount(cols.attributes['w:num'] as string | number | undefined); const gapInches = parseColumnGap(cols.attributes['w:space'] as string | number | undefined); + const withSeparator = parseColumnSeparator(cols.attributes['w:sep'] as string | number | undefined); return { count, gap: gapInches * PX_PER_INCH, + withSeparator, }; } @@ -286,7 +299,7 @@ export function extractSectionData(para: PMNode): { type?: SectionType; pageSizePx?: { w: number; h: number }; orientation?: Orientation; - columnsPx?: { count: number; gap: number }; + columnsPx?: { count: number; gap: number; withSeparator?: boolean }; titlePg?: boolean; headerRefs?: HeaderRefType; footerRefs?: HeaderRefType; diff --git a/packages/layout-engine/pm-adapter/src/sections/index.ts b/packages/layout-engine/pm-adapter/src/sections/index.ts index b21f849c6a..64b41423fb 100644 --- a/packages/layout-engine/pm-adapter/src/sections/index.ts +++ b/packages/layout-engine/pm-adapter/src/sections/index.ts @@ -17,7 +17,7 @@ export type { export { SectionType, DEFAULT_PARAGRAPH_SECTION_TYPE, DEFAULT_BODY_SECTION_TYPE } from './types.js'; // Extraction -export { extractSectionData, parseColumnCount, parseColumnGap } from './extraction.js'; +export { extractSectionData, parseColumnCount, parseColumnGap, parseColumnSeparator } from './extraction.js'; // Analysis export { diff --git a/packages/layout-engine/pm-adapter/src/sections/types.ts b/packages/layout-engine/pm-adapter/src/sections/types.ts index 40c8d9fe15..b0e3c12f3d 100644 --- a/packages/layout-engine/pm-adapter/src/sections/types.ts +++ b/packages/layout-engine/pm-adapter/src/sections/types.ts @@ -72,7 +72,7 @@ export type SectionSignature = { orientation?: 'portrait' | 'landscape'; headerRefs?: Partial>; footerRefs?: Partial>; - columnsPx?: { count: number; gap: number }; + columnsPx?: { count: number; gap: number; withSeparator?: boolean }; numbering?: { format?: 'decimal' | 'lowerLetter' | 'upperLetter' | 'lowerRoman' | 'upperRoman' | 'numberInDash'; start?: number; @@ -105,7 +105,7 @@ export interface SectionRange { } | null; pageSize: { w: number; h: number } | null; orientation: 'portrait' | 'landscape' | null; - columns: { count: number; gap: number } | null; + columns: { count: number; gap: number; withSeparator?: boolean } | null; type: SectionType; titlePg: boolean; headerRefs?: Partial>; diff --git a/packages/super-editor/src/core/presentation-editor/layout/LayoutOptionParsing.ts b/packages/super-editor/src/core/presentation-editor/layout/LayoutOptionParsing.ts index 43ac3cd7b2..f9d2b6f467 100644 --- a/packages/super-editor/src/core/presentation-editor/layout/LayoutOptionParsing.ts +++ b/packages/super-editor/src/core/presentation-editor/layout/LayoutOptionParsing.ts @@ -20,20 +20,23 @@ export function inchesToPx(value: unknown): number | undefined { /** * Parses column layout configuration from raw input. * - * Extracts column count and gap spacing from various possible property names, + * Extracts column count, gap spacing, and separator presence from various possible property names, * normalizing to a standard ColumnLayout object. Returns undefined for single-column * layouts (count <= 1) since they don't require special column handling. * * @param raw - Raw column configuration object with properties like count, num, or numberOfColumns - * @returns ColumnLayout with count and gap, or undefined if not multi-column or invalid + * @returns ColumnLayout with count, gap and separator presence, or undefined if not multi-column + * or invalid * * @remarks * - Returns undefined if raw is not an object * - Accepts count from: 'count', 'num', or 'numberOfColumns' properties * - Returns undefined if count <= 1 (single column doesn't need layout) * - Accepts gap from: 'space' or 'gap' properties (converted from inches to pixels) - * - Gap defaults to 0 if not provided or invalid + * - Accepts separator presence from: 'withSeparator' boolean property * - Column count is floored to nearest integer and minimum of 1 + * - Gap defaults to 0 if not provided or invalid + * - Separator presence defaults to false if not provided or not a boolean */ export function parseColumns(raw: unknown): ColumnLayout | undefined { if (!raw || typeof raw !== 'object') return undefined; @@ -44,5 +47,6 @@ export function parseColumns(raw: unknown): ColumnLayout | undefined { } const count = Math.max(1, Math.floor(rawCount)); const gap = inchesToPx(columnSource.space ?? columnSource.gap) ?? 0; - return { count, gap }; + const withSeparator = typeof columnSource.withSeparator === 'boolean' ? columnSource.withSeparator : false; + return { count, gap, withSeparator }; }