feat: support wide tables rendering outside of margins#2823
Conversation
superdoc-bot
Bot
added
review: thorough
review: careful
and removed
review: thorough
labels
![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: eb148ad488
ℹ️ 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".
| if (justification === 'center') { | ||
| return { x: baseX + Math.max(0, (columnWidth - width) / 2), width }; | ||
| return { x: baseX + (columnWidth - width) / 2, width }; | ||
| } | ||
| if (justification === 'right' || justification === 'end') { | ||
| return { x: baseX + Math.max(0, columnWidth - width), width }; | ||
| return { x: baseX + (columnWidth - width), width }; |
There was a problem hiding this comment.
Removing the Math.max(0, …) clamp for centered/right tables lets fragment.x move left of the table’s actual flow column when width > columnWidth (e.g., a table laid out in column 2 can start inside column 1). Several layout-bridge paths infer column index from fragment.x (notably assignFootnotesToColumns in layout-bridge/src/incrementalLayout.ts and determineColumn in layout-bridge/src/position-hit.ts), so this change can misclassify the table as belonging to the wrong column in multi-column documents, which breaks footnote placement and column-aware hit testing for those tables.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Here for any table with width > columnWidth:
- centered tables would no longer shift left into the margin
- right/end-aligned tables would no longer overflow leftward from the content area
Adding fix to support multi-column layout together with wide tables overflow.
VladaHarbour
force-pushed
the
sd-2544-2545_wide-table-prep
branch
from
eb148ad to
a06f987
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
VladaHarbour
force-pushed
the
sd-2544-2545_wide-table-prep
branch
from
a06f987 to
8f16bcf
Compare
tupizz
left a comment
tupizz
left a comment
There was a problem hiding this comment.
no blocker, just suggestions
lgtm!
VladaHarbour
force-pushed
the
sd-2544-2545_wide-table-prep
branch
from
8f16bcf to
b7ccd7d
Compare
- direct unit tests for resolveTableWidthAttr (contracts) - the new public helper had no test of its own; covers width/value field, type passthrough, zero/negative/non-finite/null rejection. - direct unit tests for resolveTableFrame and resolveRenderedTableWidth (layout-engine) - these are now exported and used by incrementalLayout, so worth testing in isolation: pct calculation, px/dxa fallback to measured, negative x for centered/right wide tables, indent + wide. - pin the quiet behavior change for tableLayout: 'fixed' without an explicit tableWidth (measuring/dom) - grid widths wider than the column now pass through; previously they were scaled to fit. - two click-to-position cases: legacy fragments without columnIndex fall back to visual x via determineColumn, and a fragment claiming a columnIndex past the document column count is clamped to the last valid column.
caio-pizzol
left a comment
caio-pizzol
left a comment
There was a problem hiding this comment.
hey @VladaHarbour, the column ownership fix looks great.
tried a 10" wide table on a 6.5" page - the last column still gets cut off at the page edge. is the rendering fix coming separately, or should it land here?
pushed a commit with tests for the new helpers and pinned a quiet behavior change. three smaller things still inline.
needs work.
|
🎉 This PR is included in @superdoc-dev/react v1.2.0-next.60 The release is available on GitHub release |
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0-next.17 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0-next.62 |
|
🎉 This PR is included in superdoc v1.30.0-next.19 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-cli v0.8.0-next.35 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0-next.21 |
|
🎉 This PR is included in superdoc-cli v0.8.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc-sdk v1.8.0 |
|
🎉 This PR is included in @superdoc-dev/mcp v0.3.0 The release is available on GitHub release |
|
🎉 This PR is included in superdoc v1.31.0 The release is available on GitHub release |
|
🎉 This PR is included in vscode-ext v2.3.0 |
|
🎉 This PR is included in @superdoc-dev/react v1.3.0 The release is available on GitHub release |
7 participants
Summary
Adds support for rendering tables wider than the content column (SD-2544 / SD-2545). Tables with explicit OOXML widths now overflow into page margins instead of being clamped to column width.
Key changes
resolveTableWidthAttr(contracts) validates table width attrs (rejects NaN/Infinity/non-positive). Used by both measuring and layout sides.resolveTableFrame/resolveRenderedTableWidth(layout-engine) resolve final rendered width and x-offset. Allows negative x for centered/right-aligned wide tables; preserves indent for left-aligned, ignores it for justified.columnIndex(logical column owner). Used byposition-hitand footnote placement so click-resolution and footnote anchoring use logical ownership instead of visual x for wide overflow tables.columnIndex, footnote placement on overflow tables, multi-column scenarios.Test plan
pnpm test:layout)pnpm test:visual)