Skip to content

Commit ca3b9d5

Browse files
committed
fix: group paragraphs with nil/none between border into continuous box
Word groups consecutive paragraphs with matching borders into a single bordered box even when w:between is nil/none — it just omits the separator. SuperDoc was skipping grouping entirely for nil/none between, rendering each paragraph as a separate bordered box. - Preserve between: {style: 'none'} during normalization so downstream can distinguish "explicitly nil/none" from "no between element" - Change grouping check to allow nil/none between through - Add suppressBottomBorder flag for nil/none groups (extend gap without drawing a separator)
1 parent a5be885 commit ca3b9d5

5 files changed

Lines changed: 217 additions & 17 deletions

File tree

packages/layout-engine/painters/dom/src/between-borders.test.ts

Lines changed: 166 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,18 @@ import {
1010
} from './features/paragraph-borders/index.js';
1111

1212
/** Helper to create BetweenBorderInfo for tests that previously passed a boolean. */
13-
const betweenOn: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 };
14-
const betweenOff: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: false, gapBelow: 0 };
13+
const betweenOn: BetweenBorderInfo = {
14+
showBetweenBorder: true,
15+
suppressTopBorder: false,
16+
suppressBottomBorder: false,
17+
gapBelow: 0,
18+
};
19+
const betweenOff: BetweenBorderInfo = {
20+
showBetweenBorder: false,
21+
suppressTopBorder: false,
22+
suppressBottomBorder: false,
23+
gapBelow: 0,
24+
};
1525
import { createDomPainter } from './index.js';
1626
import type {
1727
ParagraphBorders,
@@ -258,7 +268,12 @@ describe('applyParagraphBorderStyles — between borders', () => {
258268
left: { style: 'solid', width: 1, color: '#000' },
259269
right: { style: 'solid', width: 1, color: '#000' },
260270
};
261-
const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 };
271+
const info: BetweenBorderInfo = {
272+
showBetweenBorder: false,
273+
suppressTopBorder: true,
274+
suppressBottomBorder: false,
275+
gapBelow: 0,
276+
};
262277
applyParagraphBorderStyles(e, borders, info);
263278
expect(e.style.getPropertyValue('border-top-style')).toBe('');
264279
expect(e.style.getPropertyValue('border-left-style')).toBe('solid');
@@ -274,6 +289,49 @@ describe('applyParagraphBorderStyles — between borders', () => {
274289
expect(e.style.getPropertyValue('border-top-style')).toBe('dashed');
275290
expect(e.style.getPropertyValue('border-top-width')).toBe('3px');
276291
});
292+
293+
// --- suppressBottomBorder (nil/none between groups) ---
294+
it('skips bottom border when suppressBottomBorder is true', () => {
295+
const e = el();
296+
const borders: ParagraphBorders = {
297+
top: { style: 'solid', width: 1, color: '#000' },
298+
bottom: { style: 'solid', width: 2, color: '#F00' },
299+
left: { style: 'solid', width: 1, color: '#000' },
300+
right: { style: 'solid', width: 1, color: '#000' },
301+
};
302+
const info: BetweenBorderInfo = {
303+
showBetweenBorder: false,
304+
suppressTopBorder: false,
305+
suppressBottomBorder: true,
306+
gapBelow: 10,
307+
};
308+
applyParagraphBorderStyles(e, borders, info);
309+
expect(e.style.getPropertyValue('border-top-style')).toBe('solid');
310+
expect(e.style.getPropertyValue('border-left-style')).toBe('solid');
311+
expect(e.style.getPropertyValue('border-right-style')).toBe('solid');
312+
expect(e.style.getPropertyValue('border-bottom-style')).toBe('');
313+
});
314+
315+
it('suppresses both top and bottom for middle fragment in nil/none group', () => {
316+
const e = el();
317+
const borders: ParagraphBorders = {
318+
top: { style: 'solid', width: 1, color: '#000' },
319+
bottom: { style: 'solid', width: 1, color: '#000' },
320+
left: { style: 'solid', width: 1, color: '#000' },
321+
right: { style: 'solid', width: 1, color: '#000' },
322+
};
323+
const info: BetweenBorderInfo = {
324+
showBetweenBorder: false,
325+
suppressTopBorder: true,
326+
suppressBottomBorder: true,
327+
gapBelow: 10,
328+
};
329+
applyParagraphBorderStyles(e, borders, info);
330+
expect(e.style.getPropertyValue('border-top-style')).toBe('');
331+
expect(e.style.getPropertyValue('border-bottom-style')).toBe('');
332+
expect(e.style.getPropertyValue('border-left-style')).toBe('solid');
333+
expect(e.style.getPropertyValue('border-right-style')).toBe('solid');
334+
});
277335
});
278336

