Skip to content

fix: suppress spacing before after manual page breaks #3249

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
leandrotcawork wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: suppress spacing before after manual page breaks #3249

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
162 changes: 161 additions & 1 deletion packages/layout-engine/layout-engine/src/index.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -472,6 +472,37 @@ describe('layoutDocument', () => {
expect(layout.pages.length).toBe(1);
});

it('suppresses spacingBefore for a paragraph after an explicit page break', () => {
const introBlock: FlowBlock = {
kind: 'paragraph',
id: 'intro-on-page-one',
runs: [],
};
const pageBreak: FlowBlock = {
kind: 'pageBreak',
id: 'manual-page-break',
} as PageBreakBlock;
const headingBlock: FlowBlock = {
kind: 'paragraph',
id: 'heading-at-page-start',
runs: [],
attrs: {
spacing: { before: 24, after: 8 },
},
};

const layout = layoutDocument(
[introBlock, pageBreak, headingBlock],
[makeMeasure([20]), { kind: 'pageBreak' }, makeMeasure([20])],
DEFAULT_OPTIONS,
);

expect(layout.pages).toHaveLength(2);
expect(layout.pages[0].fragments).toHaveLength(1);
expect(layout.pages[1].fragments).toHaveLength(1);
expect(layout.pages[1].fragments[0].y).toBe(DEFAULT_OPTIONS.margins!.top);
});

it('handles spacingBefore equal to content area height (boundary condition)', () => {
// Edge case: spacingBefore exactly equals the content area height.
// This triggers the infinite loop guard after advancing to a new page.
Expand Down Expand Up @@ -2258,7 +2289,7 @@ describe('layoutDocument', () => {
{ kind: 'paragraph', id: 'p1', runs: [] },
{ kind: 'sectionBreak', id: 'sb-next', type: 'nextPage', margins: {} } as SectionBreakBlock,
{ kind: 'pageBreak', id: 'pb-before-exhibit', attrs: { source: 'pageBreakBefore' } } as PageBreakBlock,
{ kind: 'paragraph', id: 'p2', runs: [] },
{ kind: 'paragraph', id: 'p2', runs: [], attrs: { spacing: { before: 24 } } },
];

const measures: Measure[] = [
Expand All @@ -2273,6 +2304,135 @@ describe('layoutDocument', () => {
expect(layout.pages).toHaveLength(2);
expect(pageContainsBlock(layout.pages[1], 'p2')).toBe(true);
expect(layout.pages[1].fragments).toHaveLength(1);
expect(layout.pages[1].fragments[0].y).toBe(pageBreakBoundaryOptions.margins!.top + 24);
});

it('preserves spacingBefore after pageBreakBefore', () => {
const blocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'p1', runs: [] },
{ kind: 'pageBreak', id: 'pb-before-exhibit', attrs: { source: 'pageBreakBefore' } } as PageBreakBlock,
{ kind: 'paragraph', id: 'p2', runs: [], attrs: { spacing: { before: 24 } } },
];

const measures: Measure[] = [makeMeasure([40]), { kind: 'pageBreak' }, makeMeasure([40])];

const layout = layoutDocument(blocks, measures, pageBreakBoundaryOptions);

expect(layout.pages).toHaveLength(2);
expect(pageContainsBlock(layout.pages[1], 'p2')).toBe(true);
expect(layout.pages[1].fragments[0].y).toBe(pageBreakBoundaryOptions.margins!.top + 24);
});

it('suppresses spacingBefore after a manual page break when a section break precedes the paragraph', () => {
const blocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'p1', runs: [] },
{ kind: 'pageBreak', id: 'pb-manual', attrs: { lineBreakType: 'page' } } as PageBreakBlock,
{ kind: 'paragraph', id: 'p-sectpr-marker', runs: [], attrs: { sectPrMarker: true } },
{ kind: 'sectionBreak', id: 'sb-continuous', type: 'continuous', margins: {} } as SectionBreakBlock,
{ kind: 'paragraph', id: 'p2', runs: [], attrs: { spacing: { before: 24 } } },
];

const measures: Measure[] = [
makeMeasure([40]),
{ kind: 'pageBreak' },
makeMeasure([]),
{ kind: 'sectionBreak' },
makeMeasure([40]),
];

const layout = layoutDocument(blocks, measures, pageBreakBoundaryOptions);
const para = layout.pages[1].fragments.find((fragment) => fragment.blockId === 'p2') as ParaFragment;

expect(para).toBeTruthy();
expect(para.y).toBe(pageBreakBoundaryOptions.margins!.top);
});

