From 6e6852d97536943708b18e2005943efac3126d04 Mon Sep 17 00:00:00 2001 From: Dave Holladay Date: Thu, 3 Oct 2024 17:52:01 +0100 Subject: [PATCH] Take padding into consideration when deciding to break on minPresenceAhead --- packages/layout/src/node/shouldBreak.js | 5 ++- packages/layout/src/page/getContentArea.js | 6 ++- .../layout/src/steps/resolvePagination.js | 20 ++++++---- .../layout/tests/node/shouldBreak.test.js | 32 +++++++++++++++ .../tests/steps/resolvePagination.test.js | 40 +++++++++++++++++++ 5 files changed, 92 insertions(+), 11 deletions(-) diff --git a/packages/layout/src/node/shouldBreak.js b/packages/layout/src/node/shouldBreak.js index d74f6d01d..df90a4455 100644 --- a/packages/layout/src/node/shouldBreak.js +++ b/packages/layout/src/node/shouldBreak.js @@ -26,7 +26,7 @@ const getEndOfPresence = (child, futureElements) => { return Math.min(afterMinPresenceAhead, endOfFurthestFutureElement); }; -const shouldBreak = (child, futureElements, height) => { +const shouldBreak = (child, futureElements, height, paddingTop) => { if (child.props?.fixed) return false; const shouldSplit = height < child.box.top + child.box.height; @@ -36,7 +36,8 @@ const shouldBreak = (child, futureElements, height) => { const endOfPresence = getEndOfPresence(child, futureElements); // If the child is already at the top of the page, breaking won't improve its presence // (as long as react-pdf does not support breaking into differently sized containers) - const breakingImprovesPresence = child.box.top > child.box.marginTop; + const breakingImprovesPresence = + child.box.top > child.box.marginTop + paddingTop; return ( getBreak(child) || diff --git a/packages/layout/src/page/getContentArea.js b/packages/layout/src/page/getContentArea.js index d1e3d5312..1994fff7f 100644 --- a/packages/layout/src/page/getContentArea.js +++ b/packages/layout/src/page/getContentArea.js @@ -4,7 +4,11 @@ const getContentArea = (page) => { const height = page.style?.height; const { paddingTop, paddingBottom } = getPadding(page); - return height - paddingBottom - paddingTop; + return { + contentArea: height - paddingBottom - paddingTop, + paddingTop, + paddingBottom, + }; }; export default getContentArea; diff --git a/packages/layout/src/steps/resolvePagination.js b/packages/layout/src/steps/resolvePagination.js index 02b1db19e..4668fd527 100644 --- a/packages/layout/src/steps/resolvePagination.js +++ b/packages/layout/src/steps/resolvePagination.js @@ -45,7 +45,7 @@ const warnUnavailableSpace = (node) => { ); }; -const splitNodes = (height, contentArea, nodes) => { +const splitNodes = (height, contentArea, paddingTop, nodes) => { const currentChildren = []; const nextChildren = []; @@ -57,7 +57,7 @@ const splitNodes = (height, contentArea, nodes) => { const nodeTop = getTop(child); const nodeHeight = child.box.height; const isOutside = height <= nodeTop; - const shouldBreak = shouldNodeBreak(child, futureNodes, height); + const shouldBreak = shouldNodeBreak(child, futureNodes, height, paddingTop); const shouldSplit = height + SAFETY_THRESHOLD < nodeTop + nodeHeight; const canWrap = canNodeWrap(child); const fitsInsidePage = nodeHeight <= contentArea; @@ -128,17 +128,18 @@ const splitNodes = (height, contentArea, nodes) => { return [currentChildren, nextChildren]; }; -const splitChildren = (height, contentArea, node) => { +const splitChildren = (height, contentArea, paddingTop, node) => { const children = node.children || []; const availableHeight = height - getTop(node); - return splitNodes(availableHeight, contentArea, children); + return splitNodes(availableHeight, contentArea, paddingTop, children); }; -const splitView = (node, height, contentArea) => { +const splitView = (node, height, contentArea, paddingTop) => { const [currentNode, nextNode] = splitNode(node, height); const [currentChilds, nextChildren] = splitChildren( height, contentArea, + paddingTop, node, ); @@ -148,8 +149,10 @@ const splitView = (node, height, contentArea) => { ]; }; -const split = (node, height, contentArea) => - isText(node) ? splitText(node, height) : splitView(node, height, contentArea); +const split = (node, height, contentArea, paddingTop) => + isText(node) + ? splitText(node, height) + : splitView(node, height, contentArea, paddingTop); const shouldResolveDynamicNodes = (node) => { const children = node.children || []; @@ -192,13 +195,14 @@ const resolveDynamicPage = (props, page, fontStore, yoga) => { const splitPage = (page, pageNumber, fontStore, yoga) => { const wrapArea = getWrapArea(page); - const contentArea = getContentArea(page); + const { contentArea, paddingTop } = getContentArea(page); const dynamicPage = resolveDynamicPage({ pageNumber }, page, fontStore, yoga); const height = page.style.height; const [currentChilds, nextChilds] = splitNodes( wrapArea, contentArea, + paddingTop, dynamicPage.children, ); diff --git a/packages/layout/tests/node/shouldBreak.test.js b/packages/layout/tests/node/shouldBreak.test.js index d0df84f77..f4036a7c5 100644 --- a/packages/layout/tests/node/shouldBreak.test.js +++ b/packages/layout/tests/node/shouldBreak.test.js @@ -11,6 +11,7 @@ describe('node shouldBreak', () => { }, [], 1000, + 0, ); expect(result).toEqual(false); @@ -24,6 +25,7 @@ describe('node shouldBreak', () => { }, [], 1000, + 0, ); expect(result).toEqual(true); @@ -37,6 +39,7 @@ describe('node shouldBreak', () => { }, [], 1000, + 0, ); expect(result).toEqual(false); @@ -51,6 +54,7 @@ describe('node shouldBreak', () => { }, [], 1000, + 0, ); expect(result).toEqual(true); @@ -64,6 +68,7 @@ describe('node shouldBreak', () => { }, [], 1000, + 0, ); expect(result).toEqual(true); @@ -77,6 +82,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 200, marginTop: 0, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(true); @@ -90,6 +96,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 1100, height: 0, marginTop: 200, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(true); @@ -103,6 +110,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 200, marginTop: 0, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(false); @@ -116,6 +124,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 100, marginTop: 0, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(false); @@ -129,6 +138,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 1000, height: 0, marginTop: 100, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(false); @@ -142,6 +152,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 100, marginTop: 0, marginBottom: 100 } }], 1000, + 0, ); expect(result).toEqual(false); @@ -155,6 +166,21 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 200, marginTop: 0, marginBottom: 0 } }], 1000, + 0, + ); + + expect(result).toEqual(false); + }); + + test('should consider padding when breaking on minPresenceAhead', () => { + const result = shouldBreak( + { + box: { top: 550, height: 400, marginTop: 500, marginBottom: 0 }, + props: { minPresenceAhead: 400 }, + }, + [{ box: { top: 900, height: 200, marginTop: 0, marginBottom: 0 } }], + 1000, + 50, ); expect(result).toEqual(false); @@ -168,6 +194,7 @@ describe('node shouldBreak', () => { }, [{ box: { top: 900, height: 200, marginTop: 0, marginBottom: 0 } }], 1000, + 0, ); expect(result).toEqual(false); @@ -186,6 +213,7 @@ describe('node shouldBreak', () => { }, ], 1000, + 0, ); expect(result).toEqual(false); @@ -224,6 +252,7 @@ describe('node shouldBreak', () => { }, ], 811.89, + 0, ); expect(result).toEqual(false); @@ -262,6 +291,7 @@ describe('node shouldBreak', () => { }, ], 811.89, + 0, ); expect(result).toEqual(false); @@ -400,6 +430,7 @@ describe('node shouldBreak', () => { }, ], 781.89, + 0, ); expect(result).toEqual(false); @@ -428,6 +459,7 @@ describe('node shouldBreak', () => { }, ], 776.89, + 0, ); expect(result).toEqual(false); diff --git a/packages/layout/tests/steps/resolvePagination.test.js b/packages/layout/tests/steps/resolvePagination.test.js index a71d077e1..a7ca29411 100644 --- a/packages/layout/tests/steps/resolvePagination.test.js +++ b/packages/layout/tests/steps/resolvePagination.test.js @@ -289,4 +289,44 @@ describe('pagination step', () => { // If calcLayout returns then we did not hit an infinite loop expect(true).toBe(true); }); + + test('should take padding into account when splitting pages', async () => { + const yoga = await loadYoga(); + + const root = { + type: 'DOCUMENT', + yoga, + children: [ + { + type: 'PAGE', + box: {}, + style: { + paddingTop: 30, + width: 612, + height: 792, + }, + props: { wrap: true }, + children: [ + { + type: 'VIEW', + box: {}, + style: { height: 761, marginBottom: 24 }, + props: { wrap: true, break: false }, + }, + { + type: 'VIEW', + box: {}, + style: { height: 80 }, + props: { wrap: true, break: false }, + }, + ], + }, + ], + }; + + calcLayout(root); + + // If calcLayout returns then we did not hit an infinite loop + expect(true).toBe(true); + }); });