Fixed #4109 - DataTable + VirtualScroller spacer drift with rowGroupMode subheader#8579
Open
oliversluke wants to merge 1 commit into
Open
Fixed #4109 - DataTable + VirtualScroller spacer drift with rowGroupMode subheader#8579oliversluke wants to merge 1 commit into
oliversluke wants to merge 1 commit into
Conversation
… rowGroupMode subheader Add an optional getItemSize(index): number prop to VirtualScroller so it can be made variable-row-height aware. When supplied, VS builds a cumulative-size prefix-sum array and uses it for spacer total, content translation, scroll-to-index, and viewport-size calculation. When null (default), VS retains its existing uniform-height code path byte-for-byte. DataTable supplies a getItemSize that returns itemSize plus an auto- measured group-header height at cluster boundaries, and 0 for hidden rows inside collapsed clusters. The spacer-tbody subtraction sums the actually-rendered window's sizes. This corrects both drift sources: collapsed clusters no longer over-contribute to the cumulative total, and subheader rows are now accounted for in the spacer formula. Two related pre-existing bugs are addressed in the same change: - BodyRow.shouldRenderRowGroupHeader / shouldRenderRowGroupFooter use this.value[this.rowIndex - 1], which mixes a windowed value array with an absolute rowIndex in VS mode. Switched to this.index for windowed position lookup. Non-VS behavior unchanged. - TableBody's inline top on every sticky group-header causes them to stack at the same threshold when groups are dense in VS mode. Gated on isVirtualScrollerDisabled so non-VS sticky behavior is preserved. Tests: 11 new VirtualScroller specs (uniform/variable paths, cumulative correctness, binary-search boundary cases, reference-change invalidation) and 3 new DataTable specs (getItemSize boundary detection, collapsed- cluster handling). All 410 existing specs continue to pass. Signed-off-by: Oliver Sluke <22557015+oliversluke@users.noreply.github.com>
4 tasks
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
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.
Summary
Fixes #4109. Spacer math in
DataTable+VirtualScrollerdoesn't account forrowGroupMode="subheader"header rows or hidden rows in collapsed groups, producing visible row stretching, gaps between rendered rows, and full-window blanks during fast scroll.Diagnosis: #4109 (comment)
Approach
Make
VirtualScrollervariable-row-height aware via a new optionalgetItemSize(index: number): numbercallback prop. When supplied, VS builds a cumulative-size prefix-sum array and uses it for spacer total, content translation, scroll-to-index, and viewport-size calculation. Whennull(default), VS retains its existing uniform-height code path byte-for-byte — zero risk for current VS consumers (Listbox, Select, TreeSelect, AutoComplete, etc.).DataTablesupplies agetItemSizethat returnsitemSizeplus an auto-measured group-header height at cluster boundaries, and0for hidden rows in collapsed clusters. The spacer-tbody subtraction is rewritten to sum the actually-rendered window's sizes. This corrects both drift sources from #4109 with a single mechanism.Changes
VirtualScroller.vue/BaseVirtualScroller.vuegetItemSizeprop (Function, defaultnull).buildCumulativeSizes(),getItemOffset(index),getIndexAtOffset(offset)(binary search),getLastByCumulative(first, numToleratedItems).cumulativeSizesis set, falling back to the existing uniform path otherwise:setSpacerSize,setContentPosition,onScrollPositionChange.calculateCurrentIndex+calculateLast,calculateOptions.calculateLast,getRenderedRange.calculateFirstInViewport,scrollToIndex/scrollInView.DataTable.vued_rowGroupHeaderHeightmeasured viagetOuterHeight(firstHeaderRow)inmounted/updated.getItemSize(index)computed: returnsitemSize+ header contribution at group boundaries + zero-out for hidden rows in collapsed clusters.<DTVirtualScroller>via:getItemSize.calc(${spacerStyle.height} - ${getRenderedWindowSize(slotProps)}px), summing the windowedgetItemSize.BodyRow.vueshouldRenderRowGroupHeader/shouldRenderRowGroupFooterusethis.index(windowed position) instead ofthis.rowIndex(absolute, fromgetItemOptions). Previously the comparison mixed a windowedvaluearray with an absolute index in VS mode, dropping or misplacing headers depending onfirst. In non-VS modethis.index === this.rowIndexso behavior is unchanged.TableBody.vuerowGroupHeaderStylegated onisVirtualScrollerDisabled. The CSSposition: stickyrule stays; without an inlinetopit defaults totop: auto, deactivating sticky behavior. This avoids the multi-element sticky-stacking pile-up when groups are dense in VS mode (every header tries to stick at the same threshold and they overlap). Non-VS sticky group-header behavior is preserved.Test plan
getItemSizereactivity).getItemSizeboundary detection, collapsed-cluster handling).pnpm run lintclean.pnpm run format:checkclean.pnpm --filter primevue buildclean.i % 5pathological, every row its own group) and a realistic 10-groups-of-12–18-rows case. Both render uniform row heights, no gaps, no doubling, clean fast scroll, correct collapse/expand behavior.Known limitation / follow-up
Disabling sticky on row-group-headers in VS mode removes a UX cue: when scrolling deep into a long group, users can lose track of which group they're currently in. A follow-up enhancement (not in this PR) could add a single floating "currently-active group" header overlay anchored at the top of the scroll container — one sticky element instead of many, so no stacking pitfall. Mentioned here for visibility; intentionally out of scope to keep this PR focused on the spacer-math fix.