Repository navigation
refactor: keep the pivot totals row out of the tanstack row model - #9920
Merged
Merged
Conversation
The grand-totals row was prepended to the table data as row "0", so every consumer of row ids had to subtract 1 or special-case that id. The totals row is now a standalone tanstack row (`createRow`) that `FlatTable` and `NestedTable` pin to the top of the body or render in the `<tfoot>`, and data row ids match their index in the pivot data. - Remove the `hasTotalsRow` offsets from `getValuesForFlatTable`, `getValuesForExpandedKey`, `getFiltersForCell`, `addExpandedDataToPivot` and row selection; the value helpers return no values for the totals row. - Drop the sticky-row virtualizer range extractor and the `row.index === 0` skipping in the table components. - Keep the shallow copy of the data array in the table options: expanded sub-rows are merged in place and tanstack memoizes the row model on array identity.
AdityaHegde
approved these changes
Oct 2, 2026
…otals-row-separate # Conflicts: # web-common/src/features/dashboards/pivot/PivotTable.svelte
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.
Follow-up to #9915 addressing this review comment: the grand-totals row was prepended to the table data as tanstack row
"0", so every consumer of row ids had to subtract 1 or special-case that id.createRowagainst the same table, idPIVOT_TOTALS_ROW_ID), so the column cell renderers work unchanged.FlatTable.svelteandNestedTable.sveltereceive it astotalsRowand render it through the sharedpivotRowsnippet, pinned as the first body row on top or in the<tfoot>at the bottom. No morerow.index === 0skipping orrowId === "0"checks.iis now tanstack row"i": removed thehasTotalsRowoffsets fromgetValuesForFlatTable,getValuesForExpandedKey,getFiltersForCell,addExpandedDataToPivotand row selection. The value helpers return no values for the totals row id, which keeps the Explore data viewer working on a totals cell.PivotTable.sveltestill passes tanstack a shallow copy of the data array: expanded sub-rows are merged in place and tanstack memoizes the row model on array identity (without the copy nested expansion never renders the loaded rows).expanded,nestedRowLimitsandactiveCellare keyed by row id but are not persisted to the URL, presets or bookmarks, so the id shift needs no migration.pivot-click-to-filter.spec.tsshifted by one,pivot-expansion.spec.tsasserts no offset with a visible totals row. The pivot e2e specs pass unchanged since the DOM order is the same.Checklist: