Repository navigation
Conversation
Code Metrics Report
Details | | main (cc749d5) | #633 (eb896da) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 89.3% | 89.3% | +0.0% |
| Files | 228 | 228 | 0 |
| Lines | 7241 | 7258 | +17 |
+ | Covered | 6471 | 6487 | +16 |
+ | Test Execution Time | 2m54s | 2m7s | -47s |Code coverage of files in pull request scope (92.9% → 93.0%, patch 96.2%)
Reported by octocov |
There was a problem hiding this comment.
🟢 Approval recommended
Static review found no blocking issues in the additive footer API, layout integration, or accompanying coverage.
No open issues 🎉
What changed in this PR
Adds per-column totals and summaries to DataTable without synthetic data records, while keeping the existing pagination and bulk-action footer separate.
Changes:
- Introduces
Column.footerand the exportedFooterRenderContexttype. - Adds an automatic sticky summary row that follows column layout and pinning, with updated width observation.
- Adds behavior/type coverage, a snapshot, editable examples, documentation, and a minor changeset.
No blocking issues were identified in static review. Verdict: Approve
| File | Description |
|---|---|
packages/core/src/index.ts |
Exports the footer context type publicly. |
packages/core/src/components/data-table/types.ts |
Defines the footer callback and data contract. |
packages/core/src/components/data-table/index.ts |
Re-exports the footer context type. |
packages/core/src/components/data-table/data-table.tsx |
Renders sticky footers and updates column measurement. |
packages/core/src/components/data-table/data-table-footer.test.tsx |
Covers footer rendering, state, editing, and pinning. |
packages/core/__snapshots__/src__components__data-table__data-table-footer.test.tsx.snap |
Captures footer markup and styling. |
examples/vite-app/src/pages/showcase/data-table-lab/page.tsx |
Explains the totals-row demonstration. |
examples/vite-app/src/pages/showcase/data-table-lab/inline-editing-demo.tsx |
Adds live totals within a constrained scroll area. |
docs/components/data-table.md |
Updates generated footer documentation. |
docs-src/components/data-table.docs.outline.md |
Documents footer semantics and usage. |
docs-src/components/data-table.docs.examples.tsx |
Adds an editable totals example. |
docs-manifest.json |
Updates documentation exports and generation hashes. |
.changeset/bright-tables-sum.md |
Records the feature as a minor release. |
Review effort: Balanced
There was a problem hiding this comment.
Something to look out for (for consumer implementations) is a money footer cell. There are likely tables with a money column which shows mixed currencies. Could be worth documenting to watch for this and don't provide a sum on mixed currencies, OR that the sum footer cell should check that all data is the same currency before providing a number and fall back to a "N/A" / "–" etc if it's more than one currency present
I know the LineItem PR is still in-flight, which will introduce shared table utilities. We'd want this feature in LineItems, almost as the main priority. So the work here should get refactored to be sharable to LineItems
Claude mentioned that the change to Width-measuring has the widest reach. Every DataTable gets it, regardless of whether it opts in to footer functionality. No assessed specific issues though
| id: "item", | ||
| label: "Item", | ||
| type: "text", | ||
| footer: () => "Total", |
There was a problem hiding this comment.
Potentially do we want to allow string as well as a convenience for static labels?
There was a problem hiding this comment.
Agreed. I’ve implemented footer: "Total" alongside the existing (ctx) => ReactNode renderer locally, and updated the docs example and lab demo to use the shorthand. An empty string also counts as an explicitly defined footer, including while loading.
I’m keeping header unchanged: fixed header text already uses label, which also supplies column metadata; header is the custom renderer that replaces the built-in header UI, including its sort controls.
Regression tests cover strings, empty strings, loading-to-loaded transitions, and hiding the last footer column. The string-support/docs update is tested locally but is not pushed yet.
There was a problem hiding this comment.
But label can be a candidate to be deprecated by supporting "string" simply in header as footer does in this PR.
| current.map((line) => (line.id === row.id ? { ...line, quantity } : line)), | ||
| ), | ||
| }, | ||
| footer: ({ rows }) => rows.reduce((sum, row) => sum + (row.quantity ?? 0), 0), |
There was a problem hiding this comment.
We could consider providing this function rows.reduce((sum, row) => sum + (row.quantity ?? 0), 0) as an out of the box util footer option
export const footerCellSum = <T /* that has a K */>(sumFieldKey: K> => ({ rows }) => rows.reduce((sum, row) => sum + (row[sumFieldKey] ?? 0), 0)
so it can be used as:
footer: footerCellSum("reduce"),
Maybe also a helper that just counts — can pass some function that is the count condition, which defaults to (!!value > 0) || (value.length > 0)
There was a problem hiding this comment.
Yea sounds good. Let me do that in another PR!
| - **Consumer-owned content and formatting.** Return text, numbers, or JSX. `type` and `typeOptions` do not format footer results. A column without `footer`, or a renderer returning `null`, leaves an empty cell. | ||
| - **Shared column layout.** Footer cells follow column width, alignment, ordering, visibility, and left/right pinning. Hidden columns' footer callbacks are not called. The row sticks to the bottom of the table's scroll container during vertical scrolling, paired with the sticky header at the top. | ||
| - **Not data rows.** The footer has no generated editors, selection checkboxes, expansion triggers, or row-action menus; their utility columns have empty footer cells to preserve alignment. Footer cells do not trigger `onClickRow`. | ||
| - **Loading and empty states.** Loading and errors hide the footer. Loaded empty data still calls visible footer renderers with `rows: []`, so a sum can display zero. If no visible column defines `footer`, no `<tfoot>` is rendered. |
There was a problem hiding this comment.
Probably fine. Maybe just a second thought on whether we want to show 0 or an empty cell here. For Larson I think we went with "–"
There was a problem hiding this comment.
I’m keeping this consumer-owned rather than having the table choose a placeholder. With loaded empty data, the renderer receives rows: []; the zero in this example comes from reduce(..., 0), not from a table default.
A consumer can distinguish no records from a genuine zero total, for example:
footer: ({ rows }) =>
rows.length === 0
? "–"
: rows.reduce((sum, row) => sum + (row.quantity ?? 0), 0)Returning null instead leaves the cell blank. I’ve prepared a docs clarification to make this choice explicit and added regression tests for consumer-provided dash/blank values; those local changes are not pushed yet.
| footer: ({ rows }) => | ||
| rows | ||
| .reduce((sum, row) => sum + (row.weightKg ?? 0), 0) | ||
| .toLocaleString(undefined, { maximumFractionDigits: 3 }), |
There was a problem hiding this comment.
My suggestion above for a footerCellSum helper, I wonder how we can be smart to allow this sort of formatting. Could of course do
footer: ({ rows}) => footerCellSum("weightKg")(rows).toLocaleString(...)
And could provide curried* args so could call footerCellSum("weightKg", rows) as an alternative in these cases
Or why shouldn't the render default to display maxDecimals: 3 as the column's typeOptions already specifies
There was a problem hiding this comment.
Agreed on that point and will do it separately: #633 (comment)
| </p> | ||
| )} | ||
| <DataTable.Root value={table}> | ||
| <DataTable.Root value={table} className="max-h-[320px]"> |
There was a problem hiding this comment.
Claude: does this work around something that subtly breaks some visual style, or is it just to demonstrate that the footer row stickies?
There was a problem hiding this comment.
This is just for demonstration for stickiness.
| | `DataTable.Toolbar` | DataTable-specific toolbar container. Optional; pass `columnSettings` for the built-in Columns control. | | ||
| | `DataTable.ColumnSettings` | The placeable Columns control (show/hide + reorder + pin). Use it in a generic [`Toolbar`](./toolbar.md) for custom layouts. | | ||
| | `DataTable.Filters` | Add-filter panel + active filter chips, auto-generated from column filter configs. Requires `control` from `useCollectionVariables`. | | ||
| | `DataTable.Footer` | Container outside the `<table>` for pagination and other content, not the column footer. Optional. Becomes the bulk-action bar while rows are selected — see [Selection actions](#selection-actions). | |
There was a problem hiding this comment.
As discussed in standup, having a columns.footer and a DataTable.Footer could be confusing
There was a problem hiding this comment.
Agreed that the distinction needs to be explicit. For this PR, I’m keeping both names, with “footer” referring to two different scopes:
- Column footer (
column.footer): content in a column-aligned cell inside the table’s<tfoot>, such as a label, total, or note.DataTable.Tablerenders it automatically. - Table footer (
DataTable.Footer): the container outside the<table>for pagination and other table-level content, which becomes the bulk-action bar while rows are selected. It does not configure or render column footers.
There is precedent for this distinction: PrimeReact v10 exposes both Column.footer (including static labels) and DataTable.footer. TanStack Table v8 also calls the per-column content footer and accepts either a string or a renderer.
footer describes the location rather than requiring the content to be an aggregate or summary, so it also fits labels and notes. I’m adding a dedicated “Column footer vs. table footer” section to the docs to make the scopes and their independent use explicit, without renaming the existing DataTable.Footer API.
There was a problem hiding this comment.
The documentation clarification is now pushed in 4023a06: 4023a061. The new “Column footer vs. table footer” section explicitly distinguishes the column-aligned <tfoot> content from the table-level pagination/bulk-action container and explains that either can be used independently. Both API names remain unchanged.
|
|
||
| - **Current rows, not all records.** `ctx.rows` is the current `data.rows` array exposed as read-only, so a paginated table computes a page total. Selection does not narrow it. For a server-calculated total across all pages, use a closure such as `footer: () => totalQuantity` instead. | ||
| - **Current adopted values.** Updating `data.rows`, including adopting a confirmed cell edit, updates the footer. Unconfirmed editor drafts are not included, and footer callbacks must not mutate rows. | ||
| - **Consumer-owned content and formatting.** Return text, numbers, or JSX. `type` and `typeOptions` do not format footer results. A column without `footer`, or a renderer returning `null`, leaves an empty cell. |
There was a problem hiding this comment.
It would be good if we can take the type from the columns def. There may need to be some exceptions (see example below), so providing an override
Example where it wouldn't make sense: Seats reserved per person (max of 10). So type = select, which allows you to select 1-10. It is effectively a number, but you would want to show for the total row a type=number not type=select.
I know you are resolving align, but this seems like a bit of a hidden mechanism to only get some of the column data-cells attributes — and it is not immediately clear to consumer what comes for free and what does not.
There was a problem hiding this comment.
Agreed that the current distinction needs to be clear. In this PR, footers share column layout (width, alignment, ordering, visibility, and pinning), while content and formatting remain consumer-owned; type and typeOptions are not applied to the result.
I’ll revisit the footer API in a separate PR, covering both access to column settings such as maxDecimals and whether/how type and formatting should be inherited and overridden. The select-column/numeric-total example is a useful case to validate that design. That also ties into the separately planned aggregation helpers, without committing to a specific API shape here.
| // Column footer (internal — separate from DataTable.Footer) | ||
| // ============================================================================= | ||
|
|
||
| function DataTableColumnFooter() { |
There was a problem hiding this comment.
Worth putting in another file? This section adds a lot of lines to an, admittedly, already long file
There was a problem hiding this comment.
I have another ticket that plans the whole clean-up of file structure related to DataTable.
Let me work on it separately 🙏
| [columns, pinnedColumns, hasSelection, hasExpand, hasRowActions, widths, columnKeys], | ||
| ); | ||
|
|
||
| if (loading || error || !ordered.some((col) => col.footer)) return null; |
There was a problem hiding this comment.
I worry that during pagination change, we will have loading==true and the total row will flicker on and off. Can you confirm whether this would be the case out-of-the-box with our async guidance?
There was a problem hiding this comment.
Yes — with the documented loading: result.fetching wiring, the original implementation removed the footer during page changes and brought it back when fetching completed.
Fixed in 35d12cf0: the <tfoot> stays mounted during loading, with skeletons in columns that define a footer, matching the body’s loading presentation. Footer renderers are not called while loading, so we neither retain a stale page total nor calculate a temporary total from incomplete data. Errors still hide the footer.
Regression tests cover initial loading and the loaded → loading → next-page lifecycle, including retaining the same <tfoot> element, not invoking renderers during loading, and showing the new page total afterward. The loading behavior is also documented.
|
@copilot resolve the merge conflicts in this pull request |
…n-footer # Conflicts: # docs-manifest.json Co-authored-by: IzumiSy <982850+IzumiSy@users.noreply.github.com>
Motivation
Tables with editable line items often need a totals row. Without a table-aligned footer, consumers have to put synthetic summary records into
data.rows, widen their row types, and special-case those records in renderers, accessors, edit callbacks, and row interactions. A summary is presentation derived from records, not another editable record.Provide a per-column footer so consumers can keep
data.rowslimited to real records and render summaries aligned with the existing table layout. This PR is limited to column footers; it does not add editing validation, error APIs, or an add-row API.Screen shot
Design Decision
Per-column presentation hook
Add optional
Column.footer({ rows }) => ReactNodealongsideColumn.header.DataTable.Tableautomatically renders one native<tfoot>row when at least one visible column defines a footer. No additional JSX component, aggregate engine, or context export is required.The row stays at the bottom of the table scrollport and follows column widths, alignment, ordering, visibility, and left/right pinning. Selection, expansion, and row-action columns get blank cells rather than row controls. Existing
DataTable.Footerremains the separate pagination and bulk-action container outside the table.Explicit data and rendering scope
rowsis the currentdata.rowsarray, exposed as read-only: a paginated table computes a page total, not an all-pages or selected-only total. Consumers can render server-provided aggregates through a closure and own formatting. The footer reflects adopted data, not unfinished editor drafts. Loading and error states hide it; loaded empty data supplies an empty array.Measure actual column changes
A changing footer can redistribute column widths without changing the table's overall width. Observe the table's own header cells so stacked pinned offsets remain accurate, while ignoring container-only height changes during detail-row animations. Refresh those subscriptions when utility columns appear or disappear.
Summary