feat(Table): expose theme targets for the sort and filter affordances

[freddymeta]

Implements the third row of #5417 — the one with no consumer-side workaround at all.

Problem

useTableSortable rendered its control as a bare <button> wrapping a bare <span> wrapping the Icon; useTableFiltering rendered its funnel as a bare <button aria-haspopup="dialog">. Neither carried an astryx-* class, so no token and no components entry could reach them — and because both are rendered inside the plugin, there is no wrapper a consumer can interpose and no renderX prop to route around it. The only remaining move was .astryx-table-header-cell button[aria-haspopup='dialog'], a selector keyed on an ARIA attribute.

Both also re-implemented the interaction states of a button on a native <button>, which is what @rubyycheung caught.

Change

Both affordances are now the real Button / IconButton. Rest, hover, pressed, the focus ring, the scale(0.98) press and the reduced-motion guard are Button's; every hand-rolled copy is deleted. The column label stays inside the sort control, so the whole heading is still the click target.

Each keeps one table-scoped theme target on the control Button renders, the toggle-button shape:

target	element	reflects
astryx-table-sort-button	the sort control	direction
astryx-table-filter-button	the funnel trigger	active

INV5 says a part rendered by a shared primitive delegates to that primitive's target "unless the parent guarantees a distinct public visual contract". Both do, and defineTheme is why: parseStyleKey emits one compound selector on one element, no combinators, so a theme cannot write .astryx-table-header-cell .astryx-button. Delegating alone would mean "restyle every button in the app" or nothing. Beyond scope, color behaves differently on the two contracts — see below.

color on the sort target paints the glyph and leaves the column name alone. The control holds both, and the name belongs to the header cell. The value is routed to the glyph through a derived-var entry with replaces: true, so defineTheme emits --_table-sort-glyph-color and no color — the progress-bar-mark / text-area mechanism, and INV11 rather than a new public var. The filter trigger holds only its glyph, so plain color lands on it directly.

Records this moves with

Table.spec.md's anatomy-theming:v1 map dispositioned Sort control as inherits: table-header-cell and had no filter anatomy at all. Per "Change coupling", the map, the anatomy inventory and the targets move together, so this PR updates all three: Sort control and a new Filter control become target, Filter indicator glyph joins Sort indicator glyph in delegating to Icon, and the prose records why the two targets survive INV5.

The targets are declared in Table.doc.mjs rather than the plugin docs, because that is the doc check-knowledge.mjs reads for component:Table's target inventory — declared on the plugin docs they were invisible to the anatomy↔target validator.

Visual changes

