Skip to content

fix(Table): align the sortable header button with its column - #5928

Merged
ernestt merged 1 commit into
mainfrom
core-table-sortable-align
Sep 2, 2026
Merged

fix(Table): align the sortable header button with its column#5928
ernestt merged 1 commit into
mainfrom
core-table-sortable-align

Conversation

@ernestt

@ernestt ernestt commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

What

A sortable column with align: 'end' rendered its header label hard against the start of the cell, sitting above right-aligned figures. Same for align: 'center'.

The cause is that align lands as textAlign on the <th>, and the sortable plugin renders a full-width flex button inside that cell. textAlign cannot position flex children, so the alignment stopped at the cell boundary and never reached the label.

This mirrors the column's alignment onto the button's main axis via justify-content. flex-end resolves against the flex direction rather than the physical box, so it keeps mirroring correctly under RTL.

Why it's split out

Found while building an invoice template (#5865), where every numeric column is both sortable and end-aligned. Pulled into its own PR so the template stays a pure template change and this can be reviewed as the consumer-visible core fix it is.

Scope

File Change
useTableSortable.tsx +19/−2 — two StyleX rules, applied from column.align
useTableSortable.test.tsx +72 — four tests
.changeset/ patch

Only align: 'end' and align: 'center' are affected. align: 'start' and unaligned columns produce byte-identical output, which the fourth test pins.

Test plan

useTableSortable.test.tsx — 44 pass, including four new cases asserting that end and center each produce a button style distinct from the default and from each other, and that start stays identical to unaligned.

Non-sortable columns never had the bug: without the plugin there is no button, and textAlign reaches the label directly.

A column's `align` lands as `textAlign` on the <th>, which cannot
position the contents of the full-width flex button the sortable plugin
renders inside it. So an `align: 'end'` numeric column got a header
label hugging the start while its figures sat right — the heavier the
column, the more obviously wrong the pairing looked.

Mirror the column's alignment onto the button's main axis. `flex-end`
resolves against the flex direction rather than the physical box, so it
keeps mirroring correctly under RTL.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated
astryx Ready Ready Preview Sep 2, 2026 4:54pm UTC

Request Review

@github-actions github-actions Bot added the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Sep 2, 2026
ernestt added a commit that referenced this pull request Sep 2, 2026
The core change and its changeset now live in #5928, leaving this a pure
template change with no consumer-visible surface of its own.

Until that lands, the sortable numeric headers here sit start-aligned
above their right-aligned figures. That is the pre-existing behaviour
rather than a regression, and it is the honest thing for the template to
show while the fix it depends on is still in review.

Co-authored-by: Cursor <cursoragent@cursor.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Sep 2, 2026

@cixzhang cixzhang left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks, this is good. The fix stays inside the existing Table contract, and the focused checks cover the affected alignment combinations.

[Reviewed by Robohands]

@github-actions github-actions Bot removed the needs:code-review High-risk change (new package/component/API) — needs human code review before merge label Sep 2, 2026
github-actions Bot added a commit that referenced this pull request Sep 2, 2026
@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

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: No visual change across 14 compared shot(s).


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

@ernestt
ernestt merged commit 42b2fdd into main Sep 2, 2026
27 checks passed
ernestt added a commit that referenced this pull request Sep 2, 2026
…ice (#5865)

* feat(templates): rebuild the searchable table as an interrogable invoice

Rebuilds `table-page` around a single uniform document — an agency website
redesign invoice — and takes it from a C to an A on the template rubric by
removing every raw HTML tag and all but two custom CSS declarations.

What the template now demonstrates:

- Data-mode sorting and per-column filter popovers over a plain `TextInput`
  search, all narrowing one array in one pass so the totals can never describe
  a different set of rows than the table shows.
- A `Banner` that fires on filter *presence*, warning that the derived totals
  are partial, with a single control that clears every surface at once.
- Per-row `useHoverCard` detail, anchored to the whole `<tr>` and left
  uncontrolled so the card survives the pointer travelling onto it and its
  Dispute button stays clickable.

Two structural fixes are the interesting part:

- **Sitting flush takes two fixes, not one.** `LayoutContent padding={0}` stops
  the container publishing `--container-padding-inline-*`, which is what the
  table was bleeding past; the padding moves to a `VStack`, which publishes
  nothing. That removes a wrapper `<div>` that previously zeroed the variables
  inline. Cell padding still resolves to `max(var(...), spacing-2)` — a hard
  floor — so a small `useFlushEdges` plugin closes the residual 8px. Those two
  declarations are the only custom CSS left, and the floor is the one real core
  gap remaining here.
- **Totals are not a table.** They were a second `<Table>` sharing a
  hand-written `<colgroup>`, which cost two raw tags and a border declaration
  per ruled cell. A right-aligned stack lands on the same vertical line because
  the figure box is the Amount column's width, and `Divider` draws the rules —
  which also fixes a latent bug, since the hand-rolled hairline had a hardcoded
  light-mode fallback that nearly vanished in dark mode.

Core: `useTableSortable` now mirrors a column's `align` onto its header
button's main axis. The button is a full-width flex container, so the
`textAlign` that `align` puts on the `<th>` cannot position its contents, and
an `align: 'end'` numeric column got a left-hugging header over right-aligned
figures. Covered by four new tests.

Rubric: 96/100 (A), up from 74 (C). 0 raw HTML elements, 2 custom CSS
declarations, full marks on icons, layout, images, and code quality.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(templates): drop type="search" from the invoice search input

`TextInputType` is 'text' | 'password' | 'email' — 'search' is not in the
union, so the CLI template typecheck and the docsite's Next build both failed
on it. The default 'text' is what the input wants anyway: the search affordance
is carried by the leading magnifying-glass icon and the placeholder, and
type="search" would have added a UA-drawn clear button that no other Astryx
input has.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(templates): bind the invoice row detail to focus, size its title as h3

The hover card anchors to the enclosing <tr>, which the effect makes
focusable only after handing the node to the hook. `focusTrigger: 'auto'`
resolves focusability at attach time, so on that first pass it saw a plain
<tr> and bound no focus listeners. It recovers on any later re-attach —
the cleanup drops aria-describedby but leaves tabIndex behind, so the row
reads as focusable the second time around — which means the keyboard path
works today only by way of a re-render the template never asked for. Name
the intent instead, matching dashboard-project-status.

Also size the card's title as an h3 while keeping it level 2 in the
outline; the page's only other heading is the invoice h1, so a plain h3
would jump h1 to h3 and read as a missing section.

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: changeset for the sortable header alignment fix

Co-authored-by: Cursor <cursoragent@cursor.com>

* chore: move the sortable header alignment fix to its own PR

The core change and its changeset now live in #5928, leaving this a pure
template change with no consumer-visible surface of its own.

Until that lands, the sortable numeric headers here sit start-aligned
above their right-aligned figures. That is the pre-existing behaviour
rather than a regression, and it is the honest thing for the template to
show while the fix it depends on is still in review.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(templates): describe the searchable table by shape, not by its fixture

The description led on the fixture — "billing metadata", and a synonym
list of Invoice, bill, statement, receipt — which is the case the rubric
names explicitly: sample data is interchangeable, so it cannot be the
thing that distinguishes a template. Anyone reaching for a searchable
table of orders, runs or assets wants exactly this page and matched
almost none of its vocabulary.

It also missed the words people actually type. `sortable` and
`filterable` do not stem to `sort` and `filter` — the stemmer strips
only `ing|ed|ies|es|s`, so the word boundary fails — and `column`,
`record` and `list` were absent entirely. Against an 18-term probe the
old string hit 8; the new one hits 18, with both the verb and the
-able form of each behaviour present.

Differentiates on data shape (flat, uniform, one homogeneous list) and
behaviour (search, filter, sort, totals that follow the filter) instead,
and says what the siblings do differently. No billing vocabulary
remains.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix(templates): let the searchable table masthead scroll with the content

With `height="fill"` the content area is the scroll container, so
anything in the Layout `header` slot is pinned. Scrolling this page left
the title and Download button parked over a sliced-off search box and
scope toggle — the controls disappeared under a bar that had nothing to
do with them.

A document's own title is not chrome. Title, actions and the metadata
block move into the content as one masthead and scroll away together.
The `header` slot stays right for what remains useful mid-scroll: an app
bar, a toolbar, a sticky action row.

`contentWidth` still caps the column, and still does it on the Layout
rather than on the table, so full-bleed dividers keep spanning the
window. The metadata block's `paddingBlock={4}` becomes
`paddingBlockEnd={4}` on the masthead, since the top half was separating
it from a header that is no longer above it.

Co-authored-by: Cursor <cursoragent@cursor.com>

* docs(templates): reframe the searchable-table guidance around search, filter and sort

The extending notes were organised around the invoice fixture and around
whatever had been hard to build, which is the author's order, not the
reader's. Someone opening this template wants a searchable table; the
invoice is what the rows happen to contain.

Reorganised under four headings — narrowing, sorting, saying what the
reader is looking at, and structure — with the search/filter/sort
guidance expanded from incidental notes into the bulk of the file:

- when each of the three narrowing surfaces is the right one, and the
  failure mode of shipping all three so two controls do the same job
- that typed filter controls come from the search config rather than
  the column, and what an enum field missing its enumValues degrades to
- narrow once into one array, and give every control one reset
- sort after filtering, not before
- sort the value, never the label — extended past currency to dates and
  enums, with the general rule stated: keep the comparable primitive on
  the row and format it in renderCell
- the two filtered states, and detecting on filter presence rather than
  row count

The mechanical constraints worth keeping — no footer in data mode, the
two-part flush fix, per-row detail without a rowComponent, tabular
figures — are kept but demoted below the guidance and stripped of
invoice specifics.

Also corrects the ARIA claim on the partial-totals banner:
`status="warning"` renders `role="alert"`, not `role="status"`. And
drops the stale note about the header rule spanning the window, since
there is no longer a slotted header.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions
github-actions Bot deleted the core-table-sortable-align branch September 3, 2026 06:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants