From d22b1501bfe347e46afe59d0c95ed52e035ceade Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Thu, 5 Mar 2026 11:26:40 +0530 Subject: [PATCH 01/11] feat: support paragraph between borders (w:pBdr/w:between) (#2074) Flow `w:between` borders through the rendering pipeline so consecutive paragraphs sharing the same border definition display a horizontal separator line between them. - Add `between` to `ParagraphBorders` contract type - Include `between` in `normalizeParagraphBorders` side iteration - Add `bw:` segment to `hashParagraphBorders` in both hash util files - Add `computeBetweenBorderFlags` to pre-compute which fragments need the between border, following the `computeSdtBoundaries` pattern - Render between border as CSS `border-bottom` on decoration overlay - Handle header/footer sections, page splits, and patch invalidation - Add 55+ tests covering edge cases --- packages/layout-engine/contracts/src/index.ts | 1 + .../layout-bridge/src/paragraph-hash-utils.ts | 1 + .../test/paragraph-hash-utils.test.ts | 95 +++- .../painters/dom/src/between-borders.test.ts | 525 ++++++++++++++++++ .../dom/src/paragraph-hash-utils.test.ts | 36 +- .../painters/dom/src/paragraph-hash-utils.ts | 1 + .../painters/dom/src/renderer.ts | 162 +++++- .../pm-adapter/src/attributes/borders.test.ts | 29 + .../pm-adapter/src/attributes/borders.ts | 2 +- 9 files changed, 826 insertions(+), 26 deletions(-) create mode 100644 packages/layout-engine/painters/dom/src/between-borders.test.ts diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 453f393234..ee626ce37f 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -1097,6 +1097,7 @@ export type ParagraphBorders = { right?: ParagraphBorder; bottom?: ParagraphBorder; left?: ParagraphBorder; + between?: ParagraphBorder; }; export type ParagraphShading = { diff --git a/packages/layout-engine/layout-bridge/src/paragraph-hash-utils.ts b/packages/layout-engine/layout-bridge/src/paragraph-hash-utils.ts index 12a6c23df8..b42b84dc3a 100644 --- a/packages/layout-engine/layout-bridge/src/paragraph-hash-utils.ts +++ b/packages/layout-engine/layout-bridge/src/paragraph-hash-utils.ts @@ -38,6 +38,7 @@ export const hashParagraphBorders = (borders: ParagraphBorders): string => { if (borders.right) parts.push(`r:[${hashParagraphBorder(borders.right)}]`); if (borders.bottom) parts.push(`b:[${hashParagraphBorder(borders.bottom)}]`); if (borders.left) parts.push(`l:[${hashParagraphBorder(borders.left)}]`); + if (borders.between) parts.push(`bw:[${hashParagraphBorder(borders.between)}]`); return parts.join(';'); }; diff --git a/packages/layout-engine/layout-bridge/test/paragraph-hash-utils.test.ts b/packages/layout-engine/layout-bridge/test/paragraph-hash-utils.test.ts index 8e3231a42d..172c57086f 100644 --- a/packages/layout-engine/layout-bridge/test/paragraph-hash-utils.test.ts +++ b/packages/layout-engine/layout-bridge/test/paragraph-hash-utils.test.ts @@ -5,8 +5,15 @@ */ import { describe, it, expect } from 'vitest'; -import { hashBorderSpec, hashTableBorderValue, hashTableBorders, hashCellBorders } from '../src/paragraph-hash-utils'; -import type { BorderSpec, TableBorders, CellBorders } from '@superdoc/contracts'; +import { + hashBorderSpec, + hashTableBorderValue, + hashTableBorders, + hashCellBorders, + hashParagraphBorders, + hashParagraphAttrs, +} from '../src/paragraph-hash-utils'; +import type { BorderSpec, TableBorders, CellBorders, ParagraphBorders, ParagraphAttrs } from '@superdoc/contracts'; describe('hashBorderSpec', () => { it('produces deterministic hash for same border properties', () => { @@ -391,3 +398,87 @@ describe('hashCellBorders', () => { expect(hash).toContain('sp:2'); }); }); + +describe('hashParagraphBorders', () => { + it('includes between border in hash with bw: prefix', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 2, color: '#FF0000' }, + }; + const hash = hashParagraphBorders(borders); + expect(hash).toContain('t:['); + expect(hash).toContain('bw:['); + expect(hash).toContain('w:2'); + }); + + it('produces different hashes with and without between', () => { + const with_: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }; + const without_: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + }; + expect(hashParagraphBorders(with_)).not.toBe(hashParagraphBorders(without_)); + }); + + it('does not include bw: when between is undefined', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + bottom: { style: 'solid', width: 1 }, + }; + expect(hashParagraphBorders(borders)).not.toContain('bw:'); + }); + + it('places bw: after l: in hash output', () => { + const borders: ParagraphBorders = { + left: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }; + const hash = hashParagraphBorders(borders); + expect(hash.indexOf('l:[')).toBeLessThan(hash.indexOf('bw:[')); + }); +}); + +describe('hashParagraphAttrs', () => { + it('includes between border in attrs hash via borders', () => { + const attrs: ParagraphAttrs = { + borders: { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 2, color: '#F00' }, + }, + }; + const hash = hashParagraphAttrs(attrs); + expect(hash).toContain('br:'); + expect(hash).toContain('bw:['); + }); + + it('produces different hashes when between border changes', () => { + const attrs1: ParagraphAttrs = { + borders: { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }, + }; + const attrs2: ParagraphAttrs = { + borders: { + top: { style: 'solid', width: 1 }, + between: { style: 'dashed', width: 2 }, + }, + }; + expect(hashParagraphAttrs(attrs1)).not.toBe(hashParagraphAttrs(attrs2)); + }); + + it('produces different hashes when between border is added', () => { + const withoutBetween: ParagraphAttrs = { + borders: { top: { style: 'solid', width: 1 } }, + }; + const withBetween: ParagraphAttrs = { + borders: { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }, + }; + expect(hashParagraphAttrs(withoutBetween)).not.toBe(hashParagraphAttrs(withBetween)); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts new file mode 100644 index 0000000000..9f4ada987c --- /dev/null +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -0,0 +1,525 @@ +import { describe, expect, it } from 'vitest'; +import { + applyParagraphBorderStyles, + getFragmentParagraphBorders, + computeBetweenBorderFlags, + type BlockLookup, +} from './renderer.js'; +import type { + ParagraphBorders, + ParagraphBorder, + ParagraphBlock, + ListBlock, + Fragment, + ParaFragment, + ListItemFragment, + ImageFragment, +} from '@superdoc/contracts'; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +const makeParagraphBlock = (id: string, borders?: ParagraphBorders): ParagraphBlock => ({ + kind: 'paragraph', + id, + runs: [], + attrs: borders ? { borders } : undefined, +}); + +const makeListBlock = (id: string, items: { itemId: string; borders?: ParagraphBorders }[]): ListBlock => ({ + kind: 'list', + id, + listType: 'bullet', + items: items.map((item) => ({ + id: item.itemId, + marker: { text: '•' }, + paragraph: { + kind: 'paragraph', + id: `${id}-p-${item.itemId}`, + runs: [], + attrs: item.borders ? { borders: item.borders } : undefined, + }, + })), +}); + +const stubMeasure = { kind: 'paragraph' as const, lines: [], totalHeight: 0 }; +const stubListMeasure = { + kind: 'list' as const, + items: [], + totalHeight: 0, +}; + +const buildLookup = (entries: { block: ParagraphBlock | ListBlock; measure?: unknown }[]): BlockLookup => { + const map: BlockLookup = new Map(); + for (const e of entries) { + map.set(e.block.id, { + block: e.block, + measure: (e.measure ?? (e.block.kind === 'list' ? stubListMeasure : stubMeasure)) as never, + version: '1', + }); + } + return map; +}; + +const paraFragment = (blockId: string, overrides?: Partial): ParaFragment => ({ + kind: 'para', + blockId, + fromLine: 0, + toLine: 1, + x: 0, + y: 0, + width: 100, + ...overrides, +}); + +const listItemFragment = ( + blockId: string, + itemId: string, + overrides?: Partial, +): ListItemFragment => ({ + kind: 'list-item', + blockId, + itemId, + fromLine: 0, + toLine: 1, + x: 0, + y: 0, + width: 100, + markerWidth: 20, + ...overrides, +}); + +const imageFragment = (blockId: string): ImageFragment => ({ + kind: 'image', + blockId, + x: 0, + y: 0, + width: 100, + height: 100, +}); + +const MATCHING_BORDERS: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 1, color: '#000' }, +}; + +// --------------------------------------------------------------------------- +// applyParagraphBorderStyles +// --------------------------------------------------------------------------- + +describe('applyParagraphBorderStyles — between borders', () => { + const el = () => document.createElement('div'); + + // --- basic activation --- + it('does not apply between border when showBetweenBorder is false', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 2, color: '#FF0000' }, + }; + applyParagraphBorderStyles(e, borders, false); + expect(e.style.getPropertyValue('border-top-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + }); + + it('does not apply between border when showBetweenBorder is undefined', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: 2, color: '#F00' } }); + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + }); + + it('applies between border as bottom border when showBetweenBorder is true', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'dashed', width: 3, color: '#0F0' } }, true); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('dashed'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('3px'); + expect(e.style.getPropertyValue('border-bottom-color')).toBe('#0F0'); + }); + + // --- overwrite semantics --- + it('overwrites normal bottom border when between is active', () => { + const e = el(); + applyParagraphBorderStyles( + e, + { bottom: { style: 'solid', width: 1, color: '#000' }, between: { style: 'double', width: 4, color: '#F00' } }, + true, + ); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('double'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('4px'); + expect(e.style.getPropertyValue('border-bottom-color')).toBe('#F00'); + }); + + it('preserves normal bottom border when showBetweenBorder is false', () => { + const e = el(); + applyParagraphBorderStyles( + e, + { bottom: { style: 'solid', width: 1, color: '#000' }, between: { style: 'double', width: 4, color: '#F00' } }, + false, + ); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('1px'); + expect(e.style.getPropertyValue('border-bottom-color')).toBe('#000'); + }); + + it('applies all four sides plus between when active', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 1, color: '#000' }, + left: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'dashed', width: 2, color: '#F00' }, + }; + applyParagraphBorderStyles(e, borders, true); + expect(e.style.getPropertyValue('border-top-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-right-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('dashed'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('2px'); + }); + + // --- partial / degenerate border specs --- + it('handles between border with none style', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'none', width: 0, color: '#000' } }, true); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('none'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); + }); + + it('defaults width to 1px when between border has no width', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'solid', color: '#F00' } }, true); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('1px'); + }); + + it('defaults color to #000 when between border has no color', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: 2 } }, true); + expect(e.style.getPropertyValue('border-bottom-color')).toBe('#000'); + }); + + it('defaults style to solid when between border has no style', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { width: 2, color: '#F00' } }, true); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); + }); + + it('handles between border with only width', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { width: 5 } }, true); + expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('5px'); + expect(e.style.getPropertyValue('border-bottom-color')).toBe('#000'); + }); + + it('clamps negative width to 0px', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: -3 } }, true); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); + }); + + it('handles width=0 (renders as zero-width border)', () => { + const e = el(); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: 0 } }, true); + expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); + }); + + it('no-ops when showBetweenBorder=true but borders.between is undefined', () => { + const e = el(); + applyParagraphBorderStyles(e, { top: { style: 'solid', width: 1 } }, true); + // Should not crash, and no bottom border should appear + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + }); + + it('no-ops when borders is undefined', () => { + const e = el(); + applyParagraphBorderStyles(e, undefined, true); + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + }); +}); + +// --------------------------------------------------------------------------- +// getFragmentParagraphBorders +// --------------------------------------------------------------------------- + +describe('getFragmentParagraphBorders', () => { + it('returns borders from a paragraph block', () => { + const borders: ParagraphBorders = { top: { style: 'solid', width: 1 } }; + const block = makeParagraphBlock('b1', borders); + const lookup = buildLookup([{ block }]); + expect(getFragmentParagraphBorders(paraFragment('b1'), lookup)).toEqual(borders); + }); + + it('returns undefined for paragraph block without borders', () => { + const block = makeParagraphBlock('b1'); + const lookup = buildLookup([{ block }]); + expect(getFragmentParagraphBorders(paraFragment('b1'), lookup)).toBeUndefined(); + }); + + it('returns borders from a list-item block', () => { + const borders: ParagraphBorders = { between: { style: 'solid', width: 1 } }; + const block = makeListBlock('l1', [{ itemId: 'i1', borders }]); + const lookup = buildLookup([{ block }]); + expect(getFragmentParagraphBorders(listItemFragment('l1', 'i1'), lookup)).toEqual(borders); + }); + + it('returns undefined when list item is not found', () => { + const block = makeListBlock('l1', [{ itemId: 'i1' }]); + const lookup = buildLookup([{ block }]); + expect(getFragmentParagraphBorders(listItemFragment('l1', 'missing'), lookup)).toBeUndefined(); + }); + + it('returns undefined when blockId is not in lookup', () => { + const lookup = buildLookup([]); + expect(getFragmentParagraphBorders(paraFragment('missing'), lookup)).toBeUndefined(); + }); + + it('returns undefined for image fragment', () => { + const block = makeParagraphBlock('b1', { top: { style: 'solid', width: 1 } }); + const lookup = buildLookup([{ block }]); + expect(getFragmentParagraphBorders(imageFragment('b1'), lookup)).toBeUndefined(); + }); + + it('returns undefined for kind/block mismatch (para fragment with list block)', () => { + const block = makeListBlock('l1', [{ itemId: 'i1' }]); + const lookup = buildLookup([{ block }]); + // para fragment referencing a list block + expect(getFragmentParagraphBorders(paraFragment('l1'), lookup)).toBeUndefined(); + }); +}); + +// --------------------------------------------------------------------------- +// computeBetweenBorderFlags +// --------------------------------------------------------------------------- + +describe('computeBetweenBorderFlags', () => { + // --- basic matching --- + it('flags index when two adjacent paragraphs have matching between borders', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.has(0)).toBe(true); + expect(flags.size).toBe(1); + }); + + it('does not flag when between border is not defined', () => { + const noBetween: ParagraphBorders = { top: { style: 'solid', width: 1 } }; + const b1 = makeParagraphBlock('b1', noBetween); + const b2 = makeParagraphBlock('b2', noBetween); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + it('does not flag when border definitions do not match', () => { + const borders1: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 1, color: '#000' }, + }; + const borders2: ParagraphBorders = { + top: { style: 'dashed', width: 2, color: '#F00' }, + between: { style: 'dashed', width: 2, color: '#F00' }, + }; + const b1 = makeParagraphBlock('b1', borders1); + const b2 = makeParagraphBlock('b2', borders2); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + // --- page-split handling --- + it('does not flag when fragment continuesOnNext (page split)', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1', { continuesOnNext: true }), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + it('does not flag when next fragment continuesFromPrev (page split continuation)', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2', { continuesFromPrev: true })]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + // --- same-block deduplication --- + it('does not flag same blockId para fragments (same paragraph split across lines)', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }]); + const fragments: Fragment[] = [ + paraFragment('b1', { fromLine: 0, toLine: 3 }), + paraFragment('b1', { fromLine: 3, toLine: 6 }), + ]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + it('does not flag same blockId + same itemId list-item fragments', () => { + const block = makeListBlock('l1', [{ itemId: 'i1', borders: MATCHING_BORDERS }]); + const lookup = buildLookup([{ block }]); + const fragments: Fragment[] = [ + listItemFragment('l1', 'i1', { fromLine: 0, toLine: 2 }), + listItemFragment('l1', 'i1', { fromLine: 2, toLine: 4 }), + ]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + it('flags different itemIds in same list block', () => { + const block = makeListBlock('l1', [ + { itemId: 'i1', borders: MATCHING_BORDERS }, + { itemId: 'i2', borders: MATCHING_BORDERS }, + ]); + const lookup = buildLookup([{ block }]); + const fragments: Fragment[] = [listItemFragment('l1', 'i1'), listItemFragment('l1', 'i2')]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.has(0)).toBe(true); + }); + + // --- non-paragraph fragments --- + it('skips image fragments', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const imgBlock: ParagraphBlock = { kind: 'paragraph', id: 'img1', runs: [] }; + const lookup = buildLookup([{ block: b1 }, { block: imgBlock }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), imageFragment('img1'), paraFragment('b2')]; + + // Index 0 can't pair with index 1 (image), index 1 is image (skip) + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.has(0)).toBe(false); + // Index 1 is image, skipped — but index 1→2 is image→para, image is skipped + expect(flags.size).toBe(0); + }); + + // --- mixed para + list-item --- + it('flags para followed by list-item with matching borders', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const block = makeListBlock('l1', [{ itemId: 'i1', borders: MATCHING_BORDERS }]); + const lookup = buildLookup([{ block: b1 }, { block }]); + const fragments: Fragment[] = [paraFragment('b1'), listItemFragment('l1', 'i1')]; + + expect(computeBetweenBorderFlags(fragments, lookup).has(0)).toBe(true); + }); + + it('flags list-item followed by para with matching borders', () => { + const block = makeListBlock('l1', [{ itemId: 'i1', borders: MATCHING_BORDERS }]); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block }, { block: b2 }]); + const fragments: Fragment[] = [listItemFragment('l1', 'i1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).has(0)).toBe(true); + }); + + // --- multiple consecutive --- + it('flags all boundaries in a chain of three matching paragraphs', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const b3 = makeParagraphBlock('b3', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }, { block: b3 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2'), paraFragment('b3')]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.has(0)).toBe(true); + expect(flags.has(1)).toBe(true); + expect(flags.size).toBe(2); + }); + + it('breaks chain when middle paragraph has different borders', () => { + const different: ParagraphBorders = { + top: { style: 'dashed', width: 3, color: '#F00' }, + between: { style: 'dashed', width: 3, color: '#F00' }, + }; + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const b2 = makeParagraphBlock('b2', different); + const b3 = makeParagraphBlock('b3', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }, { block: b3 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2'), paraFragment('b3')]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.size).toBe(0); + }); + + // --- asymmetric between definition --- + it('does not flag when only first fragment has between border', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const noB: ParagraphBorders = { top: { style: 'solid', width: 1, color: '#000' } }; + const b2 = makeParagraphBlock('b2', noB); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + it('does not flag when only second fragment has between border', () => { + const noB: ParagraphBorders = { top: { style: 'solid', width: 1, color: '#000' } }; + const b1 = makeParagraphBlock('b1', noB); + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + // --- edge: empty / single fragment --- + it('returns empty set for empty fragment list', () => { + const lookup = buildLookup([]); + expect(computeBetweenBorderFlags([], lookup).size).toBe(0); + }); + + it('returns empty set for single fragment', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }]); + expect(computeBetweenBorderFlags([paraFragment('b1')], lookup).size).toBe(0); + }); + + // --- edge: missing block in lookup --- + it('handles missing blockId in lookup gracefully', () => { + const b2 = makeParagraphBlock('b2', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b2 }]); + // b1 is not in lookup + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + // --- edge: between borders match but other sides differ --- + it('does not flag when between borders match but other sides differ (different group)', () => { + const borders1: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 1, color: '#000' }, + }; + const borders2: ParagraphBorders = { + top: { style: 'solid', width: 2, color: '#F00' }, + between: { style: 'solid', width: 1, color: '#000' }, + }; + const b1 = makeParagraphBlock('b1', borders1); + const b2 = makeParagraphBlock('b2', borders2); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + // Full border hash differs (top is different), so not same border group + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); + + // --- edge: last fragment on page --- + it('last fragment on page is never flagged (no next to pair with)', () => { + const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); + const lookup = buildLookup([{ block: b1 }]); + const fragments: Fragment[] = [paraFragment('b1')]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.has(0)).toBe(false); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts index 2b09d080e7..4ef72e7592 100644 --- a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts +++ b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.test.ts @@ -5,8 +5,9 @@ import { getRunBooleanProp, getRunUnderlineStyle, getRunUnderlineColor, + hashParagraphBorders, } from './paragraph-hash-utils.js'; -import type { Run, TextRun } from '@superdoc/contracts'; +import type { Run, TextRun, ParagraphBorders } from '@superdoc/contracts'; describe('paragraph-hash-utils', () => { describe('getRunStringProp', () => { @@ -193,3 +194,36 @@ describe('paragraph-hash-utils', () => { }); }); }); + +describe('hashParagraphBorders', () => { + it('includes between border in hash', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 2, color: '#FF0000' }, + }; + const hash = hashParagraphBorders(borders); + expect(hash).toContain('bw:['); + expect(hash).toContain('s:solid'); + expect(hash).toContain('w:2'); + expect(hash).toContain('c:#FF0000'); + }); + + it('produces different hashes for borders with and without between', () => { + const withBetween: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }; + const withoutBetween: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + }; + expect(hashParagraphBorders(withBetween)).not.toBe(hashParagraphBorders(withoutBetween)); + }); + + it('does not include between segment when not defined', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + bottom: { style: 'solid', width: 1 }, + }; + expect(hashParagraphBorders(borders)).not.toContain('bw:'); + }); +}); diff --git a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts index 62def18ba8..55870ed7e9 100644 --- a/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts +++ b/packages/layout-engine/painters/dom/src/paragraph-hash-utils.ts @@ -29,6 +29,7 @@ export const hashParagraphBorders = (borders: ParagraphBorders): string => { if (borders.right) parts.push(`r:[${hashParagraphBorder(borders.right)}]`); if (borders.bottom) parts.push(`b:[${hashParagraphBorder(borders.bottom)}]`); if (borders.left) parts.push(`l:[${hashParagraphBorder(borders.left)}]`); + if (borders.between) parts.push(`bw:[${hashParagraphBorder(borders.between)}]`); return parts.join(';'); }; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 6b1b60ca59..344053988a 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -1972,10 +1972,11 @@ export class DomPainter { }; const sdtBoundaries = computeSdtBoundaries(page.fragments, this.blockLookup, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, this.blockLookup); page.fragments.forEach((fragment, index) => { const sdtBoundary = sdtBoundaries.get(index); - el.appendChild(this.renderFragment(fragment, contextBase, sdtBoundary)); + el.appendChild(this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.has(index))); }); this.renderDecorationsForPage(el, page, pageIndex); return el; @@ -2156,22 +2157,27 @@ export class DomPainter { pageIndex, }; + // Compute between-border flags for header/footer paragraph fragments + const betweenBorderFlags = computeBetweenBorderFlags(data.fragments, this.blockLookup); + // Separate behindDoc fragments from normal fragments. // Prefer explicit fragment.behindDoc when present. Keep zIndex===0 as a // compatibility fallback for older layouts that predate explicit metadata. - const behindDocFragments: typeof data.fragments = []; - const normalFragments: typeof data.fragments = []; + // Track original index for between-border flag lookup. + const behindDocFragments: { fragment: (typeof data.fragments)[number]; originalIndex: number }[] = []; + const normalFragments: { fragment: (typeof data.fragments)[number]; originalIndex: number }[] = []; - for (const fragment of data.fragments) { + for (let fi = 0; fi < data.fragments.length; fi += 1) { + const fragment = data.fragments[fi]; let isBehindDoc = false; if (fragment.kind === 'image' || fragment.kind === 'drawing') { isBehindDoc = fragment.behindDoc === true || (fragment.behindDoc == null && 'zIndex' in fragment && fragment.zIndex === 0); } if (isBehindDoc) { - behindDocFragments.push(fragment); + behindDocFragments.push({ fragment, originalIndex: fi }); } else { - normalFragments.push(fragment); + normalFragments.push({ fragment, originalIndex: fi }); } } @@ -2186,8 +2192,8 @@ export class DomPainter { // We can't use z-index: -1 because that goes behind the page's white background. // By inserting at the beginning and using z-index: 0, they render below body content // which also has z-index values but comes later in DOM order. - behindDocFragments.forEach((fragment) => { - const fragEl = this.renderFragment(fragment, context); + behindDocFragments.forEach(({ fragment, originalIndex }) => { + const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.has(originalIndex)); const isPageRelativeVertical = this.isPageRelativeVerticalAnchorFragment(fragment); // Page-relative anchors already carry absolute page Y coordinates. Adding decoration // container offsets would shift them twice and can push header art into body content. @@ -2203,8 +2209,8 @@ export class DomPainter { }); // Render normal fragments in the header/footer container - normalFragments.forEach((fragment) => { - const fragEl = this.renderFragment(fragment, context); + normalFragments.forEach(({ fragment, originalIndex }) => { + const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.has(originalIndex)); const isPageRelativeVertical = this.isPageRelativeVerticalAnchorFragment(fragment); if (isPageRelativeVertical) { // Convert absolute page Y back to decoration-container local coordinates. @@ -2315,6 +2321,7 @@ export class DomPainter { const existing = new Map(state.fragments.map((frag) => [frag.key, frag])); const nextFragments: FragmentDomState[] = []; const sdtBoundaries = computeSdtBoundaries(page.fragments, this.blockLookup, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, this.blockLookup); const contextBase: FragmentRenderContext = { pageNumber: page.number, @@ -2328,10 +2335,12 @@ export class DomPainter { const key = fragmentKey(fragment); const current = existing.get(key); const sdtBoundary = sdtBoundaries.get(index); + const showBetweenBorder = betweenBorderFlags.has(index); if (current) { existing.delete(key); const sdtBoundaryMismatch = shouldRebuildForSdtBoundary(current.element, sdtBoundary); + const betweenBorderMismatch = (current.element.dataset.betweenBorder === 'true') !== showBetweenBorder; // Verify the position mapping is reliable: if mapping the old pmStart doesn't produce // the expected new pmStart, the mapping is degenerate (e.g. full-document paste) and // we must rebuild to get correct span position attributes. @@ -2345,10 +2354,11 @@ export class DomPainter { this.changedBlocks.has(fragment.blockId) || current.signature !== fragmentSignature(fragment, this.blockLookup) || sdtBoundaryMismatch || + betweenBorderMismatch || mappingUnreliable; if (needsRebuild) { - const replacement = this.renderFragment(fragment, contextBase, sdtBoundary); + const replacement = this.renderFragment(fragment, contextBase, sdtBoundary, showBetweenBorder); pageEl.replaceChild(replacement, current.element); current.element = replacement; current.signature = fragmentSignature(fragment, this.blockLookup); @@ -2369,7 +2379,7 @@ export class DomPainter { return; } - const fresh = this.renderFragment(fragment, contextBase, sdtBoundary); + const fresh = this.renderFragment(fragment, contextBase, sdtBoundary, showBetweenBorder); pageEl.insertBefore(fresh, pageEl.children[index] ?? null); nextFragments.push({ key, @@ -2464,9 +2474,10 @@ export class DomPainter { }; const sdtBoundaries = computeSdtBoundaries(page.fragments, this.blockLookup, this.sdtLabelsRendered); + const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, this.blockLookup); const fragmentStates: FragmentDomState[] = page.fragments.map((fragment, index) => { const sdtBoundary = sdtBoundaries.get(index); - const fragmentEl = this.renderFragment(fragment, contextBase, sdtBoundary); + const fragmentEl = this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.has(index)); el.appendChild(fragmentEl); return { key: fragmentKey(fragment), @@ -2512,12 +2523,13 @@ export class DomPainter { fragment: Fragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, + showBetweenBorder?: boolean, ): HTMLElement { if (fragment.kind === 'para') { - return this.renderParagraphFragment(fragment, context, sdtBoundary); + return this.renderParagraphFragment(fragment, context, sdtBoundary, showBetweenBorder); } if (fragment.kind === 'list-item') { - return this.renderListItemFragment(fragment, context, sdtBoundary); + return this.renderListItemFragment(fragment, context, sdtBoundary, showBetweenBorder); } if (fragment.kind === 'image') { return this.renderImageFragment(fragment, context); @@ -2544,6 +2556,7 @@ export class DomPainter { fragment: ParaFragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, + showBetweenBorder?: boolean, ): HTMLElement { try { const lookup = this.blockLookup.get(fragment.blockId); @@ -2599,13 +2612,21 @@ export class DomPainter { // Otherwise, fall back to slicing from the original measure. const lines = fragment.lines ?? measure.lines.slice(fragment.fromLine, fragment.toLine); applyParagraphBlockStyles(fragmentEl, block.attrs); - const { shadingLayer, borderLayer } = createParagraphDecorationLayers(this.doc, fragment.width, block.attrs); + const { shadingLayer, borderLayer } = createParagraphDecorationLayers( + this.doc, + fragment.width, + block.attrs, + showBetweenBorder, + ); if (shadingLayer) { fragmentEl.appendChild(shadingLayer); } if (borderLayer) { fragmentEl.appendChild(borderLayer); } + if (showBetweenBorder) { + fragmentEl.dataset.betweenBorder = 'true'; + } if (block.attrs?.styleId) { fragmentEl.dataset.styleId = block.attrs.styleId; fragmentEl.setAttribute('styleid', block.attrs.styleId); @@ -3001,6 +3022,7 @@ export class DomPainter { fragment: ListItemFragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, + showBetweenBorder?: boolean, ): HTMLElement { try { const lookup = this.blockLookup.get(fragment.blockId); @@ -3092,13 +3114,21 @@ export class DomPainter { // Track B: preserve indent for wordLayout-based lists to show hierarchy const contentAttrs = wordLayout ? item.paragraph.attrs : stripListIndent(item.paragraph.attrs); applyParagraphBlockStyles(contentEl, contentAttrs); - const { shadingLayer, borderLayer } = createParagraphDecorationLayers(this.doc, fragment.width, contentAttrs); + const { shadingLayer, borderLayer } = createParagraphDecorationLayers( + this.doc, + fragment.width, + contentAttrs, + showBetweenBorder, + ); if (shadingLayer) { contentEl.appendChild(shadingLayer); } if (borderLayer) { contentEl.appendChild(borderLayer); } + if (showBetweenBorder) { + fragmentEl.dataset.betweenBorder = 'true'; + } // INTENTIONAL DIVERGENCE: Force list content to left alignment // Microsoft Word DOES justify list paragraphs when alignment is 'justify', // but we intentionally keep lists left-aligned to match user expectations @@ -6280,6 +6310,84 @@ const computeSdtBoundaries = ( return boundaries; }; +/** + * Extracts paragraph borders from a fragment's block data. + * Works for both 'para' and 'list-item' fragment kinds. + */ +export const getFragmentParagraphBorders = ( + fragment: Fragment, + blockLookup: BlockLookup, +): ParagraphAttrs['borders'] | undefined => { + const lookup = blockLookup.get(fragment.blockId); + if (!lookup) return undefined; + + if (fragment.kind === 'para' && lookup.block.kind === 'paragraph') { + return (lookup.block as ParagraphBlock).attrs?.borders; + } + + if (fragment.kind === 'list-item' && lookup.block.kind === 'list') { + const block = lookup.block as ListBlock; + const item = block.items.find((entry) => entry.id === fragment.itemId); + return item?.paragraph.attrs?.borders; + } + + return undefined; +}; + +/** + * Pre-computes which fragment indices should render a between-border. + * A between-border renders as a bottom border on fragment `i` when: + * 1. Fragment `i` is a para or list-item with a `between` border defined + * 2. Fragment `i` does not continue on the next page (not a page-split) + * 3. Fragment `i+1` exists, is also para or list-item, and represents a + * different logical paragraph (different block, or different list item) + * 4. Fragment `i+1` is not a continuation from a previous page + * 5. Fragment `i+1` has matching border definitions (same border group) + */ +export const computeBetweenBorderFlags = (fragments: readonly Fragment[], blockLookup: BlockLookup): Set => { + const flags = new Set(); + + for (let i = 0; i < fragments.length - 1; i += 1) { + const frag = fragments[i]; + + // Only para and list-item fragments can have between borders + if (frag.kind !== 'para' && frag.kind !== 'list-item') continue; + + // Skip page-split continuations — the visual break is a page boundary, not a paragraph boundary + if (frag.continuesOnNext) continue; + + const borders = getFragmentParagraphBorders(frag, blockLookup); + if (!borders?.between) continue; + + const next = fragments[i + 1]; + if (next.kind !== 'para' && next.kind !== 'list-item') continue; + + // Next fragment continuing from a previous page is a split, not a paragraph boundary + if (next.continuesFromPrev) continue; + + // Same paragraph split across lines — not a paragraph boundary + if (next.blockId === frag.blockId && next.kind === 'para') continue; + // Same list item split across lines — not a paragraph boundary + if ( + next.blockId === frag.blockId && + next.kind === 'list-item' && + frag.kind === 'list-item' && + (next as ListItemFragment).itemId === (frag as ListItemFragment).itemId + ) + continue; + + const nextBorders = getFragmentParagraphBorders(next, blockLookup); + if (!nextBorders?.between) continue; + + // Border definitions must match for both fragments to be in the same border group + if (hashParagraphBorders(borders) !== hashParagraphBorders(nextBorders)) continue; + + flags.add(i); + } + + return flags; +}; + const fragmentKey = (fragment: Fragment): string => { if (fragment.kind === 'para') { return `para:${fragment.blockId}:${fragment.fromLine}:${fragment.toLine}`; @@ -7043,6 +7151,7 @@ const createParagraphDecorationLayers = ( doc: Document, fragmentWidth: number, attrs?: ParagraphAttrs, + showBetweenBorder?: boolean, ): { shadingLayer?: HTMLElement; borderLayer?: HTMLElement } => { if (!attrs?.borders && !attrs?.shading) return {}; const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent); @@ -7070,14 +7179,14 @@ const createParagraphDecorationLayers = ( borderLayer.classList.add('superdoc-paragraph-border'); Object.assign(borderLayer.style, baseStyles); borderLayer.style.zIndex = '1'; - applyParagraphBorderStyles(borderLayer, attrs.borders); + applyParagraphBorderStyles(borderLayer, attrs.borders, showBetweenBorder); } return { shadingLayer, borderLayer }; }; -type BorderSide = keyof NonNullable; -const BORDER_SIDES: BorderSide[] = ['top', 'right', 'bottom', 'left']; +type CssBorderSide = 'top' | 'right' | 'bottom' | 'left'; +const BORDER_SIDES: CssBorderSide[] = ['top', 'right', 'bottom', 'left']; /** * Applies paragraph border styles to an HTML element. @@ -7102,7 +7211,11 @@ const BORDER_SIDES: BorderSide[] = ['top', 'right', 'bottom', 'left']; * }); * ``` */ -export const applyParagraphBorderStyles = (element: HTMLElement, borders?: ParagraphAttrs['borders']): void => { +export const applyParagraphBorderStyles = ( + element: HTMLElement, + borders?: ParagraphAttrs['borders'], + showBetweenBorder?: boolean, +): void => { if (!borders) return; element.style.boxSizing = 'border-box'; BORDER_SIDES.forEach((side) => { @@ -7110,9 +7223,14 @@ export const applyParagraphBorderStyles = (element: HTMLElement, borders?: Parag if (!border) return; setBorderSideStyle(element, side, border); }); + // Between border renders as a bottom border, overwriting any normal bottom border + // when the fragment is within a border group (consecutive paragraphs with matching borders) + if (showBetweenBorder && borders.between) { + setBorderSideStyle(element, 'bottom', borders.between); + } }; -const setBorderSideStyle = (element: HTMLElement, side: BorderSide, border: ParagraphBorder): void => { +const setBorderSideStyle = (element: HTMLElement, side: CssBorderSide, border: ParagraphBorder): void => { const cssSide = side; const resolvedStyle = border.style && border.style !== 'none' ? border.style : border.style === 'none' ? 'none' : 'solid'; diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts index c8a99cc4bf..a8d2b2b26c 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts @@ -549,6 +549,35 @@ describe('normalizeParagraphBorders', () => { }; expect(normalizeParagraphBorders(input)).toBeUndefined(); }); + + it('should normalize between border', () => { + const input = { + top: { val: 'single', size: 1, color: 'FF0000' }, + between: { val: 'single', size: 2, color: '0000FF' }, + }; + const result = normalizeParagraphBorders(input); + expect(result?.top).toBeDefined(); + expect(result?.between).toBeDefined(); + expect(result?.between?.style).toBe('solid'); + expect(result?.between?.color).toContain('0000FF'); + }); + + it('should normalize between border alone', () => { + const input = { + between: { val: 'dashed', size: 4, color: '00FF00' }, + }; + const result = normalizeParagraphBorders(input); + expect(result).toBeDefined(); + expect(result?.between).toBeDefined(); + expect(result?.between?.style).toBe('dashed'); + }); + + it('should return undefined when between border is nil', () => { + const input = { + between: { val: 'nil' }, + }; + expect(normalizeParagraphBorders(input)).toBeUndefined(); + }); }); describe('invalid inputs', () => { diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.ts index dfa85ab905..0f0016ce95 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.ts @@ -324,7 +324,7 @@ export function extractCellPadding(cellAttrs: Record): BoxSpaci export const normalizeParagraphBorders = (value: unknown): ParagraphAttrs['borders'] | undefined => { if (!value || typeof value !== 'object') return undefined; const source = value as Record; - const sides: Array<'top' | 'right' | 'bottom' | 'left'> = ['top', 'right', 'bottom', 'left']; + const sides: Array<'top' | 'right' | 'bottom' | 'left' | 'between'> = ['top', 'right', 'bottom', 'left', 'between']; const borders: ParagraphAttrs['borders'] = {}; sides.forEach((side) => { From 227baa4e74c6461e7e6de4f83c3f5b4caff1611c Mon Sep 17 00:00:00 2001 From: G Pardhiv Varma Date: Thu, 5 Mar 2026 22:36:56 +0530 Subject: [PATCH 02/11] test: add incremental update tests for between-border cache invalidation Cover the patch path in renderer.ts where a fragment switches in/out of a between-border group, verifying the DOM node is rebuilt when the betweenBorderMismatch flag flips. --- .../painters/dom/src/between-borders.test.ts | 129 +++++++++++++++++- 1 file changed, 128 insertions(+), 1 deletion(-) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index 9f4ada987c..d740f7453f 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -1,16 +1,20 @@ -import { describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; import { applyParagraphBorderStyles, getFragmentParagraphBorders, computeBetweenBorderFlags, type BlockLookup, } from './renderer.js'; +import { createDomPainter } from './index.js'; import type { ParagraphBorders, ParagraphBorder, ParagraphBlock, ListBlock, Fragment, + FlowBlock, + Layout, + Measure, ParaFragment, ListItemFragment, ImageFragment, @@ -523,3 +527,126 @@ describe('computeBetweenBorderFlags', () => { expect(flags.has(0)).toBe(false); }); }); + +// --------------------------------------------------------------------------- +// Incremental update — between-border cache invalidation +// --------------------------------------------------------------------------- + +describe('DomPainter between-border incremental update', () => { + let mount: HTMLElement; + + beforeEach(() => { + mount = document.createElement('div'); + document.body.appendChild(mount); + }); + + afterEach(() => { + mount.remove(); + }); + + const makeMeasure = (): Measure => ({ + kind: 'paragraph', + lines: [ + { + fromRun: 0, + fromChar: 0, + toRun: 0, + toChar: 0, + width: 100, + ascent: 14, + descent: 2, + lineHeight: 16, + }, + ], + totalHeight: 16, + }); + + const layout: Layout = { + pageSize: { w: 400, h: 500 }, + pages: [ + { + number: 1, + fragments: [ + { kind: 'para', blockId: 'b1', fromLine: 0, toLine: 1, x: 0, y: 0, width: 100 }, + { kind: 'para', blockId: 'b2', fromLine: 0, toLine: 1, x: 0, y: 16, width: 100 }, + ], + }, + ], + }; + + it('rebuilds fragment when between-border flag switches on via setData', () => { + // Initial: no between borders + const b1: FlowBlock = { kind: 'paragraph', id: 'b1', runs: [] }; + const b2: FlowBlock = { kind: 'paragraph', id: 'b2', runs: [] }; + + const painter = createDomPainter({ blocks: [b1, b2], measures: [makeMeasure(), makeMeasure()] }); + painter.paint(layout, mount); + + const page = mount.querySelector('[data-page-number="1"]') as HTMLElement; + const fragsBefore = page.querySelectorAll('[data-block-id]'); + const frag1Before = fragsBefore[0] as HTMLElement; + expect(frag1Before.dataset.betweenBorder).toBeUndefined(); + + // Update: add matching between borders to both blocks + const b1Updated: FlowBlock = { + kind: 'paragraph', + id: 'b1', + runs: [], + attrs: { borders: MATCHING_BORDERS }, + }; + const b2Updated: FlowBlock = { + kind: 'paragraph', + id: 'b2', + runs: [], + attrs: { borders: MATCHING_BORDERS }, + }; + + painter.setData!([b1Updated, b2Updated], [makeMeasure(), makeMeasure()]); + painter.paint(layout, mount); + + const fragsAfter = page.querySelectorAll('[data-block-id]'); + const frag1After = fragsAfter[0] as HTMLElement; + // Fragment was rebuilt (different DOM node) + expect(frag1After).not.toBe(frag1Before); + // Between border is now active + expect(frag1After.dataset.betweenBorder).toBe('true'); + }); + + it('rebuilds fragment when between-border flag switches off via setData', () => { + // Initial: with matching between borders + const b1: FlowBlock = { + kind: 'paragraph', + id: 'b1', + runs: [], + attrs: { borders: MATCHING_BORDERS }, + }; + const b2: FlowBlock = { + kind: 'paragraph', + id: 'b2', + runs: [], + attrs: { borders: MATCHING_BORDERS }, + }; + + const painter = createDomPainter({ blocks: [b1, b2], measures: [makeMeasure(), makeMeasure()] }); + painter.paint(layout, mount); + + const page = mount.querySelector('[data-page-number="1"]') as HTMLElement; + const fragsBefore = page.querySelectorAll('[data-block-id]'); + const frag1Before = fragsBefore[0] as HTMLElement; + expect(frag1Before.dataset.betweenBorder).toBe('true'); + + // Update: remove borders from both blocks + const b1Updated: FlowBlock = { kind: 'paragraph', id: 'b1', runs: [] }; + const b2Updated: FlowBlock = { kind: 'paragraph', id: 'b2', runs: [] }; + + painter.setData!([b1Updated, b2Updated], [makeMeasure(), makeMeasure()]); + painter.paint(layout, mount); + + const fragsAfter = page.querySelectorAll('[data-block-id]'); + const frag1After = fragsAfter[0] as HTMLElement; + // Fragment was rebuilt (different DOM node) + expect(frag1After).not.toBe(frag1Before); + // Between border is no longer active + expect(frag1After.dataset.betweenBorder).toBeUndefined(); + }); +}); From 8f1390c29ab295848ecc7942d8a5a8e8ddfdefff Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 15:37:23 -0300 Subject: [PATCH 03/11] feat: support paragraph between borders (w:pBdr/w:between) Extract paragraph border rendering into feature module at features/paragraph-borders/ with improved between-border support. - BetweenBorderInfo type replaces boolean flag, carrying gap extension and top suppression data per fragment - Border layers extend into spacing gaps for continuous group borders - Top borders suppressed for non-first group members - nil/none between borders no longer form groups - Feature registry maps OOXML elements to rendering modules --- .../painters/dom/src/between-borders.test.ts | 49 +-- .../dom/src/features/feature-registry.ts | 34 +++ .../paragraph-borders/border-layer.ts | 158 ++++++++++ .../paragraph-borders/group-analysis.ts | 183 +++++++++++ .../src/features/paragraph-borders/index.ts | 30 ++ .../src/features/paragraph-borders/types.ts | 16 + .../painters/dom/src/renderer.ts | 286 +++--------------- .../painters/dom/src/table/renderTableCell.ts | 8 +- 8 files changed, 502 insertions(+), 262 deletions(-) create mode 100644 packages/layout-engine/painters/dom/src/features/feature-registry.ts create mode 100644 packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts create mode 100644 packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts create mode 100644 packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts create mode 100644 packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index d740f7453f..062f5e6a94 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -4,7 +4,12 @@ import { getFragmentParagraphBorders, computeBetweenBorderFlags, type BlockLookup, -} from './renderer.js'; + type BetweenBorderInfo, +} from './features/paragraph-borders/index.js'; + +/** Helper to create BetweenBorderInfo for tests that previously passed a boolean. */ +const betweenOn: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 }; +const betweenOff: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: false, gapBelow: 0 }; import { createDomPainter } from './index.js'; import type { ParagraphBorders, @@ -123,7 +128,7 @@ describe('applyParagraphBorderStyles — between borders', () => { top: { style: 'solid', width: 1, color: '#000' }, between: { style: 'solid', width: 2, color: '#FF0000' }, }; - applyParagraphBorderStyles(e, borders, false); + applyParagraphBorderStyles(e, borders, betweenOff); expect(e.style.getPropertyValue('border-top-style')).toBe('solid'); expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); }); @@ -136,7 +141,7 @@ describe('applyParagraphBorderStyles — between borders', () => { it('applies between border as bottom border when showBetweenBorder is true', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'dashed', width: 3, color: '#0F0' } }, true); + applyParagraphBorderStyles(e, { between: { style: 'dashed', width: 3, color: '#0F0' } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe('dashed'); expect(e.style.getPropertyValue('border-bottom-width')).toBe('3px'); expect(e.style.getPropertyValue('border-bottom-color')).toBe('#0F0'); @@ -148,7 +153,7 @@ describe('applyParagraphBorderStyles — between borders', () => { applyParagraphBorderStyles( e, { bottom: { style: 'solid', width: 1, color: '#000' }, between: { style: 'double', width: 4, color: '#F00' } }, - true, + betweenOn, ); expect(e.style.getPropertyValue('border-bottom-style')).toBe('double'); expect(e.style.getPropertyValue('border-bottom-width')).toBe('4px'); @@ -160,7 +165,7 @@ describe('applyParagraphBorderStyles — between borders', () => { applyParagraphBorderStyles( e, { bottom: { style: 'solid', width: 1, color: '#000' }, between: { style: 'double', width: 4, color: '#F00' } }, - false, + betweenOff, ); expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); expect(e.style.getPropertyValue('border-bottom-width')).toBe('1px'); @@ -176,7 +181,7 @@ describe('applyParagraphBorderStyles — between borders', () => { left: { style: 'solid', width: 1, color: '#000' }, between: { style: 'dashed', width: 2, color: '#F00' }, }; - applyParagraphBorderStyles(e, borders, true); + applyParagraphBorderStyles(e, borders, betweenOn); expect(e.style.getPropertyValue('border-top-style')).toBe('solid'); expect(e.style.getPropertyValue('border-right-style')).toBe('solid'); expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); @@ -187,32 +192,32 @@ describe('applyParagraphBorderStyles — between borders', () => { // --- partial / degenerate border specs --- it('handles between border with none style', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'none', width: 0, color: '#000' } }, true); + applyParagraphBorderStyles(e, { between: { style: 'none', width: 0, color: '#000' } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe('none'); expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); }); it('defaults width to 1px when between border has no width', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'solid', color: '#F00' } }, true); + applyParagraphBorderStyles(e, { between: { style: 'solid', color: '#F00' } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-width')).toBe('1px'); }); it('defaults color to #000 when between border has no color', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'solid', width: 2 } }, true); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: 2 } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-color')).toBe('#000'); }); it('defaults style to solid when between border has no style', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { width: 2, color: '#F00' } }, true); + applyParagraphBorderStyles(e, { between: { width: 2, color: '#F00' } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); }); it('handles between border with only width', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { width: 5 } }, true); + applyParagraphBorderStyles(e, { between: { width: 5 } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe('solid'); expect(e.style.getPropertyValue('border-bottom-width')).toBe('5px'); expect(e.style.getPropertyValue('border-bottom-color')).toBe('#000'); @@ -220,26 +225,26 @@ describe('applyParagraphBorderStyles — between borders', () => { it('clamps negative width to 0px', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'solid', width: -3 } }, true); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: -3 } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); }); it('handles width=0 (renders as zero-width border)', () => { const e = el(); - applyParagraphBorderStyles(e, { between: { style: 'solid', width: 0 } }, true); + applyParagraphBorderStyles(e, { between: { style: 'solid', width: 0 } }, betweenOn); expect(e.style.getPropertyValue('border-bottom-width')).toBe('0px'); }); it('no-ops when showBetweenBorder=true but borders.between is undefined', () => { const e = el(); - applyParagraphBorderStyles(e, { top: { style: 'solid', width: 1 } }, true); + applyParagraphBorderStyles(e, { top: { style: 'solid', width: 1 } }, betweenOn); // Should not crash, and no bottom border should appear expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); }); it('no-ops when borders is undefined', () => { const e = el(); - applyParagraphBorderStyles(e, undefined, true); + applyParagraphBorderStyles(e, undefined, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); }); }); @@ -308,7 +313,11 @@ describe('computeBetweenBorderFlags', () => { const flags = computeBetweenBorderFlags(fragments, lookup); expect(flags.has(0)).toBe(true); - expect(flags.size).toBe(1); + expect(flags.get(0)?.showBetweenBorder).toBe(true); + // Fragment 1 also gets an entry (suppressTopBorder) + expect(flags.has(1)).toBe(true); + expect(flags.get(1)?.suppressTopBorder).toBe(true); + expect(flags.size).toBe(2); }); it('does not flag when between border is not defined', () => { @@ -436,8 +445,14 @@ describe('computeBetweenBorderFlags', () => { const flags = computeBetweenBorderFlags(fragments, lookup); expect(flags.has(0)).toBe(true); + expect(flags.get(0)?.showBetweenBorder).toBe(true); expect(flags.has(1)).toBe(true); - expect(flags.size).toBe(2); + expect(flags.get(1)?.showBetweenBorder).toBe(true); + expect(flags.get(1)?.suppressTopBorder).toBe(true); + expect(flags.has(2)).toBe(true); + expect(flags.get(2)?.suppressTopBorder).toBe(true); + expect(flags.get(2)?.showBetweenBorder).toBe(false); + expect(flags.size).toBe(3); }); it('breaks chain when middle paragraph has different borders', () => { diff --git a/packages/layout-engine/painters/dom/src/features/feature-registry.ts b/packages/layout-engine/painters/dom/src/features/feature-registry.ts new file mode 100644 index 0000000000..cf60bad70a --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/feature-registry.ts @@ -0,0 +1,34 @@ +/** + * Rendering Feature Registry + * + * Maps OOXML elements to their rendering feature modules. + * This is the primary lookup table for agents and developers. + * + * To find where an OOXML element renders: search this file. + * To add a new rendering feature: add an entry here first. + * + * Each entry specifies: + * - feature: human-readable feature name (matches folder name) + * - module: import path relative to this file + * - handles: list of OOXML element paths this feature renders + * - spec: ECMA-376 section reference + */ +export const RENDERING_FEATURES = { + // ─── Paragraph Borders ─────────────────────────────────────────── + // @spec ECMA-376 §17.3.1.24 (pBdr) + 'w:pBdr': { + feature: 'paragraph-borders', + module: './paragraph-borders', + handles: ['w:pBdr/w:top', 'w:pBdr/w:bottom', 'w:pBdr/w:left', 'w:pBdr/w:right', 'w:pBdr/w:between', 'w:pBdr/w:bar'], + spec: '§17.3.1.24', + }, + + // ─── Paragraph Shading ─────────────────────────────────────────── + // @spec ECMA-376 §17.3.1.31 (shd) + 'w:shd': { + feature: 'paragraph-borders', // shading shares the border layer module + module: './paragraph-borders', + handles: ['w:shd/@w:fill', 'w:shd/@w:val', 'w:shd/@w:color'], + spec: '§17.3.1.31', + }, +} as const; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts new file mode 100644 index 0000000000..2977b72c18 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -0,0 +1,158 @@ +/** + * Paragraph border DOM layer creation and CSS styling. + * + * Creates absolutely-positioned overlay elements for paragraph borders + * and shading, with indent-aware sizing and between-border group support. + * + * @ooxml w:pPr/w:pBdr — paragraph border properties + * @ooxml w:pPr/w:pBdr/w:top, w:bottom, w:left, w:right — side borders + * @ooxml w:pPr/w:pBdr/w:between — between border (rendered as bottom within groups) + * @ooxml w:pPr/w:shd — paragraph shading (background fill) + * @spec ECMA-376 §17.3.1.24 (pBdr), §17.3.1.31 (shd) + */ +import type { ParagraphAttrs, ParagraphBorder } from '@superdoc/contracts'; +import type { BetweenBorderInfo } from './group-analysis.js'; + +// ─── Border box sizing ───────────────────────────────────────────── + +/** + * Computes the indent-aware bounding box for paragraph border/shading layers. + * Borders hug the paragraph content, inset by left/right/firstLine/hanging indents. + */ +export const getParagraphBorderBox = ( + fragmentWidth: number, + indent?: ParagraphAttrs['indent'], +): { leftInset: number; width: number } => { + const indentLeft = Number.isFinite(indent?.left) ? indent!.left! : 0; + const indentRight = Number.isFinite(indent?.right) ? indent!.right! : 0; + const firstLine = Number.isFinite(indent?.firstLine) ? indent!.firstLine! : 0; + const hanging = Number.isFinite(indent?.hanging) ? indent!.hanging! : 0; + const firstLineOffset = firstLine - hanging; + const minLeftInset = Math.min(indentLeft, indentLeft + firstLineOffset); + const leftInset = Math.max(0, minLeftInset); + const rightInset = Math.max(0, indentRight); + return { + leftInset, + width: Math.max(0, fragmentWidth - leftInset - rightInset), + }; +}; + +// ─── Decoration layer factory ────────────────────────────────────── + +/** + * Builds overlay elements for paragraph shading and borders. + * Returns layers in the order they should be appended (shading below borders). + * + * When `betweenInfo` indicates this fragment is in a border group: + * - The border layer extends downward by `gapBelow` px into the paragraph-spacing + * gap, making left/right borders visually continuous across the group. + * - `suppressTopBorder` hides the top border for non-first group members. + * - `showBetweenBorder` replaces the bottom border with the between definition. + */ +export const createParagraphDecorationLayers = ( + doc: Document, + fragmentWidth: number, + attrs?: ParagraphAttrs, + betweenInfo?: BetweenBorderInfo, +): { shadingLayer?: HTMLElement; borderLayer?: HTMLElement } => { + if (!attrs?.borders && !attrs?.shading) return {}; + + const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent); + + // Extend layers into the spacing gap for continuous group borders + const gapExtension = betweenInfo?.showBetweenBorder ? betweenInfo.gapBelow : 0; + const bottomValue = gapExtension > 0 ? `-${gapExtension}px` : '0px'; + + const baseStyles = { + position: 'absolute', + top: '0px', + bottom: bottomValue, + left: `${borderBox.leftInset}px`, + width: `${borderBox.width}px`, + pointerEvents: 'none', + boxSizing: 'border-box', + } as const; + + let shadingLayer: HTMLElement | undefined; + if (attrs.shading) { + shadingLayer = doc.createElement('div'); + shadingLayer.classList.add('superdoc-paragraph-shading'); + Object.assign(shadingLayer.style, baseStyles); + applyParagraphShadingStyles(shadingLayer, attrs.shading); + } + + let borderLayer: HTMLElement | undefined; + if (attrs.borders) { + borderLayer = doc.createElement('div'); + borderLayer.classList.add('superdoc-paragraph-border'); + Object.assign(borderLayer.style, baseStyles); + borderLayer.style.zIndex = '1'; + applyParagraphBorderStyles(borderLayer, attrs.borders, betweenInfo); + } + + return { shadingLayer, borderLayer }; +}; + +// ─── Border CSS application ──────────────────────────────────────── + +type CssBorderSide = 'top' | 'right' | 'bottom' | 'left'; +const BORDER_SIDES: CssBorderSide[] = ['top', 'right', 'bottom', 'left']; + +/** + * Applies paragraph border styles to an HTML element. + * + * Handles between-border groups: + * - `suppressTopBorder`: skips top border for non-first group members + * - `showBetweenBorder`: replaces bottom with the between border definition + */ +export const applyParagraphBorderStyles = ( + element: HTMLElement, + borders?: ParagraphAttrs['borders'], + betweenInfo?: BetweenBorderInfo, +): void => { + if (!borders) return; + const showBetweenBorder = betweenInfo?.showBetweenBorder ?? false; + const suppressTopBorder = betweenInfo?.suppressTopBorder ?? false; + + element.style.boxSizing = 'border-box'; + BORDER_SIDES.forEach((side) => { + if (side === 'top' && suppressTopBorder) return; + const border = borders[side]; + if (!border) return; + setBorderSideStyle(element, side, border); + }); + + // Between border renders as a bottom border, overwriting any normal bottom border + // when the fragment is within a border group (consecutive paragraphs with matching borders) + if (showBetweenBorder && borders.between) { + setBorderSideStyle(element, 'bottom', borders.between); + } +}; + +const setBorderSideStyle = (element: HTMLElement, side: CssBorderSide, border: ParagraphBorder): void => { + const resolvedStyle = + border.style && border.style !== 'none' ? border.style : border.style === 'none' ? 'none' : 'solid'; + if (resolvedStyle === 'none') { + element.style.setProperty(`border-${side}-style`, 'none'); + element.style.setProperty(`border-${side}-width`, '0px'); + if (border.color) { + element.style.setProperty(`border-${side}-color`, border.color); + } + return; + } + + const width = border.width != null ? Math.max(0, border.width) : undefined; + element.style.setProperty(`border-${side}-style`, resolvedStyle); + element.style.setProperty(`border-${side}-width`, `${width ?? 1}px`); + element.style.setProperty(`border-${side}-color`, border.color ?? '#000'); +}; + +// ─── Shading CSS application ─────────────────────────────────────── + +/** + * Applies paragraph shading (background color) to an HTML element. + */ +export const applyParagraphShadingStyles = (element: HTMLElement, shading?: ParagraphAttrs['shading']): void => { + if (!shading?.fill) return; + element.style.backgroundColor = shading.fill; +}; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts new file mode 100644 index 0000000000..e5f363fd5e --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts @@ -0,0 +1,183 @@ +/** + * Paragraph border group analysis. + * + * Determines which consecutive fragments form "border groups" — runs of + * paragraphs with identical border definitions that Word renders as a + * single continuous bordered box with between-borders separating them. + * + * @ooxml w:pPr/w:pBdr/w:between — between border for grouped paragraphs + * @spec ECMA-376 §17.3.1.24 (pBdr) + */ +import type { + Fragment, + ListItemFragment, + ListBlock, + ListMeasure, + ParagraphBlock, + ParagraphAttrs, +} from '@superdoc/contracts'; +import type { BlockLookup } from './types.js'; +import { hashParagraphBorders } from '../../paragraph-hash-utils.js'; + +/** + * Per-fragment rendering info for between-border groups. + * + * - showBetweenBorder: replace bottom border with the between definition + * - suppressTopBorder: hide this fragment's top border (covered by previous fragment's extension) + * - gapBelow: px to extend the border layer downward into the paragraph-spacing gap + */ +export type BetweenBorderInfo = { + showBetweenBorder: boolean; + suppressTopBorder: boolean; + gapBelow: number; +}; + +/** + * Extracts the paragraph borders for a fragment, looking up the block data. + * Handles both paragraph and list-item fragments. + */ +export const getFragmentParagraphBorders = ( + fragment: Fragment, + blockLookup: BlockLookup, +): ParagraphAttrs['borders'] | undefined => { + const lookup = blockLookup.get(fragment.blockId); + if (!lookup) return undefined; + + if (fragment.kind === 'para' && lookup.block.kind === 'paragraph') { + return (lookup.block as ParagraphBlock).attrs?.borders; + } + + if (fragment.kind === 'list-item' && lookup.block.kind === 'list') { + const block = lookup.block as ListBlock; + const item = block.items.find((entry) => entry.id === fragment.itemId); + return item?.paragraph.attrs?.borders; + } + + return undefined; +}; + +/** + * Computes the height of a fragment from its measured line heights. + * Used to calculate the spacing gap between consecutive fragments. + */ +const getFragmentHeight = (fragment: Fragment, blockLookup: BlockLookup): number => { + if (fragment.kind === 'table' || fragment.kind === 'image' || fragment.kind === 'drawing') { + return fragment.height; + } + + const lookup = blockLookup.get(fragment.blockId); + if (!lookup) return 0; + + if (fragment.kind === 'para' && lookup.measure.kind === 'paragraph') { + const lines = fragment.lines ?? lookup.measure.lines.slice(fragment.fromLine, fragment.toLine); + let totalHeight = 0; + for (const line of lines) { + totalHeight += line.lineHeight ?? 0; + } + return totalHeight; + } + + if (fragment.kind === 'list-item' && lookup.measure.kind === 'list') { + const listMeasure = lookup.measure as ListMeasure; + const item = listMeasure.items.find((it) => it.itemId === fragment.itemId); + if (!item) return 0; + const lines = item.paragraph.lines.slice(fragment.fromLine, fragment.toLine); + let totalHeight = 0; + for (const line of lines) { + totalHeight += line.lineHeight ?? 0; + } + return totalHeight; + } + + return 0; +}; + +/** + * Whether a between border is effectively absent (nil/none or missing). + */ +const isBetweenBorderNone = (borders: ParagraphAttrs['borders']): boolean => { + if (!borders?.between) return true; + return borders.between.style === 'none'; +}; + +/** + * Pre-computes per-fragment between-border rendering info for a page. + * + * Two fragments (i, i+1) form a border group pair when: + * 1. Both are para or list-item (not table/image/drawing) + * 2. Neither is a page-split continuation + * 3. They represent different logical paragraphs + * 4. Both have a `between` border defined (not nil/none) + * 5. Their full border definitions match (same border group) + * + * For each pair, the first fragment gets: + * - showBetweenBorder: true — bottom border replaced with between definition + * - gapBelow: px distance to extend border layer into spacing gap + * + * The second fragment gets: + * - suppressTopBorder: true — the previous fragment's extension covers the boundary + * + * Middle fragments in a chain of 3+ get both flags. + */ +export const computeBetweenBorderFlags = ( + fragments: readonly Fragment[], + blockLookup: BlockLookup, +): Map => { + // Phase 1: determine which consecutive pairs form between-border groups + const pairFlags = new Set(); + + for (let i = 0; i < fragments.length - 1; i += 1) { + const frag = fragments[i]; + if (frag.kind !== 'para' && frag.kind !== 'list-item') continue; + if (frag.continuesOnNext) continue; + + const borders = getFragmentParagraphBorders(frag, blockLookup); + if (isBetweenBorderNone(borders)) continue; + + const next = fragments[i + 1]; + if (next.kind !== 'para' && next.kind !== 'list-item') continue; + if (next.continuesFromPrev) continue; + if (next.blockId === frag.blockId && next.kind === 'para') continue; + if ( + next.blockId === frag.blockId && + next.kind === 'list-item' && + frag.kind === 'list-item' && + (next as ListItemFragment).itemId === (frag as ListItemFragment).itemId + ) + continue; + + const nextBorders = getFragmentParagraphBorders(next, blockLookup); + if (isBetweenBorderNone(nextBorders)) continue; + if (hashParagraphBorders(borders!) !== hashParagraphBorders(nextBorders!)) continue; + + pairFlags.add(i); + } + + // Phase 2: build per-fragment info with gap distances and top suppression + const result = new Map(); + + for (const i of pairFlags) { + const frag = fragments[i]; + const next = fragments[i + 1]; + const fragHeight = getFragmentHeight(frag, blockLookup); + const gapBelow = Math.max(0, next.y - (frag.y + fragHeight)); + + // Current fragment: show between border on bottom, extend into gap + if (!result.has(i)) { + result.set(i, { showBetweenBorder: true, suppressTopBorder: false, gapBelow }); + } else { + const existing = result.get(i)!; + existing.showBetweenBorder = true; + existing.gapBelow = gapBelow; + } + + // Next fragment: suppress top border (previous fragment's extended layer covers boundary) + if (!result.has(i + 1)) { + result.set(i + 1, { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 }); + } else { + result.get(i + 1)!.suppressTopBorder = true; + } + } + + return result; +}; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts new file mode 100644 index 0000000000..b7ad4cef3e --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts @@ -0,0 +1,30 @@ +/** + * Paragraph Borders — rendering feature module + * + * Handles all aspects of paragraph border rendering: + * - Border group detection (between-border analysis) + * - Border/shading DOM layer creation + * - CSS border style application + * + * @ooxml w:pPr/w:pBdr — paragraph border properties + * @ooxml w:pPr/w:pBdr/w:top, w:bottom, w:left, w:right — side borders + * @ooxml w:pPr/w:pBdr/w:between — between border for grouped paragraphs + * @ooxml w:pPr/w:pBdr/w:bar — bar border (vertical line) + * @ooxml w:pPr/w:shd — paragraph shading + * @spec ECMA-376 §17.3.1.24 (pBdr), §17.3.1.31 (shd) + */ + +// Group analysis +export { computeBetweenBorderFlags, getFragmentParagraphBorders } from './group-analysis.js'; +export type { BetweenBorderInfo } from './group-analysis.js'; + +// DOM layers and CSS +export { + createParagraphDecorationLayers, + applyParagraphBorderStyles, + applyParagraphShadingStyles, + getParagraphBorderBox, +} from './border-layer.js'; + +// Shared types +export type { BlockLookup, BlockLookupEntry } from './types.js'; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts new file mode 100644 index 0000000000..272d6c5255 --- /dev/null +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts @@ -0,0 +1,16 @@ +/** + * Shared types for paragraph border rendering features. + */ +import type { FlowBlock, Measure } from '@superdoc/contracts'; + +/** + * Entry in the block lookup map. Re-exported here to avoid + * a direct import from the monolithic renderer. + */ +export type BlockLookupEntry = { + block: FlowBlock; + measure: Measure; + version: string; +}; + +export type BlockLookup = Map; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index 344053988a..fcd0a4dc3d 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -38,6 +38,9 @@ import type { ShapeTextContent, SolidFillWithAlpha, TableAttrs, + ListItemFragment, + ListBlock, + ListMeasure, TableBlock, TableCellAttrs, TableFragment, @@ -93,6 +96,15 @@ import { type SdtBoundaryOptions, } from './utils/sdt-helpers.js'; import { SdtGroupedHover } from './utils/sdt-hover.js'; +import { + computeBetweenBorderFlags, + getFragmentParagraphBorders, + createParagraphDecorationLayers, + applyParagraphBorderStyles, + applyParagraphShadingStyles, + getParagraphBorderBox, + type BetweenBorderInfo, +} from './features/paragraph-borders/index.js'; /** * Minimal type for WordParagraphLayoutOutput marker data used in rendering. @@ -1976,7 +1988,7 @@ export class DomPainter { page.fragments.forEach((fragment, index) => { const sdtBoundary = sdtBoundaries.get(index); - el.appendChild(this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.has(index))); + el.appendChild(this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.get(index))); }); this.renderDecorationsForPage(el, page, pageIndex); return el; @@ -2193,7 +2205,7 @@ export class DomPainter { // By inserting at the beginning and using z-index: 0, they render below body content // which also has z-index values but comes later in DOM order. behindDocFragments.forEach(({ fragment, originalIndex }) => { - const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.has(originalIndex)); + const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.get(originalIndex)); const isPageRelativeVertical = this.isPageRelativeVerticalAnchorFragment(fragment); // Page-relative anchors already carry absolute page Y coordinates. Adding decoration // container offsets would shift them twice and can push header art into body content. @@ -2210,7 +2222,7 @@ export class DomPainter { // Render normal fragments in the header/footer container normalFragments.forEach(({ fragment, originalIndex }) => { - const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.has(originalIndex)); + const fragEl = this.renderFragment(fragment, context, undefined, betweenBorderFlags.get(originalIndex)); const isPageRelativeVertical = this.isPageRelativeVerticalAnchorFragment(fragment); if (isPageRelativeVertical) { // Convert absolute page Y back to decoration-container local coordinates. @@ -2335,12 +2347,16 @@ export class DomPainter { const key = fragmentKey(fragment); const current = existing.get(key); const sdtBoundary = sdtBoundaries.get(index); - const showBetweenBorder = betweenBorderFlags.has(index); + const betweenInfo = betweenBorderFlags.get(index); if (current) { existing.delete(key); const sdtBoundaryMismatch = shouldRebuildForSdtBoundary(current.element, sdtBoundary); - const betweenBorderMismatch = (current.element.dataset.betweenBorder === 'true') !== showBetweenBorder; + // Detect mismatch in any between-border property + const betweenBorderMismatch = + (current.element.dataset.betweenBorder === 'true') !== (betweenInfo?.showBetweenBorder ?? false) || + (current.element.dataset.suppressTopBorder === 'true') !== (betweenInfo?.suppressTopBorder ?? false) || + (current.element.dataset.gapBelow ?? '') !== (betweenInfo?.gapBelow ? String(betweenInfo.gapBelow) : ''); // Verify the position mapping is reliable: if mapping the old pmStart doesn't produce // the expected new pmStart, the mapping is degenerate (e.g. full-document paste) and // we must rebuild to get correct span position attributes. @@ -2358,7 +2374,7 @@ export class DomPainter { mappingUnreliable; if (needsRebuild) { - const replacement = this.renderFragment(fragment, contextBase, sdtBoundary, showBetweenBorder); + const replacement = this.renderFragment(fragment, contextBase, sdtBoundary, betweenInfo); pageEl.replaceChild(replacement, current.element); current.element = replacement; current.signature = fragmentSignature(fragment, this.blockLookup); @@ -2379,7 +2395,7 @@ export class DomPainter { return; } - const fresh = this.renderFragment(fragment, contextBase, sdtBoundary, showBetweenBorder); + const fresh = this.renderFragment(fragment, contextBase, sdtBoundary, betweenInfo); pageEl.insertBefore(fresh, pageEl.children[index] ?? null); nextFragments.push({ key, @@ -2477,7 +2493,7 @@ export class DomPainter { const betweenBorderFlags = computeBetweenBorderFlags(page.fragments, this.blockLookup); const fragmentStates: FragmentDomState[] = page.fragments.map((fragment, index) => { const sdtBoundary = sdtBoundaries.get(index); - const fragmentEl = this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.has(index)); + const fragmentEl = this.renderFragment(fragment, contextBase, sdtBoundary, betweenBorderFlags.get(index)); el.appendChild(fragmentEl); return { key: fragmentKey(fragment), @@ -2523,13 +2539,13 @@ export class DomPainter { fragment: Fragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, - showBetweenBorder?: boolean, + betweenInfo?: BetweenBorderInfo, ): HTMLElement { if (fragment.kind === 'para') { - return this.renderParagraphFragment(fragment, context, sdtBoundary, showBetweenBorder); + return this.renderParagraphFragment(fragment, context, sdtBoundary, betweenInfo); } if (fragment.kind === 'list-item') { - return this.renderListItemFragment(fragment, context, sdtBoundary, showBetweenBorder); + return this.renderListItemFragment(fragment, context, sdtBoundary, betweenInfo); } if (fragment.kind === 'image') { return this.renderImageFragment(fragment, context); @@ -2556,7 +2572,7 @@ export class DomPainter { fragment: ParaFragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, - showBetweenBorder?: boolean, + betweenInfo?: BetweenBorderInfo, ): HTMLElement { try { const lookup = this.blockLookup.get(fragment.blockId); @@ -2616,7 +2632,7 @@ export class DomPainter { this.doc, fragment.width, block.attrs, - showBetweenBorder, + betweenInfo, ); if (shadingLayer) { fragmentEl.appendChild(shadingLayer); @@ -2624,8 +2640,10 @@ export class DomPainter { if (borderLayer) { fragmentEl.appendChild(borderLayer); } - if (showBetweenBorder) { - fragmentEl.dataset.betweenBorder = 'true'; + if (betweenInfo) { + if (betweenInfo.showBetweenBorder) fragmentEl.dataset.betweenBorder = 'true'; + if (betweenInfo.suppressTopBorder) fragmentEl.dataset.suppressTopBorder = 'true'; + if (betweenInfo.gapBelow) fragmentEl.dataset.gapBelow = String(betweenInfo.gapBelow); } if (block.attrs?.styleId) { fragmentEl.dataset.styleId = block.attrs.styleId; @@ -3022,7 +3040,7 @@ export class DomPainter { fragment: ListItemFragment, context: FragmentRenderContext, sdtBoundary?: SdtBoundaryOptions, - showBetweenBorder?: boolean, + betweenInfo?: BetweenBorderInfo, ): HTMLElement { try { const lookup = this.blockLookup.get(fragment.blockId); @@ -3118,7 +3136,7 @@ export class DomPainter { this.doc, fragment.width, contentAttrs, - showBetweenBorder, + betweenInfo, ); if (shadingLayer) { contentEl.appendChild(shadingLayer); @@ -3126,8 +3144,10 @@ export class DomPainter { if (borderLayer) { contentEl.appendChild(borderLayer); } - if (showBetweenBorder) { - fragmentEl.dataset.betweenBorder = 'true'; + if (betweenInfo) { + if (betweenInfo.showBetweenBorder) fragmentEl.dataset.betweenBorder = 'true'; + if (betweenInfo.suppressTopBorder) fragmentEl.dataset.suppressTopBorder = 'true'; + if (betweenInfo.gapBelow) fragmentEl.dataset.gapBelow = String(betweenInfo.gapBelow); } // INTENTIONAL DIVERGENCE: Force list content to left alignment // Microsoft Word DOES justify list paragraphs when alignment is 'justify', @@ -6310,83 +6330,7 @@ const computeSdtBoundaries = ( return boundaries; }; -/** - * Extracts paragraph borders from a fragment's block data. - * Works for both 'para' and 'list-item' fragment kinds. - */ -export const getFragmentParagraphBorders = ( - fragment: Fragment, - blockLookup: BlockLookup, -): ParagraphAttrs['borders'] | undefined => { - const lookup = blockLookup.get(fragment.blockId); - if (!lookup) return undefined; - - if (fragment.kind === 'para' && lookup.block.kind === 'paragraph') { - return (lookup.block as ParagraphBlock).attrs?.borders; - } - - if (fragment.kind === 'list-item' && lookup.block.kind === 'list') { - const block = lookup.block as ListBlock; - const item = block.items.find((entry) => entry.id === fragment.itemId); - return item?.paragraph.attrs?.borders; - } - - return undefined; -}; - -/** - * Pre-computes which fragment indices should render a between-border. - * A between-border renders as a bottom border on fragment `i` when: - * 1. Fragment `i` is a para or list-item with a `between` border defined - * 2. Fragment `i` does not continue on the next page (not a page-split) - * 3. Fragment `i+1` exists, is also para or list-item, and represents a - * different logical paragraph (different block, or different list item) - * 4. Fragment `i+1` is not a continuation from a previous page - * 5. Fragment `i+1` has matching border definitions (same border group) - */ -export const computeBetweenBorderFlags = (fragments: readonly Fragment[], blockLookup: BlockLookup): Set => { - const flags = new Set(); - - for (let i = 0; i < fragments.length - 1; i += 1) { - const frag = fragments[i]; - - // Only para and list-item fragments can have between borders - if (frag.kind !== 'para' && frag.kind !== 'list-item') continue; - - // Skip page-split continuations — the visual break is a page boundary, not a paragraph boundary - if (frag.continuesOnNext) continue; - - const borders = getFragmentParagraphBorders(frag, blockLookup); - if (!borders?.between) continue; - - const next = fragments[i + 1]; - if (next.kind !== 'para' && next.kind !== 'list-item') continue; - - // Next fragment continuing from a previous page is a split, not a paragraph boundary - if (next.continuesFromPrev) continue; - - // Same paragraph split across lines — not a paragraph boundary - if (next.blockId === frag.blockId && next.kind === 'para') continue; - // Same list item split across lines — not a paragraph boundary - if ( - next.blockId === frag.blockId && - next.kind === 'list-item' && - frag.kind === 'list-item' && - (next as ListItemFragment).itemId === (frag as ListItemFragment).itemId - ) - continue; - - const nextBorders = getFragmentParagraphBorders(next, blockLookup); - if (!nextBorders?.between) continue; - - // Border definitions must match for both fragments to be in the same border group - if (hashParagraphBorders(borders) !== hashParagraphBorders(nextBorders)) continue; - - flags.add(i); - } - - return flags; -}; +// getFragmentParagraphBorders, computeBetweenBorderFlags — moved to features/paragraph-borders/ const fragmentKey = (fragment: Fragment): string => { if (fragment.kind === 'para') { @@ -7125,129 +7069,8 @@ const applyParagraphBlockStyles = (element: HTMLElement, attrs?: ParagraphAttrs) } }; -const getParagraphBorderBox = ( - fragmentWidth: number, - indent?: ParagraphAttrs['indent'], -): { leftInset: number; width: number } => { - const indentLeft = Number.isFinite(indent?.left) ? indent!.left! : 0; - const indentRight = Number.isFinite(indent?.right) ? indent!.right! : 0; - const firstLine = Number.isFinite(indent?.firstLine) ? indent!.firstLine! : 0; - const hanging = Number.isFinite(indent?.hanging) ? indent!.hanging! : 0; - const firstLineOffset = firstLine - hanging; - const minLeftInset = Math.min(indentLeft, indentLeft + firstLineOffset); - const leftInset = Math.max(0, minLeftInset); - const rightInset = Math.max(0, indentRight); - return { - leftInset, - width: Math.max(0, fragmentWidth - leftInset - rightInset), - }; -}; - -/** - * Builds overlay elements for paragraph shading and borders with indent-aware sizing. - * Returns layers in the order they should be appended (shading below borders). - */ -const createParagraphDecorationLayers = ( - doc: Document, - fragmentWidth: number, - attrs?: ParagraphAttrs, - showBetweenBorder?: boolean, -): { shadingLayer?: HTMLElement; borderLayer?: HTMLElement } => { - if (!attrs?.borders && !attrs?.shading) return {}; - const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent); - const baseStyles = { - position: 'absolute', - top: '0px', - bottom: '0px', - left: `${borderBox.leftInset}px`, - width: `${borderBox.width}px`, - pointerEvents: 'none', - boxSizing: 'border-box', - } as const; - - let shadingLayer: HTMLElement | undefined; - if (attrs.shading) { - shadingLayer = doc.createElement('div'); - shadingLayer.classList.add('superdoc-paragraph-shading'); - Object.assign(shadingLayer.style, baseStyles); - applyParagraphShadingStyles(shadingLayer, attrs.shading); - } - - let borderLayer: HTMLElement | undefined; - if (attrs.borders) { - borderLayer = doc.createElement('div'); - borderLayer.classList.add('superdoc-paragraph-border'); - Object.assign(borderLayer.style, baseStyles); - borderLayer.style.zIndex = '1'; - applyParagraphBorderStyles(borderLayer, attrs.borders, showBetweenBorder); - } - - return { shadingLayer, borderLayer }; -}; - -type CssBorderSide = 'top' | 'right' | 'bottom' | 'left'; -const BORDER_SIDES: CssBorderSide[] = ['top', 'right', 'bottom', 'left']; - -/** - * Applies paragraph border styles to an HTML element. - * Sets CSS border properties (width, style, color) for each side specified in the borders object. - * - * @param {HTMLElement} element - The HTML element to apply border styles to - * @param {ParagraphAttrs['borders']} borders - Optional borders object containing border definitions for top, right, bottom, and left sides - * - * @remarks - * - Sets box-sizing to 'border-box' to ensure borders are included in element dimensions - * - Each side's border is processed independently - only specified sides receive border styles - * - Border width defaults to 1px if not specified, and negative widths are clamped to 0px - * - Border style defaults to 'solid' if not specified or if style is not 'none' - * - Border color defaults to '#000' (black) if not specified - * - Border style 'none' is handled specially to ensure no visible border - * - * @example - * ```typescript - * applyParagraphBorderStyles(paraElement, { - * top: { width: 2, style: 'solid', color: '#FF0000' }, - * bottom: { width: 1, style: 'dashed', color: '#0000FF' } - * }); - * ``` - */ -export const applyParagraphBorderStyles = ( - element: HTMLElement, - borders?: ParagraphAttrs['borders'], - showBetweenBorder?: boolean, -): void => { - if (!borders) return; - element.style.boxSizing = 'border-box'; - BORDER_SIDES.forEach((side) => { - const border = borders[side]; - if (!border) return; - setBorderSideStyle(element, side, border); - }); - // Between border renders as a bottom border, overwriting any normal bottom border - // when the fragment is within a border group (consecutive paragraphs with matching borders) - if (showBetweenBorder && borders.between) { - setBorderSideStyle(element, 'bottom', borders.between); - } -}; - -const setBorderSideStyle = (element: HTMLElement, side: CssBorderSide, border: ParagraphBorder): void => { - const cssSide = side; - const resolvedStyle = - border.style && border.style !== 'none' ? border.style : border.style === 'none' ? 'none' : 'solid'; - if (resolvedStyle === 'none') { - element.style.setProperty(`border-${cssSide}-style`, 'none'); - element.style.setProperty(`border-${cssSide}-width`, '0px'); - if (border.color) { - element.style.setProperty(`border-${cssSide}-color`, border.color); - } - return; - } - - const width = border.width != null ? Math.max(0, border.width) : undefined; - element.style.setProperty(`border-${cssSide}-style`, resolvedStyle); - element.style.setProperty(`border-${cssSide}-width`, `${width ?? 1}px`); - element.style.setProperty(`border-${cssSide}-color`, border.color ?? '#000'); -}; +// getParagraphBorderBox, createParagraphDecorationLayers, applyParagraphBorderStyles, +// setBorderSideStyle, applyParagraphShadingStyles — moved to features/paragraph-borders/ const stripListIndent = (attrs?: ParagraphAttrs): ParagraphAttrs | undefined => { if (!attrs?.indent || attrs.indent.left == null) { @@ -7262,30 +7085,7 @@ const stripListIndent = (attrs?: ParagraphAttrs): ParagraphAttrs | undefined => }; }; -/** - * Applies paragraph shading (background color) styles to an HTML element. - * Sets the CSS background-color property based on the shading fill value. - * - * @param {HTMLElement} element - The HTML element to apply shading styles to - * @param {ParagraphAttrs['shading']} shading - Optional shading object containing fill color definition - * - * @remarks - * - Only applies background color if shading.fill is defined - * - Currently only supports the `fill` property for solid color backgrounds - * - Theme-based shading properties (themeColor, themeTint, themeShade) are not yet supported - * - The fill value should be a valid CSS color string (hex, rgb, named color, etc.) - * - * @example - * ```typescript - * applyParagraphShadingStyles(paraElement, { - * fill: '#FFFF00' - * }); - * ``` - */ -export const applyParagraphShadingStyles = (element: HTMLElement, shading?: ParagraphAttrs['shading']): void => { - if (!shading?.fill) return; - element.style.backgroundColor = shading.fill; -}; +// applyParagraphShadingStyles — moved to features/paragraph-borders/border-layer.ts /** * Extracts and slices text runs that belong to a specific line within a paragraph block. diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index 5739336238..7708d45a62 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -22,8 +22,12 @@ import { effectiveTableCellSpacing } from '@superdoc/contracts'; import { toCssFontFamily } from '@superdoc/font-utils'; import { rescaleColumnWidths } from '@superdoc/layout-engine'; import { normalizeZIndex } from '@superdoc/pm-adapter/utilities.js'; -import type { BlockLookup, FragmentRenderContext } from '../renderer.js'; -import { applyParagraphBorderStyles, applyParagraphShadingStyles } from '../renderer.js'; +import type { FragmentRenderContext } from '../renderer.js'; +import { + applyParagraphBorderStyles, + applyParagraphShadingStyles, + type BlockLookup, +} from '../features/paragraph-borders/index.js'; import { applySquareWrapExclusionsToLines } from '../utils/anchor-helpers'; import { applyImageClipPath } from '../utils/image-clip-path.js'; import { From 80eff63ca32a1ca0e94fa20ca702e3a651073f0b Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 15:58:13 -0300 Subject: [PATCH 04/11] docs: add DomPainter feature module guidelines to CLAUDE.md --- CLAUDE.md | 5 ++-- packages/layout-engine/CLAUDE.md | 41 ++++++++++++++++++++++++++++++-- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 017c26d647..fcde499aa9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -59,7 +59,8 @@ tests/visual/ Visual regression tests (Playwright + R2 baselines) |------|----------| | React integration | `packages/react/src/SuperDocEditor.tsx` | | Editing features | `super-editor/src/extensions/` | -| Presentation mode visuals | `layout-engine/painters/dom/src/renderer.ts` | +| Presentation mode visuals | `layout-engine/painters/dom/src/features/feature-registry.ts` → feature module | +| Rendering orchestration | `layout-engine/painters/dom/src/renderer.ts` | | DOCX import/export | `super-editor/src/core/super-converter/` | | Style resolution | `layout-engine/style-engine/` | | Main entry point (Vue) | `superdoc/src/SuperDoc.vue` | @@ -79,7 +80,7 @@ tests/visual/ Visual regression tests (Playwright + R2 baselines) ## When to Modify Which System -- **Visual rendering**: Modify `pm-adapter/` (to feed data) and/or `painters/dom/` (to render it) +- **Visual rendering**: Check `painters/dom/src/features/feature-registry.ts` to find the feature module, then modify it. If no module exists yet, create one (see layout-engine CLAUDE.md). Feed data via `pm-adapter/` - **Style resolution**: Modify `style-engine/` — called by pm-adapter during conversion - **Editing commands/behavior**: Modify `super-editor/src/extensions/` - **State bridging**: Modify `PresentationEditor.ts` diff --git a/packages/layout-engine/CLAUDE.md b/packages/layout-engine/CLAUDE.md index 92e53ff4da..e06131cd5a 100644 --- a/packages/layout-engine/CLAUDE.md +++ b/packages/layout-engine/CLAUDE.md @@ -29,7 +29,8 @@ It does NOT do layout logic - that's in `layout-engine/`. | Task | Where to look | |------|---------------| -| Change how element renders | `painters/dom/src/renderer.ts` | +| Change how OOXML element renders | `painters/dom/src/features/feature-registry.ts` → feature module | +| Change rendering orchestration | `painters/dom/src/renderer.ts` | | Change pagination/layout | `layout-engine/src/index.ts` | | Add new block type | `pm-adapter/src/converters/` + `painters/dom/` | | Change style resolution | `style-engine/` | @@ -73,9 +74,45 @@ setActiveComment(commentId) → increments layoutVersion → clears pageIndexToS Maps block IDs to entries for change detection. Only changed pages re-render. See `blockIdToEntry` in `painters/dom/src/renderer.ts`. +## DomPainter Feature Modules (`painters/dom/src/features/`) + +Rendering logic for specific OOXML features is extracted into **feature modules** under `painters/dom/src/features//`. This keeps `renderer.ts` focused on orchestration while feature-specific logic lives in discoverable, self-contained modules. + +### How to find where an OOXML element renders + +1. **Search `features/feature-registry.ts`** — maps OOXML element names (e.g., `w:pBdr`, `w:shd`) to their feature module +2. Each entry has: `feature` (folder name), `module` (import path), `handles` (OOXML elements), `spec` (ECMA-376 section) +3. Open the feature's `index.ts` for its public API and `@ooxml`/`@spec` annotations + +### Adding a new rendering feature + +1. **Add a registry entry** in `features/feature-registry.ts` first — this is the source of truth +2. **Create the feature folder** at `features//`: + - `index.ts` — barrel exports with `@ooxml` and `@spec` JSDoc annotations + - Split logic into focused files (e.g., `group-analysis.ts`, `border-layer.ts`) + - `types.ts` — shared types if needed +3. **Import from the feature module** in `renderer.ts` — renderer calls feature functions, features don't import from renderer +4. **Remove extracted code** from `renderer.ts` — don't leave dead copies +5. **Update imports** in any other files that used the old renderer exports (e.g., `table/renderTableCell.ts`) + +### Feature module conventions + +- **Folder name** = human-readable feature name, matches the `feature` field in the registry +- **`@ooxml` annotations** on `index.ts` list every OOXML element the module handles +- **`@spec` annotations** reference the ECMA-376 section numbers +- **No circular imports** — features import from `@superdoc/contracts`, not from `renderer.ts` +- **Co-locate tests** as `.test.ts` next to the source + +### Existing feature modules + +| Feature | OOXML elements | Folder | +|---------|---------------|--------| +| Paragraph borders & shading | `w:pBdr`, `w:shd` | `features/paragraph-borders/` | + ## Entry Points -- `painters/dom/src/renderer.ts` - Main DOM rendering (large file) +- `painters/dom/src/renderer.ts` - Main DOM rendering orchestrator (large file — feature logic is being extracted to `features/`) +- `painters/dom/src/features/feature-registry.ts` - OOXML element → feature module lookup - `painters/dom/src/styles.ts` - CSS class definitions - `layout-bridge/src/layout-pipeline.ts` - Pipeline orchestration - `pm-adapter/src/internal.ts` - PM → FlowBlock conversion From 4793f9895b2c18ce4272e60d6b2a3e46a83bf383 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 16:34:49 -0300 Subject: [PATCH 05/11] refactor: deduplicate helpers and add column break guard - Extract getFragmentHeight from renderer.ts, reuse from group-analysis - Extract stampBetweenBorderDataset helper for consistent dataset stamping - Move BlockLookup types to single source of truth in types.ts - Add column break guard (frag.x !== next.x) to prevent cross-column grouping - Add tests for suppressTopBorder, gap extension, and column breaks --- .../painters/dom/src/between-borders.test.ts | 88 +++++++++++++++++++ .../paragraph-borders/border-layer.ts | 13 +++ .../paragraph-borders/group-analysis.ts | 5 +- .../src/features/paragraph-borders/index.ts | 3 +- .../src/features/paragraph-borders/types.ts | 9 +- .../painters/dom/src/renderer.ts | 69 ++------------- 6 files changed, 118 insertions(+), 69 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index 062f5e6a94..cdc590981a 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -3,6 +3,7 @@ import { applyParagraphBorderStyles, getFragmentParagraphBorders, computeBetweenBorderFlags, + createParagraphDecorationLayers, type BlockLookup, type BetweenBorderInfo, } from './features/paragraph-borders/index.js'; @@ -247,6 +248,65 @@ describe('applyParagraphBorderStyles — between borders', () => { applyParagraphBorderStyles(e, undefined, betweenOn); expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); }); + + // --- suppressTopBorder --- + it('skips top border when suppressTopBorder is true', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'solid', width: 2, color: '#F00' }, + left: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + }; + const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 }; + applyParagraphBorderStyles(e, borders, info); + expect(e.style.getPropertyValue('border-top-style')).toBe(''); + expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-right-style')).toBe('solid'); + }); + + it('applies top border normally when suppressTopBorder is false', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'dashed', width: 3, color: '#0F0' }, + }; + applyParagraphBorderStyles(e, borders, betweenOff); + expect(e.style.getPropertyValue('border-top-style')).toBe('dashed'); + expect(e.style.getPropertyValue('border-top-width')).toBe('3px'); + }); +}); + +// --------------------------------------------------------------------------- +// createParagraphDecorationLayers — gap extension +// --------------------------------------------------------------------------- + +describe('createParagraphDecorationLayers — gap extension', () => { + it('sets bottom to negative gapBelow when showBetweenBorder is true', () => { + const attrs = { borders: { top: { style: 'solid' as const, width: 1 } }, shading: { fill: '#EEE' } }; + const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 8 }; + const { borderLayer, shadingLayer } = createParagraphDecorationLayers(document, 400, attrs, info); + expect(borderLayer!.style.bottom).toBe('-8px'); + expect(shadingLayer!.style.bottom).toBe('-8px'); + }); + + it('sets bottom to 0px when gapBelow is 0', () => { + const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } }; + const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 }; + const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info); + expect(borderLayer!.style.bottom).toBe('0px'); + }); + + it('sets bottom to 0px when betweenInfo is undefined', () => { + const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } }; + const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs); + expect(borderLayer!.style.bottom).toBe('0px'); + }); + + it('sets bottom to 0px when showBetweenBorder is false even with gapBelow', () => { + const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } }; + const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 12 }; + const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info); + expect(borderLayer!.style.bottom).toBe('0px'); + }); }); // --------------------------------------------------------------------------- @@ -541,6 +601,34 @@ describe('computeBetweenBorderFlags', () => { const flags = computeBetweenBorderFlags(fragments, lookup); expect(flags.has(0)).toBe(false); }); + + it('does not group fragments in different columns (different x positions)', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }; + const b1 = makeParagraphBlock('b1', borders); + const b2 = makeParagraphBlock('b2', borders); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1', { y: 100, x: 0 }), paraFragment('b2', { y: 0, x: 300 })]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.size).toBe(0); + }); + + it('still groups fragments in the same column (same x positions)', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + between: { style: 'solid', width: 1 }, + }; + const b1 = makeParagraphBlock('b1', borders); + const b2 = makeParagraphBlock('b2', borders); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1', { y: 0, x: 50 }), paraFragment('b2', { y: 16, x: 50 })]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.size).toBe(2); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts index 2977b72c18..f8894d3dc2 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -147,6 +147,19 @@ const setBorderSideStyle = (element: HTMLElement, side: CssBorderSide, border: P element.style.setProperty(`border-${side}-color`, border.color ?? '#000'); }; +// ─── Dataset stamping ───────────────────────────────────────────── + +/** + * Stamps between-border info onto an element's dataset for debugging + * and incremental update cache invalidation. + */ +export const stampBetweenBorderDataset = (element: HTMLElement, betweenInfo?: BetweenBorderInfo): void => { + if (!betweenInfo) return; + if (betweenInfo.showBetweenBorder) element.dataset.betweenBorder = 'true'; + if (betweenInfo.suppressTopBorder) element.dataset.suppressTopBorder = 'true'; + if (betweenInfo.gapBelow) element.dataset.gapBelow = String(betweenInfo.gapBelow); +}; + // ─── Shading CSS application ─────────────────────────────────────── /** diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts index e5f363fd5e..47a4a4ca2d 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts @@ -60,7 +60,7 @@ export const getFragmentParagraphBorders = ( * Computes the height of a fragment from its measured line heights. * Used to calculate the spacing gap between consecutive fragments. */ -const getFragmentHeight = (fragment: Fragment, blockLookup: BlockLookup): number => { +export const getFragmentHeight = (fragment: Fragment, blockLookup: BlockLookup): number => { if (fragment.kind === 'table' || fragment.kind === 'image' || fragment.kind === 'drawing') { return fragment.height; } @@ -150,6 +150,9 @@ export const computeBetweenBorderFlags = ( if (isBetweenBorderNone(nextBorders)) continue; if (hashParagraphBorders(borders!) !== hashParagraphBorders(nextBorders!)) continue; + // Skip fragments in different columns (different x positions) + if (frag.x !== next.x) continue; + pairFlags.add(i); } diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts index b7ad4cef3e..b20f29ac89 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts @@ -15,7 +15,7 @@ */ // Group analysis -export { computeBetweenBorderFlags, getFragmentParagraphBorders } from './group-analysis.js'; +export { computeBetweenBorderFlags, getFragmentParagraphBorders, getFragmentHeight } from './group-analysis.js'; export type { BetweenBorderInfo } from './group-analysis.js'; // DOM layers and CSS @@ -24,6 +24,7 @@ export { applyParagraphBorderStyles, applyParagraphShadingStyles, getParagraphBorderBox, + stampBetweenBorderDataset, } from './border-layer.js'; // Shared types diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts index 272d6c5255..12cdf624c8 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/types.ts @@ -1,12 +1,11 @@ /** - * Shared types for paragraph border rendering features. + * Shared types for the DomPainter rendering pipeline. + * + * BlockLookup is the canonical definition — renderer.ts and feature modules + * both import from here to avoid circular dependencies. */ import type { FlowBlock, Measure } from '@superdoc/contracts'; -/** - * Entry in the block lookup map. Re-exported here to avoid - * a direct import from the monolithic renderer. - */ export type BlockLookupEntry = { block: FlowBlock; measure: Measure; diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index fcd0a4dc3d..2fa0241935 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -38,9 +38,6 @@ import type { ShapeTextContent, SolidFillWithAlpha, TableAttrs, - ListItemFragment, - ListBlock, - ListMeasure, TableBlock, TableCellAttrs, TableFragment, @@ -99,10 +96,12 @@ import { SdtGroupedHover } from './utils/sdt-hover.js'; import { computeBetweenBorderFlags, getFragmentParagraphBorders, + getFragmentHeight, createParagraphDecorationLayers, applyParagraphBorderStyles, applyParagraphShadingStyles, getParagraphBorderBox, + stampBetweenBorderDataset, type BetweenBorderInfo, } from './features/paragraph-borders/index.js'; @@ -344,20 +343,9 @@ type PainterOptions = { ruler?: RulerOptions; }; -type BlockLookupEntry = { - block: FlowBlock; - measure: Measure; - version: string; -}; - -/** - * Map of block IDs to their corresponding block data and measurements. - * Used by the renderer to efficiently look up block information during fragment rendering. - * Each entry contains the block definition, its layout measurements, and a version string for cache invalidation. - * - * @typedef {Map} BlockLookup - */ -export type BlockLookup = Map; +// BlockLookup lives in the shared types module (single source of truth) +import type { BlockLookupEntry, BlockLookup } from './features/paragraph-borders/types.js'; +export type { BlockLookup, BlockLookupEntry }; type FragmentDomState = { key: string; @@ -2640,11 +2628,7 @@ export class DomPainter { if (borderLayer) { fragmentEl.appendChild(borderLayer); } - if (betweenInfo) { - if (betweenInfo.showBetweenBorder) fragmentEl.dataset.betweenBorder = 'true'; - if (betweenInfo.suppressTopBorder) fragmentEl.dataset.suppressTopBorder = 'true'; - if (betweenInfo.gapBelow) fragmentEl.dataset.gapBelow = String(betweenInfo.gapBelow); - } + stampBetweenBorderDataset(fragmentEl, betweenInfo); if (block.attrs?.styleId) { fragmentEl.dataset.styleId = block.attrs.styleId; fragmentEl.setAttribute('styleid', block.attrs.styleId); @@ -3144,11 +3128,7 @@ export class DomPainter { if (borderLayer) { contentEl.appendChild(borderLayer); } - if (betweenInfo) { - if (betweenInfo.showBetweenBorder) fragmentEl.dataset.betweenBorder = 'true'; - if (betweenInfo.suppressTopBorder) fragmentEl.dataset.suppressTopBorder = 'true'; - if (betweenInfo.gapBelow) fragmentEl.dataset.gapBelow = String(betweenInfo.gapBelow); - } + stampBetweenBorderDataset(fragmentEl, betweenInfo); // INTENTIONAL DIVERGENCE: Force list content to left alignment // Microsoft Word DOES justify list paragraphs when alignment is 'justify', // but we intentionally keep lists left-aligned to match user expectations @@ -6232,41 +6212,6 @@ const getFragmentSdtContainerKey = (fragment: Fragment, blockLookup: BlockLookup return null; }; -const getFragmentHeight = (fragment: Fragment, blockLookup: BlockLookup): number => { - if (fragment.kind === 'table' || fragment.kind === 'image' || fragment.kind === 'drawing') { - return fragment.height; - } - - const lookup = blockLookup.get(fragment.blockId); - if (!lookup) return 0; - - if (fragment.kind === 'para' && lookup.measure.kind === 'paragraph') { - const measure = lookup.measure; - const lines = fragment.lines ?? measure.lines.slice(fragment.fromLine, fragment.toLine); - if (lines.length === 0) return 0; - let totalHeight = 0; - for (const line of lines) { - totalHeight += line.lineHeight ?? 0; - } - return totalHeight; - } - - if (fragment.kind === 'list-item' && lookup.measure.kind === 'list') { - const listMeasure = lookup.measure as ListMeasure; - const item = listMeasure.items.find((it) => it.itemId === fragment.itemId); - if (!item) return 0; - const lines = item.paragraph.lines.slice(fragment.fromLine, fragment.toLine); - if (lines.length === 0) return 0; - let totalHeight = 0; - for (const line of lines) { - totalHeight += line.lineHeight ?? 0; - } - return totalHeight; - } - - return 0; -}; - const computeSdtBoundaries = ( fragments: readonly Fragment[], blockLookup: BlockLookup, From a5be88523d5567471f5aaf9113d9ec55f788c044 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 16:55:24 -0300 Subject: [PATCH 06/11] test: add getParagraphBorderBox unit tests and remove orphaned fixture - 16 tests for indent-aware border box sizing (left, right, firstLine, hanging, NaN/Infinity clamping, combined indents) - Remove orphaned between-borders-comprehensive.docx from test-data/ --- .../painters/dom/src/between-borders.test.ts | 94 +++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index cdc590981a..855e0e2656 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -4,6 +4,7 @@ import { getFragmentParagraphBorders, computeBetweenBorderFlags, createParagraphDecorationLayers, + getParagraphBorderBox, type BlockLookup, type BetweenBorderInfo, } from './features/paragraph-borders/index.js'; @@ -631,6 +632,99 @@ describe('computeBetweenBorderFlags', () => { }); }); +// --------------------------------------------------------------------------- +// getParagraphBorderBox — indent-aware sizing +// --------------------------------------------------------------------------- + +describe('getParagraphBorderBox', () => { + const W = 600; + + it('returns full width with no indent', () => { + const box = getParagraphBorderBox(W); + expect(box).toEqual({ leftInset: 0, width: W }); + }); + + it('returns full width when indent is undefined', () => { + const box = getParagraphBorderBox(W, undefined); + expect(box).toEqual({ leftInset: 0, width: W }); + }); + + it('insets by left indent', () => { + const box = getParagraphBorderBox(W, { left: 40 }); + expect(box.leftInset).toBe(40); + expect(box.width).toBe(W - 40); + }); + + it('insets by right indent', () => { + const box = getParagraphBorderBox(W, { right: 30 }); + expect(box.leftInset).toBe(0); + expect(box.width).toBe(W - 30); + }); + + it('insets by both left and right', () => { + const box = getParagraphBorderBox(W, { left: 40, right: 30 }); + expect(box.leftInset).toBe(40); + expect(box.width).toBe(W - 40 - 30); + }); + + it('uses smaller of left and left+firstLine for leftInset', () => { + // firstLine=20 → leftInset = min(50, 50+20) = 50, width uses leftInset=50 + const box = getParagraphBorderBox(W, { left: 50, firstLine: 20 }); + expect(box.leftInset).toBe(50); + expect(box.width).toBe(W - 50); + }); + + it('reduces leftInset when hanging exceeds left indent', () => { + // hanging=60 → firstLineOffset = 0 - 60 = -60 → minLeft = min(50, 50-60) = -10 → clamped to 0 + const box = getParagraphBorderBox(W, { left: 50, hanging: 60 }); + expect(box.leftInset).toBe(0); + expect(box.width).toBe(W); + }); + + it('handles hanging smaller than left indent', () => { + // hanging=20 → firstLineOffset = 0 - 20 = -20 → minLeft = min(50, 50-20) = 30 + const box = getParagraphBorderBox(W, { left: 50, hanging: 20 }); + expect(box.leftInset).toBe(30); + expect(box.width).toBe(W - 30); + }); + + it('clamps negative leftInset to 0', () => { + // left=0, hanging=30 → firstLineOffset = -30 → minLeft = min(0, -30) = -30 → clamped to 0 + const box = getParagraphBorderBox(W, { hanging: 30 }); + expect(box.leftInset).toBe(0); + expect(box.width).toBe(W); + }); + + it('clamps negative rightInset to 0', () => { + const box = getParagraphBorderBox(W, { right: -10 }); + expect(box.leftInset).toBe(0); + expect(box.width).toBe(W); + }); + + it('clamps width to 0 when indents exceed fragment width', () => { + const box = getParagraphBorderBox(100, { left: 60, right: 60 }); + expect(box.width).toBe(0); + }); + + it('handles all indent properties together', () => { + // left=40, right=30, firstLine=10, hanging=0 + // firstLineOffset = 10 - 0 = 10, minLeft = min(40, 50) = 40 + const box = getParagraphBorderBox(W, { left: 40, right: 30, firstLine: 10, hanging: 0 }); + expect(box.leftInset).toBe(40); + expect(box.width).toBe(W - 40 - 30); + }); + + it('treats NaN indent values as 0', () => { + const box = getParagraphBorderBox(W, { left: NaN, right: NaN }); + expect(box).toEqual({ leftInset: 0, width: W }); + }); + + it('treats Infinity indent values as 0', () => { + const box = getParagraphBorderBox(W, { left: Infinity }); + expect(box).toEqual({ leftInset: 0, width: W }); + }); +}); + // --------------------------------------------------------------------------- // Incremental update — between-border cache invalidation // --------------------------------------------------------------------------- From ca3b9d506aec5052a8ec90e2098dd014f1b05fc1 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 18:22:24 -0300 Subject: [PATCH 07/11] fix: group paragraphs with nil/none between border into continuous box MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Word groups consecutive paragraphs with matching borders into a single bordered box even when w:between is nil/none — it just omits the separator. SuperDoc was skipping grouping entirely for nil/none between, rendering each paragraph as a separate bordered box. - Preserve between: {style: 'none'} during normalization so downstream can distinguish "explicitly nil/none" from "no between element" - Change grouping check to allow nil/none between through - Add suppressBottomBorder flag for nil/none groups (extend gap without drawing a separator) --- .../painters/dom/src/between-borders.test.ts | 173 +++++++++++++++++- .../paragraph-borders/border-layer.ts | 9 +- .../paragraph-borders/group-analysis.ts | 35 +++- .../pm-adapter/src/attributes/borders.test.ts | 6 +- .../pm-adapter/src/attributes/borders.ts | 11 ++ 5 files changed, 217 insertions(+), 17 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index 855e0e2656..0180d62ce7 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -10,8 +10,18 @@ import { } from './features/paragraph-borders/index.js'; /** Helper to create BetweenBorderInfo for tests that previously passed a boolean. */ -const betweenOn: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 }; -const betweenOff: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: false, gapBelow: 0 }; +const betweenOn: BetweenBorderInfo = { + showBetweenBorder: true, + suppressTopBorder: false, + suppressBottomBorder: false, + gapBelow: 0, +}; +const betweenOff: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: false, + suppressBottomBorder: false, + gapBelow: 0, +}; import { createDomPainter } from './index.js'; import type { ParagraphBorders, @@ -258,7 +268,12 @@ describe('applyParagraphBorderStyles — between borders', () => { left: { style: 'solid', width: 1, color: '#000' }, right: { style: 'solid', width: 1, color: '#000' }, }; - const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: true, + suppressBottomBorder: false, + gapBelow: 0, + }; applyParagraphBorderStyles(e, borders, info); expect(e.style.getPropertyValue('border-top-style')).toBe(''); expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); @@ -274,6 +289,49 @@ describe('applyParagraphBorderStyles — between borders', () => { expect(e.style.getPropertyValue('border-top-style')).toBe('dashed'); expect(e.style.getPropertyValue('border-top-width')).toBe('3px'); }); + + // --- suppressBottomBorder (nil/none between groups) --- + it('skips bottom border when suppressBottomBorder is true', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 2, color: '#F00' }, + left: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: false, + suppressBottomBorder: true, + gapBelow: 10, + }; + applyParagraphBorderStyles(e, borders, info); + expect(e.style.getPropertyValue('border-top-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-right-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + }); + + it('suppresses both top and bottom for middle fragment in nil/none group', () => { + const e = el(); + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 1, color: '#000' }, + left: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: true, + suppressBottomBorder: true, + gapBelow: 10, + }; + applyParagraphBorderStyles(e, borders, info); + expect(e.style.getPropertyValue('border-top-style')).toBe(''); + expect(e.style.getPropertyValue('border-bottom-style')).toBe(''); + expect(e.style.getPropertyValue('border-left-style')).toBe('solid'); + expect(e.style.getPropertyValue('border-right-style')).toBe('solid'); + }); }); // --------------------------------------------------------------------------- @@ -283,7 +341,12 @@ describe('applyParagraphBorderStyles — between borders', () => { describe('createParagraphDecorationLayers — gap extension', () => { it('sets bottom to negative gapBelow when showBetweenBorder is true', () => { const attrs = { borders: { top: { style: 'solid' as const, width: 1 } }, shading: { fill: '#EEE' } }; - const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 8 }; + const info: BetweenBorderInfo = { + showBetweenBorder: true, + suppressTopBorder: false, + suppressBottomBorder: false, + gapBelow: 8, + }; const { borderLayer, shadingLayer } = createParagraphDecorationLayers(document, 400, attrs, info); expect(borderLayer!.style.bottom).toBe('-8px'); expect(shadingLayer!.style.bottom).toBe('-8px'); @@ -291,7 +354,12 @@ describe('createParagraphDecorationLayers — gap extension', () => { it('sets bottom to 0px when gapBelow is 0', () => { const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } }; - const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 }; + const info: BetweenBorderInfo = { + showBetweenBorder: true, + suppressTopBorder: false, + suppressBottomBorder: false, + gapBelow: 0, + }; const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info); expect(borderLayer!.style.bottom).toBe('0px'); }); @@ -302,12 +370,31 @@ describe('createParagraphDecorationLayers — gap extension', () => { expect(borderLayer!.style.bottom).toBe('0px'); }); - it('sets bottom to 0px when showBetweenBorder is false even with gapBelow', () => { + it('sets bottom to 0px when showBetweenBorder is false and suppressBottomBorder is false', () => { const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } }; - const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 12 }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: true, + suppressBottomBorder: false, + gapBelow: 12, + }; const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info); expect(borderLayer!.style.bottom).toBe('0px'); }); + + it('sets bottom to negative gapBelow when suppressBottomBorder is true (nil/none between group)', () => { + const attrs = { + borders: { top: { style: 'solid' as const, width: 1 }, bottom: { style: 'solid' as const, width: 1 } }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: false, + suppressBottomBorder: true, + gapBelow: 10, + }; + const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info); + expect(borderLayer!.style.bottom).toBe('-10px'); + }); }); // --------------------------------------------------------------------------- @@ -630,6 +717,78 @@ describe('computeBetweenBorderFlags', () => { const flags = computeBetweenBorderFlags(fragments, lookup); expect(flags.size).toBe(2); }); + + // --- nil/none between grouping (continuous box without separator) --- + it('groups paragraphs with between: {style: "none"} (nil/none between)', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 1, color: '#000' }, + left: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'none' }, + }; + const b1 = makeParagraphBlock('b1', borders); + const b2 = makeParagraphBlock('b2', borders); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1', { y: 0 }), paraFragment('b2', { y: 20 })]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.size).toBe(2); + // First fragment: suppressBottomBorder (not showBetweenBorder) + expect(flags.get(0)?.showBetweenBorder).toBe(false); + expect(flags.get(0)?.suppressBottomBorder).toBe(true); + // Second fragment: suppressTopBorder + expect(flags.get(1)?.suppressTopBorder).toBe(true); + expect(flags.get(1)?.suppressBottomBorder).toBe(false); + }); + + it('groups chain of 3 paragraphs with nil/none between', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + bottom: { style: 'solid', width: 1, color: '#000' }, + left: { style: 'solid', width: 1, color: '#000' }, + right: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'none' }, + }; + const b1 = makeParagraphBlock('b1', borders); + const b2 = makeParagraphBlock('b2', borders); + const b3 = makeParagraphBlock('b3', borders); + const lookup = buildLookup([{ block: b1 }, { block: b2 }, { block: b3 }]); + const fragments: Fragment[] = [ + paraFragment('b1', { y: 0 }), + paraFragment('b2', { y: 20 }), + paraFragment('b3', { y: 40 }), + ]; + + const flags = computeBetweenBorderFlags(fragments, lookup); + expect(flags.size).toBe(3); + // First: suppress bottom, keep top + expect(flags.get(0)?.suppressBottomBorder).toBe(true); + expect(flags.get(0)?.suppressTopBorder).toBe(false); + // Middle: suppress both top and bottom + expect(flags.get(1)?.suppressTopBorder).toBe(true); + expect(flags.get(1)?.suppressBottomBorder).toBe(true); + // Last: suppress top, keep bottom + expect(flags.get(2)?.suppressTopBorder).toBe(true); + expect(flags.get(2)?.suppressBottomBorder).toBe(false); + }); + + it('does not group nil/none between with real between (different hashes)', () => { + const nilBetween: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'none' }, + }; + const realBetween: ParagraphBorders = { + top: { style: 'solid', width: 1, color: '#000' }, + between: { style: 'solid', width: 1, color: '#000' }, + }; + const b1 = makeParagraphBlock('b1', nilBetween); + const b2 = makeParagraphBlock('b2', realBetween); + const lookup = buildLookup([{ block: b1 }, { block: b2 }]); + const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')]; + + expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts index f8894d3dc2..0f8e0320d2 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -59,8 +59,10 @@ export const createParagraphDecorationLayers = ( const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent); - // Extend layers into the spacing gap for continuous group borders - const gapExtension = betweenInfo?.showBetweenBorder ? betweenInfo.gapBelow : 0; + // Extend layers into the spacing gap for continuous group borders. + // Both real between (showBetweenBorder) and nil/none between (suppressBottomBorder) + // need gap extension to keep left/right borders continuous through the spacing gap. + const gapExtension = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0; const bottomValue = gapExtension > 0 ? `-${gapExtension}px` : '0px'; const baseStyles = { @@ -113,10 +115,12 @@ export const applyParagraphBorderStyles = ( if (!borders) return; const showBetweenBorder = betweenInfo?.showBetweenBorder ?? false; const suppressTopBorder = betweenInfo?.suppressTopBorder ?? false; + const suppressBottomBorder = betweenInfo?.suppressBottomBorder ?? false; element.style.boxSizing = 'border-box'; BORDER_SIDES.forEach((side) => { if (side === 'top' && suppressTopBorder) return; + if (side === 'bottom' && suppressBottomBorder) return; const border = borders[side]; if (!border) return; setBorderSideStyle(element, side, border); @@ -157,6 +161,7 @@ export const stampBetweenBorderDataset = (element: HTMLElement, betweenInfo?: Be if (!betweenInfo) return; if (betweenInfo.showBetweenBorder) element.dataset.betweenBorder = 'true'; if (betweenInfo.suppressTopBorder) element.dataset.suppressTopBorder = 'true'; + if (betweenInfo.suppressBottomBorder) element.dataset.suppressBottomBorder = 'true'; if (betweenInfo.gapBelow) element.dataset.gapBelow = String(betweenInfo.gapBelow); }; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts index 47a4a4ca2d..64e9b6b817 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts @@ -29,6 +29,7 @@ import { hashParagraphBorders } from '../../paragraph-hash-utils.js'; export type BetweenBorderInfo = { showBetweenBorder: boolean; suppressTopBorder: boolean; + suppressBottomBorder: boolean; gapBelow: number; }; @@ -125,6 +126,7 @@ export const computeBetweenBorderFlags = ( ): Map => { // Phase 1: determine which consecutive pairs form between-border groups const pairFlags = new Set(); + const noBetweenPairs = new Set(); for (let i = 0; i < fragments.length - 1; i += 1) { const frag = fragments[i]; @@ -132,7 +134,9 @@ export const computeBetweenBorderFlags = ( if (frag.continuesOnNext) continue; const borders = getFragmentParagraphBorders(frag, blockLookup); - if (isBetweenBorderNone(borders)) continue; + // Skip if no between element at all (no grouping intent). + // between: {style: 'none'} (nil/none) IS allowed — it signals grouping without a separator. + if (!borders?.between) continue; const next = fragments[i + 1]; if (next.kind !== 'para' && next.kind !== 'list-item') continue; @@ -147,13 +151,18 @@ export const computeBetweenBorderFlags = ( continue; const nextBorders = getFragmentParagraphBorders(next, blockLookup); - if (isBetweenBorderNone(nextBorders)) continue; + if (!nextBorders?.between) continue; if (hashParagraphBorders(borders!) !== hashParagraphBorders(nextBorders!)) continue; // Skip fragments in different columns (different x positions) if (frag.x !== next.x) continue; pairFlags.add(i); + + // Track nil/none between pairs — these get suppressBottomBorder instead of showBetweenBorder + if (isBetweenBorderNone(borders) && isBetweenBorderNone(nextBorders)) { + noBetweenPairs.add(i); + } } // Phase 2: build per-fragment info with gap distances and top suppression @@ -164,19 +173,33 @@ export const computeBetweenBorderFlags = ( const next = fragments[i + 1]; const fragHeight = getFragmentHeight(frag, blockLookup); const gapBelow = Math.max(0, next.y - (frag.y + fragHeight)); + const isNoBetween = noBetweenPairs.has(i); - // Current fragment: show between border on bottom, extend into gap + // Current fragment: extend into gap. + // Real between → showBetweenBorder (replace bottom with between definition). + // Nil/none between → suppressBottomBorder (hide bottom, keep left/right continuous). if (!result.has(i)) { - result.set(i, { showBetweenBorder: true, suppressTopBorder: false, gapBelow }); + result.set(i, { + showBetweenBorder: !isNoBetween, + suppressTopBorder: false, + suppressBottomBorder: isNoBetween, + gapBelow, + }); } else { const existing = result.get(i)!; - existing.showBetweenBorder = true; + existing.showBetweenBorder = !isNoBetween; + existing.suppressBottomBorder = isNoBetween; existing.gapBelow = gapBelow; } // Next fragment: suppress top border (previous fragment's extended layer covers boundary) if (!result.has(i + 1)) { - result.set(i + 1, { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 }); + result.set(i + 1, { + showBetweenBorder: false, + suppressTopBorder: true, + suppressBottomBorder: false, + gapBelow: 0, + }); } else { result.get(i + 1)!.suppressTopBorder = true; } diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts index a8d2b2b26c..b6d1ddc3ef 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.test.ts @@ -572,11 +572,13 @@ describe('normalizeParagraphBorders', () => { expect(result?.between?.style).toBe('dashed'); }); - it('should return undefined when between border is nil', () => { + it('should preserve between: {style: "none"} when between border is nil', () => { const input = { between: { val: 'nil' }, }; - expect(normalizeParagraphBorders(input)).toBeUndefined(); + const result = normalizeParagraphBorders(input); + expect(result).toBeDefined(); + expect(result?.between).toEqual({ style: 'none' }); }); }); diff --git a/packages/layout-engine/pm-adapter/src/attributes/borders.ts b/packages/layout-engine/pm-adapter/src/attributes/borders.ts index 0f0016ce95..c1a78ba32f 100644 --- a/packages/layout-engine/pm-adapter/src/attributes/borders.ts +++ b/packages/layout-engine/pm-adapter/src/attributes/borders.ts @@ -334,6 +334,17 @@ export const normalizeParagraphBorders = (value: unknown): ParagraphAttrs['borde } }); + // Preserve between: {style: 'none'} for nil/none between borders. + // normalizeBorderSide drops 'none' sides, but for 'between' we need to keep it + // so the grouping logic can distinguish "explicitly nil/none" (group without separator) + // from "no between element at all" (don't group). + if (!borders.between && source.between) { + const style = mapBorderStyle((source.between as Record).val); + if (style === 'none') { + borders.between = { style: 'none' }; + } + } + return Object.keys(borders).length > 0 ? borders : undefined; }; From 23aa391599612a735b841804c02b2f4c97cdf065 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 18:27:19 -0300 Subject: [PATCH 08/11] feat: apply paragraph border space as padding between border and text The OOXML `space` attribute on w:pBdr sides specifies the distance between the border line and the paragraph text (in points). Word uses this to create visible padding inside bordered paragraphs. - Add computeBorderSpaceExpansion() to convert space values to px - Expand the border/shading layers outward by the space amount - Suppress top/bottom space within between-border groups where those sides are already handled by gap extension --- .../painters/dom/src/between-borders.test.ts | 81 +++++++++++++++++++ .../paragraph-borders/border-layer.ts | 47 +++++++++-- .../src/features/paragraph-borders/index.ts | 1 + 3 files changed, 124 insertions(+), 5 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/between-borders.test.ts b/packages/layout-engine/painters/dom/src/between-borders.test.ts index 0180d62ce7..dfd1c568ba 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -5,6 +5,7 @@ import { computeBetweenBorderFlags, createParagraphDecorationLayers, getParagraphBorderBox, + computeBorderSpaceExpansion, type BlockLookup, type BetweenBorderInfo, } from './features/paragraph-borders/index.js'; @@ -884,6 +885,86 @@ describe('getParagraphBorderBox', () => { }); }); +// --------------------------------------------------------------------------- +// computeBorderSpaceExpansion — border space (padding between border and text) +// --------------------------------------------------------------------------- + +describe('computeBorderSpaceExpansion', () => { + const PX_PER_PT = 96 / 72; + + it('returns zero expansion when no borders', () => { + expect(computeBorderSpaceExpansion(undefined)).toEqual({ top: 0, bottom: 0, left: 0, right: 0 }); + }); + + it('returns zero expansion when borders have no space', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1 }, + bottom: { style: 'solid', width: 1 }, + }; + expect(computeBorderSpaceExpansion(borders)).toEqual({ top: 0, bottom: 0, left: 0, right: 0 }); + }); + + it('expands all sides by space in px', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, space: 2 }, + bottom: { style: 'solid', width: 1, space: 3 }, + left: { style: 'solid', width: 1, space: 1 }, + right: { style: 'solid', width: 1, space: 4 }, + }; + const result = computeBorderSpaceExpansion(borders); + expect(result.top).toBeCloseTo(2 * PX_PER_PT); + expect(result.bottom).toBeCloseTo(3 * PX_PER_PT); + expect(result.left).toBeCloseTo(1 * PX_PER_PT); + expect(result.right).toBeCloseTo(4 * PX_PER_PT); + }); + + it('suppresses top expansion when suppressTopBorder is true', () => { + const borders: ParagraphBorders = { + top: { style: 'solid', width: 1, space: 2 }, + left: { style: 'solid', width: 1, space: 1 }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: true, + suppressBottomBorder: false, + gapBelow: 0, + }; + const result = computeBorderSpaceExpansion(borders, info); + expect(result.top).toBe(0); + expect(result.left).toBeCloseTo(1 * PX_PER_PT); + }); + + it('suppresses bottom expansion when suppressBottomBorder is true', () => { + const borders: ParagraphBorders = { + bottom: { style: 'solid', width: 1, space: 2 }, + right: { style: 'solid', width: 1, space: 1 }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: false, + suppressTopBorder: false, + suppressBottomBorder: true, + gapBelow: 10, + }; + const result = computeBorderSpaceExpansion(borders, info); + expect(result.bottom).toBe(0); + expect(result.right).toBeCloseTo(1 * PX_PER_PT); + }); + + it('suppresses bottom expansion when showBetweenBorder is true', () => { + const borders: ParagraphBorders = { + bottom: { style: 'solid', width: 1, space: 2 }, + }; + const info: BetweenBorderInfo = { + showBetweenBorder: true, + suppressTopBorder: false, + suppressBottomBorder: false, + gapBelow: 8, + }; + const result = computeBorderSpaceExpansion(borders, info); + expect(result.bottom).toBe(0); + }); +}); + // --------------------------------------------------------------------------- // Incremental update — between-border cache invalidation // --------------------------------------------------------------------------- diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts index 0f8e0320d2..3aca2c1abb 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -10,9 +10,11 @@ * @ooxml w:pPr/w:shd — paragraph shading (background fill) * @spec ECMA-376 §17.3.1.24 (pBdr), §17.3.1.31 (shd) */ -import type { ParagraphAttrs, ParagraphBorder } from '@superdoc/contracts'; +import type { ParagraphAttrs, ParagraphBorder, ParagraphBorders } from '@superdoc/contracts'; import type { BetweenBorderInfo } from './group-analysis.js'; +const PX_PER_PT = 96 / 72; + // ─── Border box sizing ───────────────────────────────────────────── /** @@ -37,6 +39,35 @@ export const getParagraphBorderBox = ( }; }; +// ─── Border space (padding between border and text) ───────────────── + +/** + * Computes the outward expansion for the border/shading layers based on + * the `space` attribute (OOXML: distance between border and text, in points). + * + * Within between-border groups, suppressed sides don't expand (the gap + * extension handles visual continuity instead). + * + * @spec ECMA-376 §17.3.1.24 — space attribute on pBdr child elements + */ +export const computeBorderSpaceExpansion = ( + borders?: ParagraphBorders, + betweenInfo?: BetweenBorderInfo, +): { top: number; bottom: number; left: number; right: number } => { + if (!borders) return { top: 0, bottom: 0, left: 0, right: 0 }; + + const suppressTop = betweenInfo?.suppressTopBorder ?? false; + const suppressBottom = betweenInfo?.suppressBottomBorder ?? false; + const showBetween = betweenInfo?.showBetweenBorder ?? false; + + return { + top: !suppressTop && borders.top?.space ? borders.top.space * PX_PER_PT : 0, + bottom: !suppressBottom && !showBetween && borders.bottom?.space ? borders.bottom.space * PX_PER_PT : 0, + left: borders.left?.space ? borders.left.space * PX_PER_PT : 0, + right: borders.right?.space ? borders.right.space * PX_PER_PT : 0, + }; +}; + // ─── Decoration layer factory ────────────────────────────────────── /** @@ -48,6 +79,9 @@ export const getParagraphBorderBox = ( * gap, making left/right borders visually continuous across the group. * - `suppressTopBorder` hides the top border for non-first group members. * - `showBetweenBorder` replaces the bottom border with the between definition. + * + * The `space` attribute on each border side expands the layer outward, + * creating padding between the border line and the paragraph text. */ export const createParagraphDecorationLayers = ( doc: Document, @@ -58,19 +92,22 @@ export const createParagraphDecorationLayers = ( if (!attrs?.borders && !attrs?.shading) return {}; const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent); + const space = computeBorderSpaceExpansion(attrs.borders, betweenInfo); // Extend layers into the spacing gap for continuous group borders. // Both real between (showBetweenBorder) and nil/none between (suppressBottomBorder) // need gap extension to keep left/right borders continuous through the spacing gap. const gapExtension = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0; - const bottomValue = gapExtension > 0 ? `-${gapExtension}px` : '0px'; + const totalBottomExpansion = gapExtension + space.bottom; + const bottomValue = totalBottomExpansion > 0 ? `-${totalBottomExpansion}px` : '0px'; + const topValue = space.top > 0 ? `-${space.top}px` : '0px'; const baseStyles = { position: 'absolute', - top: '0px', + top: topValue, bottom: bottomValue, - left: `${borderBox.leftInset}px`, - width: `${borderBox.width}px`, + left: `${borderBox.leftInset - space.left}px`, + width: `${borderBox.width + space.left + space.right}px`, pointerEvents: 'none', boxSizing: 'border-box', } as const; diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts index b20f29ac89..16dbf581f1 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/index.ts @@ -25,6 +25,7 @@ export { applyParagraphShadingStyles, getParagraphBorderBox, stampBetweenBorderDataset, + computeBorderSpaceExpansion, } from './border-layer.js'; // Shared types From 9d518a8448f9587543d7ebbfb166b655bc2a0b70 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 18:38:29 -0300 Subject: [PATCH 09/11] fix: expand border layer by border width to prevent overlap with text With box-sizing: border-box, CSS borders are drawn inside the element. The border layer must expand by space + borderWidth (not just space) so the border's inner edge sits at the correct distance from text. The shading layer only needs space expansion since it has no CSS borders. --- .../paragraph-borders/border-layer.ts | 51 +++++++++++++++---- 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts index 3aca2c1abb..0396e39fca 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -98,16 +98,18 @@ export const createParagraphDecorationLayers = ( // Both real between (showBetweenBorder) and nil/none between (suppressBottomBorder) // need gap extension to keep left/right borders continuous through the spacing gap. const gapExtension = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0; - const totalBottomExpansion = gapExtension + space.bottom; - const bottomValue = totalBottomExpansion > 0 ? `-${totalBottomExpansion}px` : '0px'; - const topValue = space.top > 0 ? `-${space.top}px` : '0px'; - const baseStyles = { + // Border widths for each rendered side. With box-sizing: border-box, CSS borders are + // drawn INSIDE the element. To position the border's inner edge at `space` distance + // from the text, the border layer must be expanded by space + borderWidth. + // The shading layer only needs space (it has no CSS borders). + const bw = computeRenderedBorderWidths(attrs.borders, betweenInfo); + + const shadingBottom = gapExtension + space.bottom; + const borderBottom = gapExtension + space.bottom + bw.bottom; + + const commonStyles = { position: 'absolute', - top: topValue, - bottom: bottomValue, - left: `${borderBox.leftInset - space.left}px`, - width: `${borderBox.width + space.left + space.right}px`, pointerEvents: 'none', boxSizing: 'border-box', } as const; @@ -116,7 +118,11 @@ export const createParagraphDecorationLayers = ( if (attrs.shading) { shadingLayer = doc.createElement('div'); shadingLayer.classList.add('superdoc-paragraph-shading'); - Object.assign(shadingLayer.style, baseStyles); + Object.assign(shadingLayer.style, commonStyles); + shadingLayer.style.top = space.top > 0 ? `-${space.top}px` : '0px'; + shadingLayer.style.bottom = shadingBottom > 0 ? `-${shadingBottom}px` : '0px'; + shadingLayer.style.left = `${borderBox.leftInset - space.left}px`; + shadingLayer.style.width = `${borderBox.width + space.left + space.right}px`; applyParagraphShadingStyles(shadingLayer, attrs.shading); } @@ -124,7 +130,11 @@ export const createParagraphDecorationLayers = ( if (attrs.borders) { borderLayer = doc.createElement('div'); borderLayer.classList.add('superdoc-paragraph-border'); - Object.assign(borderLayer.style, baseStyles); + Object.assign(borderLayer.style, commonStyles); + borderLayer.style.top = space.top + bw.top > 0 ? `-${space.top + bw.top}px` : '0px'; + borderLayer.style.bottom = borderBottom > 0 ? `-${borderBottom}px` : '0px'; + borderLayer.style.left = `${borderBox.leftInset - space.left - bw.left}px`; + borderLayer.style.width = `${borderBox.width + space.left + bw.left + space.right + bw.right}px`; borderLayer.style.zIndex = '1'; applyParagraphBorderStyles(borderLayer, attrs.borders, betweenInfo); } @@ -132,6 +142,27 @@ export const createParagraphDecorationLayers = ( return { shadingLayer, borderLayer }; }; +/** + * Computes the rendered CSS border widths per side, accounting for suppressed sides. + * Used to expand the border layer so the inner edge is at the correct `space` offset. + */ +const computeRenderedBorderWidths = ( + borders?: ParagraphBorders, + betweenInfo?: BetweenBorderInfo, +): { top: number; bottom: number; left: number; right: number } => { + if (!borders) return { top: 0, bottom: 0, left: 0, right: 0 }; + const suppressTop = betweenInfo?.suppressTopBorder ?? false; + const suppressBottom = betweenInfo?.suppressBottomBorder ?? false; + const showBetween = betweenInfo?.showBetweenBorder ?? false; + + return { + top: !suppressTop ? (borders.top?.width ?? 0) : 0, + bottom: showBetween ? (borders.between?.width ?? 0) : !suppressBottom ? (borders.bottom?.width ?? 0) : 0, + left: borders.left?.width ?? 0, + right: borders.right?.width ?? 0, + }; +}; + // ─── Border CSS application ──────────────────────────────────────── type CssBorderSide = 'top' | 'right' | 'bottom' | 'left'; From 1a901ddaa8b17d4d9f600d664febedd29766841b Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Sat, 7 Mar 2026 18:47:49 -0300 Subject: [PATCH 10/11] fix: apply between border space below to match Word paragraph spacing The between border was flush against the next paragraph's text. Now: - First paragraph reduces gap extension by between.space, positioning the between border higher in the spacing gap - Second paragraph extends upward by between.space to maintain continuous left/right borders through the gap --- .../paragraph-borders/border-layer.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts index 0396e39fca..67549b624a 100644 --- a/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts +++ b/packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts @@ -61,7 +61,15 @@ export const computeBorderSpaceExpansion = ( const showBetween = betweenInfo?.showBetweenBorder ?? false; return { - top: !suppressTop && borders.top?.space ? borders.top.space * PX_PER_PT : 0, + // When top is suppressed (non-first group member), use the between border's space + // to extend upward and fill the gap left by the previous paragraph's reduced extension. + top: suppressTop + ? borders.between?.space + ? borders.between.space * PX_PER_PT + : 0 + : borders.top?.space + ? borders.top.space * PX_PER_PT + : 0, bottom: !suppressBottom && !showBetween && borders.bottom?.space ? borders.bottom.space * PX_PER_PT : 0, left: borders.left?.space ? borders.left.space * PX_PER_PT : 0, right: borders.right?.space ? borders.right.space * PX_PER_PT : 0, @@ -97,7 +105,14 @@ export const createParagraphDecorationLayers = ( // Extend layers into the spacing gap for continuous group borders. // Both real between (showBetweenBorder) and nil/none between (suppressBottomBorder) // need gap extension to keep left/right borders continuous through the spacing gap. - const gapExtension = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0; + // + // For showBetweenBorder: reduce extension by between.space so the between border + // (drawn as CSS bottom border) sits higher in the gap, creating padding to the next + // paragraph. The next paragraph's top expansion fills the remaining gap portion. + const betweenSpaceBelow = + betweenInfo?.showBetweenBorder && attrs.borders?.between?.space ? attrs.borders.between.space * PX_PER_PT : 0; + const rawGap = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0; + const gapExtension = Math.max(0, rawGap - betweenSpaceBelow); // Border widths for each rendered side. With box-sizing: border-box, CSS borders are // drawn INSIDE the element. To position the border's inner edge at `space` distance From 28724da440f0ff67dd8127f0f108c756eb17a586 Mon Sep 17 00:00:00 2001 From: Caio Pizzol Date: Mon, 9 Mar 2026 07:59:51 -0300 Subject: [PATCH 11/11] fix: increase local test timeouts and perf variance to reduce flakiness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DOCX-loading beforeAll hooks now get 60s (up from 30s) to handle full-suite CPU contention. Performance test non-CI variance factor raised to 3× — CI targets remain the real regression gate. --- .../layout-engine/layout-bridge/test/performance.test.ts | 5 +++-- .../layout-engine/tests/src/multi-section-page-count.test.ts | 2 +- .../layout-engine/tests/src/sd-1495-auto-page-break.test.ts | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/layout-engine/layout-bridge/test/performance.test.ts b/packages/layout-engine/layout-bridge/test/performance.test.ts index ceb9123053..91793b5b97 100644 --- a/packages/layout-engine/layout-bridge/test/performance.test.ts +++ b/packages/layout-engine/layout-bridge/test/performance.test.ts @@ -24,7 +24,9 @@ beforeAll(() => { const describeIfRealCanvas = usingStub ? describe.skip : describe; const IS_CI = Boolean(process.env.CI); -const NON_CI_LATENCY_VARIANCE_FACTOR = 1.06; +// Full-suite parallel runs cause significant CPU contention locally; +// CI targets (500/700/1000 ms) are the real regression gate. +const NON_CI_LATENCY_VARIANCE_FACTOR = 3; const LATENCY_TARGETS = IS_CI ? { // CI environments are slower and more variable; use generous buffers @@ -40,7 +42,6 @@ const LATENCY_TARGETS = IS_CI const MIN_HIT_RATE = 0.95; const latencyBudget = (target: number): number => { if (IS_CI) return target; - // Full-suite runs can introduce small scheduling variance; keep a tight but non-brittle budget. return target * NON_CI_LATENCY_VARIANCE_FACTOR; }; diff --git a/packages/layout-engine/tests/src/multi-section-page-count.test.ts b/packages/layout-engine/tests/src/multi-section-page-count.test.ts index 983f1f9a3b..8e477992f4 100644 --- a/packages/layout-engine/tests/src/multi-section-page-count.test.ts +++ b/packages/layout-engine/tests/src/multi-section-page-count.test.ts @@ -150,7 +150,7 @@ describe('Multi-Section Document Page Count', () => { // Load/convert once; this conversion is expensive under full-suite parallel runs. loadedFixture = await docxToPMJson(MULTI_SECTION_DOCX_PATH); - }, 30000); + }, 60000); it('should render multi_section_doc.docx as exactly 4 pages', async () => { if (!loadedFixture) { diff --git a/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts b/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts index 2f3c02169b..a24a8472e0 100644 --- a/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts +++ b/packages/layout-engine/tests/src/sd-1495-auto-page-break.test.ts @@ -101,7 +101,7 @@ describe('SD-1495 auto page breaks', () => { loadedFixtures.forEach(({ filename, data }) => { fixtureCache.set(filename, data); }); - }, 30000); + }, 60000); it.each(FIXTURES)('pushes heading to next page for %s', async ({ filename, headingText }) => { const cachedFixture = fixtureCache.get(filename);