Skip to content

feat(data-table): sub-rows for nested data, sharing Tree's selection model - #681

Open
interacsean wants to merge 4 commits into
feat/treefrom
feat/data-table-tree
Open

interacsean wants to merge 4 commits into
feat/treefrom
feat/data-table-tree

Conversation

@interacsean

@interacsean interacsean commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Adds sub-rows to DataTable, so a table can show nested data (a chart of accounts, a BOM, locations within warehouses) as expandable rows, with the same selection model as Tree.

This is stacked on #675, so the diff is just the table work. It's the tree grid that PR deferred: a hierarchy with real columns belongs with DataTable, not Tree.

const table = useDataTable<Account>({
  columns,
  data: { rows: accounts },
  subRows: { getSubRows: (row) => row.children, column: "name" },
  rowSelection: { selectionPropagation: "full", valueStrategy: "leaves" },
});

Commits

  1. refactor(tree) moves the tree model and expansion state from components/tree into lib/tree, so Tree and DataTable share one implementation. Tree's behaviour is unchanged.
  2. feat(data-table) builds subRows on top of it.

API

  • subRows: getSubRows(row) returns a row's children, and column names the column that carries the hierarchy (defaults to the first). Open rows are controlled with expandedIds + onExpandedChange, or uncontrolled with defaultExpandedIds.
  • rowSelection takes the same selectionPropagation and valueStrategy as Tree, with the same defaults ("full" / "leaves") and the same rules about which combinations are allowed.
  • Exports: SubRowsOptions and DataTableTreeRow.
  • subRows and rowExpansion can't be combined. A row shows either its sub-rows or a detail panel, so rowExpansion is ignored with a console warning.

Behaviour worth a look

  • The hierarchy column is locked. It's always first and pinned left, even when column names a column defined later. Column settings won't hide it, unpin it, or move it or anything in front of it, and it shows a lock. Persisted column state from before sub-rows were turned on is corrected on load. Making the lock opt-in is on https://github.com/tailor-platform/app-shell/tree/feat/data-table-tree-lock-column-opt-in, held back for now.
  • Checkboxes follow the mode. Under "full" / "down" they sit in the hierarchy column, indented with the row, so a parent and the rows it ticks line up. Under "none" every row is independent, so they stay in the usual selection column.
  • Pagination is by top-level row. Sub-rows come with their parent. Select-all ticks the current page, and ids from other pages pass through untouched.
  • Accessibility: the caret is a real button with aria-expanded. A full treegrid keyboard model is a follow-up.
  • The hierarchy cell is read-only, even in an editable column, since it also holds the indent and caret.
  • Bad data warns rather than crashes. A row listed beneath itself (a root that names itself as its parent) is left out of its own sub-rows. An id that's a parent in one place and a leaf in another is reported. Each warning logs once.

Open question

Under selectionPropagation: "none", select-all also ticks sub-rows hidden inside closed parents, so a bulk action can hit rows nobody saw. That's expected under "full" and "down". I'm checking whether "none" should tick visible rows only.

Test plan

  • vitest: sub-row tests for expansion (controlled and not), the locked column, every selection mode, page-scoped select-all, cycles and warn-once. Full suite: 2,828 passing.
  • tsc, oxlint, oxfmt --check, pnpm docs:check, check-dts.
  • Checked by hand at /showcase/data-table-tree: each propagation and strategy, the locked column in the Columns popover, and alignment in each mode.
  • Adversarial review: four Major issues and three minor, all fixed.

🤖 Generated with Claude Code

interacsean and others added 2 commits November 5, 2026 15:52
The pure tree model (building, indexing, filtering, flattening, and
reading / ticking / writing a value under each propagation and value
strategy) moves from the Tree component to lib/tree, so other
tree-shaped UIs can share it, starting with DataTable sub-rows.

Expansion state moves into a useTreeExpansion hook, which can now also
be controlled (expandedIds / onExpandedChange). Tree uses it
uncontrolled, as before.

TreeSelectionPropagation, TreeValueStrategy and the
TreeSelectionBehaviorProps union are declared with the model and
re-exported from Tree, so the public API doesn't change. No behaviour
changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…odel

useDataTable takes subRows: { getSubRows, column?, expandedIds /
defaultExpandedIds / onExpandedChange }. The page's rows are the top
level, so pagination counts them and each parent's sub-rows stay on its
page.

- Hierarchy column: the first column (or subRows.column) carries the
  indent, the open / close caret and, when selection propagates, the
  checkbox. It's locked first and pinned left wherever it's defined;
  useColumns ignores hiding, unpinning or moving it (or anything in
  front of it), and column settings and the header pin menu show it as
  locked.
- Selection: rowSelection takes the same selectionPropagation and
  valueStrategy as Tree (default full / leaves), using the shared model
  to read, tick and write the selected ids. Ids from other pages pass
  through. Under full / down the checkboxes, select-all included, move
  into the hierarchy column; under none they stay in the left column.
- The caret is a button with aria-expanded, named from the hierarchy
  column's value; full treegrid keyboard navigation is a follow-up.
- subRows can't be combined with rowExpansion: subRows wins, with a
  console warning. The hierarchy cell stays read-only under inline
  editing.

Exports SubRowsOptions and DataTableTreeRow; documents Sub-rows and
Selecting sub-rows; adds a showcase page.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 5, 2026 •

Copy link
Copy Markdown
Contributor

Code Metrics Report

main (f9acc22) #681 (c327480) +/-
Coverage 89.6% 90.2% +0.6%
Test Execution Time 3m46s 2m51s -55s
Details
  |                     | main (f9acc22) | #681 (c327480) |  +/-  |
  |---------------------|----------------|----------------|-------|
+ | Coverage            |          89.6% |          90.2% | +0.6% |
  |   Files             |            269 |            281 |   +12 |
  |   Lines             |           7728 |           8315 |  +587 |
+ |   Covered           |           6930 |           7507 |  +577 |
+ | Test Execution Time |          3m46s |          2m51s |  -55s |

Code coverage of files in pull request scope (92.5% → 95.3%, patch 97.4%)

Files Coverage +/- Patch Coverage Status
packages/core/src/components/data-table/body.tsx 96.0% +0.1% 100.0% modified
packages/core/src/components/data-table/column-footer.tsx 95.4% +0.2% 100.0% modified
packages/core/src/components/data-table/column-layout.ts 100.0% 0.0% 100.0% modified
packages/core/src/components/data-table/column-settings.tsx 90.6% +0.1% 66.6% modified
packages/core/src/components/data-table/context-menu.tsx 73.5% 0.0% - modified
packages/core/src/components/data-table/context.tsx 87.5% +4.1% 100.0% modified
packages/core/src/components/data-table/header.tsx 96.0% -1.8% 90.0% modified
packages/core/src/components/data-table/i18n.ts 53.3% 0.0% - modified
packages/core/src/components/data-table/index.ts 0.0% 0.0% - modified
packages/core/src/components/data-table/root.tsx 100.0% 0.0% - modified
packages/core/src/components/data-table/rows.tsx 100.0% 0.0% 100.0% modified
packages/core/src/components/data-table/status-rows.tsx 100.0% 0.0% - modified
packages/core/src/components/data-table/sub-rows-cell.tsx 100.0% +100.0% 100.0% added
packages/core/src/components/data-table/table.tsx 100.0% 0.0% 100.0% modified
packages/core/src/components/data-table/use-columns.ts 100.0% 0.0% 100.0% modified
packages/core/src/components/data-table/use-data-table.ts 95.6% +0.5% 100.0% modified
packages/core/src/components/data-table/use-row-selection.ts 100.0% 0.0% 100.0% modified
packages/core/src/components/data-table/use-sub-rows.ts 92.8% +92.8% 92.8% added
packages/core/src/components/tree/Tree.tsx 97.9% +97.9% - modified
packages/core/src/components/tree/TreePicker.tsx 100.0% +100.0% - affected
packages/core/src/components/tree/TreeRow.tsx 100.0% +100.0% - modified
packages/core/src/components/tree/i18n.ts 75.0% +75.0% - affected
packages/core/src/components/tree/use-tree-field.ts 95.5% +95.5% - affected
packages/core/src/components/tree/use-tree-focus.ts 98.3% +98.3% - modified
packages/core/src/components/tree/use-tree-selection.ts 100.0% +100.0% 100.0% modified
packages/core/src/index.ts 0.0% 0.0% - modified
packages/core/src/lib/tree/model.ts 100.0% +100.0% - renamed
packages/core/src/lib/tree/use-tree-expansion.ts 100.0% +100.0% 100.0% added

Reported by octocov

@interacsean
interacsean force-pushed the feat/data-table-tree branch 2 times, most recently from cc4af19 to 9ec245a Compare November 6, 2026 03:10
@interacsean
interacsean marked this pull request as ready for review November 6, 2026 03:31
@interacsean
interacsean requested a review from a team as a code owner November 6, 2026 03:31
@IzumiSy

IzumiSy commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Could we consolidate selectionPropagation and valueStrategy into a single selectionStrategy prop?

There are only five valid combinations: full supports three value strategies, while down and none only support all. Exposing these as five explicit strategies would make the API easier to understand without changing the existing behavior.

Proposed names and behavior:

Assume a parent P with two leaf children, A and B. Each scenario below starts with an empty selection.

selectionStrategy Current combination IDs returned after checking P IDs returned after checking A and B individually Can P be selected independently?
leaves (default) full / leaves [A, B] [A, B] No
topmost full / topmost [P] [P] No
all full / all [P, A, B] [P, A, B] No
cascade down / all [P, A, B] [A, B] Yes
independent none / all [P] [A, B] Yes

