Skip to content

fix(Table): wire virtualizer.measureElement on rows for dynamic sizing#6379

Open
claylevering wants to merge 1 commit intonuxt:v4from
claylevering:fix/table-virtualize-measure-element
Open

fix(Table): wire virtualizer.measureElement on rows for dynamic sizing#6379
claylevering wants to merge 1 commit intonuxt:v4from
claylevering:fix/table-virtualize-measure-element

Conversation

@claylevering
Copy link
Copy Markdown
Contributor

🔗 Linked Issue

Closes #6101

📚 Description

The measureElement option on the virtualize prop is forwarded to useVirtualizer, but the callback is never invoked because the <tr> elements are missing two things TanStack Virtual needs to track them:

  1. a data-index attribute
  2. a template ref to virtualizer.measureElement

Without the ref, the virtualizer never observes the rows (no ResizeObserver.observe call), so options.measureElement is never called and dynamic row heights are impossible.

This PR:

  • Adds :ref (bound to virtualizer.measureElement) and :data-index on each virtualized <tr> so TanStack Virtual registers and measures them.
  • Stops forcing height: virtualRow.size as an inline style on each row so natural content height is actually measurable (otherwise the row would always measure exactly estimateSize regardless of content).
  • Simplifies the per-row transform to a uniform translateY(virtualItems[0].start) across the visible set. The previous formula virtualRow.start - index * virtualRow.size only produced the correct offset when every row had the same size — variable-height rows were positioned incorrectly after measurement.

Reproducing the original bug

<UTable
  virtualize
  :data="rows"
  :columns="columns"
  :virtualize="{
    estimateSize: () => 65,
    measureElement: (el) => {
      console.log('measured:', el) // never logged before this PR
      return el?.getBoundingClientRect().height || 65
    }
  }"
  class="h-[500px]"
/>

With this PR the callback fires for every mounted row and dynamic heights are respected.

🔖 Checklist

  • Read contribution guide.
  • Installed dependencies with pnpm install.
  • Code is properly formatted and linted with pnpm lint --fix.
  • Made sure the docs build correctly with pnpm docs:build.
  • Made sure tests are passing with pnpm test.

@github-actions github-actions Bot added the v4 #4488 label Apr 21, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 21, 2026

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Wires per-row DOM measurement into the table virtualizer by adding a measureRowRef to each virtualized <tr> (with data-index) and invoking a user-provided virtualize.measureElement (falling back to entry.borderBoxSize[0].blockSize or el.offsetHeight). If a measured <tr> has data-expanded="true", the immediate next <tr> height is added. Refactors virtualization state to expose virtualItems and virtualOffset, computes renderedSize from virtualItems[].size, passes index into row templates, and applies a single translateY(virtualOffset) to virtual rows. Adds docs for dynamic row heights and a measureElement example.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: wiring virtualizer.measureElement on rows for dynamic sizing, which directly addresses the PR's core objective.
Description check ✅ Passed The description provides a comprehensive explanation of the bug, the fix, and how to reproduce the original issue, directly relating to the changeset.
Linked Issues check ✅ Passed The PR fully addresses issue #6101: adds required DOM bindings (ref to virtualizer.measureElement and data-index), enables the callback invocation, removes forced height styling, fixes row positioning, and provides expansion-aware measurement.
Out of Scope Changes check ✅ Passed All changes are within scope: Table.vue modifications implement the DOM binding requirements, documentation updates explain the dynamic row heights feature without introducing unrelated functionality.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/runtime/components/Table.vue (1)

530-583: ⚠️ Potential issue | 🟠 Major

Expanded rows are not measured by the virtualizer — scroll extent and footer positioning will be inaccurate when rows are expanded.

DefineRowTemplate renders two sibling <tr> elements when row.getIsExpanded() is true (the primary row at lines 531–576 and the expansion row at lines 578–582), but measureRowRef is only attached to the primary <tr>. TanStack Virtual therefore measures only the primary row's height and ignores the expansion <tr> below it.

