Skip to content

fix: issue with vertical cells merging#2387

Open
VladaHarbour wants to merge 1 commit intomainfrom
sd-1799_cell-merging-issue
Open

fix: issue with vertical cells merging#2387
VladaHarbour wants to merge 1 commit intomainfrom
sd-1799_cell-merging-issue

Conversation

@VladaHarbour
Copy link
Contributor

@VladaHarbour VladaHarbour commented Mar 13, 2026

Table cells didn't get merged vertically
Screen Shot 2026-02-04 at 1 23 39 PM

@linear
Copy link

linear bot commented Mar 13, 2026

@github-actions
Copy link
Contributor

Status: PASS

The three elements/attributes touched by this PR are all correctly handled per the spec:

w:vMerge — The getTableCellVMerge helper returns vMerge.attributes?.['w:val'] || 'continue' when w:val is absent, which matches the spec: the default value of w:vMerge with no w:val attribute is continue (ST_Merge, §17.18.57). The two valid values (restart, continue) are the only ones checked in the rowspan loop. All correct.

w:gridBefore — Read from w:trPr as w:val (ST_DecimalNumber). The spec confirms this is a child of w:trPr with a single w:val attribute (§17.4.15), and omitting it defaults to zero — which the code handles correctly with the value > 0 guard. All correct.

w:gridSpan — Read from w:tcPr as w:val (ST_DecimalNumber). The spec states omitting it means a span of 1 (§17.4.17), which the return ... value > 0 ? value : 1 fallback handles correctly. All correct.

The logical change — matching continuation cells by grid-column position using gridBefore + gridSpan rather than by raw tc index — is the right interpretation of the spec model. The spec treats all cells as snapping to a shared column grid, so row-relative cell indexing was the bug. The new implementation is spec-conformant.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e6b461d4e6

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +107 to +108
if (node._vMergeConsumed && Number.isFinite(node._vMergeConsumedGridSpan)) {
currentColumnIndex = startColumn + node._vMergeConsumedGridSpan;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid double-advancing columns for consumed merge cells

When this branch runs, skipOccupiedColumns() has already consumed the same merged columns via activeRowSpans, so incrementing currentColumnIndex by _vMergeConsumedGridSpan advances a second time. In rows that contain a vertical-merge continuation plus later cells (for example [continue, normal]), the next real cell is encoded one column too far, which miscomputes columnWidth and can misalign subsequent merge/colspan handling.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant