Skip to content

docs(data-table): propose reworked Toolbar layout API - #451

Draft
interacsean wants to merge 4 commits into
mainfrom
feat/core/1699-datatable-toolbar-api
Draft

docs(data-table): propose reworked Toolbar layout API#451
interacsean wants to merge 4 commits into
mainfrom
feat/core/1699-datatable-toolbar-api

Conversation

@interacsean

Copy link
Copy Markdown
Contributor

Issue

tailor-inc/platform-planning#1699 — from tailor-professional-service/knowledge#350

What

Docs only. No implementation, no behaviour change. This proposes a reworked DataTable.Toolbar API by writing it as though it already ships, so it can be reviewed by reading the documentation rather than a design bullet list.

  • New docs/components/data-table-toolbar.md.
  • docs/components/data-table.md updated so the two don't contradict — sub-components table, Toolbar props section replaced by a pointer, columnSettingsshowColumnSettings in the two places it appeared.

The proposed shape

Concern API
Built-in controls, default placement showFilters, showColumnSettings
Same controls, custom placement DataTable.Filters, DataTable.ColumnSettings
Horizontal layout / multiple rows DataTable.ToolbarRow (+ endSection, gap)
Visual grouping within a row DataTable.Separator

One rule ties it together: boolean prop = default placement, sub-component = custom placement, never both for the same control.

columnSettings is deprecated in favour of showColumnSettings and removed at the next major.

Why

DataTable.Toolbar renders a row at its root but wraps children in a hardcoded astw:flex-col div, so multiple children stack vertically and stretch full width. className can't fix it — it lands on the root, not the wrapper — and cn() is bare twMerge(clsx(...)) with no astw: prefix configured, so prefixed utilities don't resolve conflicts either way. Consumers currently reach for global CSS against [data-slot="data-table-toolbar"].

Reported against 1.8.0; still reproduces on 1.11.0.

The default flip is deferred

Wrapping bare children in an implicit ToolbarRow — the flex-col → flex-row change — is held for the next major and called out inline in the doc, with a forward-compatible migration consumers can do today.

Worth knowing for review: the current column default is not purely legacy. The slot="add" / slot="chips" recipe in data-table.md deliberately relies on stacking so filter chips wrap onto their own line. 7 of 8 DataTable.Toolbar usages in this repo pass a single child and are unaffected by a flip; that recipe is the exception, and it wants column. Column has to stay expressible.

Open questions

The doc ends with an Open questions section — sub-component naming, gap props vs. configuring the twMerge prefix, endSection prop vs. a nested section sub-component, DataTable.Separator vs. exporting the existing Separator primitive, and the timing of the implicit-row flip. That section comes out before any code lands.

Notes for reviewers

  • No changeset. Nothing user-facing ships in this PR; adding one would put unbuilt components into a release note.
  • No UI Catalogue deep link was added, since check-catalogue-links.sh validates those and there's no toolbar slug yet.
  • A live repro of the current behaviour (two toolbars, one with an attempted astw:flex-row override, rendering identically) exists locally and can be added as an example page if that would help review.

🤖 Generated with Claude Code

Documents a rewritten `DataTable.Toolbar` surface as though it ships today, so
the team can review the API by reading it rather than by reading a diff:

- `showFilters` / `showColumnSettings` for default placement, with
  `DataTable.Filters` / `DataTable.ColumnSettings` as the placement escape
  hatches, under one rule: boolean prop = default, sub-component = custom.
- `DataTable.ToolbarRow` for horizontal rows and multi-row toolbars, with an
  `endSection` for right-aligned content, plus `DataTable.Separator`.
- `columnSettings` deprecated in favour of `showColumnSettings`.
- Wrapping bare children in an implicit row — the flex-col to flex-row default
  flip — is deferred to the next major and called out inline.

Open questions are kept in the doc for review and come out before any code
lands. No implementation yet.