279337
// ---------------------------------------------------------------------------
@@ -283,15 +341,25 @@ describe('applyParagraphBorderStyles — between borders', () => {
283341
describe('createParagraphDecorationLayers — gap extension', () => {
284342
it('sets bottom to negative gapBelow when showBetweenBorder is true', () => {
285343
const attrs = { borders: { top: { style: 'solid' as const, width: 1 } }, shading: { fill: '#EEE' } };
286-
const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 8 };
344+
const info: BetweenBorderInfo = {
345+
showBetweenBorder: true,
346+
suppressTopBorder: false,
347+
suppressBottomBorder: false,
348+
gapBelow: 8,
349+
};
287350
const { borderLayer, shadingLayer } = createParagraphDecorationLayers(document, 400, attrs, info);
288351
expect(borderLayer!.style.bottom).toBe('-8px');
289352
expect(shadingLayer!.style.bottom).toBe('-8px');
290353
});
291354

292355
it('sets bottom to 0px when gapBelow is 0', () => {
293356
const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } };
294-
const info: BetweenBorderInfo = { showBetweenBorder: true, suppressTopBorder: false, gapBelow: 0 };
357+
const info: BetweenBorderInfo = {
358+
showBetweenBorder: true,
359+
suppressTopBorder: false,
360+
suppressBottomBorder: false,
361+
gapBelow: 0,
362+
};
295363
const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info);
296364
expect(borderLayer!.style.bottom).toBe('0px');
297365
});
@@ -302,12 +370,31 @@ describe('createParagraphDecorationLayers — gap extension', () => {
302370
expect(borderLayer!.style.bottom).toBe('0px');
303371
});
304372

305-
it('sets bottom to 0px when showBetweenBorder is false even with gapBelow', () => {
373+
it('sets bottom to 0px when showBetweenBorder is false and suppressBottomBorder is false', () => {
306374
const attrs = { borders: { top: { style: 'solid' as const, width: 1 } } };
307-
const info: BetweenBorderInfo = { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 12 };
375+
const info: BetweenBorderInfo = {
376+
showBetweenBorder: false,
377+
suppressTopBorder: true,
378+
suppressBottomBorder: false,
379+
gapBelow: 12,
380+
};
308381
const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info);
309382
expect(borderLayer!.style.bottom).toBe('0px');
310383
});
384+
385+
it('sets bottom to negative gapBelow when suppressBottomBorder is true (nil/none between group)', () => {
386+
const attrs = {
387+
borders: { top: { style: 'solid' as const, width: 1 }, bottom: { style: 'solid' as const, width: 1 } },
388+
};
389+
const info: BetweenBorderInfo = {
390+
showBetweenBorder: false,
391+
suppressTopBorder: false,
392+
suppressBottomBorder: true,
393+
gapBelow: 10,
394+
};
395+
const { borderLayer } = createParagraphDecorationLayers(document, 400, attrs, info);
396+
expect(borderLayer!.style.bottom).toBe('-10px');
397+
});
311398
});
312399

