Fix PDF table rows splitting across lines (#370)
Root cause: the height measurement probe reimplemented wrapping logic inline, separate from the actual drawWrappingLine renderer. The two implementations could diverge, causing the predicted row height to be shorter than the actual rendered content, making cells overflow their row borders and visually bleed into the next row. Fix: - Extract shared computeVisualLines() function that both the probe and drawWrappingLine use as the single source of truth for word- wrapping and line counting. This guarantees the predicted height always matches the actual rendered height. - Save/restore doc.y around the probe loop so PDFKit's internal cursor doesn't drift between measurement and rendering passes. - Add Y-clamp in the render loop: if a cell's text cursor reaches rowBottom, stop rendering to prevent overflow as a safety net. - drawWrappingLine is now a thin wrapper that calls computeVisualLines then draws each line via drawBidiLine, keeping the code DRY. Tested with Arabic RTL and English LTR PDFs containing meetings with 10+ attendees, long titles, subheadings, and mixed bidi text. All rows stay properly aligned within their borders.
This commit is contained in:
@@ -583,15 +583,16 @@ function drawMixedLine(
|
||||
return lh;
|
||||
}
|
||||
|
||||
function drawWrappingLine(
|
||||
type WrapOpts = Pick<DrawOpts, "fontSize" | "weight" | "baseDirection" | "mapping"> & { width: number };
|
||||
|
||||
function computeVisualLines(
|
||||
doc: PDFKit.PDFDocument,
|
||||
text: string,
|
||||
opts: DrawOpts,
|
||||
): number {
|
||||
const lh = opts.lineHeight ?? opts.fontSize * 1.2;
|
||||
if (!text.trim()) return lh;
|
||||
opts: WrapOpts,
|
||||
): string[] {
|
||||
if (!text.trim()) return [""];
|
||||
const words = text.split(/\s+/).filter((w) => w.length > 0);
|
||||
if (words.length === 0) return lh;
|
||||
if (words.length === 0) return [""];
|
||||
const mOpts = {
|
||||
fontSize: opts.fontSize,
|
||||
weight: opts.weight,
|
||||
@@ -599,8 +600,7 @@ function drawWrappingLine(
|
||||
mapping: opts.mapping,
|
||||
};
|
||||
const hardBreak = (word: string): string[] => {
|
||||
const ww = measureBidiLineWidth(doc, word, mOpts);
|
||||
if (ww <= opts.width) return [word];
|
||||
if (measureBidiLineWidth(doc, word, mOpts) <= opts.width) return [word];
|
||||
const chars = [...word];
|
||||
const pieces: string[] = [];
|
||||
let cur = "";
|
||||
@@ -617,28 +617,42 @@ function drawWrappingLine(
|
||||
return pieces;
|
||||
};
|
||||
|
||||
const visualLines: string[] = [];
|
||||
const result: string[] = [];
|
||||
let current = "";
|
||||
for (let i = 0; i < words.length; i++) {
|
||||
const ww = measureBidiLineWidth(doc, words[i], mOpts);
|
||||
if (ww > opts.width) {
|
||||
if (current) { visualLines.push(current); current = ""; }
|
||||
for (const piece of hardBreak(words[i])) visualLines.push(piece);
|
||||
if (measureBidiLineWidth(doc, words[i], mOpts) > opts.width) {
|
||||
if (current) { result.push(current); current = ""; }
|
||||
for (const piece of hardBreak(words[i])) result.push(piece);
|
||||
continue;
|
||||
}
|
||||
const test = current ? current + " " + words[i] : words[i];
|
||||
if (measureBidiLineWidth(doc, test, mOpts) > opts.width) {
|
||||
visualLines.push(current);
|
||||
result.push(current);
|
||||
current = words[i];
|
||||
} else {
|
||||
current = test;
|
||||
}
|
||||
}
|
||||
if (current) visualLines.push(current);
|
||||
if (visualLines.length === 0) return lh;
|
||||
if (current) result.push(current);
|
||||
return result.length > 0 ? result : [""];
|
||||
}
|
||||
|
||||
function drawWrappingLine(
|
||||
doc: PDFKit.PDFDocument,
|
||||
text: string,
|
||||
opts: DrawOpts,
|
||||
): number {
|
||||
const lh = opts.lineHeight ?? opts.fontSize * 1.2;
|
||||
const lines = computeVisualLines(doc, text, {
|
||||
fontSize: opts.fontSize,
|
||||
weight: opts.weight,
|
||||
baseDirection: opts.baseDirection,
|
||||
mapping: opts.mapping,
|
||||
width: opts.width,
|
||||
});
|
||||
let totalH = 0;
|
||||
for (const vl of visualLines) {
|
||||
drawBidiLine(doc, vl, { ...opts, y: opts.y + totalH });
|
||||
for (const vl of lines) {
|
||||
if (vl) drawBidiLine(doc, vl, { ...opts, y: opts.y + totalH });
|
||||
totalH += lh;
|
||||
}
|
||||
return Math.max(lh, totalH);
|
||||
@@ -911,81 +925,35 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
|
||||
const cells = isRtl ? [...cellsLogical].reverse() : cellsLogical;
|
||||
const widths = isRtl ? [...cellWidths].reverse() : cellWidths;
|
||||
|
||||
// Measure the row height by laying out each cell to a temporary
|
||||
// y-cursor and taking the max. We then redraw at the final position.
|
||||
const probe = doc;
|
||||
const probeY = probe.y;
|
||||
let rowHeight = lineHeight + cellPadY * 2;
|
||||
const cellHeights: number[] = [];
|
||||
let cx = tableX;
|
||||
const probeWrapOpts = {
|
||||
const savedProbeY = doc.y;
|
||||
const wrapOpts: WrapOpts = {
|
||||
fontSize: input.font.fontSize,
|
||||
weight: input.font.fontWeight,
|
||||
baseDirection: baseDir,
|
||||
mapping,
|
||||
width: 0,
|
||||
};
|
||||
let rowHeight = lineHeight + cellPadY * 2;
|
||||
const cellHeights: number[] = [];
|
||||
for (let i = 0; i < cells.length; i++) {
|
||||
const c = cells[i];
|
||||
const startProbeY = probeY + cellPadY;
|
||||
probe.y = startProbeY;
|
||||
const lines = c.text.split("\n");
|
||||
let h = 0;
|
||||
const cellW = widths[i] - cellPadX * 2;
|
||||
for (const line of lines) {
|
||||
if (!line) {
|
||||
h += lineHeight;
|
||||
continue;
|
||||
}
|
||||
const singleW = measureBidiLineWidth(doc, line, probeWrapOpts);
|
||||
if (singleW <= cellW) {
|
||||
h += lineHeight;
|
||||
} else {
|
||||
const wds = line.split(/\s+/).filter((w: string) => w.length > 0);
|
||||
if (wds.length === 0) { h += lineHeight; continue; }
|
||||
let wrapCount = 0;
|
||||
let cur = "";
|
||||
for (let wi = 0; wi < wds.length; wi++) {
|
||||
const wordW = measureBidiLineWidth(doc, wds[wi], probeWrapOpts);
|
||||
if (wordW > cellW) {
|
||||
if (cur) { wrapCount++; cur = ""; }
|
||||
const chars = [...wds[wi]];
|
||||
let piece = "";
|
||||
for (const ch of chars) {
|
||||
const test = piece + ch;
|
||||
if (piece && measureBidiLineWidth(doc, test, probeWrapOpts) > cellW) {
|
||||
wrapCount++;
|
||||
piece = ch;
|
||||
} else {
|
||||
piece = test;
|
||||
}
|
||||
}
|
||||
if (piece) { wrapCount++; }
|
||||
continue;
|
||||
}
|
||||
const test = cur ? cur + " " + wds[wi] : wds[wi];
|
||||
if (measureBidiLineWidth(doc, test, probeWrapOpts) > cellW) {
|
||||
wrapCount++;
|
||||
cur = wds[wi];
|
||||
} else {
|
||||
cur = test;
|
||||
}
|
||||
}
|
||||
if (cur) wrapCount++;
|
||||
h += Math.max(1, wrapCount) * lineHeight;
|
||||
}
|
||||
const textLines = cells[i].text.split("\n");
|
||||
let h = 0;
|
||||
for (const tl of textLines) {
|
||||
if (!tl) { h += lineHeight; continue; }
|
||||
const vLines = computeVisualLines(doc, tl, { ...wrapOpts, width: cellW });
|
||||
h += vLines.length * lineHeight;
|
||||
}
|
||||
const finalH = h + cellPadY * 2;
|
||||
cellHeights.push(finalH);
|
||||
if (finalH > rowHeight) rowHeight = finalH;
|
||||
cx += widths[i];
|
||||
}
|
||||
doc.y = savedProbeY;
|
||||
|
||||
// Page break check. Reserve `footerReserve` so the bottom "Recorded
|
||||
// by" / date footer never overlaps the last row.
|
||||
const footerReserve =
|
||||
Math.max(11, Math.round(input.font.fontSize * 0.95)) * 1.4 * 2 + 6;
|
||||
if (
|
||||
probeY + rowHeight >
|
||||
savedProbeY + rowHeight >
|
||||
doc.page.height - doc.page.margins.bottom - footerReserve
|
||||
) {
|
||||
doc.addPage();
|
||||
@@ -993,7 +961,6 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
|
||||
}
|
||||
|
||||
const rowTop = doc.y;
|
||||
// Saved per-row tint (#288); isHighlighted is not painted.
|
||||
if (meeting.rowColor) {
|
||||
const fill = ROW_COLOR_FILL[meeting.rowColor];
|
||||
if (fill) {
|
||||
@@ -1003,7 +970,6 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
|
||||
}
|
||||
}
|
||||
|
||||
// Cell borders.
|
||||
doc.save();
|
||||
doc.lineWidth(0.5).strokeColor("#d1d5db");
|
||||
doc.rect(tableX, rowTop, tableWidth, rowHeight).stroke();
|
||||
@@ -1017,20 +983,19 @@ export async function renderSchedulePdf(input: RenderPdfInput): Promise<Buffer>
|
||||
}
|
||||
doc.restore();
|
||||
|
||||
cx = tableX;
|
||||
const rowBottom = rowTop + rowHeight;
|
||||
let cx = tableX;
|
||||
for (let i = 0; i < cells.length; i++) {
|
||||
const c = cells[i];
|
||||
const cellW = widths[i] - cellPadX * 2;
|
||||
const lines = c.text.split("\n");
|
||||
const textLines = c.text.split("\n");
|
||||
const contentH = cellHeights[i] - cellPadY * 2;
|
||||
const vOffset = Math.max(0, (rowHeight - cellPadY * 2 - contentH) / 2);
|
||||
let y = rowTop + cellPadY + vOffset;
|
||||
for (const line of lines) {
|
||||
if (!line) {
|
||||
y += lineHeight;
|
||||
continue;
|
||||
}
|
||||
const used = drawWrappingLine(doc, line, {
|
||||
for (const tl of textLines) {
|
||||
if (y >= rowBottom) break;
|
||||
if (!tl) { y += lineHeight; continue; }
|
||||
const used = drawWrappingLine(doc, tl, {
|
||||
x: cx + cellPadX,
|
||||
y,
|
||||
width: cellW,
|
||||
|
||||
Reference in New Issue
Block a user