Skip to content

refactor(painters/dom): drop list-item fragment renderer (SD-2851) #3269

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
luccas-harbour wants to merge 1 commit into
base: main
Choose a base branch
from
Open

refactor(painters/dom): drop list-item fragment renderer (SD-2851) #3269

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 4 additions & 84 deletions packages/layout-engine/painters/dom/src/between-borders.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,11 @@ import type {
ParagraphBorders,
ParagraphBorder,
ParagraphBlock,
ListBlock,
Fragment,
FlowBlock,
Layout,
Measure,
ParaFragment,
ListItemFragment,
ImageFragment,
ResolvedPaintItem,
ResolvedFragmentItem,
Expand All @@ -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<ParagraphBlock | ListBlock>;
type TestBlockList = ReadonlyArray<ParagraphBlock>;

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]));
Expand All @@ -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 = {
Expand Down Expand Up @@ -127,23 +106,6 @@ const paraFragment = (blockId: string, overrides?: Partial<ParaFragment>): ParaF
...overrides,
});

const listItemFragment = (
blockId: string,
itemId: string,
overrides?: Partial<ListItemFragment>,
): 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,
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
21 changes: 7 additions & 14 deletions packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
Loading
Loading