feat(columns): render explicit columns at authored widths with per-column gaps (SD-2629)#3635
Merged
Merged
Conversation
![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: fd141c894f
ℹ️ 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".
…ot a raw slice (SD-2629) resolveColumnCount counts usable explicit widths (finite, > 0), but resolveColumnLayout sliced the raw widths array positionally. An unusable entry before a usable one (e.g. widths [0,192,384] -> count 2) kept the 0 and dropped the 384, so the resolved metadata (widths [0,192]) re-resolved to count 1 and disagreed with the fill count: getColumnGeometryForState normalizes that metadata to one column while the fill advances into two, so columnX(1) clamps to column 0 and content overlaps. Pair each width with the gap that follows it, keep usable-width records, slice to the resolved count, emit gaps for the surviving columns (count-1). This makes resolveColumnLayout idempotent and consistent with resolveColumnCount. DOCX is unaffected (extraction already filters at source); this guards programmatic ColumnLayout inputs. Adds idempotence and gap-pairing tests.
… drive geometry (SD-2629) The semantic change SD-2629 was building toward. normalizeColumnLayout no longer scales explicit widths to fill the content area (authored widths are preserved, leaving trailing space when they underfill, to match Word); equal-mode even-division is unchanged. buildColumnGeometry now uses each column's own gap (gaps[i] ?? uniform gap) for gapAfter and the separator midpoint, and normalize carries the resolved gaps so getColumnGeometry feeds them through. Changes rendering only for unequal explicit columns and non-uniform gaps (the intended visual fix); equal columns are unaffected. Contract tests flipped to no-scale + gaps-drive- geometry. WIP on this stacked branch: downstream geometry tests (renderer separators) and the equal-column-assuming consumers (footnotes, hit-test, floating anchors, balancing) migrate to the geometry next, then F9 + the corpus diff vs the 3b baseline.
…tep-1 wording (SD-2629) The renderer-separator test pinned SCALED explicit widths ([200,952] -> [100,476], separator at 220). Under the no-scale flip the authored widths drive the separator: [200,300] stays unscaled, separator at 96+200+24=320, and the content-gate fragment moved past it. Also corrected the now-stale 'behavior-preserving / step 1' wording in getColumnGeometry's doc and the geometry test group (the flip has landed). Other explicit assertions in index.test.ts are flip-invariant (metadata via resolveColumnLayout; col0 at leftMargin; distinct-count checks); balancing/section-props use their own uniform-stride math and migrate in 4c.
…D-2629 4c) The click/selection hit-test computed the column with uniform equal-stride math (usableWidth/count), ignoring per-column widths. Migrate it to getColumnAtX(getColumnGeometry(normalizeColumnLayout(columns, pageSize.w))), preserving the prior coordinate assumptions: contentWidth = pageSize.w (margins not subtracted) and origin 0, with a gap mapping to the preceding column. Equal-column behavior is identical; explicit unequal columns now map by per-column boundaries instead of a uniform stride. This is the click path, not layout, so it does not affect layout snapshots. Added determineColumn tests (equal + unequal-width). check:types clean.
…(SD-2629 4c) Footnote ref->column assignment used a midpoint rule over uniform-gap stride, and footnote X used a uniform stride (columns.left + idx*(width+ gap)). Both now read getColumnGeometry: assignment keeps the midpoint rule but derives per-column widths/gaps from geometry (and still prefers a table fragment's columnIndex); X uses getColumnX. Placement WIDTH is left uniform (= the measurement width) on purpose - per-column footnote measurement is a separate follow-up, so this pass does not narrow placement and risk a narrow-column text overflow. Equal columns are identical; explicit unequal columns are now assigned/placed per-column, consistent with the geometry flip. check:types clean.
… stride (SD-2629 4c) Correcting an earlier wrong call that this was a no-op: the equal-width guard proves widths match, not that gaps are uniform. SD-2629's core case is equal widths with non-uniform gaps (F9: 192px columns, gaps [0,48]), which the guard still admits, so columnIndex * (columnWidth + columnGap) placed later columns wrong (col 2 at +384 instead of +432). The balancer's columnX now reads getColumnX over the resolved geometry; SectionColumnLayout carries gaps, plumbed from normalized.gaps at both call sites. Equal gaps reduce to the old stride (no change); the equal-width guard and uniform f.width are unchanged. check:types clean.
computeAnchorX, computeTableAnchorX, and the exclusion-zone columnOrigin positioned column-relative anchors with a uniform stride (columnIndex * (width + gap)) and a uniform availableWidth (columns.width). Migrate all three to getColumnX/getColumnWidth over the resolved geometry. The local ColumnLayout type gains widths?/gaps? - the float manager already receives normalizeColumns(...) output carrying them (it was just narrowly typed), so no caller change is needed. Equal columns are identical to the old stride; explicit unequal columns and non-uniform gaps now anchor per-column. check:types clean. This completes 4c: every column consumer (fill, width, separators, hit-test, footnotes, balancing, floating) now reads the single resolved geometry.
Direct layoutDocument acceptance for the step-4 flip: three authored 192px columns with per-column gaps [0, 48] in a 720px content box. Asserts the fill places the three columns at x = 40 / 232 / 472, which only holds when (a) widths are NOT scaled to fill (pre-flip scaled to ~240px -> col1 at 280) and (b) each column is positioned by its own gap (uniform-gap would put col2 at 424, not 472). This is the F9 shape (equal widths, non-uniform gaps): the case where the equal-width balancer guard passes but the old uniform stride mis-placed the trailing column. Traced end-to-end against the resolved- geometry path (clone -> resolveColumnLayout -> page.columns -> normalize -> geometry -> columnX), confirming gaps survive to placement.
…D-2629) The balancing migration (bd2ccf1) builds its geometry input directly from columnCount/columnGap/columnWidth and only attaches a widths array in explicit mode. In equal mode it passed {count, gap, width} with no widths, and getColumnGeometry fell back to a single [width] column - so every column index past 0 clamped onto column 0 and balanced content stacked on the left margin (col2 x 432 -> 96 on the last page of equal-width multi-column docs). A geometry must have exactly count columns. Expand the scalar width to count equal columns when no widths array is present rather than collapsing to one. normalizeColumnLayout already emits one width per column, so the normal engine path is unchanged; this only hardens the hand-built case. The layout corpus caught it (9 equal-width multi-column docs regressed; all restored).
…-2629) main centralized anchor-X into resolveAnchoredGraphicX with a uniform columnIndex * (width + gap) stride; the floating commit made it read the resolved geometry so every anchor path (paragraph + floating) honors authored per-column widths and gaps. This pins it: explicit unequal columns place a column-relative anchor at the authored column origin (not the uniform stride) and right-align within the authored column width (not the max width).
…he full page (SD-2629) Hit-testing resolved column boundaries from the full page width at origin 0, but the engine places fragments at marginLeft + content-relative x over the content width (page minus side margins). With non-zero margins (especially 3+ columns or asymmetric margins) a click near a boundary mapped to the wrong column. Derive geometry from the fragment's own page (margins, size, columns) so multi-section docs with varying margins stay correct; fall back to the first page's margins when no page is supplied. Thread the page through the determineColumn / determineTableColumn call sites. Adds a margin-bearing test.
…surement (SD-2629) The anchor-geometry change positioned column-relative anchors with the per-column width (getColumnWidth), but anchored images/drawings are MEASURED clamped to columns.width (the scalar max), not the per-column width (layout-image / layout-drawing). For a narrow explicit column, a max-sized object centered or right-aligned against the narrower width shifted into the margin or gap. Keep the per-column ORIGIN (getColumnX) but use the scalar column width for available width so placement matches measurement. Per-column available width waits on per-column object measurement (follow-up).
…er (SD-2629) Both balanceSectionOnPage call sites mixed normalized count/gap/width with RAW widths/equalWidth from the section config. Raw widths can be longer than the resolved count, so the equal-width balancing guard read surplus entries (and vetoed balancing of columns that render equal) while the geometry built phantom columns. Source the whole input from the normalized layout through a single toBalancingColumns() builder so the two sites cannot drift apart. Adds the headline F9 test: equal widths + non-uniform gaps [0, 48] balance with per-column x (col1 at 288, not the uniform-stride 312).
…9, Word-verified) A Word probe settled the open question (spec decision 7): Word does NOT scale explicit columns down when their widths exceed the text area. Authored two 360pt columns + 36pt gap in a 468pt content box render at 360pt each, with column 2 overflowing off the page edge; Word re-saves the w:cols unchanged. So step-4's no-scale normalize is correct in both directions (no scale-up for underfull, no scale-down for overfull). Locks it in; no scale-down code added.
caio-pizzol
force-pushed
the
caio-pizzol/sd-2629-step4-flip
branch
from
fd141c8 to
ed56edb
Compare
…ion (SD-2629) determineColumn used page.columns (the page-START config) for every fragment on a page, so a continuous section break that changes columns mid-page mapped fragments in a later region with the wrong geometry. Take the fragment's y and select the matching page.columnRegions entry when present, falling back to page.columns then layout.columns. columnRegions yStart/yEnd are page-relative, the same frame as fragment.y, so no coordinate translation is needed and every hit-test caller already has the fragment. Adds a same-page region test (a hit in the two-column region returns col 1; the single-column region returns 0).
…page-start (SD-2629) determineTableColumn clamped a table fragment's columnIndex against page.columns (the page-START count), so a table laid out in a mid-page two-column region (columnIndex 1) was snapped to 0 when the page started single-column. Factor the region selection the prior commit added into a shared resolveColumnsForHit helper and use it for the table count too, selected by the fragment's y. Export determineTableColumn and add a table-region test (region count 2 keeps columnIndex 1; the single-column region clamps to 0).
The comment claimed a page.columns -> layout.columns fallback, but the code treats a supplied page as authoritative: layout.columns is consulted only when no page is given. Falling back to the document-wide columns for an existing single-column page (the engine leaves page.columns unset for single-column pages) would reintroduce the cross-section mis-mapping the region fix resolves. The comment now matches the code and records why page.columns ?? layout.columns would be wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
|
🎉 This PR is included in superdoc-cli v0.16.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc-sdk v1.15.0 |
Contributor
|
🎉 This PR is included in @superdoc-dev/mcp v0.11.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in superdoc v1.39.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in @superdoc-dev/react v1.10.0 The release is available on GitHub release |
Contributor
|
🎉 This PR is included in vscode-ext v2.11.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explicit
<w:col>widths now render at their authored size instead of being scaled to fill the content area, and each column is positioned by its ownw:spacegap.This matches Word behavior: underfilled explicit columns leave trailing space, and overfilled explicit columns overflow rather than scaling down. Sections with non-uniform gaps no longer collapse those gaps into one uniform value.
Builds on the resolved column-geometry foundation from #3633.
normalizeColumnLayoutpreserves explicit widths in both directions, underfilled and overfilledresolveColumnLayoutfilters usable width/gap records before slicing, so malformed configs stay idempotentEqual-width columns are unchanged. Explicit-column sections move only when their authored widths or gaps differ from the old force-filled geometry.
Review note: anchored graphics intentionally keep scalar available width for now because measurement is still scalar. Per-column available width should wait for per-column image and drawing measurement.
Verified:
pnpm check:typescleanorigin/main: exactly one real geometry change, the intended explicit-column width flip; all other changes are run-to-run id noise