Refs: tailor-inc/platform-planning#1699
Refs: tailor-professional-service/knowledge#350
Comment thread docs/components/data-table-toolbar.md Outdated
</DataTable.Root>
```

Add your own content as children. It sits alongside the built-in controls:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should specify that children are left-aligned

Comment thread docs/components/data-table-toolbar.md Outdated

## Rows

`DataTable.ToolbarRow` lays its children out horizontally with a gap sized for comfortable tap targets. Use it when the toolbar needs more than one line:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

more than one line" +
"or to lay out children in one row horizontally with default spacing" (or something)

Comment thread docs/components/data-table-toolbar.md Outdated
| -------------------- | ----------- | ------- | --------------------------------------------------------------------------------------------------------------- |
| `children` | `ReactNode` | — | Toolbar content. Use `DataTable.ToolbarRow` for multi-row layouts. |
| `showFilters` | `boolean` | `false` | Render the **Add filter** trigger and active chips in their default position. Requires `control`. |
| `showColumnSettings` | `boolean` | `false` | Render the **Columns** control (show/hide + reorder + pin) anchored top-right. Persists per-user via `tableId`. |

@interacsean interacsean Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make minor reference to columnSettings prop here, and refer to deprecations below

Comment thread docs/components/data-table-toolbar.md Outdated

`DataTable.ToolbarRow` is the only element that sets a direction: it is a horizontal flex row with `gap` spacing and vertically centred items.

Children passed directly to `DataTable.Toolbar` — without a `ToolbarRow` — currently stack vertically and stretch to full width. This is why `DataTable.Filters` renders on its own line in the split-slot recipe:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add row: boolean for ergonomics, and this would wrap children in ToolbarRow, plus add col: boolean for when we make row (and wrapping) the default: col would then be the opt-out of the default wrapping

Comment thread docs/components/data-table-toolbar.md Outdated

| Prop | Type | Default | Description |
| ----------- | -------- | ------- | ------------------------------------- |
| `className` | `string` | — | Additional CSS class for the control. |

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Passing in these classNames, I think we don't want to mix the internal aswt:* prefixed classNames with classes that consumers pass in, because they don't merge well and end up unreliably resolving based on the stylesheet import order... I think

So, maybe if we want this, the implementation must mean that these classNames get dropped onto a wrapping div element (that is always there and wrapping) to allow this (it would have no astw classes on it)

- State that toolbar children are left-aligned.
- Broaden ToolbarRow's stated purpose to cover single-row horizontal layout,
  not just multi-row toolbars.
- Point `showColumnSettings` at the `columnSettings` deprecation inline.
- Add `row` / `col` on `DataTable.Toolbar`: `row` wraps children in a single
  ToolbarRow today, `col` is the no-op-today opt-out that survives the flip
  when row becomes the default. Both give a forward-compatible migration.
- Document the `className` contract: classes land on a wrapper carrying no
  app-shell classes, so they never compete with `astw:` internals or depend on
  stylesheet order. Internal layout stays prop-driven, since a wrapper can't
  reach inside.
- Rework the styling open question around that contract.

Refs: tailor-inc/platform-planning#1699
- Say explicitly that `row` is skipped when any child is a ToolbarRow,
  including the all-children-are-rows case (the ordinary multi-row toolbar).
- Make `row` / `col` mutually exclusive in the type via a discriminated union,
  with a runtime warning as the backstop for spreads and JS callers.
- Spell out which CSS a wrapper can and cannot reach: box and inherited
  properties yes, the inner element's flex-direction / gap / align-items no.
- Note that `data-slot` stays on the inner elements, so CSS already written
  against [data-slot="data-table-toolbar"] keeps matching.

Refs: tailor-inc/platform-planning#1699
Replace the `row` / `col` booleans with `direction?: "row" | "col"`. One enum
is mutually exclusive by construction — no `row col` to diagnose, and no union
whose "no overload matches" error says nothing about the actual mistake.

Scope the className wrapper by job rather than applying it everywhere:

- Placed components (`Filters`, `ColumnSettings`, `Separator`) get a wrapper.
  They are dropped into a layout the consumer wrote, so positioning and sizing
  is the need, and a bare wrapper serves it deterministically. It also makes
  `Separator className="h-6"` work, since the rule itself is `h-full`.
- Containers (`Toolbar`, `ToolbarRow`) merge onto the container. The consumer
  already owns the element these sit in, so a wrapper adds a DOM node without
  adding capability.
- Toolbar's own padding / border-bottom stay honestly documented as unreliable
  to override, rather than papered over.

Also correct the reasoning in the open question: configuring a twMerge prefix
cannot fix cross-boundary conflicts. `cn()` dedupes within one class string, so
an unprefixed `gap-4` and `astw:gap-2` are unrelated classes that both survive
and are resolved by source order either way.

Refs: tailor-inc/platform-planning#1699
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.

1 participant