313400
// ---------------------------------------------------------------------------
@@ -630,6 +717,78 @@ describe('computeBetweenBorderFlags', () => {
630717
const flags = computeBetweenBorderFlags(fragments, lookup);
631718
expect(flags.size).toBe(2);
632719
});
720+
721+
// --- nil/none between grouping (continuous box without separator) ---
722+
it('groups paragraphs with between: {style: "none"} (nil/none between)', () => {
723+
const borders: ParagraphBorders = {
724+
top: { style: 'solid', width: 1, color: '#000' },
725+
right: { style: 'solid', width: 1, color: '#000' },
726+
bottom: { style: 'solid', width: 1, color: '#000' },
727+
left: { style: 'solid', width: 1, color: '#000' },
728+
between: { style: 'none' },
729+
};
730+
const b1 = makeParagraphBlock('b1', borders);
731+
const b2 = makeParagraphBlock('b2', borders);
732+
const lookup = buildLookup([{ block: b1 }, { block: b2 }]);
733+
const fragments: Fragment[] = [paraFragment('b1', { y: 0 }), paraFragment('b2', { y: 20 })];
734+
735+
const flags = computeBetweenBorderFlags(fragments, lookup);
736+
expect(flags.size).toBe(2);
737+
// First fragment: suppressBottomBorder (not showBetweenBorder)
738+
expect(flags.get(0)?.showBetweenBorder).toBe(false);
739+
expect(flags.get(0)?.suppressBottomBorder).toBe(true);
740+
// Second fragment: suppressTopBorder
741+
expect(flags.get(1)?.suppressTopBorder).toBe(true);
742+
expect(flags.get(1)?.suppressBottomBorder).toBe(false);
743+
});
744+
745+
it('groups chain of 3 paragraphs with nil/none between', () => {
746+
const borders: ParagraphBorders = {
747+
top: { style: 'solid', width: 1, color: '#000' },
748+
bottom: { style: 'solid', width: 1, color: '#000' },
749+
left: { style: 'solid', width: 1, color: '#000' },
750+
right: { style: 'solid', width: 1, color: '#000' },
751+
between: { style: 'none' },
752+
};
753+
const b1 = makeParagraphBlock('b1', borders);
754+
const b2 = makeParagraphBlock('b2', borders);
755+
const b3 = makeParagraphBlock('b3', borders);
756+
const lookup = buildLookup([{ block: b1 }, { block: b2 }, { block: b3 }]);
757+
const fragments: Fragment[] = [
758+
paraFragment('b1', { y: 0 }),
759+
paraFragment('b2', { y: 20 }),
760+
paraFragment('b3', { y: 40 }),
761+
];
762+
763+
const flags = computeBetweenBorderFlags(fragments, lookup);
764+
expect(flags.size).toBe(3);
765+
// First: suppress bottom, keep top
766+
expect(flags.get(0)?.suppressBottomBorder).toBe(true);
767+
expect(flags.get(0)?.suppressTopBorder).toBe(false);
768+
// Middle: suppress both top and bottom
769+
expect(flags.get(1)?.suppressTopBorder).toBe(true);
770+
expect(flags.get(1)?.suppressBottomBorder).toBe(true);
771+
// Last: suppress top, keep bottom
772+
expect(flags.get(2)?.suppressTopBorder).toBe(true);
773+
expect(flags.get(2)?.suppressBottomBorder).toBe(false);
774+
});
775+
776+
it('does not group nil/none between with real between (different hashes)', () => {
777+
const nilBetween: ParagraphBorders = {
778+
top: { style: 'solid', width: 1, color: '#000' },
779+
between: { style: 'none' },
780+
};
781+
const realBetween: ParagraphBorders = {
782+
top: { style: 'solid', width: 1, color: '#000' },
783+
between: { style: 'solid', width: 1, color: '#000' },
784+
};
785+
const b1 = makeParagraphBlock('b1', nilBetween);
786+
const b2 = makeParagraphBlock('b2', realBetween);
787+
const lookup = buildLookup([{ block: b1 }, { block: b2 }]);
788+
const fragments: Fragment[] = [paraFragment('b1'), paraFragment('b2')];
789+
790+
expect(computeBetweenBorderFlags(fragments, lookup).size).toBe(0);
791+
});
633792
});
634793