Because virtualizer.getTotalSize() (line 709) is computed from measured row heights, and because the footer offset depends on this value (line 671), expanding rows will cause:

  • the scrollable container height to fall short of actual rendered content,
  • the visible-range calculations to drift as expansions accumulate,
  • the footer to under-shift.

Visually the rows stack correctly (normal flow), but scrolling behavior degrades.

Suggestion: Wrap both <tr> elements in a single measurable container (e.g., a <tbody> fragment or a div with display: contents), or measure the combined height of both rows by reading the next sibling when data-expanded="true" in a custom measureElement callback.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/components/Table.vue` around lines 530 - 583, The virtualizer
only measures the primary <tr> because measureRowRef is attached only to the
first row in DefineRowTemplate, so expanded rows (row.getIsExpanded()) are
ignored and virtualizer.getTotalSize() becomes incorrect; fix by ensuring the
measured element includes both the main and expansion row: either wrap the two
<tr> siblings in a single measurable container (e.g., a wrapper element with
display: contents) and attach measureRowRef to that wrapper, or implement a
custom measureElement callback used by measureRowRef that, when it sees a
measured element with data-expanded="true", also measures its immediate
expansion sibling (the <tr> rendered when row.getIsExpanded()) and returns the
combined height; update usages of measureRowRef/measureElement in
DefineRowTemplate so virtualizer receives the combined row height.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@src/runtime/components/Table.vue`:
- Around line 530-583: The virtualizer only measures the primary <tr> because
measureRowRef is attached only to the first row in DefineRowTemplate, so
expanded rows (row.getIsExpanded()) are ignored and virtualizer.getTotalSize()
becomes incorrect; fix by ensuring the measured element includes both the main
and expansion row: either wrap the two <tr> siblings in a single measurable
container (e.g., a wrapper element with display: contents) and attach
measureRowRef to that wrapper, or implement a custom measureElement callback
used by measureRowRef that, when it sees a measured element with
data-expanded="true", also measures its immediate expansion sibling (the <tr>
rendered when row.getIsExpanded()) and returns the combined height; update
usages of measureRowRef/measureElement in DefineRowTemplate so virtualizer
receives the combined row height.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 789ccb9e-8083-40ab-94fa-816859c5e71b

📥 Commits

Reviewing files that changed from the base of the PR and between 8d08ef3 and c1ff5b8.

📒 Files selected for processing (1)
  • src/runtime/components/Table.vue

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 21, 2026

npm i https://pkg.pr.new/@nuxt/ui@6379

commit: 102ddc2

claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row renders an expanded sibling `<tr>` (`row.getIsExpanded()`),
the virtualizer measures only the primary `<tr>` and misses the
expansion, causing `getTotalSize()` to under-report and the footer
`translateY` offset to drift.

Provide a default `measureElement` that, when the measured element has
`data-expanded="true"`, also adds the immediate next `<tr>` sibling's
height. A user-supplied `measureElement` still takes precedence for the
primary row — the expansion sum is layered on top.

Addresses CodeRabbit review feedback on nuxt#6379.
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row renders an expanded sibling `<tr>` (`row.getIsExpanded()`),
the virtualizer measures only the primary `<tr>` and misses the
expansion, causing `getTotalSize()` to under-report and the footer
`translateY` offset to drift.

Provide a default `measureElement` that, when the measured element has
`data-expanded="true"`, also adds the immediate next `<tr>` sibling's
height. A user-supplied `measureElement` still takes precedence for the
primary row — the expansion sum is layered on top.

Addresses CodeRabbit review feedback on nuxt#6379.
@claylevering
Copy link
Copy Markdown
Contributor Author

Addressed CodeRabbit's expanded-row finding in d28698f. Rather than restructuring the row template (the expansion <tr> is a sibling of the main row inside a shared <tbody>, so a display:contents wrapper or nested <tbody> would break the table DOM contract), I added a default measureElement on the virtualizer that, when data-expanded="true", adds the immediate next-sibling <tr>'s height to the base measurement. A user-supplied measureElement still takes precedence for measuring the main row — the expansion sum is layered on top, so user control is preserved.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/runtime/components/Table.vue (1)

