Skip to content

Commit 9b7d1af

Browse files
VladaHarbourVladaHarbour
committed
fix: add test coverage
1 parent 78d7ea0 commit 9b7d1af

7 files changed

Lines changed: 232 additions & 84 deletions

File tree

‎packages/layout-engine/contracts/src/graphic-placement.test.ts‎

Lines changed: 89 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,23 @@
11
import { describe, expect, it } from 'vitest';
2-
import {
3-
resolveAnchoredGraphicY,
4-
computeParagraphContentStartY,
5-
computeParagraphLayoutStartY,
6-
} from './graphic-placement.js';
2+
import { resolveAnchoredGraphicY, resolveAnchoredGraphicX } from './graphic-placement.js';
73

8-
const base = {
4+
const yBase = {
95
objectHeight: 100,
106
contentTop: 72,
117
contentBottom: 720,
128
pageBottomMargin: 72,
139
};
1410

11+
const columns = { width: 200, gap: 20, count: 2 };
12+
const margins = { left: 72, right: 72 };
13+
const pageWidth = 600;
14+
const objectWidth = 80;
15+
1516
describe('resolveAnchoredGraphicY', () => {
1617
it('positions margin-relative top with offset', () => {
1718
expect(
1819
resolveAnchoredGraphicY({
19-
...base,
20+
...yBase,
2021
anchor: { vRelativeFrom: 'margin', alignV: 'top', offsetV: 10 },
2122
}),
2223
).toBe(82);
@@ -25,7 +26,7 @@ describe('resolveAnchoredGraphicY', () => {
2526
it('positions page-relative bottom with page margin', () => {
2627
expect(
2728
resolveAnchoredGraphicY({
28-
...base,
29+
...yBase,
2930
anchor: { vRelativeFrom: 'page', alignV: 'bottom', offsetV: 5 },
3031
}),
3132
).toBe(720 + 72 - 100 + 5);
@@ -34,7 +35,7 @@ describe('resolveAnchoredGraphicY', () => {
3435
it('positions paragraph-relative center on first line', () => {
3536
expect(
3637
resolveAnchoredGraphicY({
37-
...base,
38+
...yBase,
3839
anchor: { vRelativeFrom: 'paragraph', alignV: 'center', offsetV: 0 },
3940
anchorParagraphY: 200,
4041
firstLineHeight: 24,
@@ -45,7 +46,7 @@ describe('resolveAnchoredGraphicY', () => {
4546
it('uses pre-registered fallback when vRelativeFrom is paragraph without paragraph context', () => {
4647
expect(
4748
resolveAnchoredGraphicY({
48-
...base,
49+
...yBase,
4950
anchor: { vRelativeFrom: 'paragraph', offsetV: 20 },
5051
preRegisteredFallbackToContentTop: true,
5152
}),
@@ -55,62 +56,115 @@ describe('resolveAnchoredGraphicY', () => {
5556
it('ignores paragraph alignV when pre-registered fallback has no paragraph context', () => {
5657
expect(
5758
resolveAnchoredGraphicY({
58-
...base,
59+
...yBase,
5960
anchor: { vRelativeFrom: 'paragraph', alignV: 'center', offsetV: 0 },
6061
preRegisteredFallbackToContentTop: true,
6162
}),
6263
).toBe(72);
6364
expect(
6465
resolveAnchoredGraphicY({
65-
...base,
66+
...yBase,
6667
objectHeight: 50,
6768
anchor: { vRelativeFrom: 'paragraph', alignV: 'bottom', offsetV: 10 },
6869
preRegisteredFallbackToContentTop: true,
6970
}),
7071
).toBe(82);
7172
});
7273

73-
it('legacy undefined vRelativeFrom uses anchor paragraph Y', () => {
74+
it('legacy undefined vRelativeFrom uses anchor paragraph Y plus offsetV', () => {
7475
expect(
7576
resolveAnchoredGraphicY({
76-
...base,
77+
...yBase,
7778
anchor: { offsetV: 15 },
7879
anchorParagraphY: 300,
7980
}),
8081
).toBe(315);
8182
});
82-
});
8383

84-
describe('computeParagraphLayoutStartY', () => {
85-
it('rewinds trailing then applies full spacing-before without double collapse', () => {
84+
it('legacy undefined vRelativeFrom with preRegisteredFallbackToContentTop uses contentTop', () => {
8685
expect(
87-
computeParagraphLayoutStartY({
88-
cursorY: 120,
89-
spacingBefore: 24,
90-
trailingSpacing: 12,
91-
rewindTrailingFromPrevious: true,
86+
resolveAnchoredGraphicY({
87+
...yBase,
88+
anchor: { alignV: 'center', offsetV: 20 },
89+
anchorParagraphY: 300,
90+
preRegisteredFallbackToContentTop: true,
9291
}),
93-
).toBe(132);
92+
).toBe(92);
9493
});
9594

96-
it('collapses spacing-before against trailing when previous after-spacing is kept', () => {
95+
it('legacy undefined vRelativeFrom does not use paragraph alignV without vRelativeFrom paragraph', () => {
9796
expect(
98-
computeParagraphLayoutStartY({
99-
cursorY: 100,
100-
spacingBefore: 24,
101-
trailingSpacing: 8,
102-
rewindTrailingFromPrevious: false,
97+
resolveAnchoredGraphicY({
98+
...yBase,
99+
anchor: { alignV: 'bottom', offsetV: 0 },
100+
anchorParagraphY: 200,
101+
firstLineHeight: 24,
103102
}),
104-
).toBe(116);
103+
).toBe(200);
105104
});
106105
});
107106

108-
describe('computeParagraphContentStartY', () => {
109-
it('adds spacing-before minus trailing collapse', () => {
110-
expect(computeParagraphContentStartY(100, 24, false, 8)).toBe(116);
107+
describe('resolveAnchoredGraphicX', () => {
108+
const columnIndex = 1;
109+
const columnLeft = margins.left + columnIndex * (columns.width + columns.gap);
110+
111+
describe('column-relative (default)', () => {
112+
it.each([
113+
{ alignH: 'left' as const, offsetH: 10, expected: columnLeft + 10 },
114+
{ alignH: 'center' as const, offsetH: 5, expected: columnLeft + (columns.width - objectWidth) / 2 + 5 },
115+
{ alignH: 'right' as const, offsetH: 3, expected: columnLeft + columns.width - objectWidth - 3 },
116+
])('alignH=$alignH offsetH=$offsetH', ({ alignH, offsetH, expected }) => {
117+
expect(resolveAnchoredGraphicX({ alignH, offsetH }, columnIndex, columns, objectWidth, margins, pageWidth)).toBe(
118+
expected,
119+
);
120+
});
121+
});
122+
123+
describe('margin-relative', () => {
124+
const baseX = margins.left;
125+
const availableWidth = pageWidth - margins.left - margins.right;
126+
127+
it.each([
128+
{ alignH: 'left' as const, offsetH: 10, expected: baseX + 10 },
129+
{ alignH: 'center' as const, offsetH: 5, expected: baseX + (availableWidth - objectWidth) / 2 + 5 },
130+
{ alignH: 'right' as const, offsetH: 3, expected: baseX + availableWidth - objectWidth - 3 },
131+
])('alignH=$alignH offsetH=$offsetH', ({ alignH, offsetH, expected }) => {
132+
expect(
133+
resolveAnchoredGraphicX(
134+
{ hRelativeFrom: 'margin', alignH, offsetH },
135+
columnIndex,
136+
columns,
137+
objectWidth,
138+
margins,
139+
pageWidth,
140+
),
141+
).toBe(expected);
142+
});
143+
});
144+
145+
describe('page-relative', () => {
146+
const baseX = 0;
147+
const availableWidth = pageWidth;
148+
149+
it.each([
150+
{ alignH: 'left' as const, offsetH: 10, expected: baseX + 10 },
151+
{ alignH: 'center' as const, offsetH: 5, expected: baseX + (availableWidth - objectWidth) / 2 + 5 },
152+
{ alignH: 'right' as const, offsetH: 3, expected: baseX + availableWidth - objectWidth - 3 },
153+
])('alignH=$alignH offsetH=$offsetH', ({ alignH, offsetH, expected }) => {
154+
expect(
155+
resolveAnchoredGraphicX(
156+
{ hRelativeFrom: 'page', alignH, offsetH },
157+
columnIndex,
158+
columns,
159+
objectWidth,
160+
margins,
161+
pageWidth,
162+
),
163+
).toBe(expected);
164+
});
111165
});
112166

113-
it('returns cursorY when spacing already applied', () => {
114-
expect(computeParagraphContentStartY(100, 24, true, 0)).toBe(100);
167+
it('defaults alignH to left and offsetH to zero', () => {
168+
expect(resolveAnchoredGraphicX({}, 0, columns, objectWidth, margins, pageWidth)).toBe(margins.left);
115169
});
116170
});

‎packages/layout-engine/contracts/src/graphic-placement.ts‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -102,47 +102,6 @@ export function resolveAnchoredGraphicY(input: ResolveAnchoredGraphicYInput): nu
102102
return anchorParagraphY + offsetV;
103103
}
104104

105-
/**
106-
* Y coordinate where paragraph text begins (after spacing-before collapse).
107-
*/
108-
export function computeParagraphContentStartY(
109-
cursorY: number,
110-
spacingBefore: number,
111-
appliedSpacingBefore: boolean,
112-
trailingSpacing: number | undefined,
113-
): number {
114-
if (appliedSpacingBefore || spacingBefore <= 0) {
115-
return cursorY;
116-
}
117-
const prevTrailing = trailingSpacing ?? 0;
118-
return cursorY + Math.max(spacingBefore - prevTrailing, 0);
119-
}
120-
121-
/**
122-
* Paragraph text start Y including contextual-spacing rewind from the previous paragraph.
123-
*/
124-
export function computeParagraphLayoutStartY(input: {
125-
cursorY: number;
126-
spacingBefore: number;
127-
trailingSpacing?: number;
128-
suppressSpacingBefore?: boolean;
129-
rewindTrailingFromPrevious?: boolean;
130-
}): number {
131-
let y = input.cursorY;
132-
let trailingForCollapse = input.trailingSpacing;
133-
if (input.rewindTrailingFromPrevious) {
134-
const prevTrailing = input.trailingSpacing ?? 0;
135-
if (prevTrailing > 0) {
136-
y -= prevTrailing;
137-
// Match layout-paragraph.ts: after rewind, trailingSpacing is cleared before
138-
// spacing-before is applied — do not collapse against the rewound gap again.
139-
trailingForCollapse = 0;
140-
}
141-
}
142-
const effectiveSpacingBefore = input.suppressSpacingBefore ? 0 : input.spacingBefore;
143-
return computeParagraphContentStartY(y, effectiveSpacingBefore, effectiveSpacingBefore === 0, trailingForCollapse);
144-
}
145-
146105
/**
147106
* Resolve horizontal paint position for an anchored graphic.
148107
*/

‎packages/layout-engine/contracts/src/index.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,6 @@ export { computeFragmentPmRange, computeLinePmRange, type LinePmRange } from './
8282
export {
8383
resolveAnchoredGraphicY,
8484
resolveAnchoredGraphicX,
85-
computeParagraphContentStartY,
86-
computeParagraphLayoutStartY,
8785
type ColumnLayoutForAnchor,
8886
type ResolveAnchoredGraphicYInput,
8987
} from './graphic-placement.js';

‎packages/layout-engine/layout-engine/src/layout-paragraph.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@ import {
2020
extractBlockPmRange,
2121
isEmptyTextParagraph,
2222
shouldSuppressOwnSpacing,
23+
collapseSpacingBefore,
24+
rewindPreviousParagraphTrailing,
25+
computeParagraphLayoutStartY,
2326
} from './layout-utils.js';
24-
import { resolveAnchoredGraphicY, resolveAnchoredGraphicX, computeParagraphLayoutStartY, getFragmentZIndex } from '@superdoc/contracts';
27+
import { resolveAnchoredGraphicY, resolveAnchoredGraphicX, getFragmentZIndex } from '@superdoc/contracts';
2528

2629
/** Points → CSS pixels (96 dpi / 72 pt-per-inch). */
2730
const PX_PER_PT = 96 / 72;
@@ -706,7 +709,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
706709
if (shouldSuppressOwnSpacing(state.lastParagraphStyleId, state.lastParagraphContextualSpacing, styleId)) {
707710
const prevTrailing = asSafeNumber(state.trailingSpacing);
708711
if (prevTrailing > 0) {
709-
state.cursorY -= prevTrailing;
712+
state.cursorY = rewindPreviousParagraphTrailing(state.cursorY, prevTrailing);
710713
state.trailingSpacing = 0;
711714
}
712715
}
@@ -725,8 +728,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
725728

726729
const keepLines = attrs?.keepLines === true;
727730
if (keepLines && fromLine === 0) {
728-
const prevTrailing = state.trailingSpacing ?? 0;
729-
const neededSpacingBefore = Math.max(spacingBefore - prevTrailing, 0);
731+
const neededSpacingBefore = collapseSpacingBefore(spacingBefore, state.trailingSpacing);
730732
const pageContentHeight = state.contentBottom - state.topMargin;
731733
const linesHeight = lines.reduce((sum, line) => sum + (line.lineHeight || 0), 0);
732734
const fullHeight = linesHeight + borderExpansion.top + borderExpansion.bottom;
@@ -743,7 +745,7 @@ export function layoutParagraphBlock(ctx: ParagraphLayoutContext, anchors?: Para
743745
if (!appliedSpacingBefore && spacingBefore > 0) {
744746
while (!appliedSpacingBefore) {
745747
const prevTrailing = state.trailingSpacing ?? 0;
746-
const neededSpacingBefore = Math.max(spacingBefore - prevTrailing, 0);
748+
const neededSpacingBefore = collapseSpacingBefore(spacingBefore, state.trailingSpacing);
747749
if (spacingDebugEnabled) {
748750
spacingDebugLog('spacingBefore pending', {
749751
blockId: block.id,

‎packages/layout-engine/layout-engine/src/layout-utils.test.ts‎

Lines changed: 57 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,15 @@
55

66
import { describe, it, expect } from 'bun:test';
77
import type { ParagraphBlock, TextRun, ImageRun } from '@superdoc/contracts';
8-
import { isEmptyTextParagraph, shouldSuppressSpacingForEmpty, shouldSuppressOwnSpacing } from './layout-utils.js';
8+
import {
9+
isEmptyTextParagraph,
10+
shouldSuppressSpacingForEmpty,
11+
shouldSuppressOwnSpacing,
12+
collapseSpacingBefore,
13+
rewindPreviousParagraphTrailing,
14+
computeParagraphContentStartY,
15+
computeParagraphLayoutStartY,
16+
} from './layout-utils.js';
917

1018
// ============================================================================
1119
// Empty Paragraph Detection Tests
@@ -187,3 +195,51 @@ describe('shouldSuppressOwnSpacing', () => {
187195
expect(shouldSuppressOwnSpacing('Normal', true, 'Normal')).toBe(true);
188196
});
189197
});
198+
199+
describe('collapseSpacingBefore', () => {
200+
it('subtracts trailing from spacing-before floored at zero', () => {
201+
expect(collapseSpacingBefore(24, 8)).toBe(16);
202+
expect(collapseSpacingBefore(10, 20)).toBe(0);
203+
});
204+
});
205+
206+
describe('rewindPreviousParagraphTrailing', () => {
207+
it('moves cursor up by trailing when positive', () => {
208+
expect(rewindPreviousParagraphTrailing(120, 12)).toBe(108);
209+
expect(rewindPreviousParagraphTrailing(120, 0)).toBe(120);
210+
});
211+
});
212+
213+
describe('computeParagraphLayoutStartY', () => {
214+
it('rewinds trailing then applies full spacing-before without double collapse', () => {
215+
expect(
216+
computeParagraphLayoutStartY({
217+
cursorY: 120,
218+
spacingBefore: 24,
219+
trailingSpacing: 12,
220+
rewindTrailingFromPrevious: true,
221+
}),
222+
).toBe(132);
223+
});
224+
225+
it('collapses spacing-before against trailing when previous after-spacing is kept', () => {
226+
expect(
227+
computeParagraphLayoutStartY({
228+
cursorY: 100,
229+
spacingBefore: 24,
230+
trailingSpacing: 8,
231+
rewindTrailingFromPrevious: false,
232+
}),
233+
).toBe(116);
234+
});
235+
});
236+
237+
describe('computeParagraphContentStartY', () => {
238+
it('adds spacing-before minus trailing collapse', () => {
239+
expect(computeParagraphContentStartY(100, 24, false, 8)).toBe(116);
240+
});
241+
242+
it('returns cursorY when spacing already applied', () => {
243+
expect(computeParagraphContentStartY(100, 24, true, 0)).toBe(100);
244+
});
245+
});

0 commit comments

Comments
 (0)