Skip to content

refactor: keep the pivot totals row out of the tanstack row model - #9920

Merged
nishantmonu51 merged 2 commits into
mainfrom
nishantmonu51/pivot-totals-row-separate
Oct 5, 2026
Merged

nishantmonu51 merged 2 commits into
mainfrom
nishantmonu51/pivot-totals-row-separate

Conversation

@nishantmonu51

Copy link
Copy Markdown
Collaborator

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.

  • The tanstack table is now built from the pivot data only. The totals row is a standalone tanstack row (createRow against the same table, id PIVOT_TOTALS_ROW_ID), so the column cell renderers work unchanged.
  • FlatTable.svelte and NestedTable.svelte receive it as totalsRow and render it through the shared pivotRow snippet, pinned as the first body row on top or in the <tfoot> at the bottom. No more row.index === 0 skipping or rowId === "0" checks.
  • Data row i is now tanstack row "i": removed the hasTotalsRow offsets from getValuesForFlatTable, getValuesForExpandedKey, getFiltersForCell, addExpandedDataToPivot and row selection. The value helpers return no values for the totals row id, which keeps the Explore data viewer working on a totals cell.
  • Dropped the sticky-row virtualizer range extractor and the spacer adjustment.
  • PivotTable.svelte still 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, nestedRowLimits and activeCell are keyed by row id but are not persisted to the URL, presets or bookmarks, so the id shift needs no migration.
  • Tests: nested row ids in pivot-click-to-filter.spec.ts shifted by one, pivot-expansion.spec.ts asserts no offset with a visible totals row. The pivot e2e specs pass unchanged since the DOM order is the same.

Checklist:

  • Covered by tests
  • Ran it and it works as intended
  • Reviewed the diff before requesting a review
  • Checked for unhandled edge cases
  • Linked the issues it closes
  • Checked if the docs need to be updated. If so, create a separate Linear DOCS issue
  • Intend to cherry-pick into the release branch
  • I'm proud of this work!

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.
…otals-row-separate

# Conflicts:
#	web-common/src/features/dashboards/pivot/PivotTable.svelte
@nishantmonu51
nishantmonu51 merged commit d05392d into main Oct 5, 2026
10 of 11 checks passed
@nishantmonu51
nishantmonu51 deleted the nishantmonu51/pivot-totals-row-separate branch October 5, 2026 13:45
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