545-552: ⚠️ Potential issue | 🟠 Major

Apply the virtual offset to the expanded sibling row too.

Line 651 passes the virtualization transform into ReuseRowTemplate, but Line 593 renders the expanded <tr> sibling without that style. When a virtualized expanded row is scrolled down, the main row is translated while its expanded content remains at the un-offset table-flow position.

🐛 Proposed fix
-    <tr v-if="row.getIsExpanded()" data-slot="tr" :class="ui.tr({ class: [uiProp?.tr] })">
+    <tr v-if="row.getIsExpanded()" data-slot="tr" :class="ui.tr({ class: [uiProp?.tr] })" :style="style">
       <td :colspan="row.getAllCells().length" data-slot="td" :class="ui.td({ class: [uiProp?.td] })">
         <slot name="expanded" :row="row" />
       </td>

Also applies to: 593-593, 646-653

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/components/Table.vue` around lines 545 - 552, The expanded
sibling <tr> rendered by DefineRowTemplate is missing the virtualization
transform style, so when a row is virtualized the main row gets translated but
the expanded row stays in the original flow; update the expanded-row render (the
<tr> that is created when row.getIsExpanded() is true) to receive the same style
prop used for the main row (the style object from DefineRowTemplate /
ReuseRowTemplate) by binding :style="style" (or applying the same transform
value) and ensure any reuse via ReuseRowTemplate also forwards that style so
expanded siblings stay aligned during virtualization.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/content/docs/2.components/table.md`:
- Line 688: Change the "Dynamic row heights" heading from an h4 to an h3 and
append the release badge `:badge{label="Soon" class="align-text-top"}` to that
heading; specifically update the heading text "Dynamic row heights" so it uses a
third-level markdown heading (###) instead of fourth-level (####) and include
the Soon badge inline after the heading to satisfy markdownlint and the docs
badge guideline.

---

Outside diff comments:
In `@src/runtime/components/Table.vue`:
- Around line 545-552: The expanded sibling <tr> rendered by DefineRowTemplate
is missing the virtualization transform style, so when a row is virtualized the
main row gets translated but the expanded row stays in the original flow; update
the expanded-row render (the <tr> that is created when row.getIsExpanded() is
true) to receive the same style prop used for the main row (the style object
from DefineRowTemplate / ReuseRowTemplate) by binding :style="style" (or
applying the same transform value) and ensure any reuse via ReuseRowTemplate
also forwards that style so expanded siblings stay aligned during
virtualization.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8aeed756-b0d9-4de8-ab87-ca5396ceb3a8

📥 Commits

Reviewing files that changed from the base of the PR and between d28698f and a354939.

📒 Files selected for processing (2)
  • docs/content/docs/2.components/table.md
  • src/runtime/components/Table.vue

Comment thread docs/content/docs/2.components/table.md Outdated
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row is expanded inside a virtualized table, the main <tr>
received transform: translateY(virtualOffset) but its expanded sibling
did not, so the expansion content stayed at its natural flow position
while the main row visually moved. Forward the same style prop to the
expanded row so both stay visually stacked.

Also promote 'Dynamic row heights' in the table docs from h4 to h3 to
match the rest of the page's section hierarchy and tag it with the
Soon release badge per docs convention.

Addresses CodeRabbit review feedback on nuxt#6379.
@claylevering
Copy link
Copy Markdown
Contributor Author

Addressed both findings from the latest CodeRabbit pass in a000de2:

  • Expanded <tr> now receives the same :style as its main row, so the translateY virtual offset applies to both and the expansion stays visually adjacent when scrolled.
  • Dynamic row heights heading promoted from h4 to h3 for consistency with the rest of the page, with the Soon release badge added per docs convention.

claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row is expanded inside a virtualized table, the main <tr>
received transform: translateY(virtualOffset) but its expanded sibling
did not, so the expansion content stayed at its natural flow position
while the main row visually moved. Forward the same style prop to the
expanded row so both stay visually stacked.

Also promote 'Dynamic row heights' in the table docs from h4 to h3 to
match the rest of the page's section hierarchy and tag it with the
Soon release badge per docs convention.

Addresses CodeRabbit review feedback on nuxt#6379.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/runtime/components/Table.vue (1)

102-104: Clarify interaction between user measureElement and automatic expansion handling.

The JSDoc says the virtualizer "will measure each rendered row and automatically include the expanded row's height". It's worth calling out explicitly that this addition happens on top of any user-supplied measureElement result — otherwise a user who implements their own measurement that already wraps the expansion region could be surprised by a double-count.

📝 Suggested wording tweak
-   * Pass a `measureElement` function to opt into dynamic row heights — the virtualizer
-   * will measure each rendered row and automatically include the expanded row's height
-   * when `row.getIsExpanded()` is `true`.
+   * Pass a `measureElement` function to opt into dynamic row heights. The value returned
+   * by your `measureElement` is used for the main row; when `row.getIsExpanded()` is
+   * `true`, the immediate next sibling `<tr>`'s height is added automatically on top of
+   * that value, so your callback should measure the main row only.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/components/Table.vue` around lines 102 - 104, Update the JSDoc
for measureElement to explicitly state how the virtualizer interacts with a
user-provided measureElement: explain that the virtualizer will still measure
each rendered row and, when row.getIsExpanded() is true, will add the expanded
row's height on top of the value returned by the user-supplied measureElement;
warn consumers that if their measureElement already includes the expanded region
they should avoid double-counting (or return the collapsed height) and reference
the symbols measureElement, row.getIsExpanded(), and virtualizer so readers can
locate the related code paths.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/runtime/components/Table.vue`:
- Around line 447-457: The current measureElement function doesn't trigger when
a row's expansion toggles because the expanded content is a sibling and
ResizeObserver won't fire; fix by adding a MutationObserver inside
measureElement (or during row mount) that watches the row Element's
"data-expanded" attribute and, when it changes, calls the virtualizer's
re-measure API (e.g., virtualizer.measureElement(el) or
virtualizer.measure(index)) so the cached height is refreshed; ensure you
reference the existing measureElement and virtualizerProps symbols and avoid
installing this observer when a custom userMeasure is provided (or document that
a custom measureElement must include expanded sibling height) to prevent
double-counting.

---

Nitpick comments:
In `@src/runtime/components/Table.vue`:
- Around line 102-104: Update the JSDoc for measureElement to explicitly state
how the virtualizer interacts with a user-provided measureElement: explain that
the virtualizer will still measure each rendered row and, when
row.getIsExpanded() is true, will add the expanded row's height on top of the
value returned by the user-supplied measureElement; warn consumers that if their
measureElement already includes the expanded region they should avoid
double-counting (or return the collapsed height) and reference the symbols
measureElement, row.getIsExpanded(), and virtualizer so readers can locate the
related code paths.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 677f6bfa-2a57-4ba0-b196-6f96c0ed0306

📥 Commits

Reviewing files that changed from the base of the PR and between a354939 and a000de2.

📒 Files selected for processing (2)
  • docs/content/docs/2.components/table.md
  • src/runtime/components/Table.vue
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/content/docs/2.components/table.md

Comment thread src/runtime/components/Table.vue
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
Tanstack's ResizeObserver is attached to the main <tr>, but expanding
a row inserts a *sibling* <tr> that doesn't resize the main row — the
RO never fires, so the virtualizer keeps its stale un-expanded cached
height. Watch the TanStack expanded state and call
virtualizer.measureElement on every rendered row after the DOM
updates, which picks up the new combined height via the existing
expansion-aware measureElement we ship.

Also clarify the virtualize JSDoc: expansion height is added on top
of whatever the user's measureElement returns, so user callbacks
should measure the main row only (or return collapsed height if
they already account for expansion) to avoid double-counting.

Addresses CodeRabbit review feedback on nuxt#6379.
@claylevering
Copy link
Copy Markdown
Contributor Author

Addressed both findings from the latest review in 7217b34:

  • Expanded state toggle re-measure: Added a watcher on tableApi.getState().expanded that calls virtualizer.measureElement on every rendered row after the DOM settles. As CodeRabbit noted, the ResizeObserver on the main <tr> never fires when a sibling expansion row is inserted, so the virtualizer's cached height stayed stale until the user scrolled. The watcher flushes it through the expansion-aware measureElement the PR already ships.
  • JSDoc clarification: Rewrote the virtualize JSDoc to make the composition explicit — the user's measureElement return value is used for the main row, the next-sibling <tr>'s height is added on top of it when the row is expanded, and consumers whose own measureElement already wraps the expansion region should return only the collapsed height to avoid double-counting.

claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
Tanstack's ResizeObserver is attached to the main <tr>, but expanding
a row inserts a *sibling* <tr> that doesn't resize the main row — the
RO never fires, so the virtualizer keeps its stale un-expanded cached
height. Watch the TanStack expanded state and call
virtualizer.measureElement on every rendered row after the DOM
updates, which picks up the new combined height via the existing
expansion-aware measureElement we ship.

Also clarify the virtualize JSDoc: expansion height is added on top
of whatever the user's measureElement returns, so user callbacks
should measure the main row only (or return collapsed height if
they already account for expansion) to avoid double-counting.

Addresses CodeRabbit review feedback on nuxt#6379.
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

🧹 Nitpick comments (2)
src/runtime/components/Table.vue (2)

100-109: Worth documenting the ResizeObserver limitation for dynamic expansion content.

The JSDoc already explains composition between measureElement and the auto-added sibling height, which is great. Consider also noting that TanStack Virtual's ResizeObserver is attached to the main <tr> only — so size changes inside the expanded sibling after initial mount (late-loading images, nested toggles, async content) won't trigger a re-measure automatically. Users with highly dynamic expansion content will need to call virtualizer.measure() themselves. The watcher added here covers expansion toggles but not intra-expansion resizes.

Purely a docs improvement — not a blocker.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/components/Table.vue` around lines 100 - 109, Update the JSDoc
for the measureElement/virtualization block to document that the ResizeObserver
is attached only to the main <tr> (used by the measureElement callback) so
dynamic size changes inside the expanded sibling (late-loading images, async
content, nested toggles) will not auto-trigger re-measurement; mention that the
existing watcher handles expansion toggles but not intra-expansion resizes and
recommend callers explicitly call virtualizer.measure() when expansion-region
content can change size after mount. Include references to the measureElement
function, the virtualizer.measure() method, and the expansion watcher in the
comment so users know when manual measurement is required.

479-493: Expansion watcher: redundant guard and full-sweep re-measurement.

Two small refinements on the watcher:

  1. Line 485's if (!virtualizer) return is unreachable — the entire watch(...) is already inside if (virtualizer) { ... } at line 479. Safe to drop.
  2. The callback re-measures every [data-index] row in the scroll container on any change to expanded, deep. For large virtualized viewports with frequent toggles, this is O(visibleRows) work per toggle when only one row's cached size actually changed. Consider diffing the old/new expanded state and calling virtualizer.value.measureElement only on the affected row element(s), e.g. root?.querySelector('[data-index="${index}"]') for each changed row id → index.

Not a correctness issue — functional behavior matches the goal of keeping cached heights in sync after sibling-<tr> insertion.

♻️ Minimal cleanup (drop redundant guard)
 if (virtualizer) {
   watch(
     () => tableApi.getState().expanded,
     () => {
       nextTick(() => {
         const root = rootRef.value?.$el as HTMLElement | undefined
-        if (!virtualizer) return
         root?.querySelectorAll<HTMLElement>('[data-index]').forEach((row) => {
           virtualizer.value.measureElement(row)
         })
       })
     },
     { deep: true }
   )
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/runtime/components/Table.vue` around lines 479 - 493, Remove the
redundant null-check inside the watcher (the inner `if (!virtualizer) return`)
since the whole watch is already guarded by `if (virtualizer) { ... }`, and
change the watcher callback to diff the previous and current
`tableApi.getState().expanded` values instead of re-measuring all rows: use the
old/new expanded maps to compute which row indices changed, then for each
changed index use `rootRef.value?.$el.querySelector('[data-index="${index}"]')`
(or similar) and call `virtualizer.value.measureElement` only for those
elements; keep the watcher on `tableApi.getState().expanded` but remove the deep
full-sweep and perform targeted re-measurement via
`virtualizer.value.measureElement`.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/runtime/components/Table.vue`:
- Around line 100-109: Update the JSDoc for the measureElement/virtualization
block to document that the ResizeObserver is attached only to the main <tr>
(used by the measureElement callback) so dynamic size changes inside the
expanded sibling (late-loading images, async content, nested toggles) will not
auto-trigger re-measurement; mention that the existing watcher handles expansion
toggles but not intra-expansion resizes and recommend callers explicitly call
virtualizer.measure() when expansion-region content can change size after mount.
Include references to the measureElement function, the virtualizer.measure()
method, and the expansion watcher in the comment so users know when manual
measurement is required.
- Around line 479-493: Remove the redundant null-check inside the watcher (the
inner `if (!virtualizer) return`) since the whole watch is already guarded by
`if (virtualizer) { ... }`, and change the watcher callback to diff the previous
and current `tableApi.getState().expanded` values instead of re-measuring all
rows: use the old/new expanded maps to compute which row indices changed, then
for each changed index use
`rootRef.value?.$el.querySelector('[data-index="${index}"]')` (or similar) and
call `virtualizer.value.measureElement` only for those elements; keep the
watcher on `tableApi.getState().expanded` but remove the deep full-sweep and
perform targeted re-measurement via `virtualizer.value.measureElement`.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 797d8ac2-c21e-4b2f-a619-b4e929ba2d5d

📥 Commits

Reviewing files that changed from the base of the PR and between a000de2 and 7217b34.

📒 Files selected for processing (1)
  • src/runtime/components/Table.vue

@benjamincanac
Copy link
Copy Markdown
Member

@claylevering There's an open PR that alter the Table virtualization behavior already: #6217

Could that solve your issue?

@claylevering
Copy link
Copy Markdown
Contributor Author

@benjamincanac thanks — I did look at #6217 before opening this, and I think the two PRs are complementary rather than one superseding the other. cc @ChronicStone for ack.

#6217 (ChronicStone) fixes sticky + virtualize. Structurally it's the right move: drop the outer scroll <div>, render top/bottom spacer <tr>s for padding, let the <table> be the scroll region so position: sticky on <thead>/<tfoot> works. Very clean, standard TanStack-virtual-for-tables pattern.

#6379 (this PR) fixes measureElement (#6101). It binds virtualizer.measureElement as a template ref on the <tr> plus data-index, removes forced height: virtualRow.size so natural content is measurable, adds expansion-aware measurement, and re-measures when the expanded state toggles. The branch in #6217 doesn't bind measureElement and keeps the forced height, so the original issue would still reproduce on it.

Combined PR sketch: ChronicStone's spacer-row skeleton + this PR's measurement wiring on top of it is the right shape. Rough outline:

  1. Start from feat(Table): support sticky header/footer in virtualized mode #6217's structure: no outer <div>, spacer <tr>s for virtualPaddingTop/virtualPaddingBottom, keep the v-if="centerRows[virtualRow.index]" bounds guard.
  2. Drop the forced height: virtualRow.size on the main row — let it be natural, measurement fills in the real value.
  3. Add :ref="measureRowRef" + :data-index on the virtualized row so TanStack observes it.
  4. Add the expansion-aware default measureElement (sums the next-sibling <tr>'s height when data-expanded="true") and apply :style="style" to the expanded row.
  5. Add the watcher on tableApi.getState().expanded that triggers re-measure after toggle.

I'd be happy to do any of:

Let me know which direction you'd prefer and I'll move on it.

@claylevering
Copy link
Copy Markdown
Contributor Author

Went ahead and built the combined tree as a concrete reference: claylevering:fix/table-virtualize-combined.

It's 8 commits on top of current v4:

  • 7 commits cherry-picked from @ChronicStone's features/sticky-virtual-table (rebased cleanly, no conflicts)
  • 1 measurement commit on top: fix(Table): wire virtualizer.measureElement for dynamic row heights

Validated locally: pnpm lint clean, vue-tsc --noEmit clean, pnpm test Table.spec 74/74 passing (includes @ChronicStone's new sticky-virtualize snapshot).

Happy to open this as a single PR that supersedes both #6217 and #6379 with @ChronicStone as co-author, or fold just the measurement commit into #6217 directly — whichever you both prefer. Let me know.

@ChronicStone
Copy link
Copy Markdown
Contributor

Hi @claylevering I'm totally fine with either options, whatever you prefer 😄

@benjamincanac
Copy link
Copy Markdown
Member

Ok let's merge #6217 first then! I do need to run some more tests though to validate the behavior.

@claylevering
Copy link
Copy Markdown
Contributor Author

Thoughts on release timeline @benjamincanac ?

claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row renders an expanded sibling `<tr>` (`row.getIsExpanded()`),
the virtualizer measures only the primary `<tr>` and misses the
expansion, causing `getTotalSize()` to under-report and the footer
`translateY` offset to drift.

Provide a default `measureElement` that, when the measured element has
`data-expanded="true"`, also adds the immediate next `<tr>` sibling's
height. A user-supplied `measureElement` still takes precedence for the
primary row — the expansion sum is layered on top.

Addresses CodeRabbit review feedback on nuxt#6379.
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
When a row is expanded inside a virtualized table, the main <tr>
received transform: translateY(virtualOffset) but its expanded sibling
did not, so the expansion content stayed at its natural flow position
while the main row visually moved. Forward the same style prop to the
expanded row so both stay visually stacked.

Also promote 'Dynamic row heights' in the table docs from h4 to h3 to
match the rest of the page's section hierarchy and tag it with the
Soon release badge per docs convention.

Addresses CodeRabbit review feedback on nuxt#6379.
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
Tanstack's ResizeObserver is attached to the main <tr>, but expanding
a row inserts a *sibling* <tr> that doesn't resize the main row — the
RO never fires, so the virtualizer keeps its stale un-expanded cached
height. Watch the TanStack expanded state and call
virtualizer.measureElement on every rendered row after the DOM
updates, which picks up the new combined height via the existing
expansion-aware measureElement we ship.

Also clarify the virtualize JSDoc: expansion height is added on top
of whatever the user's measureElement returns, so user callbacks
should measure the main row only (or return collapsed height if
they already account for expansion) to avoid double-counting.

Addresses CodeRabbit review feedback on nuxt#6379.
claylevering added a commit to claylevering/ui that referenced this pull request Apr 21, 2026
…imits

Address CodeRabbit nitpicks on nuxt#6379:

- Watcher: diff the before/after TanStack expanded state and only call
  virtualizer.measureElement on rows whose expanded flag actually
  toggled, instead of sweeping every visible row on every change. When
  the state is the boolean sentinel `true` (expand-all) we fall back
  to the full sweep since there are no individual row ids to diff.
- Watcher: drop the redundant inner `if (!virtualizer)` guard — the
  whole watch is already inside `if (virtualizer) { ... }`.
- JSDoc: note that TanStack's ResizeObserver is attached to the main
  <tr> only, and the built-in expansion watcher fires on expand/collapse
  toggles but not on intra-expansion size changes (late-loading images,
  async content, nested toggles). Users whose expansion region can
  resize after mount should call `virtualizer.measure()` themselves.
@claylevering claylevering force-pushed the fix/table-virtualize-measure-element branch from 5f28bbe to 8d86a67 Compare April 21, 2026 11:20
@claylevering
Copy link
Copy Markdown
Contributor Author

Addressed both CodeRabbit nitpicks from the most recent review in 8d86a67:

  • Targeted re-measure. Watcher now diffs the before/after TanStack expanded state and calls virtualizer.measureElement only on rows whose expanded flag actually toggled, instead of sweeping every [data-index] row on each change. Boolean sentinel states (true = expand-all) fall back to a full sweep since there are no individual row ids to diff against.
  • Dropped redundant guard. The inner if (!virtualizer) return inside the watcher was unreachable (outer if (virtualizer) { … } already gates it). Removed.
  • JSDoc. Added a note that TanStack Virtual's ResizeObserver is attached to the main <tr> only, so size changes inside the expansion sibling (late-loading images, async content, nested toggles) won't auto-trigger a re-measure — users should call virtualizer.measure() themselves in those cases. The built-in watcher covers expansion toggles, not intra-expansion resizes.

Same refinements mirrored onto the combined branch (bd9882b on fix/table-virtualize-combined) so whichever path maintainers pick stays in sync.

@benjamincanac
Copy link
Copy Markdown
Member

@claylevering I've merged #6217 but there are some conflicts to be fixed.

Rebased on top of nuxt#6217's spacer-row structure.

- Bind virtualizer.measureElement as a template ref plus data-index on
  the virtualized <tr> so TanStack Virtual observes it and invokes the
  measureElement option. Without this the user's measureElement option
  was typed but never called, blocking dynamic heights entirely (nuxt#6101).
- Stop forcing the virtualRow.size style on the main row so natural
  content height is measurable; estimateSize becomes a true initial
  guess only.
- Default measureElement adds the immediate next-sibling <tr>'s height
  when data-expanded="true", so expansion rows are included in the
  cached size. A user-supplied measureElement still measures the main
  row; the expansion sum is layered on top.
- Forward the per-row :style to the expanded <tr> as well so it stays
  adjacent to its main row under any future transform.
- Watcher on tableApi.getState().expanded diffs before/after and
  re-measures only rows whose expanded flag toggled. Expand-all sentinel
  (boolean true) falls back to a full sweep.
- JSDoc notes that TanStack's ResizeObserver is attached to the main
  <tr> only — intra-expansion resizes (late-loading images, nested
  toggles) need a manual virtualizer.measure() call.
- Docs: new 'Dynamic row heights' subsection with a measureElement
  example and the auto-sum note for expanded rows.

Closes nuxt#6101.
@claylevering claylevering force-pushed the fix/table-virtualize-measure-element branch from 8d86a67 to 102ddc2 Compare April 22, 2026 16:43
@claylevering
Copy link
Copy Markdown
Contributor Author

Rebased onto the new v4 that includes #6217, reduced to a single clean commit on top (102ddc2). All conflicts resolved — the old wrapper-div / per-row translate is gone, and the measurement work now sits on the spacer-row structure as planned in the combined-branch sketch.

Summary of what's on the PR now:

  • :ref=measureRowRef + :data-index on virtualized <tr>, so TanStack Virtual observes rows and the user's measureElement option actually fires.
  • No more forced height: virtualRow.size on the main row — natural content height is measurable.
  • Default measureElement adds the immediate next-sibling <tr>'s height when data-expanded="true".
  • :style forwarded to the expanded sibling too.
  • Expand-toggle watcher scoped to just the rows whose expanded flag changed (with a full-sweep fallback for the boolean expand-all sentinel).
  • JSDoc + new "Dynamic row heights" docs subsection documenting composition, the expansion auto-sum, and the ResizeObserver-only-on-main-row limitation.

pnpm lint, vue-tsc --noEmit, and pnpm test Table.spec (74/74 including the sticky+virtualize snapshots from #6217) all clean locally.

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

Labels

v4 #4488

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Table] virtualize.measureElement is typed but not functional - DOM binding missing

3 participants