From a06abeec6fff364e1d77b96023b5ae5c6cb42554 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Mon, 29 Dec 2025 10:04:39 -0500 Subject: [PATCH 1/7] chore: change applyBorder signature to take lowercase side --- .../dom/src/table/border-utils.test.ts | 34 +++++++++---------- .../painters/dom/src/table/border-utils.ts | 33 +++++++++++------- 2 files changed, 37 insertions(+), 30 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.test.ts b/packages/layout-engine/painters/dom/src/table/border-utils.test.ts index ae9f439f1..793338a15 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.test.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.test.ts @@ -30,91 +30,91 @@ describe('applyBorder', () => { it('should apply border with single style (converts to solid)', () => { const border: BorderSpec = { style: 'single', width: 2, color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); // Browsers may normalize colors to rgb() format expect(element.style.borderTop).toMatch(/2px solid (#FF0000|rgb\(255,\s*0,\s*0\))/i); }); it('should apply border with double style', () => { const border: BorderSpec = { style: 'double', width: 2, color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/2px double (#FF0000|rgb\(255,\s*0,\s*0\))/i); }); it('should apply border with dashed style', () => { const border: BorderSpec = { style: 'dashed', width: 1, color: '#00FF00' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/1px dashed (#00FF00|rgb\(0,\s*255,\s*0\))/i); }); it('should apply border with dotted style', () => { const border: BorderSpec = { style: 'dotted', width: 1, color: '#0000FF' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/1px dotted (#0000FF|rgb\(0,\s*0,\s*255\))/i); }); it('should convert triple to solid CSS', () => { const border: BorderSpec = { style: 'triple', width: 2, color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/2px solid (#FF0000|rgb\(255,\s*0,\s*0\))/i); }); it('should handle thick border with width multiplier', () => { const border: BorderSpec = { style: 'thick', width: 1, color: '#000000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); // Thick borders use max(width * 2, 3) expect(element.style.borderTop).toMatch(/3px solid (#000000|rgb\(0,\s*0,\s*0\))/i); }); it('should handle thick border with larger width', () => { const border: BorderSpec = { style: 'thick', width: 3, color: '#000000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/6px solid (#000000|rgb\(0,\s*0,\s*0\))/i); }); it('should set border to none for none style', () => { const border: BorderSpec = { style: 'none', width: 2, color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); // Setting border to 'none' results in empty string or 'none' depending on browser expect(element.style.borderTop === 'none' || element.style.borderTop === '').toBe(true); }); it('should set border to none for zero width', () => { const border: BorderSpec = { style: 'single', width: 0, color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); // Setting border to 'none' results in empty string or 'none' depending on browser expect(element.style.borderTop === 'none' || element.style.borderTop === '').toBe(true); }); it('should sanitize invalid hex color to black', () => { const border: BorderSpec = { style: 'single', width: 1, color: 'invalid' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/1px solid (#000000|rgb\(0,\s*0,\s*0\))/i); }); it('should default width to 1 if missing', () => { const border: BorderSpec = { style: 'single', color: '#FF0000' }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/1px solid (#FF0000|rgb\(255,\s*0,\s*0\))/i); }); it('should default color to black if missing', () => { const border: BorderSpec = { style: 'single', width: 2 }; - applyBorder(element, 'Top', border); + applyBorder(element, 'top', border); expect(element.style.borderTop).toMatch(/2px solid (#000000|rgb\(0,\s*0,\s*0\))/i); }); it('should do nothing if border is undefined', () => { - applyBorder(element, 'Top', undefined); + applyBorder(element, 'top', undefined); expect(element.style.borderTop).toBe(''); }); it('should apply to all four sides', () => { const border: BorderSpec = { style: 'single', width: 1, color: '#FF0000' }; - applyBorder(element, 'Top', border); - applyBorder(element, 'Right', border); - applyBorder(element, 'Bottom', border); - applyBorder(element, 'Left', border); + applyBorder(element, 'top', border); + applyBorder(element, 'right', border); + applyBorder(element, 'bottom', border); + applyBorder(element, 'left', border); const pattern = /1px solid (#FF0000|rgb\(255,\s*0,\s*0\))/i; expect(element.style.borderTop).toMatch(pattern); expect(element.style.borderRight).toMatch(pattern); diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.ts b/packages/layout-engine/painters/dom/src/table/border-utils.ts index eaa075ef6..11dd6b66c 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.ts @@ -49,6 +49,13 @@ const borderStyleToCSS = (style?: BorderStyle): string => { const isValidHexColor = (color: string): boolean => /^#[0-9A-Fa-f]{6}$/.test(color); +const BORDER_CSS_ATTRS = { + top: 'borderTop', + right: 'borderRight', + bottom: 'borderBottom', + left: 'borderLeft', +} as const; + /** * Applies a border specification to one side of an HTML element. * @@ -57,24 +64,24 @@ const isValidHexColor = (color: string): boolean => /^#[0-9A-Fa-f]{6}$/.test(col * and special cases like 'thick' borders which use doubled width. * * @param element - The HTML element to apply the border to - * @param side - Which side of the element to apply the border ('Top', 'Right', 'Bottom', or 'Left') + * @param side - Which side of the element to apply the border ('top', 'right', 'bottom', or 'left') * @param border - The border specification to apply, or undefined to skip * * @example * ```typescript * const cell = document.createElement('td'); - * applyBorder(cell, 'Top', { style: 'single', width: 2, color: '#FF0000' }); + * applyBorder(cell, 'top', { style: 'single', width: 2, color: '#FF0000' }); * // Sets cell.style.borderTop = '2px solid #FF0000' * ``` */ export const applyBorder = ( element: HTMLElement, - side: 'Top' | 'Right' | 'Bottom' | 'Left', + side: 'top' | 'right' | 'bottom' | 'left', border?: BorderSpec, ): void => { if (!border) return; if (border.style === 'none' || border.width === 0) { - element.style[`border${side}`] = 'none'; + element.style[BORDER_CSS_ATTRS[side]] = 'none'; return; } @@ -83,7 +90,7 @@ export const applyBorder = ( const color = border.color ?? '#000000'; const safeColor = isValidHexColor(color) ? color : '#000000'; const actualWidth = border.style === 'thick' ? Math.max(width * 2, 3) : width; - element.style[`border${side}`] = `${actualWidth}px ${style} ${safeColor}`; + element.style[BORDER_CSS_ATTRS[side]] = `${actualWidth}px ${style} ${safeColor}`; }; /** @@ -107,10 +114,10 @@ export const applyBorder = ( */ export const applyCellBorders = (element: HTMLElement, borders?: CellBorders): void => { if (!borders) return; - applyBorder(element, 'Top', borders.top); - applyBorder(element, 'Right', borders.right); - applyBorder(element, 'Bottom', borders.bottom); - applyBorder(element, 'Left', borders.left); + applyBorder(element, 'top', borders.top); + applyBorder(element, 'right', borders.right); + applyBorder(element, 'bottom', borders.bottom); + applyBorder(element, 'left', borders.left); }; /** @@ -228,10 +235,10 @@ export const createTableBorderOverlay = ( overlay.style.pointerEvents = 'none'; overlay.style.zIndex = '1'; - applyBorder(overlay, 'Top', top); - applyBorder(overlay, 'Right', right); - applyBorder(overlay, 'Bottom', bottom); - applyBorder(overlay, 'Left', left); + applyBorder(overlay, 'top', top); + applyBorder(overlay, 'right', right); + applyBorder(overlay, 'bottom', bottom); + applyBorder(overlay, 'left', left); return overlay; }; From b14052884e36f48c1ec1aade8c3308d6dcf404a0 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Mon, 29 Dec 2025 11:33:58 -0500 Subject: [PATCH 2/7] fix: handling of table borders * Fix bug with border size units: eliminate eighthPointsToPixels conversion in processInlineCellBorders (legacy-handle-table-cell-node.js) * Eliminate redunant logic in renderTableRow: have a single case handle any combination of explicit borders & table borders * Collapse borders by only painting right and bottom borders (omitting left and top) unless the cell is the first column or the first row (or treated as the first row, because it is first on the page) --- packages/layout-engine/contracts/src/index.ts | 38 +++++++++++++- .../layout-engine/measuring/dom/src/index.ts | 41 ++++++++++------ .../measuring/dom/src/table-utils.ts | 11 +++++ .../painters/dom/src/table/border-utils.ts | 2 +- .../painters/dom/src/table/renderTableCell.ts | 18 ++++--- .../painters/dom/src/table/renderTableRow.ts | 49 ++++--------------- .../helpers/legacy-handle-table-cell-node.js | 3 +- .../src/extensions/table-cell/table-cell.js | 2 + shared/common/index.ts | 3 ++ shared/common/type-helpers.ts | 7 +++ 10 files changed, 110 insertions(+), 64 deletions(-) create mode 100644 packages/layout-engine/measuring/dom/src/table-utils.ts create mode 100644 shared/common/type-helpers.ts diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index 6cabb71d6..fcbe3c89f 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -1,4 +1,5 @@ import type { TabStop } from './engines/tabs.js'; +import type { DeepReadonly, DeepRequired } from '@superdoc/common'; export { computeTabStops, layoutWithTabs, calculateTabWidth } from './engines/tabs.js'; // Re-export TabStop for external consumers @@ -437,10 +438,30 @@ export type TableCellAttrs = { borders?: CellBorders; padding?: BoxSpacing; verticalAlign?: 'top' | 'middle' | 'center' | 'bottom'; - background?: string; + background?: string | null; tableCellProperties?: Record; }; +export type CompleteTableCellAttrs = DeepRequired; + +export const defaultTableCellAttrs: DeepReadonly = { + borders: { + top: { style: 'single', width: 0.5, color: 'auto', space: 0 }, + right: { style: 'single', width: 0.5, color: 'auto', space: 0 }, + bottom: { style: 'single', width: 0.5, color: 'auto', space: 0 }, + left: { style: 'single', width: 0.5, color: 'auto', space: 0 }, + }, + padding: { + top: 2, + left: 4, + right: 4, + bottom: 2, + }, + verticalAlign: 'top', + background: null, + tableCellProperties: {}, +}; + export type TableAttrs = { borders?: TableBorders; borderCollapse?: 'collapse' | 'separate'; @@ -461,6 +482,15 @@ export type TableCell = { attrs?: TableCellAttrs; }; +export type CompleteTableCell = DeepRequired; + +export const defaultTableCell: DeepReadonly> = { + blocks: [], + rowSpan: 1, + colSpan: 1, + attrs: defaultTableCellAttrs, +}; + export type TableRowAttrs = { tableRowProperties?: Record; rowHeight?: { @@ -1310,11 +1340,17 @@ export type TableCellMeasure = { colSpan?: number; /** Number of rows this cell spans */ rowSpan?: number; + /** Bottom border */ + borderBottom: number; + /** Top border */ + borderTop: number; }; export type TableRowMeasure = { cells: TableCellMeasure[]; height: number; + borderTop: number; + borderBottom: number; }; export type TableMeasure = { diff --git a/packages/layout-engine/measuring/dom/src/index.ts b/packages/layout-engine/measuring/dom/src/index.ts index 7f6540136..661679f98 100644 --- a/packages/layout-engine/measuring/dom/src/index.ts +++ b/packages/layout-engine/measuring/dom/src/index.ts @@ -57,6 +57,7 @@ import type { DrawingGeometry, DropCapDescriptor, } from '@superdoc/contracts'; +import { defaultTableCell } from '@superdoc/contracts'; import type { WordParagraphLayoutOutput } from '@superdoc/word-layout'; import { Engines } from '@superdoc/contracts'; import { @@ -73,7 +74,9 @@ import { toCssFontFamily } from '@superdoc/font-utils'; export { installNodeCanvasPolyfill } from './setup.js'; import { clearMeasurementCache, getMeasuredTextWidth, setCacheSize } from './measurementCache.js'; import { getFontMetrics, clearFontMetricsCache, type FontInfo } from './fontMetricsCache.js'; +import { getBorderWidth } from './table-utils'; +export { getBorderWidth }; export { clearFontMetricsCache }; const { computeTabStops } = Engines; @@ -688,6 +691,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P leaders?: Line['leaders']; /** Count of breakable spaces already included on this line (for justify-aware fitting) */ spaceCount: number; + naturalWidth?: number; } | null = null; // Helper to calculate effective available width based on current line count. @@ -865,8 +869,8 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P lineToTrim.width = roundValue(Math.max(0, lineToTrim.width - delta)); lineToTrim.spaceCount = Math.max(0, lineToTrim.spaceCount - trimCount); - if ((lineToTrim as any).naturalWidth != null && typeof (lineToTrim as any).naturalWidth === 'number') { - (lineToTrim as any).naturalWidth = roundValue(Math.max(0, (lineToTrim as any).naturalWidth - delta)); + if (lineToTrim.naturalWidth != undefined && typeof lineToTrim.naturalWidth === 'number') { + lineToTrim.naturalWidth = roundValue(Math.max(0, lineToTrim.naturalWidth - delta)); } }; @@ -1745,7 +1749,7 @@ async function measureParagraphBlock(block: ParagraphBlock, maxWidth: number): P (shouldIncludeDelimiterSpace ? ((run as TextRun).letterSpacing ?? 0) : 0); // Preserve natural width when compression is applied for justify calculations if (compressedWidth != null) { - (currentLine as any).naturalWidth = roundValue(totalWidthWithWord); + currentLine.naturalWidth = roundValue(totalWidthWithWord); } currentLine.width = roundValue(targetWidth); currentLine.maxFontInfo = updateMaxFontInfo(currentLine.maxFontSize, currentLine.maxFontInfo, run); @@ -2135,8 +2139,8 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai let gridColIndex = 0; // Track position in the grid for (const cell of row.cells) { - const colspan = cell.colSpan ?? 1; - const rowspan = cell.rowSpan ?? 1; + const colspan = cell.colSpan ?? defaultTableCell.colSpan; + const rowspan = cell.rowSpan ?? defaultTableCell.rowSpan; // Skip grid columns that are occupied by rowspans from previous rows // before processing this cell @@ -2159,12 +2163,14 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai } } - // Get cell padding for height calculation - const cellPadding = cell.attrs?.padding ?? { top: 2, left: 4, right: 4, bottom: 2 }; - const paddingTop = cellPadding.top ?? 2; - const paddingBottom = cellPadding.bottom ?? 2; - const paddingLeft = cellPadding.left ?? 4; - const paddingRight = cellPadding.right ?? 4; + // Get cell padding and borders for width and height calculations + const paddingTop = cell.attrs?.padding?.top ?? defaultTableCell.attrs.padding.top; + const paddingBottom = cell.attrs?.padding?.bottom ?? defaultTableCell.attrs.padding.bottom; + const paddingLeft = cell.attrs?.padding?.left ?? defaultTableCell.attrs.padding.left; + const paddingRight = cell.attrs?.padding?.right ?? defaultTableCell.attrs.padding.right; + + const borderTop = getBorderWidth(cell.attrs?.borders?.top ?? defaultTableCell.attrs.borders.top); + const borderBottom = getBorderWidth(cell.attrs?.borders?.bottom ?? defaultTableCell.attrs.borders.bottom); // Content width accounts for horizontal padding const contentWidth = Math.max(1, cellWidth - paddingLeft - paddingRight); @@ -2184,7 +2190,7 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai * * Height Calculation: * - contentHeight = sum of all block.totalHeight values - * - totalCellHeight = contentHeight + paddingTop + paddingBottom + * - totalCellHeight = contentHeight + paddingTop + paddingBottom + borderTop * * Example: * ``` @@ -2217,7 +2223,7 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai } // Total cell height includes vertical padding - const totalCellHeight = contentHeight + paddingTop + paddingBottom; + const totalCellHeight = contentHeight + paddingTop + paddingBottom + borderTop; cellMeasures.push({ blocks: blockMeasures, @@ -2228,6 +2234,8 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai gridColumnStart: gridColIndex, colSpan: colspan, rowSpan: rowspan, + borderTop, + borderBottom, }); if (rowspan === 1) { @@ -2247,7 +2255,12 @@ async function measureTableBlock(block: TableBlock, constraints: MeasureConstrai } } - rows.push({ cells: cellMeasures, height: 0 }); + rows.push({ + cells: cellMeasures, + height: 0, + borderTop: Math.max(0, ...cellMeasures.map((cm) => cm.borderTop)), + borderBottom: Math.max(0, ...cellMeasures.map((cm) => cm.borderBottom)), + }); } const rowHeights = [...rowBaseHeights]; diff --git a/packages/layout-engine/measuring/dom/src/table-utils.ts b/packages/layout-engine/measuring/dom/src/table-utils.ts new file mode 100644 index 000000000..d625dfe32 --- /dev/null +++ b/packages/layout-engine/measuring/dom/src/table-utils.ts @@ -0,0 +1,11 @@ +import { BorderSpec } from '@superdoc/contracts'; + +export function getBorderWidth(border: BorderSpec | null | undefined): number { + if (!border || border.style === 'none') { + return 0; + } + + // TODO: Is there a reason the default is different from the default border weight in defaultTableCellAttrs.borders? + // (This is the same default value used in applyBorder() in painters/dom/src/table/border-utils.ts) + return border.width ?? 1; +} diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.ts b/packages/layout-engine/painters/dom/src/table/border-utils.ts index 11dd6b66c..516e31bc9 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.ts @@ -86,7 +86,7 @@ export const applyBorder = ( } const style = borderStyleToCSS(border.style); - const width = border.width ?? 1; + const width = border.width ?? 1; // TODO: Is there a reason this is different from the default border weight in defaultTableCellAttrs.borders? const color = border.color ?? '#000000'; const safeColor = isValidHexColor(color) ? color : '#000000'; const actualWidth = border.style === 'thick' ? Math.max(width * 2, 3) : width; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index 2b4c83006..e0a7eb5b8 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -9,6 +9,8 @@ import type { TableBlock, TableMeasure, } from '@superdoc/contracts'; +import { defaultTableCell } from '@superdoc/contracts'; +import { getBorderWidth } from '@superdoc/measuring-dom'; import { applyCellBorders } from './border-utils.js'; import type { FragmentRenderContext } from '../renderer.js'; import { toCssFontFamily } from '@superdoc/font-utils'; @@ -216,6 +218,8 @@ type TableCellRenderDependencies = { fromLine?: number; /** Ending line index for partial row rendering (exclusive), -1 means render to end */ toLine?: number; + /** Maximum top border for the row, used to add extra padding to the cell */ + rowBorderTop: number; }; /** @@ -297,21 +301,21 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen applySdtDataset, fromLine, toLine, + rowBorderTop, } = deps; - const attrs = cell?.attrs; - const padding = attrs?.padding || { top: 2, left: 4, right: 4, bottom: 2 }; - const paddingLeft = padding.left ?? 4; - const paddingTop = padding.top ?? 2; - const paddingRight = padding.right ?? 4; - const paddingBottom = padding.bottom ?? 2; + const paddingTop = + (cell?.attrs?.padding?.top ?? defaultTableCell.attrs.padding.top) + rowBorderTop - getBorderWidth(borders?.top); + const paddingBottom = cell?.attrs?.padding?.bottom ?? defaultTableCell.attrs.padding.bottom; + const paddingLeft = cell?.attrs?.padding?.left ?? defaultTableCell.attrs.padding.left; + const paddingRight = cell?.attrs?.padding?.right ?? defaultTableCell.attrs.padding.right; const cellEl = doc.createElement('div'); cellEl.style.position = 'absolute'; cellEl.style.left = `${x}px`; cellEl.style.top = `${y}px`; cellEl.style.width = `${cellMeasure.width}px`; - cellEl.style.height = `${rowHeight}px`; + cellEl.style.height = `${rowHeight + getBorderWidth(borders?.bottom)}px`; cellEl.style.boxSizing = 'border-box'; // Cell clips all overflow - no scrollbars, content just gets clipped at boundaries cellEl.style.overflow = 'hidden'; diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index 28cd60244..bef0993aa 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -9,7 +9,7 @@ import type { TableMeasure, } from '@superdoc/contracts'; import { renderTableCell } from './renderTableCell.js'; -import { resolveTableCellBorders, borderValueToSpec } from './border-utils.js'; +import { borderValueToSpec } from './border-utils.js'; import type { FragmentRenderContext } from '../renderer.js'; type TableRowMeasure = TableMeasure['rows'][number]; @@ -242,8 +242,9 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { let resolvedBorders; if (hasBordersAttribute && !hasExplicitBorders) { // Cell explicitly has borders={} meaning "no borders" + // TODO: Is this actually accurate? Shouldn't borders={} mean "don't override any borders"? resolvedBorders = undefined; - } else if (hasExplicitBorders && tableBorders) { + } else { // Merge cell's explicit borders with table's outer borders for edge cells // This handles DOCX files that use right/bottom ownership model const isFirstRow = rowIndex === 0; @@ -257,47 +258,16 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { resolvedBorders = { // For top: use cell's if defined, otherwise use table's top border for first row OR continuation - top: cellBordersAttr.top ?? borderValueToSpec(treatAsFirstRow ? tableBorders.top : tableBorders.insideH), + top: treatAsFirstRow ? (cellBordersAttr?.top ?? borderValueToSpec(tableBorders?.top)) : undefined, // For bottom: use cell's if defined, otherwise use table's bottom border for last row OR before continuation - bottom: cellBordersAttr.bottom ?? borderValueToSpec(treatAsLastRow ? tableBorders.bottom : undefined), + bottom: cellBordersAttr?.bottom ?? borderValueToSpec(treatAsLastRow ? tableBorders?.bottom : undefined), // For left: use cell's if defined, otherwise use table's left for first col - left: cellBordersAttr.left ?? borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV), + left: isFirstCol + ? (cellBordersAttr?.left ?? borderValueToSpec(isFirstCol ? tableBorders?.left : tableBorders?.insideV)) + : undefined, // For right: use cell's if defined, otherwise use table's right for last col only - right: cellBordersAttr.right ?? borderValueToSpec(isLastCol ? tableBorders.right : undefined), - }; - } else if (hasExplicitBorders) { - // Cell has explicit borders but no table borders to merge with - // Use cell borders as-is (no table borders to add for continuations) - resolvedBorders = { - top: cellBordersAttr.top, - bottom: cellBordersAttr.bottom, - left: cellBordersAttr.left, - right: cellBordersAttr.right, + right: cellBordersAttr?.right ?? borderValueToSpec(isLastCol ? tableBorders?.right : undefined), }; - } else if (tableBorders) { - // For continuation handling: treat split boundaries as table edges - const isFirstRow = rowIndex === 0; - const isLastRow = rowIndex === totalRows - 1; - const treatAsFirstRow = isFirstRow || continuesFromPrev; - const treatAsLastRow = isLastRow || continuesOnNext; - - // Get base borders, then override for continuations - const baseBorders = resolveTableCellBorders(tableBorders, rowIndex, gridColIndex, totalRows, totalCols); - - if (baseBorders) { - resolvedBorders = { - // If this is a continuation (continuesFromPrev), use table's top border - top: treatAsFirstRow ? borderValueToSpec(tableBorders.top) : baseBorders.top, - // If this continues on next (continuesOnNext), use table's bottom border - bottom: treatAsLastRow ? borderValueToSpec(tableBorders.bottom) : baseBorders.bottom, - left: baseBorders.left, - right: baseBorders.right, - }; - } else { - resolvedBorders = undefined; - } - } else { - resolvedBorders = undefined; } // Calculate cell height - use rowspan height if cell spans multiple rows @@ -334,6 +304,7 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { applySdtDataset, fromLine, toLine, + rowBorderTop: rowMeasure.borderTop, }); container.appendChild(cellElement); diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js index 4d80b3acf..2ed5269c5 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js @@ -1,4 +1,4 @@ -import { eighthPointsToPixels, twipsToPixels } from '@converter/helpers'; +import { twipsToPixels } from '@converter/helpers'; import { translator as tcPrTranslator } from '../../tcPr'; /** @@ -254,7 +254,6 @@ const processInlineCellBorders = (borders, rowBorders) => { if (borderAttrs && borderAttrs['val'] !== 'nil') { const color = borderAttrs['color']; let size = borderAttrs['size']; - if (size) size = eighthPointsToPixels(size); acc[direction] = { color, size, val: borderAttrs['val'] }; return acc; } diff --git a/packages/super-editor/src/extensions/table-cell/table-cell.js b/packages/super-editor/src/extensions/table-cell/table-cell.js index 313e9ef25..0f4249928 100644 --- a/packages/super-editor/src/extensions/table-cell/table-cell.js +++ b/packages/super-editor/src/extensions/table-cell/table-cell.js @@ -144,6 +144,7 @@ export const TableCell = Node.create({ }, cellMargins: { + // TODO: apply eighthPointsToPixels conversion for borders renderDOM({ cellMargins, borders }) { if (!cellMargins) return {}; const sides = ['top', 'right', 'bottom', 'left']; @@ -162,6 +163,7 @@ export const TableCell = Node.create({ }, }, + // TODO: apply eighthPointsToPixels conversion for borders borders: { default: () => createCellBorders(), renderDOM({ borders }) { diff --git a/shared/common/index.ts b/shared/common/index.ts index 58ccd811c..8e2c33ff4 100644 --- a/shared/common/index.ts +++ b/shared/common/index.ts @@ -49,3 +49,6 @@ export type { // Collaboration/Awareness export * from './collaboration/awareness'; + +// TypeScript utilities +export * from './type-helpers'; diff --git a/shared/common/type-helpers.ts b/shared/common/type-helpers.ts new file mode 100644 index 000000000..0135dd242 --- /dev/null +++ b/shared/common/type-helpers.ts @@ -0,0 +1,7 @@ +export type DeepRequired = Required<{ + [K in keyof T]: DeepRequired; +}>; + +export type DeepReadonly = Readonly<{ + [K in keyof T]: DeepReadonly; +}>; From 98d3087e755d558b7b467cfd0e137536e8bf0719 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Mon, 29 Dec 2025 19:21:31 -0500 Subject: [PATCH 3/7] fix: add eighthPointsToPixels conversion for legacy table-cell borders --- .../super-editor/src/extensions/table-cell/table-cell.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/super-editor/src/extensions/table-cell/table-cell.js b/packages/super-editor/src/extensions/table-cell/table-cell.js index 0f4249928..670d63cbd 100644 --- a/packages/super-editor/src/extensions/table-cell/table-cell.js +++ b/packages/super-editor/src/extensions/table-cell/table-cell.js @@ -39,6 +39,7 @@ import { Node, Attribute } from '@core/index.js'; import { createCellBorders } from './helpers/createCellBorders.js'; +import { eighthPointsToPixels } from '@core/super-converter/helpers.js'; /** * Cell margins configuration @@ -144,7 +145,6 @@ export const TableCell = Node.create({ }, cellMargins: { - // TODO: apply eighthPointsToPixels conversion for borders renderDOM({ cellMargins, borders }) { if (!cellMargins) return {}; const sides = ['top', 'right', 'bottom', 'left']; @@ -153,7 +153,7 @@ export const TableCell = Node.create({ const margin = cellMargins?.[side] ?? 0; const border = borders?.[side]; // TODO: this should include table-level borders as well for the first/last cell in the row - const borderSize = border && border.val !== 'none' ? Math.ceil(border.size) : 0; + const borderSize = border && border.val !== 'none' ? Math.ceil(eighthPointsToPixels(border.size)) : 0; if (margin) return `padding-${side}: ${Math.max(0, margin - borderSize)}px;`; return ''; @@ -163,7 +163,6 @@ export const TableCell = Node.create({ }, }, - // TODO: apply eighthPointsToPixels conversion for borders borders: { default: () => createCellBorders(), renderDOM({ borders }) { @@ -175,7 +174,7 @@ export const TableCell = Node.create({ if (border && border.val === 'none') return `border-${side}: ${border.val};`; let color = border?.color || 'black'; if (color === 'auto') color = 'black'; - if (border) return `border-${side}: ${Math.ceil(border.size)}px solid ${color};`; + if (border) return `border-${side}: ${Math.ceil(eighthPointsToPixels(border.size))}px solid ${color};`; return ''; }) .join(' '); From e8ce1ba727bcacb9b02e8b4bf363043dd7f75900 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Mon, 29 Dec 2025 19:57:11 -0500 Subject: [PATCH 4/7] fix: update default table border size in @superdoc/contracts Unclear where the original 0.5 value came from; change it to 1 to reflect the default value used in rendering table borders. --- packages/layout-engine/contracts/src/index.ts | 8 ++++---- packages/layout-engine/measuring/dom/src/table-utils.ts | 2 -- .../layout-engine/painters/dom/src/table/border-utils.ts | 2 +- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/layout-engine/contracts/src/index.ts b/packages/layout-engine/contracts/src/index.ts index fcbe3c89f..6e40ced0b 100644 --- a/packages/layout-engine/contracts/src/index.ts +++ b/packages/layout-engine/contracts/src/index.ts @@ -446,10 +446,10 @@ export type CompleteTableCellAttrs = DeepRequired; export const defaultTableCellAttrs: DeepReadonly = { borders: { - top: { style: 'single', width: 0.5, color: 'auto', space: 0 }, - right: { style: 'single', width: 0.5, color: 'auto', space: 0 }, - bottom: { style: 'single', width: 0.5, color: 'auto', space: 0 }, - left: { style: 'single', width: 0.5, color: 'auto', space: 0 }, + top: { style: 'single', width: 1, color: 'auto', space: 0 }, + right: { style: 'single', width: 1, color: 'auto', space: 0 }, + bottom: { style: 'single', width: 1, color: 'auto', space: 0 }, + left: { style: 'single', width: 1, color: 'auto', space: 0 }, }, padding: { top: 2, diff --git a/packages/layout-engine/measuring/dom/src/table-utils.ts b/packages/layout-engine/measuring/dom/src/table-utils.ts index d625dfe32..4667bc0b8 100644 --- a/packages/layout-engine/measuring/dom/src/table-utils.ts +++ b/packages/layout-engine/measuring/dom/src/table-utils.ts @@ -5,7 +5,5 @@ export function getBorderWidth(border: BorderSpec | null | undefined): number { return 0; } - // TODO: Is there a reason the default is different from the default border weight in defaultTableCellAttrs.borders? - // (This is the same default value used in applyBorder() in painters/dom/src/table/border-utils.ts) return border.width ?? 1; } diff --git a/packages/layout-engine/painters/dom/src/table/border-utils.ts b/packages/layout-engine/painters/dom/src/table/border-utils.ts index 516e31bc9..11dd6b66c 100644 --- a/packages/layout-engine/painters/dom/src/table/border-utils.ts +++ b/packages/layout-engine/painters/dom/src/table/border-utils.ts @@ -86,7 +86,7 @@ export const applyBorder = ( } const style = borderStyleToCSS(border.style); - const width = border.width ?? 1; // TODO: Is there a reason this is different from the default border weight in defaultTableCellAttrs.borders? + const width = border.width ?? 1; const color = border.color ?? '#000000'; const safeColor = isValidHexColor(color) ? color : '#000000'; const actualWidth = border.style === 'thick' ? Math.max(width * 2, 3) : width; From 6cd71254e9b945e6e3643f902bc3978b5123b08f Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Wed, 31 Dec 2025 07:39:26 -0500 Subject: [PATCH 5/7] fix: borders are parsed as eights-of-a-point, not pixels --- .../handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js index 76b4a684e..59df18ae7 100644 --- a/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js +++ b/packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.test.js @@ -137,7 +137,7 @@ describe('legacy-handle-table-cell-node', () => { // borders merged: inline bottom overrides; left set to none inherits from row with val=none expect(out.attrs.borders.bottom.color).toBe('#FF0000'); - expect(out.attrs.borders.bottom.size).toBeCloseTo(4, 3); + expect(out.attrs.borders.bottom.size).toBeCloseTo(24, 3); expect(out.attrs.borders.left.val).toBe('none'); // untouched right comes from rowBorders expect(out.attrs.borders.right).toEqual(rowBorders.right); From f99a9db96ddb41dbe78cdbea75662f58a3082de4 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Wed, 31 Dec 2025 08:32:50 -0500 Subject: [PATCH 6/7] fix: improve collapsing of vertical table borders Revert earlier change which disabled rendering of top/left borders, because this would cause the height of a thick left border to be too short for a cell with added top/bottom borders (e.g. see "Thick border all around" in SD-1131-table-borders.docx). Instead, render all four borders, but allow them to overlap, to simulate collapsed borders. --- .../painters/dom/src/table/renderTableCell.ts | 15 ++++++++------- .../painters/dom/src/table/renderTableRow.ts | 13 ++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts index e0a7eb5b8..c68a84628 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableCell.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableCell.ts @@ -307,23 +307,24 @@ export const renderTableCell = (deps: TableCellRenderDependencies): TableCellRen const paddingTop = (cell?.attrs?.padding?.top ?? defaultTableCell.attrs.padding.top) + rowBorderTop - getBorderWidth(borders?.top); const paddingBottom = cell?.attrs?.padding?.bottom ?? defaultTableCell.attrs.padding.bottom; - const paddingLeft = cell?.attrs?.padding?.left ?? defaultTableCell.attrs.padding.left; + const paddingLeft = + (cell?.attrs?.padding?.left ?? defaultTableCell.attrs.padding.left) - getBorderWidth(borders?.left) / 2; const paddingRight = cell?.attrs?.padding?.right ?? defaultTableCell.attrs.padding.right; const cellEl = doc.createElement('div'); cellEl.style.position = 'absolute'; - cellEl.style.left = `${x}px`; + cellEl.style.left = `${x - getBorderWidth(borders?.left) / 2}px`; cellEl.style.top = `${y}px`; - cellEl.style.width = `${cellMeasure.width}px`; + cellEl.style.width = `${cellMeasure.width + getBorderWidth(borders?.left) / 2 + getBorderWidth(borders?.right) / 2}px`; cellEl.style.height = `${rowHeight + getBorderWidth(borders?.bottom)}px`; cellEl.style.boxSizing = 'border-box'; // Cell clips all overflow - no scrollbars, content just gets clipped at boundaries cellEl.style.overflow = 'hidden'; // Apply padding directly to cell so content is positioned correctly - cellEl.style.paddingLeft = `${paddingLeft}px`; - cellEl.style.paddingTop = `${paddingTop}px`; - cellEl.style.paddingRight = `${paddingRight}px`; - cellEl.style.paddingBottom = `${paddingBottom}px`; + cellEl.style.paddingLeft = `${Math.max(0, paddingLeft)}px`; + cellEl.style.paddingTop = `${Math.max(0, paddingTop)}px`; + cellEl.style.paddingRight = `${Math.max(0, paddingRight)}px`; + cellEl.style.paddingBottom = `${Math.max(paddingBottom, 0)}px`; if (borders) { applyCellBorders(cellEl, borders); diff --git a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts index bef0993aa..a9e6a71e2 100644 --- a/packages/layout-engine/painters/dom/src/table/renderTableRow.ts +++ b/packages/layout-engine/painters/dom/src/table/renderTableRow.ts @@ -1,4 +1,5 @@ import type { + CellBorders, DrawingBlock, Line, ParagraphBlock, @@ -239,7 +240,7 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { // - If continuesFromPrev=true: draw TOP border (table's top border) to close the top // - If continuesOnNext=true: draw BOTTOM border (table's bottom border) to close the bottom // This means both fragments at a split have their edge borders drawn. - let resolvedBorders; + let resolvedBorders: CellBorders | undefined; if (hasBordersAttribute && !hasExplicitBorders) { // Cell explicitly has borders={} meaning "no borders" // TODO: Is this actually accurate? Shouldn't borders={} mean "don't override any borders"? @@ -258,15 +259,13 @@ export const renderTableRow = (deps: TableRowRenderDependencies): void => { resolvedBorders = { // For top: use cell's if defined, otherwise use table's top border for first row OR continuation - top: treatAsFirstRow ? (cellBordersAttr?.top ?? borderValueToSpec(tableBorders?.top)) : undefined, + top: borderValueToSpec(cellBordersAttr?.top ?? (treatAsFirstRow ? tableBorders?.top : undefined)), // For bottom: use cell's if defined, otherwise use table's bottom border for last row OR before continuation - bottom: cellBordersAttr?.bottom ?? borderValueToSpec(treatAsLastRow ? tableBorders?.bottom : undefined), + bottom: borderValueToSpec(cellBordersAttr?.bottom ?? (treatAsLastRow ? tableBorders?.bottom : undefined)), // For left: use cell's if defined, otherwise use table's left for first col - left: isFirstCol - ? (cellBordersAttr?.left ?? borderValueToSpec(isFirstCol ? tableBorders?.left : tableBorders?.insideV)) - : undefined, + left: borderValueToSpec(cellBordersAttr?.left ?? (isFirstCol ? tableBorders?.left : undefined)), // For right: use cell's if defined, otherwise use table's right for last col only - right: cellBordersAttr?.right ?? borderValueToSpec(isLastCol ? tableBorders?.right : undefined), + right: borderValueToSpec(cellBordersAttr?.right ?? (isLastCol ? tableBorders?.right : undefined)), }; } From e88ea8dc466c1cb8517a35371f75e1b0c090f709 Mon Sep 17 00:00:00 2001 From: Tony Novak Date: Wed, 31 Dec 2025 10:12:18 -0500 Subject: [PATCH 7/7] fix: add missing dependency: painter-dom depends on measuring-dom --- packages/layout-engine/painters/dom/package.json | 1 + pnpm-lock.yaml | 3 +++ 2 files changed, 4 insertions(+) diff --git a/packages/layout-engine/painters/dom/package.json b/packages/layout-engine/painters/dom/package.json index 799c56068..9c467d3dc 100644 --- a/packages/layout-engine/painters/dom/package.json +++ b/packages/layout-engine/painters/dom/package.json @@ -19,6 +19,7 @@ "dependencies": { "@superdoc/contracts": "workspace:*", "@superdoc/font-utils": "workspace:*", + "@superdoc/measuring-dom": "workspace:*", "@superdoc/preset-geometry": "workspace:*", "@superdoc/url-validation": "workspace:*" }, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 52f4a3b44..92de13691 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -580,6 +580,9 @@ importers: '@superdoc/font-utils': specifier: workspace:* version: link:../../../../shared/font-utils + '@superdoc/measuring-dom': + specifier: workspace:* + version: link:../../measuring/dom '@superdoc/preset-geometry': specifier: workspace:* version: link:../../../preset-geometry