Repository navigation
docs(useTableSortableState): add hook docs and a multi-sort orders example block - #9911
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
community
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| TableBody | 3.4 kB (0) · 3.3 kB CSS (0) | — | ✓ · ✓ · ✗ |
| TableFooter | 3.4 kB (0) · 3.3 kB CSS (0) | — | ✓ · ✓ · ✗ |
| TableHeader | 3.4 kB (0) · 3.3 kB CSS (0) | — | ✓ · ✓ · ✗ |
5 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| TableRow | 6.5 kB (0) · 4.0 kB CSS (0) | — | ✓ · ✓ · ✗ |
| Table | 83.5 kB (0) · 16.1 kB CSS (0) | — | ✓ · ✓ · — |
| TableCell | 75.1 kB (0) · 15.9 kB CSS (0) | — | ✓ · ✓ · ✓ |
| TableHeaderCell | 74.7 kB (0) · 15.6 kB CSS (0) | — | ✓ · ✓ · ✓ |
| TableSelectionToolbar | 54.4 kB (0) · 11.8 kB CSS (0) | — | ✓ · ✓ · ✓ |
Package size:
- @astryxdesign/cli 3.34 MB packed (+720 B); no new dependencies
- @astryxdesign/core 6.31 MB packed (+1.3 kB); no new dependencies
API changes: none in this pull request
Committed API report behind this build: cli 8 rows, theme-butter 1 row, theme-chocolate 1 row, theme-gothic 1 row, theme-matcha 1 row, theme-neutral 1 row, theme-stone 1 row, theme-y2k 1 row; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | View full report
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for this: the hook doc and the multi-sort orders block close #9480 well, and the block gets all of its sorting from the hook. One change is needed before approval:
- The
comparatorsdescription says null, undefined and NaN sort last (useTableSortableState.doc.mjslines 58 and 142), but in a descending sort the hook puts them first, as its own test "sorts NaN to the start in descending order, like null" asserts, so please change both strings to "last in ascending order and first in descending".
While you are there (optional): with an empty sort state, sortedData is data itself, not a copy. CI has not run on this head yet. It waits for a maintainer to add the ci:run label.
[Automated review]
| name: 'config.comparators', | ||
| type: 'Partial<Record<TSortKey, (a: T, b: T) => number>>', | ||
| description: | ||
| 'Ascending comparators per sort key; the hook flips the result for descending. Keys without one compare numbers numerically, compare other values as strings with a numeric collator in the active InternationalizationProvider locale, and put null, undefined and NaN last.', |
There was a problem hiding this comment.
In a descending sort the hook negates the default comparator's result too, so null, undefined and NaN come first, not last. useTableSortableState.test.tsx asserts this in "sorts NaN to the start in descending order, like null". Suggested ending: ...and put null, undefined and NaN last in ascending order and first in descending.
| 'config.sort': 'controlled sort state; pair with onSortChange.', | ||
| 'config.onSortChange': 'called with next sort state; required with sort.', | ||
| 'config.comparators': | ||
| 'ascending comparators per key (flipped for descending). Else numbers numeric, others numeric-collator strings in provider locale; null/undefined/NaN last.', |
There was a problem hiding this comment.
Same here: in a descending sort, missing values come first. Suggested ending: null/undefined/NaN last ascending, first descending.
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, so a maintainer needs to add the ci:run label for it to run.
Thanks for the quick fix. The comparators row now says missing values sort last in ascending order and first in descending, which matches the hook, and the hook doc and multi-sort orders block meet the hook-example rule (INV28): the block gets all of its sorting from the hook.
- Optional: with an empty sort state the hook returns
dataitself, so theconfig.dataandsortedDatarows could say "a sorted copy of data, or data itself while unsorted".
[Automated review]
| name: 'sortedData', | ||
| type: 'T[]', | ||
| description: | ||
| 'A sorted copy of data, memoized on data and the sort state. Ties keep their order in data.', |
There was a problem hiding this comment.
Optional: with an empty sort state the hook returns data itself (useTableSortableState.tsx lines 212-213), not a copy. Consider "A sorted copy of data, or data itself while unsorted, …" here and in the config.data row and the two dense lines.
cixzhang
added
the
ci:run
github-actions
Bot
removed
the
needs:code-review
Reader impact
useTableSortableStatehas no HookDoc, so it has no docsite page andastryx hook useTableSortableStatefinds nothing. After this change it has a page with its config, its four return values and when to useuseTableSortableinstead, and an example. An orders Table has no sort code of its own: the hook owns the sort state, returns the sorted rows and handsuseTableSortableits config. Clicking Customer and then Shift-clicking Total sorts by customer, then by total within each customer, and a line above the table reads the current sort state from the hook. It pairs with theuseTableSortablepage, which shows the controlled path.astryx template useTableSortableStateHookUsage <path>copies it.Source of truth
packages/core/src/Table/plugins/sortable/useTableSortableState.tsx(config, default comparator, controlled and uncontrolled state,applySort) anduseTableSortable.tsxfor the header click behavior the passed-through options control.architecture:knowledge-contractsINV28.Correction
packages/core/src/Table/plugins/sortable/useTableSortableState.doc.mjsnext to the source, as the issue asks (aHookDoc,docs+docsDense,group: 'Table',importPath: '@astryxdesign/core/Table').blocks/components/Table/useTableSortableStateHookUsage(exampleFor: 'useTableSortableState'). It callsuseTableSortableState<Order>({data: orders, isMultiSortEnabled: true}), passessortConfigtouseTableSortable, renderssortedData, and printssort. Seven orders across three customers, so the Total tiebreaker changes the order within each customer.useTableSortableStateinpackages/cli/api/component/__fixtures__/resolutions/modules.txt(it now opens its own doc instead of falling back toTable/Table.doc.mjs), as test: main is green again: Tokenizer's sheet-row test passes presentation, and the recorded module resolutions follow core #9851 did foruseLightbox.Evidence
Real Chromium (Playwright against the docsite dev server,
/components/useTableSortableState, 1280x900).aria-sortNo page errors.
Test (
packages/cli/api/template/use-table-sortable-state-hook-usage.test.tsx, 3 cases). It asserts the same three orders and sort lines. With the hook replaced by the unsorted data and a fixed empty sort, the two sorting cases fail.packages/cli/api/build/tree.test.mjspins the blocksastryx buildpicks for "analytics dashboard with a KPI row and a sortable table". A first description with "rows" and "Table" in it pushedTableStripedTableout of that list, so the block's description talks about orders instead. The pinned list is unchanged.Scope
internal/shadcn-registry/routes.lock.json, the block's sandbox route inapps/sandbox/scripts/next-export-routes.json, and the recorded resolution above.Testing
vitest run --project node packages/cli/foundation/discovery packages/cli/api/template packages/cli/assets/templates internal/shadcn-registry apps/sandbox/src/route-manifest.test.ts packages/cli/api/build packages/cli/test/experimental-isolation.test.mjs packages/cli/api/component packages/cli/api/docs packages/cli/api/hook: 1689 passed, 0 failed.vitest run --project ui packages/core/src/docPropLiterals.test.ts packages/core/src/docPropReferences.test.ts packages/core/src/Table/plugins/sortable: 92 passed.apps/docsite:pnpm generate, thenvitest run src/__tests__/data-extraction.test.ts src/__tests__/example-coverage.test.ts: 136 passed, 0 failed. The route lock check is clean afterUPDATE_SHADCN_ROUTE_LOCK=1added the two routes.pnpm lint:strict: clean (0 errors).pnpm -F @astryxdesign/cli typecheck: clean.node scripts/release/changeset-coverage.mjs pr --base upstream/main --head HEAD --released v0.7.7: passes. Changeset names@astryxdesign/coreand@astryxdesign/cli, patch,[docs]..github/scripts/check-public-text.mjsover this commit, title and description: no findings.pnpm testsuite.Notes for reviewers
useTableSortable,useTableSelectionStateand so on) are documented asComponentDocs withsubComponentOf: 'Table'inpackages/core/src/Table/. The issue asks for aHookDocnext to the source, with params and returns, so that is what this adds. If the Table family should stay on its own shape, the same content can move toTable/useTableSortableState.doc.mjsas asubComponentOfdoc.allowUnsortedStatecomment says@default true, and the plugin does fall back totrue(cfg.allowUnsortedState ?? true), so the new doc saystrue.useTableSortable.doc.mjslists that option's default asfalse, which looks stale. I left it alone here.Fixes #9480