From a156110f992ddde9637fcbe6f48294b9572b18da Mon Sep 17 00:00:00 2001 From: Luccas Correa Date: Wed, 13 May 2026 14:57:02 -0300 Subject: [PATCH] refactor(painters/dom): drop list-item fragment renderer List rendering is now unified through the paragraph fragment path, so the list-item-specific renderer and its supporting helpers are no longer reachable. Removes: - `renderListItemFragment` and `applyResolvedListItemWrapperFrame` from `renderer.ts`, along with the `LIST_MARKER_GAP` constant and the `stripListIndent` helper. - list-item branches from `fragmentKey`, `deriveBlockVersion`, and the fragment-frame dispatch (now treats `list-item` as unsupported). - list-item handling in `computeBetweenBorderFlags` (border grouping now only pairs `para` fragments). - list-item fixtures and assertions from `between-borders.test.ts`, `renderer-dispatch.test.ts`, and `index.test.ts`. - the now-stale `renderer-known-divergences.test.ts`. --- .../painters/dom/src/between-borders.test.ts | 88 +---- .../paragraph-borders/group-analysis.ts | 21 +- .../painters/dom/src/index.test.ts | 315 ----------------- .../dom/src/renderer-dispatch.test.ts | 68 ---- .../dom/src/renderer-hanging-indent.test.ts | 2 +- .../src/renderer-known-divergences.test.ts | 317 ------------------ .../painters/dom/src/renderer.ts | 262 ++------------- .../dom/src/test-utils/normalize-line.ts | 2 +- 8 files changed, 36 insertions(+), 1039 deletions(-) delete mode 100644 packages/layout-engine/painters/dom/src/renderer-known-divergences.test.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 9030fe8f47..52627ac04e 100644 --- a/packages/layout-engine/painters/dom/src/between-borders.test.ts +++ b/packages/layout-engine/painters/dom/src/between-borders.test.ts @@ -27,13 +27,11 @@ import type { ParagraphBorders, ParagraphBorder, ParagraphBlock, - ListBlock, Fragment, FlowBlock, Layout, Measure, ParaFragment, - ListItemFragment, ImageFragment, ResolvedPaintItem, ResolvedFragmentItem, @@ -50,35 +48,19 @@ const makeParagraphBlock = (id: string, borders?: ParagraphBorders): ParagraphBl 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, - }, - })), -}); - /** * Test surrogate for the old BlockLookup — a list of blocks keyed by id that * `buildResolvedItems` consumes to synthesize per-fragment ResolvedPaintItems. */ -type TestBlockList = ReadonlyArray; +type TestBlockList = ReadonlyArray; -const buildLookup = (entries: { block: ParagraphBlock | ListBlock; measure?: unknown }[]): TestBlockList => +const buildLookup = (entries: { block: ParagraphBlock; measure?: unknown }[]): TestBlockList => entries.map((e) => e.block); /** * Build resolved items aligned 1:1 with the given fragments. - * Looks up each fragment's block (+ list item) to extract paragraph borders, - * then produces a ResolvedFragmentItem carrying the borders and a border hash. + * Looks up each fragment's block to extract paragraph borders, then produces a + * ResolvedFragmentItem carrying the borders and a border hash. */ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockList): ResolvedPaintItem[] => { const byId = new Map(blocks.map((b) => [b.id, b])); @@ -88,9 +70,6 @@ const buildResolvedItems = (fragments: readonly Fragment[], blocks: TestBlockLis if (fragment.kind === 'para' && block?.kind === 'paragraph') { borders = block.attrs?.borders; - } else if (fragment.kind === 'list-item' && block?.kind === 'list') { - const item = block.items.find((listItem) => listItem.id === fragment.itemId); - borders = item?.paragraph.attrs?.borders; } const item: ResolvedFragmentItem = { @@ -127,23 +106,6 @@ const paraFragment = (blockId: string, overrides?: Partial): ParaF ...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, @@ -517,29 +479,6 @@ describe('computeBetweenBorderFlags', () => { expect(runFlags(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(runFlags(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 = runFlags(fragments, lookup); - expect(flags.has(0)).toBe(true); - }); - // --- non-paragraph fragments --- it('skips image fragments', () => { const b1 = makeParagraphBlock('b1', MATCHING_BORDERS); @@ -555,25 +494,6 @@ describe('computeBetweenBorderFlags', () => { 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(runFlags(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(runFlags(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); 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 eaa1327e91..e6186becf0 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 @@ -8,7 +8,7 @@ * @ooxml w:pPr/w:pBdr/w:between — between border for grouped paragraphs * @spec ECMA-376 §17.3.1.24 (pBdr) */ -import type { ListItemFragment, ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts'; +import type { ResolvedPaintItem, ResolvedFragmentItem } from '@superdoc/contracts'; import { hashParagraphBorders } from '../../paragraph-hash-utils.js'; /** @@ -34,8 +34,8 @@ const isBetweenBorderNone = (borders: ResolvedFragmentItem['paragraphBorders']): }; /** - * Helper: check whether a resolved item is a ResolvedFragmentItem (para/list-item) - * with pre-computed paragraph border data. + * Helper: check whether a resolved item is a ResolvedFragmentItem with + * pre-computed paragraph border data. */ function isResolvedFragmentWithBorders( item: ResolvedPaintItem | undefined, @@ -49,7 +49,7 @@ function isResolvedFragmentWithBorders( * 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) + * 1. Both are para fragments (not table/image/drawing) * 2. Neither is a page-split continuation * 3. They represent different logical paragraphs * 4. Both have border definitions @@ -79,7 +79,7 @@ export const computeBetweenBorderFlags = ( const resolvedCur = resolvedItems[i]; if (resolvedCur.kind !== 'fragment') continue; const frag = resolvedCur.fragment; - if (frag.kind !== 'para' && frag.kind !== 'list-item') continue; + if (frag.kind !== 'para') continue; if (frag.continuesOnNext) continue; if (!isResolvedFragmentWithBorders(resolvedCur)) continue; @@ -88,16 +88,9 @@ export const computeBetweenBorderFlags = ( const resolvedNext = resolvedItems[i + 1]; if (resolvedNext.kind !== 'fragment') continue; const next = resolvedNext.fragment; - if (next.kind !== 'para' && next.kind !== 'list-item') continue; + if (next.kind !== 'para') 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; + if (next.blockId === frag.blockId) continue; if (!isResolvedFragmentWithBorders(resolvedNext)) continue; const nextBorders = resolvedNext.paragraphBorders; diff --git a/packages/layout-engine/painters/dom/src/index.test.ts b/packages/layout-engine/painters/dom/src/index.test.ts index 9c7c3cb49f..b88790d228 100644 --- a/packages/layout-engine/painters/dom/src/index.test.ts +++ b/packages/layout-engine/painters/dom/src/index.test.ts @@ -249,24 +249,6 @@ const withFallbackFragment = ( const fromLine = 'fromLine' in item && typeof item.fromLine === 'number' ? item.fromLine : 0; const toLine = 'toLine' in item && typeof item.toLine === 'number' ? item.toLine : fromLine + 1; - if (item.fragmentKind === 'list-item') { - return { - ...item, - fragment: { - kind: 'list-item', - blockId: item.blockId, - itemId: item.itemId, - markerText: item.markerText ?? '', - markerWidth: item.markerWidth ?? 0, - fromLine, - toLine, - x: item.x, - y: item.y, - width: item.width, - }, - }; - } - return { ...item, fragment: { @@ -5453,224 +5435,6 @@ describe('DomPainter', () => { }); }); - it('renders list fragments with markers', () => { - const listBlock: FlowBlock = { - kind: 'list', - id: 'list-1', - listType: 'number', - items: [ - { - id: 'item-1', - marker: { kind: 'number', text: '1.', level: 0, order: 1 }, - paragraph: block, - }, - ], - }; - - const listMeasure: Measure = { - kind: 'list', - items: [ - { - itemId: 'item-1', - markerWidth: 30, - markerTextWidth: 18, - indentLeft: 0, - paragraph: measure as ParagraphMeasure, - }, - ], - totalHeight: measure.totalHeight, - }; - - const listLayout: Layout = { - pageSize: layout.pageSize, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-1', - itemId: 'item-1', - fromLine: 0, - toLine: 1, - x: 100, - y: 40, - width: 260, - markerWidth: 30, - }, - ], - }, - ], - }; - - const painter = createTestPainter({ blocks: [listBlock], measures: [listMeasure] }); - painter.paint(listLayout, mount); - - const marker = mount.querySelector('.superdoc-list-marker'); - expect(marker?.textContent).toBe('1.'); - }); - - it('preserves marker-adjusted list-item wrapper geometry during resolved incremental updates', () => { - const listBlock: FlowBlock = { - kind: 'list', - id: 'list-1', - listType: 'number', - items: [ - { - id: 'item-1', - marker: { kind: 'number', text: '1.', level: 0, order: 1 }, - paragraph: block, - }, - ], - }; - - const listMeasure: Measure = { - kind: 'list', - items: [ - { - itemId: 'item-1', - markerWidth: 30, - markerTextWidth: 18, - indentLeft: 0, - paragraph: measure as ParagraphMeasure, - }, - ], - totalHeight: measure.totalHeight, - }; - - const initialLayout: Layout = { - pageSize: layout.pageSize, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-1', - itemId: 'item-1', - fromLine: 0, - toLine: 1, - x: 100, - y: 40, - width: 260, - markerWidth: 30, - }, - ], - }, - ], - }; - - const updatedLayout: Layout = { - pageSize: layout.pageSize, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-1', - itemId: 'item-1', - fromLine: 0, - toLine: 1, - x: 120, - y: 55, - width: 280, - markerWidth: 30, - }, - ], - }, - ], - }; - - const initialResolvedLayout: ResolvedLayout = { - version: 1, - flowMode: 'paginated', - pageGap: 0, - pages: [ - { - id: 'page-0', - index: 0, - number: 1, - width: 400, - height: 500, - items: [ - { - kind: 'fragment', - id: 'list-item:list-1:item-1:0:1', - pageIndex: 0, - x: 100, - y: 40, - width: 260, - height: 20, - fragmentKind: 'list-item', - fragment: initialLayout.pages[0].fragments[0], - blockId: 'list-1', - fragmentIndex: 0, - markerWidth: 30, - block: listBlock as import('@superdoc/contracts').ListBlock, - measure: listMeasure as import('@superdoc/contracts').ListMeasure, - }, - ], - }, - ], - }; - - const updatedResolvedLayout: ResolvedLayout = { - version: 1, - flowMode: 'paginated', - pageGap: 0, - pages: [ - { - id: 'page-0', - index: 0, - number: 1, - width: 400, - height: 500, - items: [ - { - kind: 'fragment', - id: 'list-item:list-1:item-1:0:1', - pageIndex: 0, - x: 120, - y: 55, - width: 280, - height: 20, - fragmentKind: 'list-item', - fragment: updatedLayout.pages[0].fragments[0], - blockId: 'list-1', - fragmentIndex: 0, - markerWidth: 30, - block: listBlock as import('@superdoc/contracts').ListBlock, - measure: listMeasure as import('@superdoc/contracts').ListMeasure, - }, - ], - }, - ], - }; - - const painter = createTestPainter({ blocks: [listBlock], measures: [listMeasure] }); - - painter.setResolvedLayout(initialResolvedLayout); - painter.paint(initialLayout, mount); - - const initialWrapper = mount.querySelector('.superdoc-fragment-list-item') as HTMLElement; - expect(initialWrapper.style.left).toBe('70px'); - expect(initialWrapper.style.top).toBe('40px'); - expect(initialWrapper.style.width).toBe('290px'); - - painter.setResolvedLayout(updatedResolvedLayout); - painter.paint(updatedLayout, mount); - - const updatedWrapper = mount.querySelector('.superdoc-fragment-list-item') as HTMLElement; - const updatedLine = updatedWrapper.querySelector('.superdoc-line') as HTMLElement; - expect(updatedWrapper).not.toBe(initialWrapper); - expect(updatedWrapper.style.left).toBe('90px'); - expect(updatedWrapper.style.top).toBe('55px'); - expect(updatedWrapper.style.width).toBe('310px'); - expect(updatedWrapper.dataset.layoutEpoch).toBeTruthy(); - expect(updatedLine.dataset.layoutEpoch).toBe(updatedWrapper.dataset.layoutEpoch); - }); - it('applies resolved zIndex only to anchored media fragments', () => { const anchoredDrawingBlock: FlowBlock = { kind: 'drawing', @@ -6744,85 +6508,6 @@ describe('DomPainter', () => { expectCssColor(shadingLayer.style.backgroundColor, '#ffeeaa'); }); - it('strips indent padding when rendering list content', () => { - const listBlock: FlowBlock = { - kind: 'list', - id: 'list-indent', - listType: 'number', - items: [ - { - id: 'item-1', - marker: { kind: 'number', text: '1.', level: 1, order: 1 }, - paragraph: { - kind: 'paragraph', - id: 'paragraph-list', - runs: [{ text: 'Indented body', fontFamily: 'Arial', fontSize: 16 }], - attrs: { indent: { left: 36, hanging: 18 } }, - }, - }, - ], - }; - - const paragraphMeasure: ParagraphMeasure = { - kind: 'paragraph', - lines: [ - { - fromRun: 0, - fromChar: 0, - toRun: 0, - toChar: 13, - width: 140, - ascent: 12, - descent: 4, - lineHeight: 18, - }, - ], - totalHeight: 18, - }; - - const listMeasure: Measure = { - kind: 'list', - items: [ - { - itemId: 'item-1', - markerWidth: 30, - markerTextWidth: 14, - indentLeft: 36, - paragraph: paragraphMeasure, - }, - ], - totalHeight: 18, - }; - - const listLayout: Layout = { - pageSize: layout.pageSize, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-indent', - itemId: 'item-1', - fromLine: 0, - toLine: 1, - x: 80, - y: 40, - width: 180, - markerWidth: 30, - }, - ], - }, - ], - }; - - const painter = createTestPainter({ blocks: [listBlock], measures: [listMeasure] }); - painter.paint(listLayout, mount); - - const content = mount.querySelector('.superdoc-list-content') as HTMLElement; - expect(content.style.paddingLeft).toBe(''); - }); - describe('line-level paragraph indent handling', () => { it('applies paragraph left/right indent to each line element', () => { const indentBlock: FlowBlock = { diff --git a/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts b/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts index ea641ee9d4..eeea052fc7 100644 --- a/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-dispatch.test.ts @@ -50,64 +50,6 @@ function paragraphFixtures() { return { blocks: [block], measures: [measure], layout }; } -function listItemFixtures() { - const block: FlowBlock = { - kind: 'list', - id: 'list-dispatch', - listType: 'bullet', - items: [ - { - id: 'item-dispatch', - marker: { kind: 'bullet', text: '•', level: 0 }, - paragraph: { - kind: 'paragraph', - id: 'list-para-dispatch', - runs: [{ text: 'Item', fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 5 }], - }, - }, - ], - }; - const measure: Measure = { - kind: 'list', - items: [ - { - itemId: 'item-dispatch', - markerWidth: 20, - markerTextWidth: 10, - indentLeft: 36, - paragraph: { - kind: 'paragraph', - lines: [{ fromRun: 0, fromChar: 0, toRun: 0, toChar: 4, width: 40, ascent: 10, descent: 4, lineHeight: 16 }], - totalHeight: 16, - }, - }, - ], - totalHeight: 16, - }; - const layout: Layout = { - pageSize: { w: 400, h: 500 }, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-dispatch', - itemId: 'item-dispatch', - fromLine: 0, - toLine: 1, - x: 50, - y: 0, - width: 250, - markerWidth: 20, - }, - ], - }, - ], - }; - return { blocks: [block], measures: [measure], layout }; -} - function imageFixtures() { const block: FlowBlock = { kind: 'image', @@ -337,16 +279,6 @@ describe('renderFragment dispatch', () => { expect(spy.mock.calls[0]![0].kind).toBe('para'); }); - it('routes list-item fragment to renderListItemFragment', () => { - const dummyDiv = document.createElement('div'); - const spy = vi.spyOn(DomPainter.prototype as any, 'renderListItemFragment').mockReturnValue(dummyDiv); - const { blocks, measures, layout } = listItemFixtures(); - const painter = createDomPainter({ blocks, measures }); - painter.paint(layout, container); - expect(spy).toHaveBeenCalledTimes(1); - expect(spy.mock.calls[0]![0].kind).toBe('list-item'); - }); - it('routes image fragment to renderImageFragment', () => { const dummyDiv = document.createElement('div'); const spy = vi.spyOn(DomPainter.prototype as any, 'renderImageFragment').mockReturnValue(dummyDiv); diff --git a/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts b/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts index eb79122ec9..e668ed19eb 100644 --- a/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts +++ b/packages/layout-engine/painters/dom/src/renderer-hanging-indent.test.ts @@ -692,7 +692,7 @@ describe('DomPainter hanging indent with tabs', () => { // paddingLeft should be: left (100) + firstLine (500) = 600px expect(lineEl.style.paddingLeft).toBe('600px'); - // Tab element should exist and have width equal to LIST_MARKER_GAP (8px) + // Tab element should preserve the computed marker-to-text gap. const tabEl = lineEl.querySelector('.superdoc-tab') as HTMLElement; expect(tabEl).toBeTruthy(); expect(tabEl.style.width).toBe('10px'); diff --git a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts b/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts deleted file mode 100644 index 92706dc9c5..0000000000 --- a/packages/layout-engine/painters/dom/src/renderer-known-divergences.test.ts +++ /dev/null @@ -1,317 +0,0 @@ -/** - * Known divergence lock tests. - * - * Rule: This file contains ONLY intentional mismatches between rendering - * paths. Every test has a Resolution target comment. When a future PR - * resolves the divergence, it deletes the corresponding test(s). - * - * If a new divergence is discovered during PR 0 implementation, it is added - * here with a resolution target — not fixed in production code. - */ - -import { describe, it, expect } from 'vitest'; -import { createTestPainter as createDomPainter } from './_test-utils.js'; -import type { FlowBlock, Measure, Layout, Line } from '@superdoc/contracts'; -import { normalizeLines } from './test-utils/normalize-line.js'; - -// --------------------------------------------------------------------------- -// Shared constants and helpers -// --------------------------------------------------------------------------- - -const JUSTIFY_TEXT = 'The quick brown fox jumps over'; -const JUSTIFY_TEXT_LEN = JUSTIFY_TEXT.length; -const LINE_WIDTH = 150; -const FRAGMENT_WIDTH = 300; - -function makeLine(fromChar: number, toChar: number, overrides?: Partial): Line { - return { - fromRun: 0, - fromChar, - toRun: 0, - toChar, - width: LINE_WIDTH, - maxWidth: FRAGMENT_WIDTH, - ascent: 10, - descent: 4, - lineHeight: 16, - ...overrides, - }; -} - -// --------------------------------------------------------------------------- -// List-item fixtures -// --------------------------------------------------------------------------- - -function listItemFixtures(opts: { alignment?: string; lines?: Line[] }) { - const halfLen = Math.floor(JUSTIFY_TEXT_LEN / 2); - const lines = opts.lines ?? [makeLine(0, halfLen), makeLine(halfLen, JUSTIFY_TEXT_LEN)]; - - const block: FlowBlock = { - kind: 'list', - id: 'list-div', - listType: 'number', - items: [ - { - id: 'item-div', - marker: { kind: 'number', text: '1.', level: 0 }, - paragraph: { - kind: 'paragraph', - id: 'list-para-div', - runs: [{ text: JUSTIFY_TEXT, fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 1 + JUSTIFY_TEXT_LEN }], - ...(opts.alignment ? { attrs: { alignment: opts.alignment } } : {}), - }, - }, - ], - }; - - const measure: Measure = { - kind: 'list', - items: [ - { - itemId: 'item-div', - markerWidth: 24, - markerTextWidth: 12, - indentLeft: 36, - paragraph: { - kind: 'paragraph', - lines, - totalHeight: lines.reduce((h, l) => h + l.lineHeight, 0), - }, - }, - ], - totalHeight: lines.reduce((h, l) => h + l.lineHeight, 0), - }; - - const layout: Layout = { - pageSize: { w: 600, h: 800 }, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'list-item', - blockId: 'list-div', - itemId: 'item-div', - fromLine: 0, - toLine: lines.length, - x: 60, - y: 40, - width: FRAGMENT_WIDTH, - markerWidth: 24, - }, - ], - }, - ], - }; - - return { blocks: [block], measures: [measure], layout }; -} - -// --------------------------------------------------------------------------- -// Body/table-cell fixtures (for three-way comparison) -// --------------------------------------------------------------------------- - -function bodyFixtures(alignment: string) { - const halfLen = Math.floor(JUSTIFY_TEXT_LEN / 2); - const lines = [makeLine(0, halfLen), makeLine(halfLen, JUSTIFY_TEXT_LEN)]; - - const block: FlowBlock = { - kind: 'paragraph', - id: 'body-para-div', - runs: [{ text: JUSTIFY_TEXT, fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 1 + JUSTIFY_TEXT_LEN }], - attrs: { alignment }, - }; - - const measure: Measure = { - kind: 'paragraph', - lines, - totalHeight: lines.reduce((h, l) => h + l.lineHeight, 0), - }; - - const layout: Layout = { - pageSize: { w: 600, h: 800 }, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'para', - blockId: 'body-para-div', - fromLine: 0, - toLine: lines.length, - x: 30, - y: 40, - width: FRAGMENT_WIDTH, - pmStart: 1, - pmEnd: 1 + JUSTIFY_TEXT_LEN, - }, - ], - }, - ], - }; - - return { blocks: [block], measures: [measure], layout }; -} - -function tableCellFixtures(alignment: string) { - const halfLen = Math.floor(JUSTIFY_TEXT_LEN / 2); - const lines = [makeLine(0, halfLen), makeLine(halfLen, JUSTIFY_TEXT_LEN)]; - - const block: FlowBlock = { - kind: 'table', - id: 'table-div', - rows: [ - { - id: 'row-0', - cells: [ - { - id: 'cell-0', - blocks: [ - { - kind: 'paragraph', - id: 'table-para-div', - runs: [ - { text: JUSTIFY_TEXT, fontFamily: 'Arial', fontSize: 12, pmStart: 1, pmEnd: 1 + JUSTIFY_TEXT_LEN }, - ], - attrs: { alignment }, - }, - ], - attrs: {}, - }, - ], - }, - ], - }; - - const measure: Measure = { - kind: 'table', - rows: [ - { - height: 40, - cells: [ - { - width: FRAGMENT_WIDTH, - height: 40, - gridColumnStart: 0, - blocks: [ - { - kind: 'paragraph', - lines, - totalHeight: lines.reduce((h, l) => h + l.lineHeight, 0), - }, - ], - }, - ], - }, - ], - columnWidths: [FRAGMENT_WIDTH], - totalWidth: FRAGMENT_WIDTH, - totalHeight: 40, - }; - - const layout: Layout = { - pageSize: { w: 600, h: 800 }, - pages: [ - { - number: 1, - fragments: [ - { - kind: 'table', - blockId: 'table-div', - fromRow: 0, - toRow: 1, - x: 30, - y: 40, - width: FRAGMENT_WIDTH, - height: 40, - }, - ], - }, - ], - }; - - return { blocks: [block], measures: [measure], layout }; -} - -function renderAndNormalize(fixtures: { blocks: FlowBlock[]; measures: Measure[]; layout: Layout }) { - const container = document.createElement('div'); - const painter = createDomPainter({ blocks: fixtures.blocks, measures: fixtures.measures }); - painter.paint(fixtures.layout, container); - return normalizeLines(container); -} - -// --------------------------------------------------------------------------- -// Tests -// --------------------------------------------------------------------------- - -describe('known divergences (frozen — delete when resolved)', () => { - // ----------------------------------------------------------------------- - // Resolution target: PR 7 (collapse list-item parallel paint model) - // ----------------------------------------------------------------------- - - describe('list-item force-left alignment — Resolution target: PR 7 (collapse list-item parallel paint model)', () => { - it('list-item fragment forces textAlign to left even when paragraph alignment is justify', () => { - const container = document.createElement('div'); - const fix = listItemFixtures({ alignment: 'justify' }); - const painter = createDomPainter({ blocks: fix.blocks, measures: fix.measures }); - painter.paint(fix.layout, container); - - // The list-item content div overrides textAlign to 'left' - const contentEl = container.querySelector('.superdoc-list-content') as HTMLElement | null; - expect(contentEl).not.toBeNull(); - expect(contentEl!.style.textAlign).toBe('left'); - }); - - it('list-item fragment forces textAlign to left even when paragraph alignment is center', () => { - const container = document.createElement('div'); - const fix = listItemFixtures({ alignment: 'center' }); - const painter = createDomPainter({ blocks: fix.blocks, measures: fix.measures }); - painter.paint(fix.layout, container); - - const contentEl = container.querySelector('.superdoc-list-content') as HTMLElement | null; - expect(contentEl).not.toBeNull(); - expect(contentEl!.style.textAlign).toBe('left'); - }); - }); - - // ----------------------------------------------------------------------- - // Resolution target: PR 7 (collapse list-item parallel paint model) - // ----------------------------------------------------------------------- - - describe('list-item skip-justify on all lines — Resolution target: PR 7 (collapse list-item parallel paint model)', () => { - it('multi-line list-item produces zero wordSpacing on every line including non-last', () => { - // 2-line list item with alignment: 'justify' and multi-word text - const listLines = renderAndNormalize(listItemFixtures({ alignment: 'justify' })); - - expect(listLines.length).toBe(2); - // ALL lines have no word-spacing — body/table-cell would justify non-last lines - for (const line of listLines) { - expect(line.wordSpacing).toBe(''); - } - }); - }); - - // ----------------------------------------------------------------------- - // Resolution target: PR 5–7 (shared flow + list-item collapse) - // ----------------------------------------------------------------------- - - describe('three-way justify snapshot — Resolution target: PR 5–7 (shared flow + list-item collapse)', () => { - it('body and table-cell justify non-last lines; list-item does not', () => { - // Same logical paragraph (alignment: 'justify', 2 lines, multi-word text) - const bodyLines = renderAndNormalize(bodyFixtures('justify')); - const tableLines = renderAndNormalize(tableCellFixtures('justify')); - const listLines = renderAndNormalize(listItemFixtures({ alignment: 'justify' })); - - expect(bodyLines.length).toBe(2); - expect(tableLines.length).toBe(2); - expect(listLines.length).toBe(2); - - // Parity: body and table-cell line 0 wordSpacing should match and be > 0 - expect(bodyLines[0]!.wordSpacing).not.toBe(''); - expect(bodyLines[0]!.wordSpacing).toBe(tableLines[0]!.wordSpacing); - - // Divergence: list-item line 0 wordSpacing is empty (no justify) - expect(listLines[0]!.wordSpacing).toBe(''); - }); - }); -}); diff --git a/packages/layout-engine/painters/dom/src/renderer.ts b/packages/layout-engine/painters/dom/src/renderer.ts index a9903dde5e..4ce4d07110 100644 --- a/packages/layout-engine/painters/dom/src/renderer.ts +++ b/packages/layout-engine/painters/dom/src/renderer.ts @@ -19,9 +19,6 @@ import type { ImageRun, Line, LineSegment, - ListBlock, - ListItemFragment, - ListMeasure, PageMargins, ParaFragment, ParagraphAttrs, @@ -799,7 +796,6 @@ function collectLineTabsForSnapshot(lineEl: HTMLElement): PaintSnapshotTabStyle[ return tabs; } -const LIST_MARKER_GAP = 8; /** * Default page height in pixels (11 inches at 96 DPI). * Used as a fallback when page size information is not available for ruler rendering. @@ -3006,15 +3002,6 @@ export class DomPainter { resolvedItem as ResolvedFragmentItem | undefined, ); } - if (fragment.kind === 'list-item') { - return this.renderListItemFragment( - fragment, - context, - sdtBoundary, - betweenInfo, - resolvedItem as ResolvedFragmentItem | undefined, - ); - } if (fragment.kind === 'image') { return this.renderImageFragment(fragment, context, resolvedItem as ResolvedImageItem | undefined); } @@ -3603,165 +3590,6 @@ export class DomPainter { return dropCapEl; } - private renderListItemFragment( - fragment: ListItemFragment, - context: FragmentRenderContext, - sdtBoundary?: SdtBoundaryOptions, - betweenInfo?: BetweenBorderInfo, - resolvedItem?: ResolvedFragmentItem, - ): HTMLElement { - try { - if (!this.doc) { - throw new Error('DomPainter: document is not available'); - } - - // Pre-extracted block/measure from the resolved item. - if (resolvedItem?.block?.kind !== 'list' || resolvedItem?.measure?.kind !== 'list') { - throw new Error(`DomPainter: missing resolved list block/measure for fragment ${fragment.blockId}`); - } - const block = resolvedItem.block as ListBlock; - const measure = resolvedItem.measure as ListMeasure; - const item = block.items.find((entry) => entry.id === fragment.itemId); - const itemMeasure = measure.items.find((entry) => entry.itemId === fragment.itemId); - if (!item || !itemMeasure) { - throw new Error(`DomPainter: missing list item ${fragment.itemId}`); - } - - // Prefer resolved item metadata over legacy fragment reads - const listContinuesFromPrev = resolvedItem?.continuesFromPrev; - const listContinuesOnNext = resolvedItem?.continuesOnNext; - // Default to 0 (no marker gutter) when absent — used directly in Math.max below. - const listMarkerWidth = resolvedItem?.markerWidth ?? 0; - - const fragmentEl = this.doc.createElement('div'); - fragmentEl.classList.add(CLASS_NAMES.fragment, `${CLASS_NAMES.fragment}-list-item`); - applyStyles(fragmentEl, fragmentStyles); - if (resolvedItem) { - this.applyResolvedListItemWrapperFrame(fragmentEl, fragment, resolvedItem, context.section); - } else { - fragmentEl.style.left = `${fragment.x - fragment.markerWidth}px`; - fragmentEl.style.top = `${fragment.y}px`; - fragmentEl.style.width = `${fragment.markerWidth + fragment.width}px`; - fragmentEl.dataset.blockId = fragment.blockId; - applySourceAnchorDataset(fragmentEl, fragment.sourceAnchor); - } - fragmentEl.dataset.itemId = fragment.itemId; - - const paragraphMetadata = item.paragraph.attrs?.sdt; - this.applySdtDataset(fragmentEl, paragraphMetadata); - - // Apply SDT container styling (document sections, structured content blocks) - applySdtContainerStyling( - this.doc, - fragmentEl, - paragraphMetadata, - item.paragraph.attrs?.containerSdt, - sdtBoundary, - ); - - if (listContinuesFromPrev) { - fragmentEl.dataset.continuesFromPrev = 'true'; - } - if (listContinuesOnNext) { - fragmentEl.dataset.continuesOnNext = 'true'; - } - - const markerEl = this.doc.createElement('span'); - markerEl.classList.add(DOM_CLASS_NAMES.LIST_MARKER); - applySourceAnchorDataset(markerEl, item.marker.sourceAnchor ?? item.sourceAnchor ?? resolvedItem?.sourceAnchor); - - // Track B: Use marker styling from wordLayout if available - const wordLayout: MinimalWordLayout | undefined = item.paragraph.attrs?.wordLayout as - | MinimalWordLayout - | undefined; - const marker = wordLayout?.marker; - if (marker) { - markerEl.textContent = marker.markerText ?? null; - markerEl.style.display = 'inline-block'; - markerEl.style.width = `${Math.max(0, listMarkerWidth - LIST_MARKER_GAP)}px`; - markerEl.style.paddingRight = `${LIST_MARKER_GAP}px`; - markerEl.style.textAlign = marker.justification ?? 'left'; - - // Apply marker run styling with font fallback chain - markerEl.style.fontFamily = toCssFontFamily(marker.run.fontFamily) ?? marker.run.fontFamily; - markerEl.style.fontSize = `${marker.run.fontSize}px`; - if (marker.run.bold) markerEl.style.fontWeight = 'bold'; - if (marker.run.italic) markerEl.style.fontStyle = 'italic'; - if (marker.run.color) markerEl.style.color = marker.run.color; - if (marker.run.letterSpacing) markerEl.style.letterSpacing = `${marker.run.letterSpacing}px`; - } else { - // Fallback: legacy behavior - markerEl.textContent = item.marker.text; - markerEl.style.display = 'inline-block'; - markerEl.style.width = `${Math.max(0, listMarkerWidth - LIST_MARKER_GAP)}px`; - markerEl.style.paddingRight = `${LIST_MARKER_GAP}px`; - if (item.marker.align) { - markerEl.style.textAlign = item.marker.align; - } - } - fragmentEl.appendChild(markerEl); - - const contentEl = this.doc.createElement('div'); - contentEl.classList.add('superdoc-list-content'); - this.applySdtDataset(contentEl, paragraphMetadata); - contentEl.style.display = 'inline-block'; - contentEl.style.position = 'relative'; - contentEl.style.width = `${fragment.width}px`; - const lines = itemMeasure.paragraph.lines.slice(fragment.fromLine, fragment.toLine); - // 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, - betweenInfo, - ); - if (shadingLayer) { - contentEl.appendChild(shadingLayer); - } - if (borderLayer) { - contentEl.appendChild(borderLayer); - } - 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 - // and current behavior. This is a documented design decision, not a bug. - // Applied AFTER applyParagraphBlockStyles (which may set justify from paragraph properties). - contentEl.style.textAlign = 'left'; - // Override alignment to left for list content rendering - const paraForList: ParagraphBlock = { - ...item.paragraph, - attrs: { ...(item.paragraph.attrs || {}), alignment: 'left' }, - }; - const expandedRunsForList = expandRunsForInlineNewlines(paraForList.runs); - lines.forEach((line, idx) => { - const lineEl = this.renderLine( - paraForList, - line, - context, - fragment.width, - fragment.fromLine + idx, - true, - expandedRunsForList, - ); - this.capturePaintSnapshotLine(lineEl, context, { - inTableFragment: false, - inTableParagraph: false, - sourceAnchor: resolvedItem?.sourceAnchor, - }); - contentEl.appendChild(lineEl); - }); - fragmentEl.appendChild(contentEl); - - return fragmentEl; - } catch (error) { - console.error('[DomPainter] List item fragment rendering failed:', { fragment, error }); - return this.createErrorPlaceholder(fragment.blockId, error); - } - } - private renderImageFragment( fragment: ImageFragment, context: FragmentRenderContext, @@ -7008,11 +6836,6 @@ export class DomPainter { // Narrow to fragment-kind resolved items (excludes ResolvedGroupItem) const fragmentItem = resolvedItem?.kind === 'fragment' ? resolvedItem : undefined; - if (fragment.kind === 'list-item' && fragmentItem) { - this.applyResolvedListItemWrapperFrame(el, fragment, fragmentItem as ResolvedFragmentItem, section); - return; - } - if (fragmentItem) { this.applyResolvedFragmentFrame(el, fragmentItem, fragment, section); } else { @@ -7209,33 +7032,12 @@ export class DomPainter { this.applyFragmentPmAttributes(el, fragment, section, item); } - /** - * Applies the resolved wrapper frame for a list-item fragment. - * - * List-item wrappers intentionally extend into the marker gutter. The resolved - * fragment item stores the paragraph content box, so the marker-width expansion - * must be applied consistently on both initial render and incremental updates. - */ - private applyResolvedListItemWrapperFrame( - el: HTMLElement, - fragment: ListItemFragment, - item: ResolvedFragmentItem, - section?: 'body' | 'header' | 'footer', - ): void { - this.applyResolvedFragmentFrame(el, item, fragment, section); - // Default to 0 (no marker gutter expansion) when markerWidth is absent — the resolve - // stage populates this for list items that have a measured marker (SD-2957). - const mw = item.markerWidth ?? 0; - el.style.left = `${item.x - mw}px`; - el.style.width = `${item.width + mw}px`; - } - /** * Estimates the height of a fragment when explicit height is not available. * * This method provides fallback height calculations for footer bottom-alignment - * by consulting measure data for paragraphs and list items, or using the - * fragment's height property for tables, images, and drawings. + * from resolved layout data, or using the fragment's height property for + * tables, images, and drawings. * * @param fragment - The fragment to estimate height for * @returns Estimated height in pixels, or 0 if height cannot be determined @@ -7510,29 +7312,28 @@ const computeSdtBoundaries = ( // computeBetweenBorderFlags — moved to features/paragraph-borders/ const fragmentKey = (fragment: Fragment): string => { - if (fragment.kind === 'para') { - return `para:${fragment.blockId}:${fragment.fromLine}:${fragment.toLine}`; - } - if (fragment.kind === 'list-item') { - return `list-item:${fragment.blockId}:${fragment.itemId}:${fragment.fromLine}:${fragment.toLine}`; - } - if (fragment.kind === 'image') { - return `image:${fragment.blockId}:${fragment.x}:${fragment.y}`; - } - if (fragment.kind === 'drawing') { - return `drawing:${fragment.blockId}:${fragment.x}:${fragment.y}`; - } - if (fragment.kind === 'table') { - // Include row range and partial row info to uniquely identify table fragments - // This is critical for mid-row splitting where multiple fragments can exist for the same table - const partialKey = fragment.partialRow - ? `:${fragment.partialRow.fromLineByCell.join(',')}-${fragment.partialRow.toLineByCell.join(',')}` - : ''; - return `table:${fragment.blockId}:${fragment.fromRow}:${fragment.toRow}${partialKey}`; + switch (fragment.kind) { + case 'para': + return `para:${fragment.blockId}:${fragment.fromLine}:${fragment.toLine}`; + case 'list-item': + throw new Error(`DomPainter: unsupported fragment kind ${fragment.kind}`); + case 'image': + return `image:${fragment.blockId}:${fragment.x}:${fragment.y}`; + case 'drawing': + return `drawing:${fragment.blockId}:${fragment.x}:${fragment.y}`; + case 'table': { + // Include row range and partial row info to uniquely identify table fragments + // This is critical for mid-row splitting where multiple fragments can exist for the same table + const partialKey = fragment.partialRow + ? `:${fragment.partialRow.fromLineByCell.join(',')}-${fragment.partialRow.toLineByCell.join(',')}` + : ''; + return `table:${fragment.blockId}:${fragment.fromRow}:${fragment.toRow}${partialKey}`; + } + default: { + const _exhaustiveCheck: never = fragment; + return _exhaustiveCheck; + } } - // Exhaustive check - all fragment kinds should be handled above - const _exhaustiveCheck: never = fragment; - return _exhaustiveCheck; }; const hasFragmentGeometryChanged = (previous: Fragment, next: Fragment): boolean => @@ -7789,10 +7590,6 @@ const deriveBlockVersion = (block: FlowBlock): string => { return parts.join('|'); } - if (block.kind === 'list') { - return block.items.map((item) => `${item.id}:${item.marker.text}:${deriveBlockVersion(item.paragraph)}`).join('|'); - } - if (block.kind === 'image') { const imgSdt = (block as ImageBlock).attrs?.sdt; const imgSdtVersion = getSdtMetadataVersion(imgSdt); @@ -8221,19 +8018,6 @@ const applyParagraphBlockStyles = (element: HTMLElement, attrs?: ParagraphAttrs) // getParagraphBorderBox, createParagraphDecorationLayers, applyParagraphBorderStyles, // setBorderSideStyle, applyParagraphShadingStyles — moved to features/paragraph-borders/ -const stripListIndent = (attrs?: ParagraphAttrs): ParagraphAttrs | undefined => { - if (!attrs?.indent || attrs.indent.left == null) { - return attrs; - } - const nextIndent = { ...attrs.indent }; - delete nextIndent.left; - - return { - ...attrs, - indent: Object.keys(nextIndent).length > 0 ? nextIndent : undefined, - }; -}; - // applyParagraphShadingStyles — moved to features/paragraph-borders/border-layer.ts const applyStyles = (el: HTMLElement, styles: Partial): void => { diff --git a/packages/layout-engine/painters/dom/src/test-utils/normalize-line.ts b/packages/layout-engine/painters/dom/src/test-utils/normalize-line.ts index 2484c34486..9910c119d1 100644 --- a/packages/layout-engine/painters/dom/src/test-utils/normalize-line.ts +++ b/packages/layout-engine/painters/dom/src/test-utils/normalize-line.ts @@ -1,6 +1,6 @@ /** * Test-only normalization helpers for comparing rendered line output - * across different rendering contexts (body, table-cell, list-item). + * across different rendering contexts (body, table-cell). * * DO NOT import this file from production code. Only *.test.ts and * other test-utils/ files may import from here.