A few important distinctions:

  • leaves, topmost, and all derive the parent's checkbox state from its descendant leaves. Unchecking A after checking P makes P indeterminate.
  • With topmost, [P] represents the entire selected subtree, not P alone.
  • cascade gives the parent its own selection state while propagating parent toggles to descendants. Unchecking A after checking P leaves [P, B], with P still checked.
  • independent selects each row separately. The name also avoids confusing “no propagation” with “selection disabled.”

This would replace the two public props with one, while retaining the existing propagation and value-formatting logic internally. Since these behaviors are shared with Tree and TreePicker, it would be worth keeping the API consistent across those components.

@IzumiSy

IzumiSy commented Nov 6, 2026

Copy link
Copy Markdown
Contributor

Following up on the selectionStrategy proposal, I would like to propose two additional changes.

1. Define a dedicated subRows.hierarchyColumn

Currently, subRows.column selects an existing column, falling back to the first column when omitted. That column is then forced to the front, pinned left, and made non-movable, non-hideable, and non-editable.

This creates a hidden dependency: a column can declare pin: "right" but end up pinned left because of the separate subRows configuration.

Instead, replace the key-based subRows.column with a required subRows.hierarchyColumn definition.

This follows the same ownership model as rowActions: that feature adds its own dedicated column on the right, rather than turning an existing data column into an actions column. Similarly, subRows should add its own hierarchy column on the left without modifying regular data columns.

For example, using the selectionStrategy API proposed in the earlier comment:

type Account = {
  id: string;
  name: string;
  code: string;
  balance: number;
  children?: Account[];
};

const table = useDataTable<Account>({
  columns: [
    {
      id: "code",
      label: "Code",
      type: "text",
      accessor: (row) => row.code,
    },
    {
      id: "balance",
      label: "Balance",
      type: "money",
      accessor: (row) => row.balance,
      pin: "right",
    },
  ],
  data: { rows: accounts },
  subRows: {
    getSubRows: (row) => row.children,
    hierarchyColumn: {
      label: "Account",
      type: "link",
      typeOptions: {
        href: (row) => `/accounts/${row.id}`,
      },
      accessor: (row) => row.name,
      width: 280,
      truncate: true,
      headerProps: () => ({
        title: "Account hierarchy",
      }),
      cellProps: (row) => ({
        className: row.children?.length ? "astw:font-semibold" : undefined,
      }),
      footer: ({ rows }) => `${rows.length} top-level accounts`,
    },
  },
  rowSelection: {
    selectionStrategy: "leaves",
  },
});

Unlike an actions column, the hierarchy column also displays row data. It should keep indentation, the expand/collapse control, and the row label together, especially with deeper nesting.

Its layout should be owned by the feature, just like the actions column:

  • Insert it before regular data columns and pin it left.
  • Exclude it from Column Settings and regular column ordering, visibility, pinning, and layout persistence.
  • Do not override any existing column’s configuration.
  • Remove the implicit fallback to the first data column.

This is a distinction in layout ownership, not a reason to create a separate rendering system. hierarchyColumn should reuse a subset of the regular column’s display APIs:

  • Required label and accessor
  • render
  • type and typeOptions
  • width and truncate
  • header and footer
  • headerProps, footerProps, and cellProps

The header and footer renderers should use the same contexts and semantics as regular columns. Custom header content should not replace feature-owned controls such as the select-all checkbox. The footer’s rows should remain the current page’s top-level data.rows, not the flattened list of visible descendants.

Preserve the regular typing relationships and type-dependent requirements between type, typeOptions, and accessor. Do not expose id or pin; keep edit, sort, and filter outside the scope of this change.

Selection placement should retain the existing behavior, expressed using the proposed strategy names:

  • For leaves, topmost, all, and cascade, keep checkboxes inside the hierarchy column, indented with each row, and select-all in its header.
  • For independent, keep the separate selection column before the hierarchy column, with select-all in the selection header.

Sizing should reuse existing column behavior: natural sizing without width, and truncation applied only to the label—not indentation or interactive controls—with the existing tooltip behavior preserved. No new placement, depth-limit, or indentation options are needed.

Because width is not a strict maximum under automatic table layout, deep nesting, narrow viewports, and long labels should be visually checked.

2. Enforce mutual exclusivity in UseDataTableOptions

Keep the other options shared, and intersect them with a union that makes rowExpansion and subRows mutually exclusive:

type DataTableExpansionOptions<TRow extends Record<string, unknown>> =
  | {
      rowExpansion?: RowExpansionOptions<TRow>;
      subRows?: never;
    }
  | {
      rowExpansion?: never;
      subRows: SubRowsOptions<TRow>;
    };

This union would be part of UseDataTableOptions, rather than a new public API.

  • Neither supplied: valid.
  • Either one supplied: valid.
  • Both supplied: a TypeScript error.

The explicit never properties reject simultaneous configuration even when the options come from typed variables or object spreads, rather than only relying on excess-property checks for object literals.

This prevents the conflicting configuration that currently produces a runtime warning and gives subRows precedence.

Passing both is now a compile error rather than a runtime warning. The
warning stays for untyped callers.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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