Not "no visual change" — the changeset lists them. Hover and pressed are now Button's overlay (a background image, not a background colour) plus scale(0.98); the filter trigger gains the shared focus ring and Button's sm square box (28×28 measured, above WCAG 2.2 2.5.8's 24px); the sort control's label is measured identical to its <th> on colour, white-space, overflow, text-overflow, font size and weight, so headings are unchanged.

Test plan

• pnpm exec vitest run packages/core/src packages/lab/src, pnpm -F @astryxdesign/core build, typecheck, typecheck:docs, pnpm check:repo, pnpm lint:strict — output quoted in the review reply.
• .github/scripts/affordance-color-reach.js against a built Storybook, in Chromium. The probe stylesheet is no longer hand-written: it is what defineTheme + generateThemeCSS emit for a theme setting color on the target, so the derived-var expansion is exercised rather than assumed.
• Negative controls, both quoted in the reply: removing the derived-var entry makes the guard report the colour landing on the button, the label following it and the glyph unmoved — the exact failure @cixzhang described; forcing a hardcoded stroke on the geometry makes it report the painted glyph ignoring the target.
• check-knowledge.mjs negative control: reverting Sort control to inherits while the target stays declared fails with current target "table-sort-button" has no anatomy entry with a target disposition.

Still needs a maintainer

The CI wiring for the guard edits .github/workflows/ci.yml, which my token cannot push (no workflow scope). The block is in the PR discussion.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
astryx	Ready	Preview	Sep 2, 2026 12:59pm UTC

[github-actions]

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

Modified Components

Table _{(@astryxdesign/core)} · View in Storybook

Metric	Before	After	Delta
Bundle Size (ESM)	N/A	N/A	N/A
Lines of Code	N/A	1950	-
Complexity	N/A	Very High (112)	-

Bundle Size Summary

Package	Size (ESM)	Size (CJS)	Gzipped
@astryxdesign/core	N/A	4.8KB	1.2KB

Accessibility Audit

Status: No accessibility violations detected.

Visual Regression

Status: Skipped — Broad stable scope is deferred to the daily release gate. It covers 4332 trusted baseline shots instead of recapturing them for this PR. View the report

---

_{Generated by PR Enrichment workflow | Storybook | Sandbox | View full report}

[cixzhang]

Thanks — the problem is right, and keeping the guard fix here is right too; I checked and the old one stays green with these three unchecked.

The two sort targets are one axis, and neither carries the glyph. Set a colour on astryx-table-sort-button — the obvious one — and nothing happens; the funnel has the same problem with only one target. Both icons want color="inherit", then the colour goes on the buttons and astryx-table-sort-indicator comes out. Spinner went this way in #5408.

colour on the button today	with the label pinned

Drop the resting opacity: 0.35 with it — inline below. Composited, the arrows sit at 1.57:1 against the header where AA wants 3:1; --color-icon-secondary alone is 4.74:1.

Then a browser test to hold it: colour on the button target, assert the glyph paints it, hover, assert it follows. theme-var-reachability.js is the shape.

If you'd rather talk it through, we're in Discord.

[Reviewed by Robohands]

[cixzhang]

color="inherit" on this one (it's secondary two lines down) and the button's colour reaches the glyph — the registry SVGs are already currentColor.

[cixzhang]

This resting opacity: 0.35 is what puts the arrows at 1.57:1. Drop it and let secondary render — the header label span then needs its own colour so the button's doesn't bleed into it.

[cixzhang]

Same here — the funnel's Icon sets its own colour, so the single target above doesn't reach it.

[cixzhang]

Thanks — my last review asked for one target per affordance, button-owned color, inherited SVG paint, no resting opacity, and a browser guard. The runtime work now matches all five asks.

The guard is still the hold. It is not called by CI, and on this head it reads before Storybook renders and fails both cases. After waiting, it checks svg.color, so a fixed stroke still passes while the glyph paints the wrong color. Could a maintainer wire it into the Storybook Chromium job, wait for each target, and assert the painted stroke/fill?

[Reviewed by Robohands]

[rubyycheung]

Keeping the sort and filter affordances visible at rest is the right accessibility improvement, but these are buttons and should mirror the standard Button/IconButton interaction states. The current change removes the distinct pointer-hover feedback entirely.

Please apply the shared hover and pressed treatments, retain the focus-visible ring for keyboard users, and reserve the accent styling for the active sorted/filtered state. This gives the controls a clear rest → hover → pressed interaction model without returning to the inaccessible 35% resting opacity.

[freddymeta]

(Replaces my previous comment, which was far too long.)

Both reviews came down to one question — who owns these two controls — and the answer is toggle-button's shape: the real primitive, carrying one target of its own. Head 864ecc8c4, merged from main.

@rubyycheung — now the real Button / IconButton. You were right about drift, and it was already measurable: the hand-rolled :hover/:active used background-color, where the shared interactionOverlayStyles uses a background-image gradient. Sort is <Button variant="ghost" size="sm"> (label as children, glyph as endContent); filter is <IconButton>. Every hand-rolled state deleted. Filter trigger measures 28×28 (>24px, WCAG 2.5.8). Accent stays reserved for sorted/filtered.

@cixzhang — color on the target no longer repaints the label. A derivedVarRegistry entry with replaces: true makes defineTheme emit --_table-sort-glyph-color and no color, so the heading keeps inheriting astryx-table-header-cell:

theme table-sort-button { color }	glyph	column label
before	moves	moves too ← the bug
after	moves	stays with the <th>

Guard now builds its probe with the real defineTheme + generateThemeCSS instead of writing the private var. Probe theme regenerated (274 targets). #5784 merged and arrives via main.

Why keep the targets rather than delegate. parseStyleKey emits one compound selector on one element — no combinators. So delegation offers a themer .astryx-button (every button in the app) or nothing; .astryx-table-header-cell .astryx-button isn't expressible. That's INV5's "distinct public visual contract" met by construction, and toggle-button is the in-repo precedent.

Your call, and the one thing I'd flag: this amends the Table.spec.md anatomy map you approved on 30 Aug — Sort control moves from inherits to its own target, and filter parts are added. If you'd rather keep the map as approved, the alternative is dropping both targets and delegating to Button globally; say the word and I'll do that instead.

Negative controls: remove the derived-var entry → the label follows the colour again (your exact bug); revert Sort control to inherits → check-knowledge fails with "no anatomy entry with a target disposition".

Gates: 8957 tests pass, build/typecheck/typecheck:docs/check:repo clean, lint:strict 0 errors (86 warnings, none in touched files). pr-rtl is the repo-wide soft-red.

Still blocked on me: the guard's CI step needs .github/workflows/ci.yml, which my token can't push (no workflow scope) — block is in the thread for a maintainer.

[cixzhang]

Thanks — the Button/IconButton move fixes the interaction drift. Two things still block this head.

The new affordance guard is only a package script; no workflow invokes it, so the target/color contract can regress without CI noticing. Please wire it into the existing Storybook Chromium job.

Table.spec.md also still says “documentation only,” eight targets, and no runtime/target change while this head records ten targets and changes the controls. Please align its compatibility, ownership, verification, and decision sections.

[Reviewed by Robohands]

[freddymeta]

Both asks addressed. The spec half is pushed as 9c0da1e1b; the CI half is a patch below, because my account cannot push it.

Table.spec.md

It still read "additive documentation only", "eight current targets" in three places, and a Decision log of "None ... without introducing a component-local visual or runtime change".

• Compatibility — additive targets plus the intentional visual changes, itemised: Button/IconButton's hover and pressed overlay and scale(0.98) press, the filter trigger's shared focus ring and sm square box, and the removal of the resting opacity: 0.35. Records that nothing is removed or renamed, and that the column heading is measured unchanged against its <th>.
• Ownership, FR4, verification map — eight to ten, and FR4 now names table-sort-button and table-filter-button with the states they reflect.
• Decision log — four decisions replacing "None": own targets rather than delegating to button (DEC-1), the derived-var routing that colours the glyph without the label (DEC-2), building on Button (DEC-3), the opacity removal as a contrast fix rather than a preference (DEC-4).

Editing an authority: current record means spec-owner-approval will now ask for your approval where the previous head did not. That is the gate working; I have not touched the approval metadata.

The guard in CI — needs you

I cannot push it:

! [remote rejected] refusing to allow a Personal Access Token to create or
  update workflow `.github/workflows/ci.yml` without `workflow` scope

Your 08-26 review already framed this as a maintainer action, so here is the patch, verified locally rather than sketched.

It goes in pr-a11y, beside modal-close-visibility and theme-var-reachability, for the reason they are there — the built Storybook is already in hand. One wiring detail that is easy to miss: the guard builds its probe stylesheet with the real defineTheme out of packages/core/dist, and pr-a11y builds nothing, so without the extra download it dies on its own "Built core not found" check. build-storybook already publishes dists-<hash> with core's dist for pr-visual, so this reuses it — no second build.

The patch

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index a2ad46c07..cc7a75bfd 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -525,6 +525,17 @@ jobs:
           name: storybook-${{ needs.build-storybook.outputs.short_hash }}
           path: apps/storybook/dist
 
+      # For the affordance-colour guard below, which generates its probe
+      # stylesheet with the real `defineTheme` out of core's dist rather than
+      # hand-writing one. build-storybook has already built it; downloading
+      # keeps this job on an artifact instead of a second full build, the same
+      # trade pr-visual makes.
+      - name: Download built themes and core
+        uses: actions/download-artifact@v8
+        with:
+          name: dists-${{ needs.build-storybook.outputs.short_hash }}
+          path: packages
+
       - name: Install Playwright
         run: pnpm install --frozen-lockfile && npx playwright install chromium
 
@@ -569,6 +580,18 @@ jobs:
       - name: Run theme var reachability guard
         run: node .github/scripts/theme-var-reachability.js --storybook-dir apps/storybook/dist
 
+      # A theme target naming a button that holds one icon promises that a
+      # colour set on it reaches the glyph. An `Icon` that names its own colour
+      # writes `color` on the glyph and beats what the button inherits down, so
+      # the target resolves, the rule emits, and nothing moves — which is how
+      # Table's sort and filter affordances shipped. jsdom resolves no cascade,
+      # so a unit test can assert the class is on the button and the rule is in
+      # the CSS text while the glyph still paints its own colour; Chromium is
+      # where the promise is checkable. Sibling to the two guards above, and
+      # here for the same reason: this job already has the built Storybook.
+      - name: Run affordance colour reach guard
+        run: node .github/scripts/affordance-color-reach.js --storybook-dir apps/storybook/dist
+
   # The layer order Astryx promises is Astryx < theme < product, and which
   # declaration paints is a cascade fact no jsdom test can settle: the build has
   # been wrong in both directions inside one week — unsplit, so StyleX outranked

Verified, not asserted

Ran it exactly as the new step invokes it, against a real Storybook build:

$ node .github/scripts/affordance-color-reach.js --storybook-dir apps/storybook/dist
✓ Table sort affordance — rest rgb(115,115,115) on rgb(255,255,255) — 4.74:1;
  themed → painted rgb(1,2,3); hovered → painted rgb(1,2,3);
  data-direction="ascending" → painted rgb(1,2,3);
  the header label held at rgb(82,82,82)
✓ Table filter affordance — rest 4.74:1; themed → painted; hovered → painted
All 2 affordance targets colour their own glyph.                      exit 0

The 4.74:1 and the held label are the two things the last two rounds doubted — measured here.

Negative control, since this guard's integrity has been questioned twice. Restored the original defect (color="secondary" on the sort Icon), rebuilt core and Storybook, reran:

✗ ... did not reach the PAINTED glyph — stroke/fill stayed rgb(115,115,115)
✗ ... the themed colour is lost on hover
✗ ... the themed colour is lost once the control reflects a state
Failing: 1 affordance target(s).                                      exit 1

So it fails on the defect it exists for, in all three of rest, hover and state, and it is not passing by writing the private var directly.

Also: check-knowledge.mjs aligned; themingTargets.test.ts + derivedVarRegistry.test.ts 518 tests pass.

[cixzhang]

Thanks — the spec now matches the implementation. This exact head still defines guard:affordance-color, but no workflow invokes it, so the target/color contract can regress without CI noticing. The prepared pr-a11y wiring still needs to land before merge.

[Reviewed by Robohands]

[freddymeta]

Follow-up on f45cefe7e — CI is green again (21 pass; only the review gates pending).

The branch was DIRTY, which is why CI had stopped running entirely. targets.json conflicted with main, where Typeahead and Tokenizer D2 targets have since landed. Merged, kept both sides, rebuilt, and re-ran both guards on the merged tree:

affordance-color-reach:  All 2 affordance targets colour their own glyph.   exit 0
rtl-audit --filter Table: COV 1 measured / 0 gap                            exit 0

pr-rtl was also failing on the previous head, unrelated to your review — COV: 0 measured / 1 gap, since the audit requires every touched component to be measured or carry a written reason and Table was neither. Added a D2 target over core-table--column-alignment: column order genuinely mirrors, cell centres LTR 305 < 1024, RTL 795 > 76.

One trap worth passing on, because the selector looks obviously right and is not: tbody tr:first-child td:first-child matches Storybook’s own hidden docs prop-table first (inside .sb-preparing-docs, width 0), so boundingBox() returns null and the dimension reports “prev/next not found” — an N-A that still counts as a gap rather than erroring. Scoping to .astryx-table fixes it.

For the record, pr-rtl carries continue-on-error: true, so it was never blocking this PR — I fixed it because it was red, not because it gated.

Still needs you for the two things I cannot do: the ci.yml patch in my previous comment (no workflow scope on my token), and spec-owner-approval, which the Table.spec.md edit now requires.