fix(tables): fix autofit column scaling, cell width overflow, and page break splitting #1987
Conversation
Fix autofit tables dropping columns when rows use rowspan/colspan patterns and improve page break splitting to match Word behavior. Column rendering fixes: - Prefer grid values over colwidth for column widths (grid sums to page width) - Use colSpan sums for column count instead of physical cell count - Distribute colspan tcW proportionally across grid columns during DOCX import Page break improvements: - Rowspan-aware split point selection (prefer breaks at rowspan boundaries) - Ghost cell rendering for rowspan cells spanning page breaks - Skip header repeat when it would prevent a cantSplit row from fitting - Fix fullPageHeight to use actual content area (contentBottom - topMargin) Cell height refinement: - Skip last paragraph spacing.after in cell measuring and rendering SD-1797
|
Based on my review of the code changes and my knowledge of ECMA-376 WordprocessingML specification, here is my assessment: Status: PASS The changes in What ChangedThe modification (lines 73-90, 97-98) adds logic to handle cells with OOXML Spec ComplianceThe implementation correctly handles the ECMA-376 table model:
Minor Observations
The code correctly interprets the relationship between For details on these elements, see https://ooxml.dev/spec?q=tcW and https://ooxml.dev/spec?q=gridSpan. |
…ccurate-rendering-of-autofit-tables
…797, SD-1859, SD-1895) Fix three table rendering bugs: - SD-1797: Autofit tables losing columns due to wrong column width priority, incorrect colspan counting, missing scale-up logic, and page breaks splitting rowspan groups - SD-1859: Percent-width tables overflowing in portrait pages of mixed-orientation documents because cell widths used measurement values instead of rescaled fragment column widths - SD-1895: Autofit tables from DOCX rendering narrow instead of filling page width due to cell width hints taking priority over the authoritative tblGrid values Add three visual regression tests covering all fixed scenarios.
tupizz
changed the title
…7, SD-1859, SD-1895) Introduce new visual tests to validate fixes for table rendering bugs: - SD-1797: Ensure autofit tables with colspan preserve all columns. - SD-1859: Verify percent-width tables fit within portrait page bounds in mixed-orientation documents. - SD-1895: Confirm autofit tables fill the page width as expected. Tests are conditionally skipped if the required documents are not available.
tupizz
requested review from
andrii-harbour,
artem-harbour,
caio-pizzol,
Copilot,
harbournick and
luccas-harbour
There was a problem hiding this comment.
Pull request overview
Fixes several table rendering/layout issues across measurement, layout, and DOM painting to prevent column loss, cell overflow in mixed-orientation docs, and incorrect autofit scaling; also improves table splitting behavior and adds regression/visual coverage.
Changes:
- Correct table measurement/scaling (colspan-aware column count; scale auto-layout tables up/down to effective width; adjust spacing.after handling).
- Add per-fragment column width rescaling for clamped tables and use those widths when computing/rendering cell widths.
- Improve page splitting for rowspans (clean split points + ghost cells) and add regression + visual tests/data.
Reviewed changes
Copilot reviewed 20 out of 24 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/visual/tests/rendering/table-fixes.spec.ts | Adds visual rendering coverage for SD-1797/1859/1895 using doc-based screenshots. |
| packages/super-editor/src/tests/regression/table-autofit-colspan.test.js | Adds editor-level regression test ensuring colspan/autofit tables preserve grid columns. |
| packages/super-editor/src/tests/data/table.docx | Adds DOCX fixture for table-related tests. |
| packages/super-editor/src/tests/data/table-autofit-colspan.docx | Adds DOCX fixture reproducing colspan/autofit grid truncation. |
| packages/super-editor/src/tests/data/pci-table.docx | Adds DOCX fixture for rowspan/column-count scenarios. |
| packages/super-editor/src/core/super-converter/v3/handlers/w/tc/helpers/legacy-handle-table-cell-node.js | Adjusts cell colwidth generation to optionally scale tcW across grid spans when tcW exceeds grid total. |
| packages/layout-engine/pm-adapter/src/converters/table.ts | Changes width-priority to prefer OOXML tblGrid over cell colwidth when userEdited is false. |
| packages/layout-engine/pm-adapter/src/converters/table.test.ts | Updates converter tests to reflect new grid-over-colwidth priority and twips→px expectations. |
| packages/layout-engine/painters/dom/src/table/renderTableRow.ts | Computes rendered cell width from (possibly rescaled) columnWidths and passes it to cell renderer. |
| packages/layout-engine/painters/dom/src/table/renderTableFragment.ts | Uses per-fragment columnWidths when present; adds ghost-cell rendering for rowspan continuations. |
| packages/layout-engine/painters/dom/src/table/renderTableFragment.test.ts | Adds tests ensuring fragment.columnWidths drives rendered cell widths. |
| packages/layout-engine/painters/dom/src/table/renderTableCell.ts | Accepts optional computed cellWidth; changes spacing.after behavior for last paragraph in a cell. |
| packages/layout-engine/painters/dom/src/table/renderTableCell.test.ts | Updates tests to reflect last-paragraph spacing.after behavior change. |
| packages/layout-engine/measuring/dom/src/index.ts | Makes max column count colspan-aware; scales auto-layout tables up/down; updates cell spacing.after measurement. |
| packages/layout-engine/measuring/dom/src/index.test.ts | Adds tests for colspan/rowspan column preservation and updated spacing.after measurement expectations. |
| packages/layout-engine/layout-engine/src/layout-table.ts | Adds rescaleColumnWidths for clamped tables; improves split-point selection for rowspans; adds header-skip for cantSplit interactions. |
| packages/layout-engine/layout-engine/src/layout-table.test.ts | Adds tests validating per-fragment column width rescaling behavior. |
| packages/layout-engine/layout-bridge/test/resolveMeasurementConstraints.test.ts | Adds tests ensuring measurement width uses max section content width in mixed-orientation docs. |
| packages/layout-engine/layout-bridge/src/incrementalLayout.ts | Rescales fragment column widths during incremental layout when tables are clamped. |
| packages/layout-engine/contracts/src/index.ts | Extends TableFragment contract with optional per-fragment columnWidths. |
| devtools/visual-testing/tests/interactions/stories/tables/percent-width-table-overflow.ts | Adds interaction story verifying percent-width tables don’t overflow in portrait sections. |
| devtools/visual-testing/tests/interactions/stories/tables/autofit-table-fill-width.ts | Adds interaction story verifying inserted auto-layout tables fill available width. |
| devtools/visual-testing/tests/interactions/stories/tables/autofit-table-docx-rendering.ts | Adds interaction story validating DOCX autofit tables fill page width. |
| devtools/visual-testing/pnpm-lock.yaml | Updates lockfile for local superdoc tarball integrity. |
Files not reviewed (1)
- devtools/visual-testing/pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const table = findFirstTable(editor.state.doc); | ||
| expect(table).toBeDefined(); | ||
|
|
||
| // The table has a 4-column grid | ||
| const grid = Array.isArray(table.attrs?.grid) ? table.attrs.grid : []; | ||
| expect(grid.length).toBe(4); |
There was a problem hiding this comment.
expect(table).toBeDefined() will still pass when table is null (Vitest only checks for !== undefined). Since findFirstTable() returns null when not found, this assertion won't fail in that case; use a null-safe assertion (e.g. toBeTruthy() / not.toBeNull()), so the test correctly fails when no table is present.
|
|
||
| if (hasExplicitBorders && tableBorders) { | ||
| // Use cell's borders, with table top border for continuation | ||
| applyBorder(ghostDiv, 'Top', cellBordersAttr.top ?? borderValueToSpec(tableBorders.top)); | ||
| applyBorder( | ||
| ghostDiv, | ||
| 'Left', | ||
| cellBordersAttr.left ?? borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV), | ||
| ); | ||
| applyBorder( | ||
| ghostDiv, | ||
| 'Right', | ||
| cellBordersAttr.right ?? borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV), | ||
| ); | ||
| if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { | ||
| applyBorder(ghostDiv, 'Bottom', cellBordersAttr.bottom ?? borderValueToSpec(tableBorders.insideH)); | ||
| } | ||
| } else if (tableBorders) { | ||
| // Resolve from table borders | ||
| applyBorder(ghostDiv, 'Top', borderValueToSpec(tableBorders.top)); | ||
| applyBorder(ghostDiv, 'Left', borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV)); | ||
| applyBorder(ghostDiv, 'Right', borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV)); | ||
| if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { |
There was a problem hiding this comment.
Ghost cell border resolution currently only runs when tableBorders is present; if a table has no tableBorders but a cell has explicit borders, the ghost cell renders borderless. Also, for rowspans that continue past this fragment, the ghost cell never draws a bottom border at the fragment boundary, which can leave a visible gap when fragment.continuesOnNext closes the table on the page break. Consider matching renderTableRow’s continuation/explicit-border logic for ghost cells (handle explicit borders without table borders, and apply a bottom border when the fragment ends on a page break).
| if (hasExplicitBorders && tableBorders) { | |
| // Use cell's borders, with table top border for continuation | |
| applyBorder(ghostDiv, 'Top', cellBordersAttr.top ?? borderValueToSpec(tableBorders.top)); | |
| applyBorder( | |
| ghostDiv, | |
| 'Left', | |
| cellBordersAttr.left ?? borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV), | |
| ); | |
| applyBorder( | |
| ghostDiv, | |
| 'Right', | |
| cellBordersAttr.right ?? borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV), | |
| ); | |
| if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { | |
| applyBorder(ghostDiv, 'Bottom', cellBordersAttr.bottom ?? borderValueToSpec(tableBorders.insideH)); | |
| } | |
| } else if (tableBorders) { | |
| // Resolve from table borders | |
| applyBorder(ghostDiv, 'Top', borderValueToSpec(tableBorders.top)); | |
| applyBorder(ghostDiv, 'Left', borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV)); | |
| applyBorder(ghostDiv, 'Right', borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV)); | |
| if (effectiveEnd <= fragment.toRow && spanEndRow <= fragment.toRow) { | |
| const shouldDrawBottomBorder = | |
| effectiveEnd <= fragment.toRow && | |
| (spanEndRow <= fragment.toRow || fragment.continuesOnNext === true); | |
| if (hasExplicitBorders) { | |
| if (tableBorders) { | |
| // Use cell's borders, with table borders for continuation | |
| applyBorder(ghostDiv, 'Top', cellBordersAttr.top ?? borderValueToSpec(tableBorders.top)); | |
| applyBorder( | |
| ghostDiv, | |
| 'Left', | |
| cellBordersAttr.left ?? borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV), | |
| ); | |
| applyBorder( | |
| ghostDiv, | |
| 'Right', | |
| cellBordersAttr.right ?? borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV), | |
| ); | |
| if (shouldDrawBottomBorder) { | |
| applyBorder(ghostDiv, 'Bottom', cellBordersAttr.bottom ?? borderValueToSpec(tableBorders.insideH)); | |
| } | |
| } else { | |
| // No table borders; use only explicit cell borders | |
| if (cellBordersAttr.top !== undefined) { | |
| applyBorder(ghostDiv, 'Top', cellBordersAttr.top); | |
| } | |
| if (cellBordersAttr.left !== undefined) { | |
| applyBorder(ghostDiv, 'Left', cellBordersAttr.left); | |
| } | |
| if (cellBordersAttr.right !== undefined) { | |
| applyBorder(ghostDiv, 'Right', cellBordersAttr.right); | |
| } | |
| if (shouldDrawBottomBorder && cellBordersAttr.bottom !== undefined) { | |
| applyBorder(ghostDiv, 'Bottom', cellBordersAttr.bottom); | |
| } | |
| } | |
| } else if (tableBorders) { | |
| // Resolve from table borders | |
| applyBorder(ghostDiv, 'Top', borderValueToSpec(tableBorders.top)); | |
| applyBorder(ghostDiv, 'Left', borderValueToSpec(isFirstCol ? tableBorders.left : tableBorders.insideV)); | |
| applyBorder(ghostDiv, 'Right', borderValueToSpec(isLastCol ? tableBorders.right : tableBorders.insideV)); | |
| if (shouldDrawBottomBorder) { |
| const hasPage2 = await page.evaluate(() => { | ||
| const pages = document.querySelectorAll('.superdoc-page'); | ||
| if (pages.length > 1) { | ||
| const container = document.querySelector('.harness-main'); |
There was a problem hiding this comment.
Unused variable container.
| const container = document.querySelector('.harness-main'); |
| // force a page break so the remaining rows start on the next page instead of | ||
| // continuing to fill the current page with another fragment. | ||
| if (forcePageBreak && currentRow < block.rows.length) { | ||
| state = advanceColumn(state); |
There was a problem hiding this comment.
The value assigned to state here is unused.
| state = advanceColumn(state); | |
| advanceColumn(state); |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a97b2c396
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…endering tests Modified the visual tests for table rendering issues (SD-1797, SD-1859, SD-1895) to improve document availability checks. Tests now skip with a more generic message if the required documents are not found, enhancing clarity and maintainability.
Visual diffs detectedPixel differences were found in visual tests. This is not blocking — reproduce locally with |
1 participant
Summary
Fixes three interrelated table rendering bugs that caused columns to disappear, cells to overflow page boundaries, and autofit tables to render at incorrect widths.
Closes SD-1797, SD-1859, SD-1895
SD-1797: Autofit tables losing columns
cells.lengthinstead ofcolSpansums — rows with rowspan continuations have fewer physical cells, dropping the rightmost columncolwidthvalues (scaled fromtcWduring import) preferred over authoritativew:tblGridvaluesSD-1859: Percent-width table cell overflow in mixed-orientation docs
CleanShot.2026-02-12.at.11.53.59.mp4
SD-1895: Autofit DOCX tables rendering narrow
tcWcell hints took priority overw:tblGrid, making tables render at their hint widths instead of filling the pageChanges
Column width priority (
measuring/dom/index.ts)maxCellCountnow sumscolSpanvalues per row instead of counting physical cellseffectiveTargetWidthPer-fragment column rescaling (
layout-table.ts,incrementalLayout.ts)rescaleColumnWidths()proportionally scales column widths when measured width exceeds fragment widthTableFragment.columnWidthsadded to contracts — carries rescaled widths per fragmentCell width from column widths (
renderTableRow.ts,renderTableCell.ts)renderTableRowcomputes cell width by summingcolumnWidths[gridStart..gridStart+colSpan]renderTableCellaccepts optionalcellWidthparameter, using it instead ofcellMeasure.widthrenderTableFragmentusesfragment.columnWidths ?? measure.columnWidthsaseffectiveColumnWidthsPage break improvements (
layout-table.ts)maxRowspanEndandlastCleanFitRowto avoid splitting rowspan groupscantSplitrows when repeated headers would prevent fittingVisual regression tests
percent-width-table-overflow— validates 100% width table doesn't overflow in portrait pagesautofit-table-fill-width— validates auto-layout tables fill page width (programmatic)autofit-table-docx-rendering— validates SD-1895 DOCX table renders correctlyTest plan
pnpm --filter @superdoc/layout-engine test -- --run layout-table— rowspan-aware breaks, rescaleColumnWidthspnpm --filter @superdoc/painter-dom test -- --run renderTableFragment— effectiveColumnWidths, cell width from columnWidthspnpm --filter @superdoc/layout-bridge test— measurement constraint testspnpm build)