it('suppresses spacingBefore after a manual page break when an anchored image precedes the paragraph', () => {
const anchoredImage: ImageBlock = {
kind: 'image',
id: 'anchored-image',
src: 'data:image/png;base64,xxx',
anchor: {
isAnchored: true,
hRelativeFrom: 'column',
vRelativeFrom: 'paragraph',
offsetH: 0,
offsetV: 0,
},
wrap: {
type: 'Square',
wrapText: 'right',
},
attrs: {
anchorParagraphId: 'p2',
},
};
const imageMeasure: ImageMeasure = {
kind: 'image',
width: 40,
height: 40,
};
const blocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'p1', runs: [] },
{ kind: 'pageBreak', id: 'pb-manual', attrs: { lineBreakType: 'page' } } as PageBreakBlock,
anchoredImage,
{ kind: 'paragraph', id: 'p2', runs: [], attrs: { spacing: { before: 24 } } },
];

const measures: Measure[] = [makeMeasure([40]), { kind: 'pageBreak' }, imageMeasure, makeMeasure([40])];

const layout = layoutDocument(blocks, measures, pageBreakBoundaryOptions);
const para = layout.pages[1].fragments.find((fragment) => fragment.blockId === 'p2') as ParaFragment;

expect(para).toBeTruthy();
expect(para.y).toBe(pageBreakBoundaryOptions.margins!.top);
});

it('suppresses spacingBefore after a manual page break when an anchored drawing precedes the paragraph', () => {
const anchoredDrawing: FlowBlock = {
kind: 'drawing',
id: 'anchored-drawing',
drawingKind: 'vectorShape',
geometry: { width: 40, height: 40, rotation: 0 },
anchor: {
isAnchored: true,
hRelativeFrom: 'column',
vRelativeFrom: 'paragraph',
offsetH: 0,
offsetV: 0,
},
wrap: {
type: 'Square',
wrapText: 'right',
},
attrs: {
anchorParagraphId: 'p2',
},
};
const drawingMeasure: DrawingMeasure = {
kind: 'drawing',
drawingKind: 'vectorShape',
width: 40,
height: 40,
scale: 1,
naturalWidth: 40,
naturalHeight: 40,
geometry: { width: 40, height: 40, rotation: 0, flipH: false, flipV: false },
};
const blocks: FlowBlock[] = [
{ kind: 'paragraph', id: 'p1', runs: [] },
{ kind: 'pageBreak', id: 'pb-manual', attrs: { lineBreakType: 'page' } } as PageBreakBlock,
anchoredDrawing,
{ kind: 'paragraph', id: 'p2', runs: [], attrs: { spacing: { before: 24 } } },
];

const measures: Measure[] = [makeMeasure([40]), { kind: 'pageBreak' }, drawingMeasure, makeMeasure([40])];

const layout = layoutDocument(blocks, measures, pageBreakBoundaryOptions);
const para = layout.pages[1].fragments.find((fragment) => fragment.blockId === 'p2') as ParaFragment;

expect(para).toBeTruthy();
expect(para.y).toBe(pageBreakBoundaryOptions.margins!.top);
});

it('still honors manual page breaks after a fresh page boundary', () => {
Expand Down
81 changes: 67 additions & 14 deletions packages/layout-engine/layout-engine/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,6 +634,70 @@ const shouldSkipRedundantPageBreakBefore = (block: PageBreakBlock, state: PageSt
return isAtTopOfFreshPage;
};

const isManualPageBreak = (block: FlowBlock | undefined): block is PageBreakBlock => {
return block?.kind === 'pageBreak' && block.attrs?.source !== 'pageBreakBefore';
};

const sectionBreakForcesPageBoundary = (sectionBreak: SectionBreakBlock | null | undefined): boolean => {
const breakType = sectionBreak?.type ?? (sectionBreak?.attrs?.source === 'sectPr' ? 'nextPage' : undefined);
return (
sectionBreak != null &&
(breakType === 'nextPage' ||
breakType === 'evenPage' ||
breakType === 'oddPage' ||
sectionBreak.attrs?.requirePageBoundary === true)
);
};

const isEmptyParagraphBlock = (block: FlowBlock): block is ParagraphBlock => {
if (block.kind !== 'paragraph') return false;

const runs = block.runs ?? [];
return (
runs.length === 0 ||
(runs.length === 1 &&
(!runs[0].kind || runs[0].kind === 'text') &&
(!(runs[0] as { text?: string }).text || (runs[0] as { text?: string }).text === ''))
);
};

const shouldSkipParagraphInManualBreakScan = (blocks: FlowBlock[], index: number): boolean => {
const block = blocks[index];
if (!isEmptyParagraphBlock(block)) return false;

const isSectPrMarker = block.attrs?.sectPrMarker === true;
const prevBlock = index > 0 ? blocks[index - 1] : undefined;
const nextBlock = index < blocks.length - 1 ? blocks[index + 1] : undefined;
const nextSectionBreak = nextBlock?.kind === 'sectionBreak' ? (nextBlock as SectionBreakBlock) : undefined;

return (
(isSectPrMarker && sectionBreakForcesPageBoundary(nextSectionBreak)) ||
(prevBlock?.kind === 'pageBreak' && nextSectionBreak != null)
);
};

const hasManualPageBreakBeforeParagraph = (
blocks: FlowBlock[],
paragraphIndex: number,
anchorsForPara?: { block: ImageBlock | DrawingBlock }[],
): boolean => {
if (isManualPageBreak(blocks[paragraphIndex - 1])) return true;

const anchorIds = new Set(anchorsForPara?.map((entry) => entry.block.id) ?? []);
for (let index = paragraphIndex - 1; index >= 0; index -= 1) {
const block = blocks[index];
if ((block.kind === 'image' || block.kind === 'drawing') && anchorIds.has(block.id)) {
continue;
}
if (block.kind === 'sectionBreak' || shouldSkipParagraphInManualBreakScan(blocks, index)) {
continue;
}
return isManualPageBreak(block);
Comment thread
leandrotcawork marked this conversation as resolved.
}

return false;
};

const hasOnlySectionBreakBlocks = (blocks: readonly FlowBlock[]): boolean => {
return blocks.length > 0 && blocks.every((block) => block.kind === 'sectionBreak');
};
Expand Down Expand Up @@ -2161,12 +2225,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
// Skip empty paragraphs that appear between a pageBreak and a sectionBreak
// (Word sectPr marker paragraphs should not create visible content)
const paraBlock = block as ParagraphBlock;
const isEmpty =
!paraBlock.runs ||
paraBlock.runs.length === 0 ||
(paraBlock.runs.length === 1 &&
(!paraBlock.runs[0].kind || paraBlock.runs[0].kind === 'text') &&
(!(paraBlock.runs[0] as { text?: string }).text || (paraBlock.runs[0] as { text?: string }).text === ''));
const isEmpty = isEmptyParagraphBlock(paraBlock);

if (isEmpty) {
const isSectPrMarker = paraBlock.attrs?.sectPrMarker === true;
Expand All @@ -2175,14 +2234,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
const nextBlock = index < blocks.length - 1 ? blocks[index + 1] : null;

const nextSectionBreak = nextBlock?.kind === 'sectionBreak' ? (nextBlock as SectionBreakBlock) : null;
const nextBreakType =
nextSectionBreak?.type ?? (nextSectionBreak?.attrs?.source === 'sectPr' ? 'nextPage' : undefined);
const nextBreakForcesPage =
nextSectionBreak &&
(nextBreakType === 'nextPage' ||
nextBreakType === 'evenPage' ||
nextBreakType === 'oddPage' ||
nextSectionBreak.attrs?.requirePageBoundary === true);
const nextBreakForcesPage = sectionBreakForcesPageBoundary(nextSectionBreak);

if (isSectPrMarker && nextBreakForcesPage) {
continue;
Expand Down Expand Up @@ -2365,6 +2417,7 @@ export function layoutDocument(blocks: FlowBlock[], measures: Measure[], options
floatManager,
remeasureParagraph: options.remeasureParagraph,
overrideSpacingAfter,
suppressSpacingBeforeAtPageTop: hasManualPageBreakBeforeParagraph(blocks, index, anchorsForPara),
},
anchorsForPara
? {
Expand Down
Loading