Repository navigation
feat(lab): ListInput reorders through core's createReorderable - #8163
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| ContextMenu | 108.4 kB (+61 B) · 17.4 kB CSS (0) | — | — · — · — |
| lab/TransferListSelector | 109.1 kB (+50 B) · 1.4 kB CSS (0) | — | — · — · — |
| lab/TransferList | 101.6 kB (+49 B) · 1.2 kB CSS (0) | — | — · — · — |
13 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Markdown | 187.4 kB (+48 B) · 20.6 kB CSS (0) | — | — · — · — |
| List | 18.1 kB (+43 B) · 790 B CSS (0) | — | ✓ · ✓ · — |
| Table | 124.7 kB (+41 B) · 18.3 kB CSS (0) | — | — · — · — |
| CheckboxList | 68.0 kB (+40 B) · 7.7 kB CSS (0) | — | — · — · — |
| TableHeaderCell | 109.2 kB (+36 B) · 17.7 kB CSS (0) | — | — · — · — |
| TableCell | 109.6 kB (+20 B) · 17.9 kB CSS (0) | — | — · — · — |
| PowerSearch | 286.6 kB (+6 B) · 24.1 kB CSS (0) | — | — · — · — |
| lab/Comments | 350.0 kB (+6 B) · 2.5 kB CSS (0) | — | — · — · — |
| DropdownMenu | 97.2 kB (0) · 17.6 kB CSS (0) | — | — · — · — |
| ListItem | 70.4 kB (0) · 11.4 kB CSS (0) | — | ✓ · ✓ · ✓ |
| MoreMenu | 97.5 kB (0) · 17.6 kB CSS (0) | — | — · — · — |
| lab/CommentThread | 343.1 kB (−15 B) · 2.4 kB CSS (0) | — | — · — · — |
| lab/ListInput · Storybook | 88.3 kB (−13.9 kB) · 922 B CSS (−763 B) | 1,083 (−678) | — · ✓ · — |
Package size: @astryxdesign/core 7.19 MB packed (+3.5 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 29 rows, core 34 rows, theme-butter 1 row, theme-y2k 1 row; run pnpm api:report.
Touched components checked; add ci:run-visual 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.
Approval waits on an engineering owner's decision on direction that no current spec covers: draft spec:AST-094 FR1–FR23 with DEC-1 and DEC-2, and the four ListInput decisions the description marks as pending owner review. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever that decision is, these still need changes:
- Land or retarget the stack so this diff is only the ListInput change; it now carries 34,464 authored lines, most of them from eight other pull requests.
- Add the real-browser test that
useListReorder.tsandListReorder.test.tsxpoint to (ListReorder.interaction.browser.spec.ts), covering the slide, autoscroll, drop and pointer cancel. - Remove the document touch listeners
ListReorderHandleinstalls once no handle needs them, and fix AST-094's references to aspec:AST-090that does not exist.
[Automated review]
c79ed6b to
79c4b3c
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on direction no current spec covers: draft spec:AST-094 FR1–FR23 with DEC-1 and DEC-2, and the four ListInput decisions the description marks as pending owner review. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever that decision is, these still need changes:
- Land or retarget the stack so this diff is only the ListInput change; it now carries 34,443 authored lines, most of them from other pull requests.
- Add the real-browser test that
useListReorder.tsandListReorder.test.tsxpoint to (ListReorder.interaction.browser.spec.ts), covering the slide, autoscroll, drop and cancel. - Remove the document touch listeners
ListReorderHandleinstalls once no handle needs them, and fix AST-094's references to aspec:AST-090that does not exist.
[Automated review]
|
On the predecessor stack: this pull request is stacked, as its first line says, and only its last commit (79c4b3c, 5 files, +359 −1,392) is its own. It rebases onto main as each predecessor lands, and the diff then shrinks to that commit. |
79c4b3c to
dd38d7d
Compare
dd38d7d to
969b566
Compare
969b566 to
9bbfef4
Compare
9bbfef4 to
e75eada
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on direction no current spec covers: draft spec:AST-094 FR1–FR23 with DEC-1 and DEC-2 (including whether FR4's untunable touch motion or the themable reorder.slide.spring stands) and the ListInput reorder decisions the description marks as pending owner review, now including the handle's moves menu. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision is, these still need changes:
- Land or retarget the stack so this diff is only the ListInput change; it now carries 48,303 authored lines, most of them from ten other open pull requests.
- The reorder handle announces itself as a menu button, but Enter and Space pick the record up and never open its moves menu; give that role its keyboard activation or drop
aria-haspopup. - Add a browser test for the moves menu's focus path and the drag autoscroll, and correct the theme template's
platformaxis, the stale sentence in thetouch-motion-theme-roleschangeset, and AST-094's references to aspec:AST-090that does not exist.
[Automated review]
e75eada to
148a984
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on direction no current spec covers: draft spec:AST-094 FR1–FR23 with DEC-1 and DEC-2 (including whether FR4's untunable touch motion or the themable reorder.slide.spring stands) and the ListInput reorder decisions the description marks as pending owner review, including the handle's new look and drag for mouse users. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Whatever that decision is, these still need changes:
- Land or retarget the stack so this diff is only the ListInput change; it now carries 48,192 authored lines, most of them from nine other open pull requests.
- The reorder handle announces itself as a menu button, but Enter and Space pick the record up and never open its moves menu; give that role its keyboard activation or drop
aria-haspopup. - Add browser evidence for the moves menu's focus path, the drag autoscroll and the mouse drag after the change, and mark or drop the new
@astryxdesign/core/motion/reducedMotion.stylexexport. - Correct the changesets that say no component uses the gesture and motion code yet, and AST-094's references to a
spec:AST-090that does not exist.
[Automated review]
Where the primary pointer is coarse, a reorder mode move forces one style pass for each row that glides for the first time: the shared animator reads the row's will-change then. The List record names this as an implementation gap.
|
/approve-spec 3a1c5e5 |
Main's meta#8215 rewrote the zoom section of motion/MEASUREMENTS.md, the last section before the reorder section this pull request appends. The resolution keeps main's zoom text and table, then the reorder section as it was.
The reorder section and the reorder constants' sources named their measurement batches by a label the repository never defines. They now give the lab, device and date, as the other measured sources do.
|
/approve-spec d858d64 |
There was a problem hiding this comment.
Approved pending CI for the whole stack: the build and test workflow has not run on this head yet.
#8139 — approve. The drop and reorder-mode move now read every row before they write, and CI passes on its head.
#8143 — approve. Its own change is unchanged and still looks right.
#8163 — approve. Its own change is unchanged and still looks right.
Whole stack — passes. Next: packages/core/src/motion/MEASUREMENTS.md (#8139) now conflicts with main, where the Lightbox change appended its own table, so merging main into #8139 and up the stack lets CI run. A pull request lands only when it and every pull request below it are approved.
Non-blocking: the INV29 comment at ListInput.tsx:593-596 can keep only its constraint, and #8143's and #8163's descriptions still name the heads they merged before.
[Automated review]
|
/approve-spec d3b8e98 |
There was a problem hiding this comment.
Approved for the whole stack: CI passes on this head.
#8139 — approve. Merging main resolved the MEASUREMENTS.md conflict, and the reorder measurements now cite their lab, device and date; CI passes on its head.
#8143 — approve. Its own change is unchanged and still looks right, and this head now contains it, so the stack holds again; CI passes on its head.
#8163 — approve. Its own change is unchanged and still looks right.
Whole stack — passes. A pull request lands only when it and every pull request below it are approved.
Non-blocking: the comment at ListInput.tsx:593-596 can keep only its constraint, and this description still names the heads it merged before.
[Automated review]
|
/approve-spec d3b8e98 |
1 similar comment
|
/approve-spec d3b8e98 |
Main's meta#10586 put List.doc.mjs in Prettier's format and changed edge's bleed text, beside the reorderable prop and anatomy entries this pull request adds. The resolution keeps main's file and adds this pull request's entries in that format.
The reorder handle and the reorder session each kept a flag of their own to warn once. Core's createDevWarnings now owns per-instance warn-once, as Dialog, Resizable and TabList use it, so both call it and the flags go. Each still warns once: a handle about a row with no name, a session about rows with no item key. The two warning tests now render twice, so a second warning fails them.
This pull request's Core changes ship to consumers: the internal reorder entry, and the reorder session's support for an object a Lab collection adapts. Lab is private, so the Changeset names @astryxdesign/core alone, as an internal chore.
|
/approve-spec 52f7a74 |
meta#8143's head now adds a Changeset for its Core changes. This takes it in a plain merge. This pull request's own changes stay as they were.
This pull request's Core changes ship to consumers: the reorder scope a Lab collection's list joins, its English message, and the internal reorder entry's support for it. Lab is private, so the Changeset names @astryxdesign/core alone, as an internal chore.
There was a problem hiding this comment.
Changes requested. ListInput's and TransferList's move onto Core's reorder engine looks right and its tests pass; two things block merging, and merging #8143's current head into this branch addresses both.
- Regression: the Lint check's changeset coverage fails, because this pull request changes Core's source, its English catalog (
@astryx.listInput.itemName) and itsexports(./internal/reorder) with no Changeset naming@astryxdesign/core; #8143's newest commit adds one, which could also name ListInput's catalog message andListReorderScope. - Regression: the stack declaration no longer holds, because #8143's head moved to that Changeset commit and this head does not contain it, so the 5,413 authored lines, 2,550 of them #8143's, count as one change; once this branch contains #8143's head, size is only a note.
- Non-blocking: the comment at
ListInput.tsx:593-596can keep only its constraint ("A disabled or loading list keeps its handles focusable but inert."). - Miss: non-blocking, the header of
internal/reorder.ts(lines 3-19) can drop the@outputprose that its functions' JSDoc repeats.
[Automated review]
|
/approve-spec d50b386 |
There was a problem hiding this comment.
Approved. Both changes the last review asked for are in: each pull request now carries a Changeset naming @astryxdesign/core, so the Lint check passes, and this branch contains #8143's current head, so the stack holds.
#8143
Approve. No blocking findings.
#8163
Approve. No blocking findings.
- Non-blocking: the comment at
ListInput.tsx:593-596can keep only its constraint ("A disabled or loading list keeps its handles focusable but inert."). - Non-blocking: the header of
internal/reorder.ts(lines 3-19) can drop the@outputprose that its functions' JSDoc repeats.
Whole stack
Pass: ListInput, TransferList and TransferListSelector share Core's one reorder engine through the internal entry, with nothing left between the two pull requests. Each pull request lands only when it and every pull request below it are approved.
[Automated review]
|
/approve-spec 9bb411b |
There was a problem hiding this comment.
Approved. The head only merges main, where #8143 has landed, and touches none of this pull request's files, so its own changes are the ones approved before; CI passes on this head.
#8143
Merged into main; its approval stands.
#8163
Approve. No blocking findings.
- Non-blocking: the comment at
ListInput.tsx:593-596can keep only its constraint ("A disabled or loading list keeps its handles focusable but inert."). - Non-blocking: the header of
internal/reorder.ts(lines 3-19) can drop the@outputprose that its functions' JSDoc repeats.
Whole stack
Pass: ListInput, TransferList and TransferListSelector share Core's one reorder engine through the one internal entry. Each pull request lands only when it and every pull request below it are approved.
[Automated review]
Review only this PR's change: vjeux/astryx@36b8f6e...9bb411b. Main's disabled Button change (#7906) edits the lines beside this PR's disabled-handle case in
ListInput.test.tsx: that case keeps main'saria-disabledassertion for Add and adds this PR's handle assertions.Why
ListInput lets people build an ordered list of records, such as guests. Its grip grabbed every touch at once, so swiping over grips on a phone didn't scroll.
What led to it
This is part of the UIKit-parity program, which makes Astryx's components behave like their iOS counterparts on touch screens. Moving Lab's reorders onto core's is part of it. ListInput had its own drag code, duplicating core
List's reorder. Following Cindy's direction (her 1272d6a is here), it takesreorderable, thecreateReorderableobject, keyed bygetItemKey. Each record ends with core's handle ("Reorder guest 2"), after Remove and in Remove's cell, so the handle is the one element the reorder adds to a record. The handle comes with the owner's object, so a ListInput withoutreorderableloads no reorder code. Each move goes toonReorder({key, to}), and disabled or loading lists keep their handles inert. The list takes the session's style, so its insertion line marks a drop, and a record's place in reorder mode. Lab reaches core's reorder session through@astryxdesign/core/internal/reorder, which gains the session for rows that are not ListItems.architecture:react-component-runtimeINV13 lists that session among its known public-API gaps (#8143's record change), asspec:AST-107FR7 asks. List and that session share the same two hooks, so the session's wiring lives in one place.Before:
<ListInput isReorderable onChange={...} />. After:<ListInput reorderable={createReorderable({onReorder})} getItemKey={...} />.Options and tradeoffs
Why this choice
Reusing core's reorder means one implementation, and ListInput gets the hold, the keyboard path, the hint and the announcements. The cost is the API change; Lab is private, so there's no changeset. The handle goes after Remove, where UIKit puts it. The design record drops the removed
reorderStyles.tspath and its "Implementation gap" paragraph, which this closes, for Cindy's review.The record columns
With
reorderable, the Add row reused the records'autocontrol track, which is empty there, so the Add button ran 56 px past the record fields. Each handle and its focus ring also ended 8 px past the list: the handle reaches into aListItemrow's inline inset, and a record has none. The control track is now fixed (Remove, the gap and the 28 px handle), and records set that inset to zero. So the fields column is 8 px narrower than on the previous head, and the handle stays inside the list.A record that stays put
A record that the owner's
isReorderablekeeps in place has no handle, so its control cell holds Remove alone. The cell centred it in the fixed control track, 16 px past every other record's Remove. The cell now starts its controls at the track's start, so every Remove sits in one column. A new fixture,A11y/ListInput › Fixed record, keeps its first record in place; it stays out of the visual matrix.How to check it
In Storybook
Lab/ListInput:This head, 9bb411b, merges main (36b8f6e), where #8143 squash-merged, with #8143's final head (1e21069) as the merge base, with no conflict, and leaves this pull request's own changes as they were. Lab is private, so the Changeset names
@astryxdesign/corealone, as an internal chore ([chore] Internal:). At this head: Core and Lab typecheck 0,check:knowledge,check:changesets, the Changeset coverage check against main, the comment-provenance check, the CLI search goldens, the Lab and Core reorder units (305), the TransferList and ListInput reorder specs (13), List's interaction specs in Chromium (94, 1 skipped), the full UI project (23,203 tests) and the node project (17,699 tests, no failures).Gates at the top of the stack. The full run is at 4fd0951, on main 1c8292c, whose tree differs from this head only in the comment-provenance baseline and one file's formatting; at this head lint, the baseline check, typecheck and the List and Item units ran again:
check:knowledge,check:changesets,check:repo,check:i18n-catalog,check:i18n-runtime,check:use-clientandcheck:package-boundaries: 0.pnpm install --frozen-lockfile: every Lab test (TransferList, TransferListSelector and ListInput included), core List, motion, Layer, Core's utils and internal entries, the keyboard module, Button andcreateReorderable, 5,510 of 5,510. The full UI project passes, 22,672 tests. The full node project passes 17,448 tests; its one failure, a CLI block-picker case (build tree› "login form with single sign-on buttons"), fails the same way on main (1c8292c).Earlier, at ac07610:
experimental-isolation, 44 of 44) pass: feat(core): createReorderable makes List rows reorderable #8139 counts its new hook doc in them.Writing rubric results
For
design:ordered-collection-reordering:Evidence
Before: the grip drags at once, and on iOS 26.2: after, hold, drag and drop, and loading: the grip is inert.
Pull request size
1,284 lines added and 1,587 removed in 16 files: 428 added of source and 679 of tests. ListInput's own drag code and the shared Lab reorder styles go.