Skip to content

fix(Table): exclude hidden columns from colspan (nuxt/ui@2e8f533) - #550

Merged
IgorShevchik merged 1 commit into
mainfrom
sync/nuxt-2e8f533d
Sep 8, 2026
Merged

IgorShevchik merged 1 commit into
mainfrom
sync/nuxt-2e8f533d

Conversation

@IgorShevchik

Copy link
Copy Markdown
Collaborator

Port of nuxt/ui@2e8f533d — first of ten upstream commits in this run, taken alone since the next is a chore(deps) batch with nothing in common.

Five colspan expressions counted all columns while the body rendered only the visible ones — Table.vue:569 already used row.getVisibleCells(). The row body and the colspan were reading different column sets, which is the whole defect: hide a column and every full-width cell was one column too wide, so the <td> overhung the row it was meant to span.

  • row.getAllCells()row.getVisibleCells() — the expanded row
  • tableApi.getAllLeafColumns()getVisibleLeafColumns()empty, loading, and the two virtualizer padding rows

Applies verbatim; our pre-image has the same five sites and the same two method names, and this fork's naming is not on any changed line.

Coverage was measured per site, and the measurement changed the PR

reverted alone upstream's test
expanded 2 red
empty 2 red
loading 78 green
the two virtualizer rows 78 green

So three of the five were riding on a test that cannot see them. One assertion added for loading — it reads the count from the same object as empty but through a different branch, so a fix applied to one and not the other would still have passed. It now turns 2 red when reverted alone.

The two virtualizer padding rows stay uncovered, and that is stated rather than glossed. They are aria-hidden spacers that exist only when the virtualizer is active, so reaching them means standing up a virtualizer harness — out of proportion to a one-word change, and upstream has no such test either. Recorded in .sync/log/ so the gap is known rather than rediscovered.

Worth recording how the added assertion nearly went in wrong: it first passed ...props, which carries data. The loading row is a sibling v-else-if of the rows, so it renders only when there are none — the rows mounted instead and the query came back empty. It failed on correct code, which is the good failure mode; a test passing for the wrong reason would not have shown itself.

Bookkeeping

  • .sync/nuxt-ui.json — cursor 726e14202e8f533d, entry with the pending-merge placeholder.
  • .sync/dep-parity.json — refreshed at the new cursor; only the cursor line changes.
  • .sync/log/2e8f533d….md — the long-form reasoning.

Local gate green: lint · typecheck · build (3.87 MB) · test (347 files, 7865 passed, 6 skipped) · test:module · repl:build.

Still ahead in this run

Nine more upstream commits: two chore(deps) batches, fix(InputNumber), fix(components) for the virtualizer, an upstream release commit, and four docs-only commits.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JS8ypVfQSFzYVZzkTHhURb


Generated by Claude Code

Five `colspan` expressions counted all columns while the body rendered only the
visible ones — `Table.vue:569` already used `row.getVisibleCells()`. Hide a
column and every full-width cell was one column too wide, so the `<td>` overhung
the row it was meant to span.

`getAllCells()` → `getVisibleCells()` for the expanded row, and
`getAllLeafColumns()` → `getVisibleLeafColumns()` for `empty`, `loading` and the
two virtualizer padding rows.

Coverage measured per site rather than assumed, and the measurement changed what
got written. Upstream's case turns red for `expanded` and `empty`, but reverting
`loading` alone left 78 green, as did reverting both virtualizer rows — three of
the five were riding on a test that cannot see them. One assertion added for
`loading`, which reads the count from the same object as `empty` but through a
different branch; it now turns 2 red when reverted alone.

The two virtualizer padding rows stay uncovered. They are `aria-hidden` spacers
that exist only when the virtualizer is active, so reaching them means standing
up a virtualizer harness — out of proportion to a one-word change, and upstream
has no such test either. Recorded in the log rather than left to be rediscovered.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JS8ypVfQSFzYVZzkTHhURb
@IgorShevchik
IgorShevchik merged commit bf25c1e into main Sep 8, 2026
2 checks passed
@IgorShevchik
IgorShevchik deleted the sync/nuxt-2e8f533d branch September 8, 2026 07:37
IgorShevchik pushed a commit that referenced this pull request Sep 8, 2026
`2e8f533d` went in with `pending-merge` in both fields, as PORTING.md §6 step 4
requires — the entry is written with the port, before its PR has a number or a
squash SHA. That PR has now merged:

- `2e8f533d` → #550 / `bf25c1eb`

Carried here rather than in its own bookkeeping PR, which is the mechanism §6
step 4 describes: the next port in a run reconciles the previous entry, and only
the last entry needs a PR of its own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JS8ypVfQSFzYVZzkTHhURb
IgorShevchik added a commit that referenced this pull request Sep 8, 2026
…d4adf) (#551)

* chore(deps): update non-major dependencies, pin happy-dom (nuxt/ui@ebd4adf)

Every package this fork shares with upstream in the same section sat at
upstream's exact pre-image, checked package by package rather than inferred from
`dep-parity.json` being green. Six of upstream's names are absent here, and the
`@internationalized/*` peer ranges are `^3.0.0` on both sides — upstream did not
touch those either.

`playgrounds/demo` is ours alone and declares four of the bumped packages. Two
sat at upstream's pre-image; the other two had drifted behind *this fork's own
root* — `@ai-sdk/vue` `^4.0.77` against our `^4.0.85`, `ai` `^7.0.77` against
`^7.0.85`. Brought up with the rest. That is the "one-time divergence becomes
permanent" failure `dep-parity.json` describes, and demo is invisible to that
file because upstream has no counterpart to compare against.

`happy-dom` is NOT taken, and the reason outlives this commit.

Upstream goes `^20.12.0` → `^20.14.0`. Taking it turns 1450 tests and 136
snapshots red across 64 files:

    TypeError: 'defineProperty' on proxy: trap returned falsish
    for property '__v_skip'

Vue's `markRaw` against a happy-dom proxy, raised from reka's `Presence`
watcher. Isolated rather than guessed — with every other bump applied and
happy-dom alone moved back, the suite is green — then bisected: 20.12.0 passes;
20.12.1, 20.12.2, 20.13.0, 20.13.1 and 20.14.0 all fail.

The declared range was `^20.12.0`, which admits every one of those. It worked
only because the lockfile happened to hold 20.12.0 — any fresh resolution would
have taken 20.14.0 and reddened the suite with no source change to point at.
`~20.12.0` is not enough either, since 20.12.1 already breaks. So the
declaration becomes an exact pin and the reason goes in `dep-parity.json`'s
`exceptions`, where the next deps port will read it. This closes a landmine
rather than adding a restriction.

Upstream ships `^20.14.0` and is presumably green, so whatever differs is on our
side — reka's version or the vitest environment. Finding it is not something a
dependency-bump port can do; recorded for a follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JS8ypVfQSFzYVZzkTHhURb

* chore(sync): reconcile the #550 ledger entry

`2e8f533d` went in with `pending-merge` in both fields, as PORTING.md §6 step 4
requires — the entry is written with the port, before its PR has a number or a
squash SHA. That PR has now merged:

- `2e8f533d` → #550 / `bf25c1eb`

Carried here rather than in its own bookkeeping PR, which is the mechanism §6
step 4 describes: the next port in a run reconciles the previous entry, and only
the last entry needs a PR of its own.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JS8ypVfQSFzYVZzkTHhURb

---------

Co-authored-by: Shevchik Igor <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants