From c35d70da7f23a8558b19426968795f3f887a39e3 Mon Sep 17 00:00:00 2001 From: Kevin Van Cott Date: Sun, 2 Aug 2026 21:57:02 -0500 Subject: [PATCH] fix(table-core): correct expanded/paginated state contents and sorting toggle defaults Two independent clusters of default-behavior bugs from the beta triage. Expanded and paginated state contents: Expand-all materialized every id in `rowsById`, including leaf rows that can never expand, so a serialized `ExpandedState` was polluted with dead keys and `getExpandedDepth` was skewed by them. Materialization now writes only ids where `row.getCanExpand()` is true, chosen over `subRows.length` so `getRowCanExpand` lazy-load overrides stay expandable. `getExpandedDepth` filters the same way, and `getIsAllRowsExpanded` now only considers expandable rows so the materialized map still round-trips as "all expanded" (a map of stale ids with no expandable rows is false). Paginated `flatRows` pushed each row and then recursed into its subRows, but page rows already contain expanded descendants inline, so those rows appeared twice. The rebuild now dedupes by row id. Collapsed descendants stay included, consistent with every other row model, where `flatRows` ignores expansion state. `row.toggleExpanded(bool)` and `table.toggleAllRowsExpanded(bool)` fired `onExpandedChange` even when the requested state already matched, so controlled consumers got spurious callbacks. Both now early-return. Neither toggle consulted `row.getCanExpand()`, unlike their row-selection equivalents, so a non-expandable row could be written into expanded state imperatively. The expand direction is now guarded; collapsing is always allowed so stale expanded ids can still be cleaned up. Sorting toggle defaults: `column.toggleSorting()` called `column.getNextSortingOrder(column)` with no `multi` argument, so `enableMultiRemove` was dead on every path. The argument is now forwarded as `multi && column.getCanMultiSort()`, matching the multi-mode condition the updater itself uses. The public `getNextSortingOrder` type now accepts the `multi` argument it always supported at runtime. `column.getAutoSortDir()` sampled only `flatRows[0]`, so a leading null or a manual-sorting data swap flipped the inferred first direction mid-cycle and silently dropped a state from the toggle cycle. It now samples the first 10 rows for a non-nullish value, matching `getAutoSortFn`. Docs: `sortUndefined: false` was documented as "considered tied" in all framework sorting guides; undefined values are actually passed straight to the sorting function with no special handling. Corrected, and the missing `'first'`/`'last'` bullets were added to the generated reference. Also fixes three crashes in the MRT examples surfaced while testing the sorting changes. The MUI and Mantine sort labels read `sorting` off `state`, which does not carry it, and crashed on `sorting.length`; they now read it through `table.Subscribe` on `table.atoms.sorting`, which also makes the badge reactive. The row-action edit handler passed a shallow row copy to `setEditingRow`, breaking the row identity the edit modal needs. `mrtRefsFeature` seeded the keyed `editInputRefs`/`filterInputRefs` bags with `null` instead of `{}`, crashing on first keyed write. New e2e coverage pins all three plus the sort-direction indicator. Closes #6115 Closes #5833 Closes #6136 Closes #4939 Closes #4946 Closes #5147 Closes #5832 Co-Authored-By: Claude Opus 5 (1M context) --- beta-window-triage.md | 14 +- docs/framework/alpine/guide/sorting.md | 2 +- docs/framework/angular/guide/sorting.md | 2 +- docs/framework/ember/guide/sorting.md | 2 +- docs/framework/lit/guide/sorting.md | 2 +- docs/framework/octane/guide/sorting.md | 2 +- docs/framework/preact/guide/sorting.md | 2 +- docs/framework/react/guide/sorting.md | 2 +- docs/framework/solid/guide/sorting.md | 2 +- docs/framework/svelte/guide/sorting.md | 2 +- docs/framework/vue/guide/sorting.md | 2 +- .../index/interfaces/ColumnDef_RowSorting.md | 6 +- .../index/interfaces/Column_RowSorting.md | 14 +- .../functions/column_getAutoSortDir.md | 9 +- .../functions/column_toggleSorting.md | 4 +- .../buttons/MRT_ToggleRowActionMenuButton.tsx | 2 +- .../head/MRT_TableHeadCellSortLabel.tsx | 117 ++++++++------ .../tests/e2e/smoke.spec.ts | 50 +++++- .../react/material-react-table/src/main.tsx | 2 +- .../buttons/MRT_ToggleRowActionMenuButton.tsx | 2 +- .../head/MRT_TableHeadCellSortLabel.tsx | 143 +++++++++-------- .../features/mrtRefsFeature.ts | 8 +- .../tests/e2e/smoke.spec.ts | 71 ++++++++- .../flex-render/flex-render-table.test.ts | 27 ++-- .../tests/integration/flex-render.test.gts | 4 + .../rowExpandingFeature.utils.ts | 68 ++++++-- .../row-pagination/createPaginatedRowModel.ts | 6 + .../row-sorting/rowSortingFeature.types.ts | 12 +- .../row-sorting/rowSortingFeature.utils.ts | 30 ++-- .../createPaginatedRowModel.test.ts | 29 ++++ .../rowExpandingFeature.utils.test.ts | 149 ++++++++++++++++-- .../rowSortingFeature.utils.test.ts | 77 +++++++++ 32 files changed, 665 insertions(+), 199 deletions(-) diff --git a/beta-window-triage.md b/beta-window-triage.md index 568df36a0d..861ccab112 100644 --- a/beta-window-triage.md +++ b/beta-window-triage.md @@ -36,12 +36,14 @@ Implemented on beta per maintainer decisions (opt-in flag, prune-only, select-al 1. **[#5968](https://github.com/TanStack/table/issues/5968) (high) — land FIRST.** `memo` (`utils.ts:176-201`) has no first-run suppression, so the first `getRowModel()` schedules `table_autoResetExpanded`/`table_autoResetPageIndex` on mount, wiping controlled `expanded` and initial `pageIndex`. Also `table_autoResetPageIndex` calls `table_resetPageIndex(table, true)` (`rowPaginationFeature.utils.ts:49`), hard-coding 0 and ignoring `initialState`. Fix: restore first-run guard + drop the `true`. 2. **[#5801](https://github.com/TanStack/table/issues/5801) (med-high) — land SECOND, via PR [#6443](https://github.com/TanStack/table/pull/6443).** `table_autoResetExpanded` is only wired from the grouped row model; wire into `createCoreRowModel`. Landing this before the [#5968](https://github.com/TanStack/table/issues/5968) guard would extend the mount-wipe bug to all non-grouped tables. -### Cluster 3: Expanded/pagination state contents +### Cluster 3: Expanded/pagination state contents — FIX IMPLEMENTED 2026-08-02 (pending PR) -- **[#6115](https://github.com/TanStack/table/issues/6115) (med)** — expand-all materializes every id from `rowsById` including non-expandable rows (`rowExpandingFeature.utils.ts:263-266`), polluting serialized `ExpandedState` and skewing `getExpandedDepth`. Port [#6116](https://github.com/TanStack/table/pull/6116) approach; decide `row_getCanExpand` vs `subRows.length` semantics. -- **[#5833](https://github.com/TanStack/table/issues/5833) (med)** — paginated `flatRows` duplicates expanded sub-rows (`createPaginatedRowModel.ts:71-80` pushes then recurses into already-flattened rows). Changes `flatRows` contents; needs small semantic decision on collapsed descendants. -- **[#6136](https://github.com/TanStack/table/issues/6136) (low-med)** — `row.toggleExpanded(bool)` fires `onExpandedChange` on no-ops (`rowExpandingFeature.utils.ts:258`); controlled consumers get spurious callbacks. Port [#6184](https://github.com/TanStack/table/pull/6184) approach. -- **[#4939](https://github.com/TanStack/table/issues/4939) (med)** — imperative `row_toggleExpanded`/`toggleAllRowsExpanded` never consult `row_getCanExpand`, unlike the selection twin. Guard turns currently-succeeding calls into no-ops. +All four implemented on beta in one coordinated change (`rowExpandingFeature.utils.ts` + `createPaginatedRowModel.ts`), with new unit/implementation tests, all-frameworks expanding-guide note, and changeset `expanding-pagination-state-contents`: + +- **[#6115](https://github.com/TanStack/table/issues/6115)** — expand-all materialization (`row_toggleExpanded` old===true branch) now only writes ids where `row_getCanExpand` is true (chosen over `subRows.length` so `getRowCanExpand` lazy-load overrides stay expandable). `table_getExpandedDepth` filters the same way for expanded-all. Companion: `table_getIsAllRowsExpanded` now only considers expandable rows so the materialized map still round-trips as "all expanded" (stale-ids-with-no-expandable-rows returns false). +- **[#5833](https://github.com/TanStack/table/issues/5833)** — paginated `flatRows` rebuild dedupes via seen-id set. Semantic decision: collapsed descendants of page rows stay included (consistent with every other row model where `flatRows` ignores expansion state); only the duplication is removed. +- **[#6136](https://github.com/TanStack/table/issues/6136)** — `row_toggleExpanded` early-returns (no `onExpandedChange`) when the target state matches current atom state; `table_toggleAllRowsExpanded` gets symmetric no-op guards (already `true`, or collapse with nothing expanded). +- **[#4939](https://github.com/TanStack/table/issues/4939)** — expand direction of `row_toggleExpanded` now guarded on `row_getCanExpand`; `table_toggleAllRowsExpanded` expand branch guarded on `table_getCanSomeRowsExpand`. Collapse always allowed (stale-id cleanup). Angular/ember flex-render tests that drove expansion on flat rows updated with `getRowCanExpand: () => true`. ### Cluster 4: Sorting defaults @@ -110,6 +112,6 @@ Implemented on beta per maintainer decisions (opt-in flag, prune-only, select-al 4. **[#6313](https://github.com/TanStack/table/pull/6313)** rebase + merge ([#6007](https://github.com/TanStack/table/issues/6007)); **[#6361](https://github.com/TanStack/table/pull/6361)** fix CI + merge ([#5987](https://github.com/TanStack/table/issues/5987)); **[#6443](https://github.com/TanStack/table/pull/6443)** rebase + merge ([#5801](https://github.com/TanStack/table/issues/5801)). 5. `_valuesCache`/`defaultColumn` invalidation pair ([#5363](https://github.com/TanStack/table/issues/5363)/[#4485](https://github.com/TanStack/table/issues/4485) + [#5275](https://github.com/TanStack/table/issues/5275)). 6. Sorting defaults ([#4946](https://github.com/TanStack/table/issues/4946) one-liner; [#5147](https://github.com/TanStack/table/issues/5147)/[#5832](https://github.com/TanStack/table/issues/5832) auto-dir sampling). -7. Remaining semantics decisions: [#5909](https://github.com/TanStack/table/issues/5909) (undefined = uncontrolled), [#5778](https://github.com/TanStack/table/issues/5778), [#6115](https://github.com/TanStack/table/issues/6115), [#5833](https://github.com/TanStack/table/issues/5833), [#6101](https://github.com/TanStack/table/issues/6101), [#6081](https://github.com/TanStack/table/issues/6081), [#4939](https://github.com/TanStack/table/issues/4939), [#6136](https://github.com/TanStack/table/issues/6136). +7. Remaining semantics decisions: [#5909](https://github.com/TanStack/table/issues/5909) (undefined = uncontrolled), [#5778](https://github.com/TanStack/table/issues/5778), [#6101](https://github.com/TanStack/table/issues/6101), [#6081](https://github.com/TanStack/table/issues/6081). (Cluster 3 — [#6115](https://github.com/TanStack/table/issues/6115), [#5833](https://github.com/TanStack/table/issues/5833), [#4939](https://github.com/TanStack/table/issues/4939), [#6136](https://github.com/TanStack/table/issues/6136) — implemented 2026-08-02, pending PR.) 8. Type changes batch: [#5908](https://github.com/TanStack/table/issues/5908), [#5971](https://github.com/TanStack/table/issues/5971), [#6302](https://github.com/TanStack/table/issues/6302) (TS-perf gate). 9. [#6078](https://github.com/TanStack/table/issues/6078) via corrected [#6445](https://github.com/TanStack/table/pull/6445) (urgent but non-breaking — do not let it slip just because it is "anytime"). diff --git a/docs/framework/alpine/guide/sorting.md b/docs/framework/alpine/guide/sorting.md index 48f9e5bd04..ce8645c768 100644 --- a/docs/framework/alpine/guide/sorting.md +++ b/docs/framework/alpine/guide/sorting.md @@ -442,7 +442,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/angular/guide/sorting.md b/docs/framework/angular/guide/sorting.md index 0e2f43da78..bc1263056c 100644 --- a/docs/framework/angular/guide/sorting.md +++ b/docs/framework/angular/guide/sorting.md @@ -434,7 +434,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/ember/guide/sorting.md b/docs/framework/ember/guide/sorting.md index 29c338333c..7004db2fd5 100644 --- a/docs/framework/ember/guide/sorting.md +++ b/docs/framework/ember/guide/sorting.md @@ -426,7 +426,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/lit/guide/sorting.md b/docs/framework/lit/guide/sorting.md index 5d1723fda7..9a8dc9201c 100644 --- a/docs/framework/lit/guide/sorting.md +++ b/docs/framework/lit/guide/sorting.md @@ -451,7 +451,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/octane/guide/sorting.md b/docs/framework/octane/guide/sorting.md index 526cd35626..aa82b35804 100644 --- a/docs/framework/octane/guide/sorting.md +++ b/docs/framework/octane/guide/sorting.md @@ -427,7 +427,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/preact/guide/sorting.md b/docs/framework/preact/guide/sorting.md index 781e098aa0..0091c8ef78 100644 --- a/docs/framework/preact/guide/sorting.md +++ b/docs/framework/preact/guide/sorting.md @@ -427,7 +427,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/react/guide/sorting.md b/docs/framework/react/guide/sorting.md index afabc7656a..558b908120 100644 --- a/docs/framework/react/guide/sorting.md +++ b/docs/framework/react/guide/sorting.md @@ -427,7 +427,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/solid/guide/sorting.md b/docs/framework/solid/guide/sorting.md index c27d60e2bb..09e2cdc3dd 100644 --- a/docs/framework/solid/guide/sorting.md +++ b/docs/framework/solid/guide/sorting.md @@ -426,7 +426,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/svelte/guide/sorting.md b/docs/framework/svelte/guide/sorting.md index f3f05db56b..3799e6efcd 100644 --- a/docs/framework/svelte/guide/sorting.md +++ b/docs/framework/svelte/guide/sorting.md @@ -440,7 +440,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/framework/vue/guide/sorting.md b/docs/framework/vue/guide/sorting.md index fdbcc4b6cf..ea4ded5678 100644 --- a/docs/framework/vue/guide/sorting.md +++ b/docs/framework/vue/guide/sorting.md @@ -429,7 +429,7 @@ If not specified, the default value for `sortUndefined` is `1`, and undefined va - `'first'` - Undefined values will be pushed to the beginning of the list - `'last'` - Undefined values will be pushed to the end of the list -- `false` - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) +- `false` - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) diff --git a/docs/reference/index/interfaces/ColumnDef_RowSorting.md b/docs/reference/index/interfaces/ColumnDef_RowSorting.md index 3b1ad97805..9e5328c34d 100644 --- a/docs/reference/index/interfaces/ColumnDef_RowSorting.md +++ b/docs/reference/index/interfaces/ColumnDef_RowSorting.md @@ -91,8 +91,12 @@ Defined in: [features/row-sorting/rowSortingFeature.types.ts:153](https://github The priority of undefined values when sorting this column. - `false` - - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) + - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them - `-1` - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) - `1` - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) +- `'first'` + - Undefined values will be pushed to the beginning of the list regardless of sort direction +- `'last'` + - Undefined values will be pushed to the end of the list regardless of sort direction diff --git a/docs/reference/index/interfaces/Column_RowSorting.md b/docs/reference/index/interfaces/Column_RowSorting.md index ba10e53780..1313910604 100644 --- a/docs/reference/index/interfaces/Column_RowSorting.md +++ b/docs/reference/index/interfaces/Column_RowSorting.md @@ -134,12 +134,20 @@ Reads this column's current sort direction, or `false` when unsorted. ### getNextSortingOrder() ```ts -getNextSortingOrder: () => false | SortDirection; +getNextSortingOrder: (multi?) => false | SortDirection; ``` -Defined in: [features/row-sorting/rowSortingFeature.types.ts:191](https://github.com/TanStack/table/blob/main/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts#L191) +Defined in: [features/row-sorting/rowSortingFeature.types.ts:193](https://github.com/TanStack/table/blob/main/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts#L193) -Returns the next sorting order. +Returns the next sorting order. Pass `multi` to resolve the order for a +multi-sort toggle, where `enableMultiRemove` governs whether the cycle can +remove the sort. + +#### Parameters + +##### multi? + +`boolean` #### Returns diff --git a/docs/reference/static-functions/functions/column_getAutoSortDir.md b/docs/reference/static-functions/functions/column_getAutoSortDir.md index d98a9f2d85..5e43c0c910 100644 --- a/docs/reference/static-functions/functions/column_getAutoSortDir.md +++ b/docs/reference/static-functions/functions/column_getAutoSortDir.md @@ -11,10 +11,13 @@ function column_getAutoSortDir(column): "asc" | "desc" Defined in: [features/row-sorting/rowSortingFeature.utils.ts:154](https://github.com/TanStack/table/blob/main/packages/table-core/src/features/row-sorting/rowSortingFeature.utils.ts#L154) -Chooses the default first sort direction from the first filtered row value. +Chooses the default first sort direction from sampled filtered row values. -String columns start ascending so alphabetical order is natural; other value -types start descending. +The first non-nullish value among the sampled rows decides: string columns +start ascending so alphabetical order is natural; other value types (or +columns with no non-nullish sample) start descending. Sampling past leading +nullish values keeps the toggle cycle stable when sorting or a data swap +moves an empty value into the first row. ## Type Parameters diff --git a/docs/reference/static-functions/functions/column_toggleSorting.md b/docs/reference/static-functions/functions/column_toggleSorting.md index e7a277dae1..d03fde271d 100644 --- a/docs/reference/static-functions/functions/column_toggleSorting.md +++ b/docs/reference/static-functions/functions/column_toggleSorting.md @@ -17,8 +17,8 @@ Defined in: [features/row-sorting/rowSortingFeature.utils.ts:221](https://github Applies the next sorting state for this column. The toggle can add, replace, flip, or remove this column's sort entry. Multi -sorting respects `enableMultiSort`, `maxMultiSortColCount`, and the `multi` -argument. +sorting respects `enableMultiSort`, `enableMultiRemove`, +`maxMultiSortColCount`, and the `multi` argument. ## Type Parameters diff --git a/examples/react/mantine-react-table/src/mantine-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx b/examples/react/mantine-react-table/src/mantine-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx index 403c50aaac..3003664d35 100644 --- a/examples/react/mantine-react-table/src/mantine-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx +++ b/examples/react/mantine-react-table/src/mantine-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx @@ -44,7 +44,7 @@ export const MRT_ToggleRowActionMenuButton = ({ const handleStartEditMode = (event: MouseEvent) => { event.stopPropagation() - setEditingRow({ ...row }) + setEditingRow(row) } const showEditActionButtons = diff --git a/examples/react/mantine-react-table/src/mantine-react-table/components/head/MRT_TableHeadCellSortLabel.tsx b/examples/react/mantine-react-table/src/mantine-react-table/components/head/MRT_TableHeadCellSortLabel.tsx index b1fd889008..576792a526 100644 --- a/examples/react/mantine-react-table/src/mantine-react-table/components/head/MRT_TableHeadCellSortLabel.tsx +++ b/examples/react/mantine-react-table/src/mantine-react-table/components/head/MRT_TableHeadCellSortLabel.tsx @@ -1,6 +1,7 @@ import clsx from 'clsx' import { ActionIcon, Indicator, Tooltip } from '@mantine/core' +import { Subscribe } from '@tanstack/react-table' import { dataVariable } from '../../utils/style.utils' import classes from './MRT_TableHeadCellSortLabel.module.css' import type { ActionIconProps } from '@mantine/core' @@ -18,7 +19,6 @@ export const MRT_TableHeadCellSortLabel = ({ ...rest }: Props) => { const { - state, options: { icons: { IconArrowsSort, IconSortAscending, IconSortDescending }, localization, @@ -26,58 +26,73 @@ export const MRT_TableHeadCellSortLabel = ({ } = table const column = header.column const { columnDef } = column - const { sorting } = state - const sorted = column.getIsSorted() - const sortIndex = column.getSortIndex() - const sortTooltip = sorted - ? sorted === 'desc' - ? localization.sortedByColumnDesc.replace('{column}', columnDef.header) - : localization.sortedByColumnAsc.replace('{column}', columnDef.header) - : column.getNextSortingOrder() === 'desc' - ? localization.sortByColumnDesc.replace('{column}', columnDef.header) - : localization.sortByColumnAsc.replace('{column}', columnDef.header) + return ( + + {(sorting) => { + const sorted = column.getIsSorted() + const sortIndex = column.getSortIndex() - const SortActionButton = ( - - {sorted === 'desc' ? ( - - ) : sorted === 'asc' ? ( - - ) : ( - - )} - - ) + const sortTooltip = sorted + ? sorted === 'desc' + ? localization.sortedByColumnDesc.replace( + '{column}', + columnDef.header, + ) + : localization.sortedByColumnAsc.replace( + '{column}', + columnDef.header, + ) + : column.getNextSortingOrder() === 'desc' + ? localization.sortByColumnDesc.replace( + '{column}', + columnDef.header, + ) + : localization.sortByColumnAsc.replace('{column}', columnDef.header) - return ( - - {sorting.length < 2 || sortIndex === -1 ? ( - SortActionButton - ) : ( - - {SortActionButton} - - )} - + const SortActionButton = ( + + {sorted === 'desc' ? ( + + ) : sorted === 'asc' ? ( + + ) : ( + + )} + + ) + + return ( + + {sorting.length < 2 || sortIndex === -1 ? ( + SortActionButton + ) : ( + + {SortActionButton} + + )} + + ) + }} + ) } diff --git a/examples/react/mantine-react-table/tests/e2e/smoke.spec.ts b/examples/react/mantine-react-table/tests/e2e/smoke.spec.ts index 807e215de6..e8d254be09 100644 --- a/examples/react/mantine-react-table/tests/e2e/smoke.spec.ts +++ b/examples/react/mantine-react-table/tests/e2e/smoke.spec.ts @@ -1,7 +1,7 @@ -import { expect, test } from '@playwright/test' -import type { Page } from '@playwright/test' import path from 'node:path' +import { expect, test } from '@playwright/test' import { startExampleServer } from '../../../../../tests/e2e/helpers/startExampleServer' +import type { Page } from '@playwright/test' const exampleDir = path.resolve() @@ -97,6 +97,52 @@ test('paginates without resetting to the first page', async ({ page }) => { } }) +test('updates the sort direction indicator', async ({ page }) => { + const { errors, server } = await openExample(page) + + try { + const firstNameHeader = page + .locator('thead th') + .filter({ hasText: 'First Name' }) + const sortTarget = firstNameHeader.locator('.mrt-table-head-cell-labels') + const sortLabel = firstNameHeader.locator('.mrt-table-head-sort-button') + + await sortTarget.click() + await expect(sortLabel).toHaveAttribute('data-sorted', 'asc') + await expect(sortLabel).toHaveAttribute( + 'aria-label', + 'Sorted by First Name ascending', + ) + + await sortTarget.click() + await expect(sortLabel).toHaveAttribute('data-sorted', 'desc') + await expect(sortLabel).toHaveAttribute( + 'aria-label', + 'Sorted by First Name descending', + ) + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) + +test('opens the edit row modal without crashing', async ({ page }) => { + const { errors, server } = await openExample(page) + + try { + await page.getByRole('button', { name: 'Row Actions' }).first().click() + await page.getByRole('menuitem', { name: 'Edit', exact: true }).click() + + await expect(page.getByRole('dialog')).toBeVisible() + await expect( + page.getByRole('textbox', { name: 'First Name' }), + ).toBeVisible() + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) + test('groups and ungroups a column without crashing', async ({ page }) => { const { errors, server } = await openExample(page) diff --git a/examples/react/material-react-table/src/main.tsx b/examples/react/material-react-table/src/main.tsx index 869f82f228..9202c7ce59 100644 --- a/examples/react/material-react-table/src/main.tsx +++ b/examples/react/material-react-table/src/main.tsx @@ -144,7 +144,7 @@ function App() { View Profile , diff --git a/examples/react/material-react-table/src/material-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx b/examples/react/material-react-table/src/material-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx index f0440f8154..6c1cdcfc7b 100644 --- a/examples/react/material-react-table/src/material-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx +++ b/examples/react/material-react-table/src/material-react-table/components/buttons/MRT_ToggleRowActionMenuButton.tsx @@ -69,7 +69,7 @@ export const MRT_ToggleRowActionMenuButton = ({ const handleStartEditMode = (event: MouseEvent) => { event.stopPropagation() - setEditingRow({ ...row }) + setEditingRow(row) setAnchorEl(null) } diff --git a/examples/react/material-react-table/src/material-react-table/components/head/MRT_TableHeadCellSortLabel.tsx b/examples/react/material-react-table/src/material-react-table/components/head/MRT_TableHeadCellSortLabel.tsx index 6db54a5b52..307724c100 100644 --- a/examples/react/material-react-table/src/material-react-table/components/head/MRT_TableHeadCellSortLabel.tsx +++ b/examples/react/material-react-table/src/material-react-table/components/head/MRT_TableHeadCellSortLabel.tsx @@ -26,72 +26,87 @@ export const MRT_TableHeadCellSortLabel = ({ } = table const { column } = header const { columnDef } = column - const { isLoading, showSkeletons, sorting } = state + const { isLoading, showSkeletons } = state - const isSorted = !!column.getIsSorted() - - const sortTooltip = - isLoading || showSkeletons - ? '' - : column.getIsSorted() - ? column.getIsSorted() === 'desc' - ? localization.sortedByColumnDesc.replace( - '{column}', - columnDef.header, - ) - : localization.sortedByColumnAsc.replace('{column}', columnDef.header) - : column.getNextSortingOrder() === 'desc' - ? localization.sortByColumnDesc.replace('{column}', columnDef.header) - : localization.sortByColumnAsc.replace('{column}', columnDef.header) + return ( + + {(sorting) => { + const isSorted = !!column.getIsSorted() + const sortTooltip = + isLoading || showSkeletons + ? '' + : column.getIsSorted() + ? column.getIsSorted() === 'desc' + ? localization.sortedByColumnDesc.replace( + '{column}', + columnDef.header, + ) + : localization.sortedByColumnAsc.replace( + '{column}', + columnDef.header, + ) + : column.getNextSortingOrder() === 'desc' + ? localization.sortByColumnDesc.replace( + '{column}', + columnDef.header, + ) + : localization.sortByColumnAsc.replace( + '{column}', + columnDef.header, + ) - const direction = isSorted - ? (column.getIsSorted() as 'asc' | 'desc') - : undefined + const direction = isSorted + ? (column.getIsSorted() as 'asc' | 'desc') + : undefined - return ( - - 1 ? column.getSortIndex() + 1 : 0} - overlap="circular" - > - ( - - ) - : ArrowDownwardIcon - } - active - aria-label={sortTooltip} - direction={direction} - onClick={(e) => { - e.stopPropagation() - header.column.getToggleSortingHandler()?.(e) - }} - {...rest} - sx={(theme) => ({ - '.MuiTableSortLabel-icon': { - color: `${ - theme.palette.mode === 'dark' - ? theme.palette.text.primary - : theme.palette.text.secondary - } !important`, - }, - flex: '0 0', - opacity: isSorted ? 1 : 0.3, - transition: 'all 150ms ease-in-out', - width: '3ch', - ...(parseFromValuesOrFunc(rest?.sx, theme) as any), - })} - /> - - + return ( + + 1 ? column.getSortIndex() + 1 : 0} + overlap="circular" + > + ( + + ) + : ArrowDownwardIcon + } + active + aria-label={sortTooltip} + direction={direction} + onClick={(e) => { + e.stopPropagation() + header.column.getToggleSortingHandler()?.(e) + }} + {...rest} + sx={(theme) => ({ + '.MuiTableSortLabel-icon': { + color: `${ + theme.palette.mode === 'dark' + ? theme.palette.text.primary + : theme.palette.text.secondary + } !important`, + }, + flex: '0 0', + opacity: isSorted ? 1 : 0.3, + transition: 'all 150ms ease-in-out', + width: '3ch', + ...(parseFromValuesOrFunc(rest?.sx, theme) as any), + })} + /> + + + ) + }} + ) } diff --git a/examples/react/material-react-table/src/material-react-table/features/mrtRefsFeature.ts b/examples/react/material-react-table/src/material-react-table/features/mrtRefsFeature.ts index 86d5e52f0d..0a3d968646 100644 --- a/examples/react/material-react-table/src/material-react-table/features/mrtRefsFeature.ts +++ b/examples/react/material-react-table/src/material-react-table/features/mrtRefsFeature.ts @@ -38,8 +38,8 @@ declare module '@tanstack/react-table' { /** * The v9 table instance is constructed once (via `useState(() => - * constructTable(...))`), so a bag of plain mutable `{ current: null }` refs - * assigned here persists for the table's lifetime — no `useRef` needed. This + * constructTable(...))`), so the plain mutable ref objects assigned here + * persist for the table's lifetime — no `useRef` needed. This * replaces the twelve `useRef`s MRT threaded through `useMRT_TableInstance`. */ export const mrtRefsFeature: TableFeature = { @@ -47,8 +47,8 @@ export const mrtRefsFeature: TableFeature = { ;(table as unknown as MRT_Table_Refs).refs = { actionCellRef: { current: null }, bottomToolbarRef: { current: null }, - editInputRefs: { current: null }, - filterInputRefs: { current: null }, + editInputRefs: { current: {} }, + filterInputRefs: { current: {} }, lastSelectedRowId: { current: null }, searchInputRef: { current: null }, tableContainerRef: { current: null }, diff --git a/examples/react/material-react-table/tests/e2e/smoke.spec.ts b/examples/react/material-react-table/tests/e2e/smoke.spec.ts index 3fad41b789..1b51e2eff0 100644 --- a/examples/react/material-react-table/tests/e2e/smoke.spec.ts +++ b/examples/react/material-react-table/tests/e2e/smoke.spec.ts @@ -1,7 +1,7 @@ -import { expect, test } from '@playwright/test' -import type { Page } from '@playwright/test' import path from 'node:path' +import { expect, test } from '@playwright/test' import { startExampleServer } from '../../../../../tests/e2e/helpers/startExampleServer' +import type { Page } from '@playwright/test' const exampleDir = path.resolve() @@ -97,6 +97,73 @@ test('paginates without resetting to the first page', async ({ page }) => { } }) +test('updates the sort direction indicator', async ({ page }) => { + const { errors, server } = await openExample(page) + + try { + const firstNameHeader = page + .locator('thead th') + .filter({ hasText: 'First Name' }) + const sortTarget = firstNameHeader.locator( + '.Mui-TableHeadCell-Content-Labels', + ) + const sortLabel = firstNameHeader.locator('.MuiTableSortLabel-root') + + await sortTarget.click() + await expect(firstNameHeader).toHaveAttribute('aria-sort', 'ascending') + await expect(firstNameHeader).toHaveAttribute('data-sort', 'asc') + await expect(sortLabel).toHaveClass(/MuiTableSortLabel-directionAsc/) + await expect(sortLabel).toHaveAttribute( + 'aria-label', + 'Sorted by First Name ascending', + ) + + await sortTarget.click() + await expect(firstNameHeader).toHaveAttribute('aria-sort', 'descending') + await expect(firstNameHeader).toHaveAttribute('data-sort', 'desc') + await expect(sortLabel).toHaveClass(/MuiTableSortLabel-directionDesc/) + await expect(sortLabel).toHaveAttribute( + 'aria-label', + 'Sorted by First Name descending', + ) + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) + +test('shows column filters without crashing', async ({ page }) => { + const { errors, server } = await openExample(page) + + try { + await page.getByRole('button', { name: 'Show/Hide filters' }).click() + + await expect( + page.getByRole('textbox', { name: 'Filter by First Name' }), + ).toBeVisible() + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) + +test('opens the edit row modal without crashing', async ({ page }) => { + const { errors, server } = await openExample(page) + + try { + await page.getByRole('button', { name: 'Row Actions' }).first().click() + await page.getByRole('menuitem', { name: 'Edit', exact: true }).click() + + await expect(page.getByRole('dialog')).toBeVisible() + await expect( + page.getByRole('textbox', { name: 'First Name' }), + ).toBeVisible() + expect(errors).toEqual([]) + } finally { + await server.close() + } +}) + test('groups and ungroups a column without crashing', async ({ page }) => { const { errors, server } = await openExample(page) diff --git a/packages/angular-table/tests/flex-render/flex-render-table.test.ts b/packages/angular-table/tests/flex-render/flex-render-table.test.ts index 5239dce24e..e4862bda9e 100644 --- a/packages/angular-table/tests/flex-render/flex-render-table.test.ts +++ b/packages/angular-table/tests/flex-render/flex-render-table.test.ts @@ -193,16 +193,21 @@ describe('FlexRenderDirective', () => { }) class EmptyCell {} - const { fixture } = createTestTable(defaultData, [ - { - id: 'cell', - header: 'Header', - cell: (context) => { - contextCaptor(context) - return flexRenderComponent(EmptyCell) + const { fixture } = createTestTable( + defaultData, + [ + { + id: 'cell', + header: 'Header', + cell: (context) => { + contextCaptor(context) + return flexRenderComponent(EmptyCell) + }, }, - }, - ]) + ], + // The flat test rows have no subRows, so opt them into expandability + () => ({ getRowCanExpand: () => true }), + ) const latestCall = () => contextCaptor.mock.lastCall![0] as CellContext< @@ -303,6 +308,8 @@ describe('FlexRenderDirective', () => { coreRowModel: createCoreRowModel(), }, state: { expanded: this.expandState() }, + // The flat test rows have no subRows, so opt them into expandability + getRowCanExpand: () => true, onExpandedChange: (updaterOrValue) => { typeof updaterOrValue === 'function' ? this.expandState.update(updaterOrValue) @@ -404,6 +411,8 @@ describe('FlexRenderDirective', () => { coreRowModel: createCoreRowModel(), }, state: { expanded: this.expandState() }, + // The flat test rows have no subRows, so opt them into expandability + getRowCanExpand: () => true, onExpandedChange: (updaterOrValue) => { typeof updaterOrValue === 'function' ? this.expandState.update(updaterOrValue) diff --git a/packages/ember-table/tests/integration/flex-render.test.gts b/packages/ember-table/tests/integration/flex-render.test.gts index d8f220947c..545175578a 100644 --- a/packages/ember-table/tests/integration/flex-render.test.gts +++ b/packages/ember-table/tests/integration/flex-render.test.gts @@ -464,6 +464,8 @@ module('Integration | FlexRender', function (hooks) { table = useTable(() => ({ data: this.data, features: stockFeatures, + // The flat test rows have no subRows, so opt them into expandability + getRowCanExpand: () => true, columns: [ { id: 'expand', @@ -525,6 +527,8 @@ module('Integration | FlexRender', function (hooks) { table = useTable(() => ({ data: this.data, features: stockFeatures, + // The flat test rows have no subRows, so opt them into expandability + getRowCanExpand: () => true, columns: [ { id: 'expand', diff --git a/packages/table-core/src/features/row-expanding/rowExpandingFeature.utils.ts b/packages/table-core/src/features/row-expanding/rowExpandingFeature.utils.ts index ac8f826195..832d98138f 100644 --- a/packages/table-core/src/features/row-expanding/rowExpandingFeature.utils.ts +++ b/packages/table-core/src/features/row-expanding/rowExpandingFeature.utils.ts @@ -77,6 +77,9 @@ export function table_setExpanded< * an empty map. Omitting the value toggles based on whether all rows are * currently expanded. * + * The call is a no-op (no `onExpandedChange`) when no row can expand or when + * the requested state matches the current state exactly. + * * @example * ```ts * table_toggleAllRowsExpanded(table) @@ -86,9 +89,14 @@ export function table_toggleAllRowsExpanded< TFeatures extends TableFeatures, TData extends RowData, >(table: Table_Internal, expanded?: boolean) { + const currentExpanded = table.atoms.expanded?.get() ?? {} + if (expanded ?? !table_getIsAllRowsExpanded(table)) { + if (currentExpanded === true) return + if (!table_getCanSomeRowsExpand(table)) return table_setExpanded(table, true) } else { + if (currentExpanded !== true && !Object.keys(currentExpanded).length) return table_setExpanded(table, makeObjectMap()) } } @@ -179,10 +187,12 @@ export function table_getIsSomeRowsExpanded< } /** - * Checks whether every row in the current row model is expanded. + * Checks whether every expandable row in the current row model is expanded. * * The special expanded-all value `true` returns true immediately. Empty - * expanded state returns false. + * expanded state returns false. Rows that cannot expand are ignored, so a + * materialized expanded-all map (which only contains expandable row ids) + * still counts as all rows expanded. * * @example * ```ts @@ -204,8 +214,16 @@ export function table_getIsAllRowsExpanded< return false } - // If any row is not expanded, return false - if (table.getRowModel().flatRows.some((row) => !row_getIsExpanded(row))) { + const expandableRows = table + .getRowModel() + .flatRows.filter((row) => row_getCanExpand(row)) + + if (!expandableRows.length) { + return false + } + + // If any expandable row is not expanded, return false + if (expandableRows.some((row) => !row_getIsExpanded(row))) { return false } @@ -216,8 +234,8 @@ export function table_getIsAllRowsExpanded< /** * Computes the deepest expanded row id depth. * - * Row ids are split on `.`; expanded-all state scans the current row model, - * while explicit expanded state scans its expanded id keys. + * Row ids are split on `.`; expanded-all state scans the current row model's + * expandable rows, while explicit expanded state scans its expanded id keys. * * @example * ```ts @@ -230,10 +248,14 @@ export function table_getExpandedDepth< >(table: Table_Internal) { let maxDepth = 0 + const expanded = table.atoms.expanded?.get() + const rowIds = - table.atoms.expanded?.get() === true - ? Object.keys(table.getRowModel().rowsById) - : Object.keys(table.atoms.expanded?.get() ?? {}) + expanded === true + ? Object.values(table.getRowModel().rowsById) + .filter((row) => row_getCanExpand(row)) + .map((row) => row.id) + : Object.keys(expanded ?? {}) rowIds.forEach((id) => { const splitId = id.split('.') @@ -247,8 +269,12 @@ export function table_getExpandedDepth< * Expands or collapses this row. * * Omitting `expanded` toggles the row. If the current state is expanded-all, - * the function first materializes that state into a row-id map before applying - * the row-specific change. + * the function first materializes that state into a row-id map (containing + * only expandable row ids) before applying the row-specific change. + * + * The call is a no-op (no `onExpandedChange`) when the requested state matches + * the current state, or when expanding a row that cannot expand. Collapsing is + * always allowed so stale expanded ids can be cleaned up. * * @example * ```ts @@ -259,27 +285,35 @@ export function row_toggleExpanded< TFeatures extends TableFeatures, TData extends RowData, >(row: Row, expanded?: boolean) { + const currentExpanded = row.table.atoms.expanded?.get() ?? {} + const currentExists = + currentExpanded === true || isExpandedRowId(currentExpanded, row.id) + const targetExpanded = expanded ?? !currentExists + + if (targetExpanded === currentExists) return + if (targetExpanded && !row_getCanExpand(row)) return + table_setExpanded(row.table, (old) => { const exists = old === true ? true : isExpandedRowId(old, row.id) let oldExpanded: ExpandedStateList = makeObjectMap() if (old === true) { - Object.keys(row.table.getRowModel().rowsById).forEach((rowId) => { - oldExpanded[rowId] = true + Object.values(row.table.getRowModel().rowsById).forEach((rowModelRow) => { + if (row_getCanExpand(rowModelRow)) { + oldExpanded[rowModelRow.id] = true + } }) } else { oldExpanded = Object.assign(makeObjectMap(), old) } - expanded = expanded ?? !exists - - if (!exists && expanded) { + if (!exists && targetExpanded) { oldExpanded[row.id] = true return oldExpanded } - if (exists && !expanded) { + if (exists && !targetExpanded) { const rest: ExpandedStateList = makeObjectMap() const rowIds = Object.keys(oldExpanded) for (let i = 0; i < rowIds.length; i++) { diff --git a/packages/table-core/src/features/row-pagination/createPaginatedRowModel.ts b/packages/table-core/src/features/row-pagination/createPaginatedRowModel.ts index bd991529cf..33b9dad7a9 100644 --- a/packages/table-core/src/features/row-pagination/createPaginatedRowModel.ts +++ b/packages/table-core/src/features/row-pagination/createPaginatedRowModel.ts @@ -70,7 +70,13 @@ function _createPaginatedRowModel< paginatedRowModel.flatRows = [] + // The page rows can contain expanded sub-rows inline alongside their + // parents, so track seen ids to keep each row unique in flatRows. + const seenFlatRows = new Set() + const handleRow = (row: Row) => { + if (seenFlatRows.has(row.id)) return + seenFlatRows.add(row.id) paginatedRowModel.flatRows.push(row) if (row.subRows.length) { row.subRows.forEach(handleRow) diff --git a/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts b/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts index 682c684dcb..3f5eb1f0c0 100644 --- a/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts +++ b/packages/table-core/src/features/row-sorting/rowSortingFeature.types.ts @@ -144,11 +144,15 @@ export interface ColumnDef_RowSorting< /** * The priority of undefined values when sorting this column. * - `false` - * - Undefined values will be considered tied and need to be sorted by the next column filter or original index (whichever applies) + * - Undefined values will be passed to the sorting function like any other value with no special handling; the sorting function is responsible for handling them * - `-1` * - Undefined values will be sorted with higher priority (ascending) (if ascending, undefined will appear on the beginning of the list) * - `1` * - Undefined values will be sorted with lower priority (descending) (if ascending, undefined will appear on the end of the list) + * - `'first'` + * - Undefined values will be pushed to the beginning of the list regardless of sort direction + * - `'last'` + * - Undefined values will be pushed to the end of the list regardless of sort direction */ sortUndefined?: false | -1 | 1 | 'first' | 'last' } @@ -186,9 +190,11 @@ export interface Column_RowSorting< */ getIsSorted: () => false | SortDirection /** - * Returns the next sorting order. + * Returns the next sorting order. Pass `multi` to resolve the order for a + * multi-sort toggle, where `enableMultiRemove` governs whether the cycle can + * remove the sort. */ - getNextSortingOrder: () => SortDirection | false + getNextSortingOrder: (multi?: boolean) => SortDirection | false /** * Finds this column's position in the ordered sorting state. */ diff --git a/packages/table-core/src/features/row-sorting/rowSortingFeature.utils.ts b/packages/table-core/src/features/row-sorting/rowSortingFeature.utils.ts index 32e3507e58..9efe5080ab 100644 --- a/packages/table-core/src/features/row-sorting/rowSortingFeature.utils.ts +++ b/packages/table-core/src/features/row-sorting/rowSortingFeature.utils.ts @@ -165,10 +165,13 @@ export function column_getAutoSortFn< } /** - * Chooses the default first sort direction from the first filtered row value. + * Chooses the default first sort direction from sampled filtered row values. * - * String columns start ascending so alphabetical order is natural; other value - * types start descending. + * The first non-nullish value among the sampled rows decides: string columns + * start ascending so alphabetical order is natural; other value types (or + * columns with no non-nullish sample) start descending. Sampling past leading + * nullish values keeps the toggle cycle stable when sorting or a data swap + * moves an empty value into the first row. * * @example * ```ts @@ -180,12 +183,16 @@ export function column_getAutoSortDir< TData extends RowData, TValue extends CellData = CellData, >(column: Column_Internal) { - const firstRow = column.table.getFilteredRowModel().flatRows[0] + const firstRows = column.table.getFilteredRowModel().flatRows.slice(0, 10) + + for (let i = 0; i < firstRows.length; i++) { + const value = firstRows[i]!.getValue(column.id) - const value = firstRow ? firstRow.getValue(column.id) : undefined + if (value == null) { + continue + } - if (typeof value === 'string') { - return 'asc' + return typeof value === 'string' ? 'asc' : 'desc' } return 'desc' @@ -234,8 +241,8 @@ export function column_getSortFn< * Applies the next sorting state for this column. * * The toggle can add, replace, flip, or remove this column's sort entry. Multi - * sorting respects `enableMultiSort`, `maxMultiSortColCount`, and the `multi` - * argument. + * sorting respects `enableMultiSort`, `enableMultiRemove`, + * `maxMultiSortColCount`, and the `multi` argument. * * @example * ```ts @@ -252,7 +259,10 @@ export function column_toggleSorting< multi?: boolean, ) { // this needs to be outside of table.setSorting to be in sync with rerender - const nextSortingOrder = column_getNextSortingOrder(column) + const nextSortingOrder = column_getNextSortingOrder( + column, + multi && column_getCanMultiSort(column), + ) const hasManualValue = typeof desc !== 'undefined' table_setSorting(column.table, (old) => { diff --git a/packages/table-core/tests/implementation/features/row-pagination/createPaginatedRowModel.test.ts b/packages/table-core/tests/implementation/features/row-pagination/createPaginatedRowModel.test.ts index e9ed3f5284..bb08830179 100644 --- a/packages/table-core/tests/implementation/features/row-pagination/createPaginatedRowModel.test.ts +++ b/packages/table-core/tests/implementation/features/row-pagination/createPaginatedRowModel.test.ts @@ -199,6 +199,35 @@ describe('createPaginatedRowModel', () => { table.getPaginatedRowModel().flatRows.map((row) => row.id), ).toEqual(['0', '0.0', '0.1', '1', '1.0', '1.1']) }) + + it('should not duplicate expanded sub-rows in flatRows (default paginateExpandedRows)', () => { + const table = createTable({ data: makeNestedData(), pageSize: 3 }) + + table.getRow('0').toggleExpanded() + + // The page slice is ['0', '0.0', '0.1']; the sub-rows appear both + // inline and under their parent's subRows, but flatRows must list each + // row exactly once + expect( + table.getPaginatedRowModel().flatRows.map((row) => row.id), + ).toEqual(['0', '0.0', '0.1']) + }) + + it('should not duplicate expanded sub-rows in flatRows with paginateExpandedRows: false', () => { + const table = createTable({ + data: makeNestedData(), + pageSize: 2, + paginateExpandedRows: false, + }) + + table.getRow('0').toggleExpanded() + + // Page rows are ['0', '0.0', '0.1', '1']; collapsed sub-rows of '1' + // still join flatRows once via the subRows walk + expect( + table.getPaginatedRowModel().flatRows.map((row) => row.id), + ).toEqual(['0', '0.0', '0.1', '1', '1.0', '1.1']) + }) }) describe('rowsById passthrough', () => { diff --git a/packages/table-core/tests/unit/features/row-expanding/rowExpandingFeature.utils.test.ts b/packages/table-core/tests/unit/features/row-expanding/rowExpandingFeature.utils.test.ts index 16a12e50a5..edf1aeeb5b 100644 --- a/packages/table-core/tests/unit/features/row-expanding/rowExpandingFeature.utils.test.ts +++ b/packages/table-core/tests/unit/features/row-expanding/rowExpandingFeature.utils.test.ts @@ -131,12 +131,54 @@ describe('table_toggleAllRowsExpanded', () => { it('should apply an explicit value without toggling', () => { const onExpandedChange = vi.fn() - const table = makeTable({ onExpandedChange }) + const table = makeTable({ + onExpandedChange, + initialState: { expanded: { '0': true } }, + }) table_toggleAllRowsExpanded(table, false) expect(onExpandedChange).toHaveBeenCalledWith({}) }) + + it('should be a no-op when collapsing with nothing expanded', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange }) + + table_toggleAllRowsExpanded(table, false) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op when already in the expanded-all state', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ + onExpandedChange, + initialState: { expanded: true }, + }) + + table_toggleAllRowsExpanded(table, true) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op when no rows can expand', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange }, [3]) + + table_toggleAllRowsExpanded(table, true) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op when expanding is disabled', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange, enableExpanding: false }) + + table_toggleAllRowsExpanded(table, true) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) }) describe('table_getCanSomeRowsExpand', () => { @@ -196,11 +238,27 @@ describe('table_getIsAllRowsExpanded', () => { expect(table_getIsAllRowsExpanded(table)).toBe(true) }) - it('should return false when any row is collapsed', () => { + it('should return true when only every expandable row id is expanded', () => { + // A materialized expanded-all map only contains expandable row ids, so + // leaf rows must not count against the all-expanded check + const table = makeTable({ + initialState: { expanded: { '0': true, '1': true, '2': true } }, + }) + + expect(table_getIsAllRowsExpanded(table)).toBe(true) + }) + + it('should return false when any expandable row is collapsed', () => { const table = makeTable({ initialState: { expanded: { '0': true } } }) expect(table_getIsAllRowsExpanded(table)).toBe(false) }) + + it('should return false for stale expanded ids when no rows can expand', () => { + const table = makeTable({ initialState: { expanded: { '0': true } } }, [3]) + + expect(table_getIsAllRowsExpanded(table)).toBe(false) + }) }) describe('table_getExpandedDepth', () => { @@ -223,10 +281,16 @@ describe('table_getExpandedDepth', () => { ).toBe(2) }) - it('should measure depth from the row model for the expanded-all state', () => { + it('should measure depth from the row model expandable rows for the expanded-all state', () => { + // With 2 levels of data only the root rows can expand, so the deepest + // expandable (and therefore expanded) id depth is 1, not the leaf depth const table = makeTable({ initialState: { expanded: true } }) - expect(table_getExpandedDepth(table)).toBe(2) + expect(table_getExpandedDepth(table)).toBe(1) + + const deepTable = makeTable({ initialState: { expanded: true } }, [2, 2, 2]) + + expect(table_getExpandedDepth(deepTable)).toBe(2) }) }) @@ -254,7 +318,7 @@ describe('row_toggleExpanded', () => { ).toEqual({ '1': true }) }) - it('should return the previous state unchanged for a redundant expand', () => { + it('should be a no-op for a redundant expand', () => { const onExpandedChange = vi.fn() const table = makeTable({ onExpandedChange, @@ -263,11 +327,75 @@ describe('row_toggleExpanded', () => { row_toggleExpanded(table.getRow('0'), true) - const old = { '0': true } - expect(getUpdaterResult(onExpandedChange, old)).toBe(old) + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op for a redundant collapse', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange }) + + row_toggleExpanded(table.getRow('0'), false) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op for a redundant expand in the expanded-all state', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ + onExpandedChange, + initialState: { expanded: true }, + }) + + row_toggleExpanded(table.getRow('0'), true) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op when expanding a row that cannot expand', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange }) + + row_toggleExpanded(table.getRow('0.0', true)) + row_toggleExpanded(table.getRow('0.0', true), true) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should be a no-op when expanding is disabled', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ onExpandedChange, enableExpanding: false }) + + row_toggleExpanded(table.getRow('0')) + + expect(onExpandedChange).not.toHaveBeenCalled() + }) + + it('should let options.getRowCanExpand allow expanding a row without subRows', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ + onExpandedChange, + getRowCanExpand: (row) => row.id === '0.0', + }) + + row_toggleExpanded(table.getRow('0.0', true)) + + expect(getUpdaterResult(onExpandedChange, {})).toEqual({ '0.0': true }) + }) + + it('should still collapse an expanded row that can no longer expand', () => { + const onExpandedChange = vi.fn() + const table = makeTable({ + onExpandedChange, + enableExpanding: false, + initialState: { expanded: { '0': true } }, + }) + + row_toggleExpanded(table.getRow('0')) + + expect(getUpdaterResult(onExpandedChange, { '0': true })).toEqual({}) }) - it('should materialize the expanded-all state when collapsing one row', () => { + it('should materialize the expanded-all state with only expandable row ids when collapsing one row', () => { const onExpandedChange = vi.fn() const table = makeTable({ onExpandedChange, @@ -279,7 +407,10 @@ describe('row_toggleExpanded', () => { const result = getUpdaterResult(onExpandedChange, true) expect(result['0']).toBeUndefined() expect(result['1']).toBe(true) - expect(result['0.1']).toBe(true) + // Leaf rows cannot expand, so they must not leak into the materialized map + expect(result['0.1']).toBeUndefined() + expect(result['1.0']).toBeUndefined() + expect(result).toEqual({ '1': true, '2': true }) }) }) diff --git a/packages/table-core/tests/unit/features/row-sorting/rowSortingFeature.utils.test.ts b/packages/table-core/tests/unit/features/row-sorting/rowSortingFeature.utils.test.ts index 1e0bbc5b4b..2cef9ce5ba 100644 --- a/packages/table-core/tests/unit/features/row-sorting/rowSortingFeature.utils.test.ts +++ b/packages/table-core/tests/unit/features/row-sorting/rowSortingFeature.utils.test.ts @@ -323,6 +323,37 @@ describe('column_getAutoSortDir', () => { expect(column_getAutoSortDir(table.getColumn('firstName')!)).toBe('desc') }) + + it('should skip leading nullish values when inferring the direction', () => { + // Regression #5147/#5832: sampling only the first row flipped the toggle + // cycle when a sort or data swap moved a null value into the first row + const data = [ + { firstName: null, lastName: 'young', age: 40 }, + { firstName: undefined, lastName: 'xi', age: 30 }, + { firstName: 'amy', lastName: 'zulu', age: 20 }, + ] as unknown as Array + const table = constructTable({ + data, + columns: personColumns, + features, + }) + + expect(column_getAutoSortDir(table.getColumn('firstName')!)).toBe('asc') + }) + + it('should return desc when all sampled values are nullish', () => { + const data = [ + { firstName: null, lastName: 'young', age: 40 }, + { firstName: undefined, lastName: 'xi', age: 30 }, + ] as unknown as Array + const table = constructTable({ + data, + columns: personColumns, + features, + }) + + expect(column_getAutoSortDir(table.getColumn('firstName')!)).toBe('desc') + }) }) describe('column_getFirstSortDir', () => { @@ -537,6 +568,52 @@ describe('column_toggleSorting', () => { ]) }) + it('should flip instead of removing in multi mode when enableMultiRemove is false', () => { + // Regression #4946: toggleSorting never forwarded `multi` to + // getNextSortingOrder, so enableMultiRemove was ignored on every path + const sorting: SortingState = [{ id: 'firstName', desc: true }] + const { table, onSortingChange } = makeTableWithMockOnSortingChange({ + enableMultiRemove: false, + initialState: { sorting }, + }) + + column_toggleSorting(table.getColumn('firstName')!, undefined, true) + + expect(getUpdaterResult(onSortingChange, sorting)).toEqual([ + { id: 'firstName', desc: false }, + ]) + }) + + it('should remove at the end of the cycle in multi mode by default', () => { + const sorting: SortingState = [ + { id: 'age', desc: true }, + { id: 'firstName', desc: true }, + ] + const { table, onSortingChange } = makeTableWithMockOnSortingChange({ + initialState: { sorting }, + }) + + column_toggleSorting(table.getColumn('firstName')!, undefined, true) + + expect(getUpdaterResult(onSortingChange, sorting)).toEqual([ + { id: 'age', desc: true }, + ]) + }) + + it('should ignore enableMultiRemove when the column cannot multi-sort', () => { + const sorting: SortingState = [{ id: 'firstName', desc: true }] + const { table, onSortingChange } = makeTableWithMockOnSortingChange({ + enableMultiRemove: false, + enableMultiSort: false, + initialState: { sorting }, + }) + + column_toggleSorting(table.getColumn('firstName')!, undefined, true) + + // Falls back to single-sort semantics, where enableSortingRemoval governs + expect(getUpdaterResult(onSortingChange, sorting)).toEqual([]) + }) + it('should keep only the latest maxMultiSortColCount columns', () => { const sorting: SortingState = [ { id: 'age', desc: true },