635794
// ---------------------------------------------------------------------------

packages/layout-engine/painters/dom/src/features/paragraph-borders/border-layer.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,10 @@ export const createParagraphDecorationLayers = (
5959

6060
const borderBox = getParagraphBorderBox(fragmentWidth, attrs.indent);
6161

62-
// Extend layers into the spacing gap for continuous group borders
63-
const gapExtension = betweenInfo?.showBetweenBorder ? betweenInfo.gapBelow : 0;
62+
// Extend layers into the spacing gap for continuous group borders.
63+
// Both real between (showBetweenBorder) and nil/none between (suppressBottomBorder)
64+
// need gap extension to keep left/right borders continuous through the spacing gap.
65+
const gapExtension = betweenInfo?.showBetweenBorder || betweenInfo?.suppressBottomBorder ? betweenInfo!.gapBelow : 0;
6466
const bottomValue = gapExtension > 0 ? `-${gapExtension}px` : '0px';
6567

6668
const baseStyles = {
@@ -113,10 +115,12 @@ export const applyParagraphBorderStyles = (
113115
if (!borders) return;
114116
const showBetweenBorder = betweenInfo?.showBetweenBorder ?? false;
115117
const suppressTopBorder = betweenInfo?.suppressTopBorder ?? false;
118+
const suppressBottomBorder = betweenInfo?.suppressBottomBorder ?? false;
116119

117120
element.style.boxSizing = 'border-box';
118121
BORDER_SIDES.forEach((side) => {
119122
if (side === 'top' && suppressTopBorder) return;
123+
if (side === 'bottom' && suppressBottomBorder) return;
120124
const border = borders[side];
121125
if (!border) return;
122126
setBorderSideStyle(element, side, border);
@@ -157,6 +161,7 @@ export const stampBetweenBorderDataset = (element: HTMLElement, betweenInfo?: Be
157161
if (!betweenInfo) return;
158162
if (betweenInfo.showBetweenBorder) element.dataset.betweenBorder = 'true';
159163
if (betweenInfo.suppressTopBorder) element.dataset.suppressTopBorder = 'true';
164+
if (betweenInfo.suppressBottomBorder) element.dataset.suppressBottomBorder = 'true';
160165
if (betweenInfo.gapBelow) element.dataset.gapBelow = String(betweenInfo.gapBelow);
161166
};
162167

packages/layout-engine/painters/dom/src/features/paragraph-borders/group-analysis.ts

Lines changed: 29 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import { hashParagraphBorders } from '../../paragraph-hash-utils.js';
2929
export type BetweenBorderInfo = {
3030
showBetweenBorder: boolean;
3131
suppressTopBorder: boolean;
32+
suppressBottomBorder: boolean;
3233
gapBelow: number;
3334
};
3435

@@ -125,14 +126,17 @@ export const computeBetweenBorderFlags = (
125126
): Map<number, BetweenBorderInfo> => {
126127
// Phase 1: determine which consecutive pairs form between-border groups
127128
const pairFlags = new Set<number>();
129+
const noBetweenPairs = new Set<number>();
128130

129131
for (let i = 0; i < fragments.length - 1; i += 1) {
130132
const frag = fragments[i];
131133
if (frag.kind !== 'para' && frag.kind !== 'list-item') continue;
132134
if (frag.continuesOnNext) continue;
133135

134136
const borders = getFragmentParagraphBorders(frag, blockLookup);
135-
if (isBetweenBorderNone(borders)) continue;
137+
// Skip if no between element at all (no grouping intent).
138+
// between: {style: 'none'} (nil/none) IS allowed — it signals grouping without a separator.
139+
if (!borders?.between) continue;
136140

137141
const next = fragments[i + 1];
138142
if (next.kind !== 'para' && next.kind !== 'list-item') continue;
@@ -147,13 +151,18 @@ export const computeBetweenBorderFlags = (
147151
continue;
148152

149153
const nextBorders = getFragmentParagraphBorders(next, blockLookup);
150-
if (isBetweenBorderNone(nextBorders)) continue;
154+
if (!nextBorders?.between) continue;
151155
if (hashParagraphBorders(borders!) !== hashParagraphBorders(nextBorders!)) continue;
152156

153157
// Skip fragments in different columns (different x positions)
154158
if (frag.x !== next.x) continue;
155159

156160
pairFlags.add(i);
161+
162+
// Track nil/none between pairs — these get suppressBottomBorder instead of showBetweenBorder
163+
if (isBetweenBorderNone(borders) && isBetweenBorderNone(nextBorders)) {
164+
noBetweenPairs.add(i);
165+
}
157166
}
158167

159168
// Phase 2: build per-fragment info with gap distances and top suppression
@@ -164,19 +173,33 @@ export const computeBetweenBorderFlags = (
164173
const next = fragments[i + 1];
165174
const fragHeight = getFragmentHeight(frag, blockLookup);
166175
const gapBelow = Math.max(0, next.y - (frag.y + fragHeight));
176+
const isNoBetween = noBetweenPairs.has(i);
167177

168-
// Current fragment: show between border on bottom, extend into gap
178+
// Current fragment: extend into gap.
179+
// Real between → showBetweenBorder (replace bottom with between definition).
180+
// Nil/none between → suppressBottomBorder (hide bottom, keep left/right continuous).
169181
if (!result.has(i)) {
170-
result.set(i, { showBetweenBorder: true, suppressTopBorder: false, gapBelow });
182+
result.set(i, {
183+
showBetweenBorder: !isNoBetween,
184+
suppressTopBorder: false,
185+
suppressBottomBorder: isNoBetween,
186+
gapBelow,
187+
});
171188
} else {
172189
const existing = result.get(i)!;
173-
existing.showBetweenBorder = true;
190+
existing.showBetweenBorder = !isNoBetween;
191+
existing.suppressBottomBorder = isNoBetween;
174192
existing.gapBelow = gapBelow;
175193
}
176194

177195
// Next fragment: suppress top border (previous fragment's extended layer covers boundary)
178196
if (!result.has(i + 1)) {
179-
result.set(i + 1, { showBetweenBorder: false, suppressTopBorder: true, gapBelow: 0 });
197+
result.set(i + 1, {
198+
showBetweenBorder: false,
199+
suppressTopBorder: true,
200+
suppressBottomBorder: false,
201+
gapBelow: 0,
202+
});
180203
} else {
181204
result.get(i + 1)!.suppressTopBorder = true;
182205
}

packages/layout-engine/pm-adapter/src/attributes/borders.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -572,11 +572,13 @@ describe('normalizeParagraphBorders', () => {
572572
expect(result?.between?.style).toBe('dashed');
573573
});
574574

575-
it('should return undefined when between border is nil', () => {
575+
it('should preserve between: {style: "none"} when between border is nil', () => {
576576
const input = {
577577
between: { val: 'nil' },
578578
};
579-
expect(normalizeParagraphBorders(input)).toBeUndefined();
579+
const result = normalizeParagraphBorders(input);
580+
expect(result).toBeDefined();
581+
expect(result?.between).toEqual({ style: 'none' });
580582
});
581583
});
582584

packages/layout-engine/pm-adapter/src/attributes/borders.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -334,6 +334,17 @@ export const normalizeParagraphBorders = (value: unknown): ParagraphAttrs['borde
334334
}
335335
});
336336

337+
// Preserve between: {style: 'none'} for nil/none between borders.
338+
// normalizeBorderSide drops 'none' sides, but for 'between' we need to keep it
339+
// so the grouping logic can distinguish "explicitly nil/none" (group without separator)
340+
// from "no between element at all" (don't group).
341+
if (!borders.between && source.between) {
342+
const style = mapBorderStyle((source.between as Record<string, unknown>).val);
343+
if (style === 'none') {
344+
borders.between = { style: 'none' };
345+
}
346+
}
347+
337348
return Object.keys(borders).length > 0 ? borders : undefined;
338349
};
339350

0 commit comments

Comments
 (0)