Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
e4d7ed7 to
2c3fd8d
Compare
There was a problem hiding this comment.
Approval waits on an owner decision. TransferList's move to core List's reorder model (owner decisions 1–4 in the description) and the draft AST-094 have no approved spec, so an engineering owner needs to provide or identify the governing records and record the decision by commenting /approve-spec <full head SHA> with this pull request's current head. The author may do this if they are an engineering owner. Whatever the decision, these changes are needed:
- Land #8139 and #8126 first, or rebase once they merge, so this pull request carries only its 1,023-line TransferList commit instead of 32,385 lines.
- In
reorderSelected, keep values that have no option in their own slots: today Space, Home, Escape on Owner with['name', 'retired', 'owner', 'status']reports['name', 'owner', 'retired', 'status']. - With
renderOption, name each reorder handle after its option rather than "Reorder item N". - The description says the keys keep their behavior, but arrow keys now move a row without a pick-up, so please include that in the owner decision.
[Automated review]
| // Next to the row it lands on, in the full value (which may hold | ||
| // values without an option). | ||
| const nextValue = currentValueRef.current.filter(item => item !== moved); | ||
| const at = nextValue.indexOf(anchor); | ||
| nextValue.splice(toIndex > fromIndex ? at + 1 : at, 0, moved); | ||
| commit(nextValue); |
There was a problem hiding this comment.
Escape replays the inverse move through onReorder(index, lifted.origin), but re-inserting the moved value next to its anchor lets entries with no option drift: with ['name', 'retired', 'owner', 'status'], Space, Home, Escape on Owner ends at ['name', 'owner', 'retired', 'status'] while List announces "back to position 2 of 3", and ArrowDown then ArrowUp on Name ends at ['retired', 'name', 'owner', 'status']. Writing the moved visible order back into the visible values' own indices keeps hidden entries in their slots and makes every move reversible.
| return ( | ||
| <ListItem | ||
| key={option.value} | ||
| label={renderOption?.(option) ?? option.label} |
There was a problem hiding this comment.
With renderOption this label is a node, so ListReorderHandle names every handle "Reorder item N" by position (ListReorderHandle.tsx:197-211), while the row's other action still reads "Remove Name"; the old grip was always "Reorder {option.label}". Giving the handle the option's text, and asserting Reorder Name in the renderOption test, would keep the handles distinguishable for screen-reader and voice-control users.
| useIsomorphicLayoutEffect(() => { | ||
| const element = listRef.current; | ||
| if (element == null) { | ||
| return; | ||
| } | ||
| const rows = rowsOf(element); | ||
| const places = new WeakMap<HTMLElement, number>(); | ||
| for (const row of rows) { | ||
| // Inside the list: content above it changing size moves no row. | ||
| const base = row.offsetParent === element ? 0 : element.offsetTop; | ||
| places.set(row, row.offsetTop - base); | ||
| } | ||
| const before = placesRef.current; | ||
| placesRef.current = places; | ||
| const height = element.offsetHeight; | ||
| const grew = height - heightRef.current; | ||
| heightRef.current = height; | ||
| // The first commit, motion off, or a reorder session that owns the rows: | ||
| // remember where rows are, move nothing. | ||
| if ( | ||
| before == null || |
There was a problem hiding this comment.
This layout Effect reads every row's offsetTop and the list's offsetHeight after every commit before it checks animates, so every List, including those without hasAnimatedChanges, forces a layout on each commit. Returning before the reads when enabled is false (and clearing the remembered places) limits the cost to lists that opt in.
2c3fd8d to
d0c7163
Compare
There was a problem hiding this comment.
Approval waits on an owner decision: TransferList's move to core List's reorder model (owner decisions 1–4 in the description, plus arrow keys that now move a row without a Space pick-up) and the draft AST-094 have no approved spec. An engineering owner needs to provide or identify the governing records and record the decision by commenting /approve-spec <full head SHA> with this pull request's current head; the author may do so if they are an engineering owner. Whatever the decision, these changes are needed:
- Land #8139 and #8126 first, or rebase once they merge, so this pull request carries only its 1,023-line TransferList commit instead of 32,445 lines.
- In
reorderSelected, keep values that have no option in their own slots: Space, Home, Escape on Owner with['name', 'retired', 'owner', 'status']reports['name', 'owner', 'retired', 'status']. - Keep the selected rows' Remove buttons in one column and centred: beside the 44 px handle, movable rows grow to 44 px and Remove moves 36 px inward and 4 px up (visible in the Locked story).
- With
renderOption, name each reorder handle after its option rather than "Reorder item N".
[Automated review]
| <ListItem | ||
| key={option.value} | ||
| label={renderOption?.(option) ?? option.label} | ||
| endContent={optionActions(option, side, index)} |
There was a problem hiding this comment.
The List handle that follows this end content is at least 44 × 44 (ListReorderHandle.tsx:56-64), so in this compact list a movable row grows to 44 px while fixed rows and every available row stay 36 px, and its Remove button sits 36 px further from the edge and 4 px above the row's centre. In the Locked story the Remove column zigzags between Owner/Last updated and Name/Status. Reserving the handle's slot on rows without one, and letting the handle fit a compact row instead of growing it, would keep one Remove column and one row height.
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
d0c7163 to
03b352e
Compare
There was a problem hiding this comment.
Approval waits on an owner decision: TransferList's move to core List's reorder model (owner decisions 1–4 in the description, plus arrow keys that now move a row without a Space pick-up) and the draft AST-094 have no approved spec, so an engineering owner needs to provide or identify the governing records and record the decision by commenting /approve-spec <full head SHA> on this pull request, which the author may do if they are an engineering owner. Whatever the decision, these changes are needed:
- Land #8139 first, or rebase once it merges, so this pull request carries only its 1,128-line TransferList commit instead of 32,359 lines.
- In
reorderSelected, keep values that have no option in their own slots: Space, Home, Escape on Owner with['name', 'retired', 'owner', 'status']reports['name', 'owner', 'retired', 'status']. - Keep the selected rows' Remove buttons in one column beside the 44 px handle, and name each handle after its option under
renderOptioninstead of "Reorder item N". - Add the List drag browser spec that
ListReorder.test.tsxanduseListReorder.tscite, or correct those references.
[Automated review]
| * nothing, and a disabled row's handle stays focusable but inert | ||
| * @position Tests for List/useListReorder.ts and List/ListReorderHandle.tsx; | ||
| * the touch and mouse drags are proven in a real browser by | ||
| * ListReorder.interaction.browser.spec.ts |
There was a problem hiding this comment.
This names ListReorder.interaction.browser.spec.ts as the proof of the touch and mouse drags, but no such spec exists in this pull request (useListReorder.ts:43-44 and TransferList.test.tsx:6 point to it too). The mouse drag that replaced TransferList's pointer tests has no test; adding the spec, or correcting the three references, would keep the coverage claim true.
|
On the predecessor stack: this pull request is stacked, as its first line says, and only its last commit (03b352e, 4 files, +156 −972) is its own. It rebases onto main as each predecessor lands, and the diff then shrinks to that commit. |
03b352e to
1a2c33c
Compare
1a2c33c to
60cfda0
Compare
There was a problem hiding this comment.
Changes requested: TransferList's move onto createReorderable follows spec:AST-103 and the current reordering design, and the issues from the last review (the fixed rows' Remove column, the drag guidance and the drag test) are fixed. What remains is scope: the compare still carries about 41,000 lines from the open predecessor stack (#8337, #8562 and the pull requests under them) beside TransferList's 2,123, so please land those first, or rebase onto main as each one merges, so that review and CI cover only TransferList's change.
[Automated review]
There was a problem hiding this comment.
Changes requested: TransferList's move onto createReorderable follows spec:AST-103 and the current reordering design, and the issues raised earlier in TransferList's own files are fixed. Two things remain, both from the stacked predecessors this branch carries:
- Land #8337, #8562 and the pull requests under them first, or rebase onto
mainas each one merges, so this pull request carries only TransferList's 2,122 changed lines instead of 41,791. scripts/touch-support-bundle.test.mjs("is absent from the root entry") fails on this head and passes onmain, because the rootcreateReorderableexport now brings the gesture host into Core's root entry; please resolve it in the predecessors before this lands.
[Automated review]
| @@ -172,6 +172,9 @@ export * from './Outline'; | |||
| // Overflow list | |||
| export * from './OverflowList'; | |||
|
|
|||
| // Reorder (a collection's `reorderable` prop) | |||
| export * from './reorderable'; | |||
There was a problem hiding this comment.
With this export, Core's root entry carries the reorder engine and with it 14 gesture modules (GestureHost.ts, GestureArena.ts and more), so scripts/touch-support-bundle.test.mjs › "is absent from the root entry" fails at this head while it passes on main. Built alone, @astryxdesign/core/reorderable carries 15 gesture modules; TransferList's and List's entries carry none. Keeping the engine out of the root entry, or changing the test with an owner's agreement, would let the node tests pass.
There was a problem hiding this comment.
Changes requested: TransferList's move onto createReorderable follows spec:AST-103 and the current reordering design, and no defect turned up in TransferList's own files. Two things remain, both from the stacked predecessors this branch carries:
- Land #8337, #8562 and the pull requests under them first, or rebase onto
mainas each one merges, so this pull request carries only TransferList's 2,122 changed lines instead of 42,120. scripts/touch-support-bundle.test.mjs("is absent from the root entry") fails on this head and passes onmain, because the rootcreateReorderableexport brings the gesture host into Core's root entry; please resolve it in #8139 before this lands.
[Automated review]
| @@ -172,6 +172,9 @@ export * from './Outline'; | |||
| // Overflow list | |||
| export * from './OverflowList'; | |||
|
|
|||
| // Reorder (a collection's `reorderable` prop) | |||
| export * from './reorderable'; | |||
There was a problem hiding this comment.
This root export brings createReorderable's engine, and with it 14 gesture modules (GestureHost.ts, GestureArena.ts, LongPressGestureRecognizer.ts among them), into Core's root entry, so scripts/touch-support-bundle.test.mjs › "is absent from the root entry" fails on this head and passes on main. Please settle it in #8139, by keeping the engine out of the root entry or by changing the check with an owner's agreement; the description points to the open #8671 for that.
|
Posted by Cindy's agent on her behalf: closed and reopened only to restart this PR's CI on the current workflow after tonight's queue-wide cancel; nothing in the PR changed. |
|
Thanks, and sorry for the late reply. These are all in: |
There was a problem hiding this comment.
Changes requested. TransferList's own switch to createReorderable matches spec:AST-103 and the direction given on this pull request, but the branch as issued can't land.
Blocking:
- The compare carries 42,120 authored lines, 40,012 of them from the predecessor pull requests (#8337, #8139, #8138 and those below) and the closed #8562, and no longer merges with
main; land those first or rebase ontomainas each merges, so this pull request carries only TransferList's 2,122-line increment. scripts/touch-support-bundle.test.mjsfails at this head ("is absent from the root entry") because the rootcreateReorderableexport pulls the gesture host into Core's root entry; fix it in #8139 againstmain's current version of that check.- Desktop behavior change needs an owner decision: with default props, TransferList no longer lets a mouse drag selected rows to reorder (the old
isReorderabledefaulted totrue); gate it to touch input or cite the decision that covers it in the description. - The carried List reorder engine gives a pen on a desktop (fine primary pointer) no drag at all (
listReorderEngine.tsx:877,:1907), while currentspec:AST-103FR3 says such a pen drags without a hold; fix it in #8139.
The Stack (merge in this order) line differs across the listed pull requests, so this pull request is reviewed as its own change. Two non-blocking comment suggestions are inline.
[Automated review]
| * the move. Paused while a search filters the panel. Without it, the | ||
| * selected options keep their order. | ||
| */ | ||
| reorderable?: Reorderable<T>; |
There was a problem hiding this comment.
Desktop behavior change needs an owner decision: with default props a mouse can no longer drag selected rows to reorder (the old isReorderable defaulted to true). Gate it to touch input, or cite the decision that covers default-off reordering in the description.
| } | ||
| }, | ||
| [beginReorder, finishReorder], | ||
| // The selected panel's rows, in order, as List reports a move among them. |
There was a problem hiding this comment.
Non-blocking (comment style): this line describes nothing at isOrdering. Suggest deleting it and keeping only the reason:
// Reordering is off while a search filters the panel. A hidden row could
// otherwise move past the shown rows.| // The owner's object, adapted so List's moves among the shown rows reach | ||
| // the owner in the value's terms; its engine is the owner's, so nothing | ||
| // here loads the reorder (spec:AST-103 FR2), and the owner's `start(key)` | ||
| // finds this list (FR4a). |
There was a problem hiding this comment.
Non-blocking (comment style): one 40-word sentence with three topics. Suggest:
// The engine is the owner's, so this adapter loads no reorder code
// (spec:AST-103 FR2). The owner's `start(key)` finds this list (FR4a).|
/approve-spec 5cb0176 |
There was a problem hiding this comment.
Changes requested. TransferList's own switch to createReorderable follows spec:AST-103 and the direction given on this pull request, but the branch as issued can't land yet.
Blocking:
- The compare carries 12,026 authored lines, 9,899 of them from the open #8106, #8139 and #8337, whose
Stack:lines differ, so land those first (or rebase ontomainas each merges) and this pull request carries only TransferList's 2,133-line increment. - Desktop behavior change needs an owner decision: with default props, TransferList no longer lets a mouse drag selected rows to reorder (the old
isReorderabledefaulted totrue), so gate the change to touch input or cite the decision that covers it in the description. - The List reorder engine gives a pen on a desktop (fine primary pointer) no drag at all (
listReorderEngine.tsx:887), while currentspec:AST-103FR3 says such a pen drags without a hold; fix it in #8139 with a browser test for a pen on a fine and on a coarse primary pointer. spec:AST-103FR2 asks for the measured cost to be recorded in the pull request, and the description records none for this head, so add List's and TransferList's import size with and withoutreorderable.
One non-blocking comment suggestion is inline.
[Automated review]
| return () => {}; | ||
| } | ||
| const onPointerDown = (event: PointerEvent) => { | ||
| if (event.pointerType !== 'mouse' || event.button !== 0) { |
There was a problem hiding this comment.
spec:AST-103 FR3 says "with a mouse, or a pen where the primary pointer is fine, a drag from the handle starts without a hold". This returns for every pointer but a mouse, and the touch hold attaches only under the touch presentation, so a pen on a desktop gets no drag. Accept pen here when the primary pointer is fine, and add a browser test for a pen on a fine and on a coarse primary pointer (FR3's verification row).
| } | ||
| }, | ||
| [announce, isReorderable, setReorderSession, t], | ||
| // The selected panel's rows, in order, as List reports a move among them. |
There was a problem hiding this comment.
Non-blocking (INV29): this first sentence describes nothing in isOrdering. Suggest keeping only the reason: "Reordering is off while a search filters the panel. A hidden row could otherwise move past the shown rows."
|
/approve-spec fddf9c6 |
A List that takes the handle useAnimatedLayout returns animates its row changes on one shared motion, for every pointer. An inserted row enters, a removed row collapses in place, and a row that crosses rows moving the other way rides above them. The rows after a change, and the content after the list, glide to their new places. Rows match across renders by their itemKey; a row without one changes at once. The row marks the update module writes (item key, leaving, travelling) live in their own private-attribute owner modules. A list under the handle isolates for every pointer, so a sliding row stays below a page-level sticky header. The List and ListItem records now say the row updates and itemKey are implemented, and their verified_by lists the files that verify them.
There was a problem hiding this comment.
Changes requested. TransferList's own switch to createReorderable follows spec:AST-103 and the direction given on this pull request, and the desktop pen drag and the measured bundle cost are now in, but the branch as issued can't land yet.
Blocking:
- The compare carries 12,292 authored lines, 10,166 of them from the open #8106, #8139 and #8337, and the stack declaration does not hold because #8106 has no
Stack:line and #8163's line differs, so add the identicalStack: #8106 → #8139 → #8337 → #8143 → #8163line to both (or land the predecessors first) so this pull request carries only TransferList's 2,132-line increment. - Desktop behavior change needs an owner decision: with default props, TransferList no longer lets a mouse drag selected rows to reorder (the old
isReorderabledefaulted totrue,TransferList.tsx:135), so gate the change to touch input or cite the decision that covers it in the description.
One non-blocking comment suggestion is inline.
[Automated review]
| * strings and presentation current, and attaches the list's listeners. The | ||
| * touch hold attaches only where the primary pointer is coarse. It also |
There was a problem hiding this comment.
Non-blocking (comment): this, and the file header at lines 52-53, still say the hold attaches only where the primary pointer is coarse, but attachTouch(isTouch) below now attaches it under every primary pointer. Suggested wording: "It attaches the list's listeners. The touch hold takes a finger under every primary pointer, and a pen where the primary pointer is coarse."
|
/approve-spec 82a3af6 |
There was a problem hiding this comment.
Changes requested. TransferList's own switch to createReorderable follows spec:AST-103 and the direction given on this pull request, but the branch as issued can't land yet.
Blocking:
- The compare carries 12,378 authored lines, 10,220 of them from the open #8106, #8139 and #8337, and the stack declaration does not hold because #8163's
Stack:line differs and its branch does not contain this head, so put the identicalStack: #8106 → #8139 → #8337 → #8143 → #8163line in #8163 and rebase it onto this pull request (or land the predecessors first) so this pull request carries only TransferList's 2,164-line increment. - Desktop behavior change needs an owner decision: with default props, TransferList no longer lets a mouse drag selected rows to reorder (the old
isReorderabledefaulted totrue,TransferList.tsx:135), so gate the change to touch input or cite the decision that covers it in the description. - The doc page's new "Migrating from isReorderable to reorderable" example calls
createReorderable({...})with no type argument, so itsReorderable<string | number>fails to type-check against TransferList'sreorderable(TransferList.doc.mjs:594); writecreateReorderable<string>(...)there and in the description's migration snippet. - Miss: the "Independent transfer and reorder constraints" example has the same type error against TransferListSelector (
TransferList.doc.mjs:556), so type its call the same way.
[Automated review]
| * search filters the panel. Without it, the selected options keep their | ||
| * order. | ||
| */ | ||
| reorderable?: Reorderable<T>; |
There was a problem hiding this comment.
Desktop behavior change needs an owner decision: with default props, a mouse can no longer drag selected rows to reorder (the old isReorderable defaulted to true). Gate it to touch input, or cite the decision that covers it in the description.
There was a problem hiding this comment.
The description now cites the owner basis: "Owner rule, Nov 4 2026 (Cindy): anything Lab-only gets an OK."
| // AFTER: reordering is opt-in through the object createReorderable makes. | ||
| const reorderable = useMemo( | ||
| () => | ||
| createReorderable({ |
There was a problem hiding this comment.
Without a type argument this object is Reorderable<string | number>, which TransferList's reorderable?: Reorderable<T> refuses (TS2322). Every story and test writes it typed:
createReorderable<string>({There was a problem hiding this comment.
Typed as createReorderable<string> in 164d15d, and in the description's snippet.
| code: `<TransferListSelector | ||
| code: `const reorderable = useMemo( | ||
| () => | ||
| createReorderable({ |
There was a problem hiding this comment.
Miss: the same untyped call here gives TransferListSelector a Reorderable<string | number>, which its reorderable?: Reorderable<T> refuses (TS2322). Type it as createReorderable<string>({...}).
|
/approve-spec 164d15d |
Owner decision, Nov 6 2026 (Cindy): the layout glide takes --duration-medium. The shared glide played over --duration-medium-min; it now plays over --duration-medium: 300 ms in the Neutral theme and 410 ms with no theme, instead of 225 ms and 310 ms, both scaled by tempo. Collapsible, TreeList, Table's row expansion, AppShell's content area and List share it. AST-105 FR4 names the token, and its DEC-3 records the decision.
Settling a swiped row read the next row's transform after writing the row before it, and read every row's place after all the writes. One pass now reads each near row's translation and place, and then the settles are written. A settled row's place is its box less its own translation.
createReorderable({onReorder, isReorderable}) makes a collection's
reorder behavior, and List takes it as `reorderable`. Each ListItem
with an itemKey gets a reorder handle at its end, and each committed
move calls onReorder({key, to}) once.
Under a finger, a 150 ms hold on a handle picks its row up, and a
flick on it still scrolls. The rows stay still while a dimmed source,
a preview and an insertion line show the move. On release they glide
on the shared settle role. A pen holds the same way where the primary
pointer is coarse. A mouse, or a pen where the primary pointer is
fine, drags a handle at once. A click, Space or Enter turns on reorder
mode: the insertion line marks the row's place, the arrow keys move
the row and announce its place, and Escape ends the mode.
The reorder's machinery ships with the object, so a List without
`reorderable` loads none of it. The UIKit table's reorder values and
the drop model come in here with their first user. With an
animatedLayout handle, a drag's commits stay the reorder's own. The
CLI lists createReorderable with the hooks.
While a reorderable List row is in reorder mode, a hint anchored below its handle shows Move up, Move down and Done until the mode ends. Each move does what its key does, with the same announcement, and leaves the focus on the handle. Done ends the mode as Escape does, keeping the moves. So a pointer or a finger can reorder by taps alone, as WCAG 2.5.7 asks. A screen reader that moves its focus into the hint keeps the mode, and the focus goes back to the handle. The hint shares the keyboard hint's layer: hooks/anchoredHint.tsx holds the anchored hint both render, and useKeyboardHint renders its hint through it unchanged. The hint joins the shared dismissal stack, so an Escape in it ends the mode and leaves a Dialog below it open.
A row named by its place, as ListInput names its records, gets a new name after each move. The handle now records its name apart from its registration, so a new name leaves the row in reorder mode. A move is announced by the names the rows have when it lands. Comments now say that a drag ends reorder mode with its announcement, and test comments match the hold and blur behavior.
createReorderable's discovery rule lists a create*.doc.mjs documented as a hook. createAnimator, createPanGesture and createDragToSnapPoints are documented as listed beside their own hooks, and each names that hook in relatedHooks. Such a factory now stays out of the hook list, so hook --list matches main for them and adds only createReorderable. The CLI's search goldens match main's again.
Five comments in the reorder's engine, touch glide, drop model and UIKit table each held one sentence over 25 words, and the reorder mode spec's header ran two sentences together. Each is now split, with the same meaning.
The selected panel is a core List with `reorderable`, its rows
ListItems keyed by value, each ending with core's reorder handle. The
panel's own pointer drag, preview, grip and keyboard session go. A
finger holds the handle a moment before it drags, so a flick scrolls
and a pinch zooms. Reorder mode moves a row from the keyboard or the
hint's buttons.
TransferList and TransferListSelector take the owner's object as
`reorderable`: `isReorderable(value)` says which values move, and each
move goes to `onReorder({key, to})`. A staged selector aims the moves at
its draft, so Cancel drops them. An adapter in core's internal reorder
entry runs the owner's engine with those moves, and the owner's
`start(key)` still finds the panel's list.
Main's TransferList isDisabled was written for TransferList's own reorder grip, which this change replaces with core List's handle. It now works on the createReorderable path: - Every option is transfer-disabled without its own message, and the search field is disabled but focusable. - Reordering pauses as it does while a search filters the panel. No handle shows, so none stays pressed. - A handle with focus unmounts, so focus moves to its row's Remove button, which stays focusable. - A move or a cancel that ends after the list became disabled changes nothing.
Three comments in TransferList and its reorder specs each held one sentence over 25 words. Each is now split, with the same meaning.
There was a problem hiding this comment.
Changes requested. TransferList's switch to createReorderable follows spec:AST-103, but one change at this head needs a fix before it can land.
- A disabled TransferList now removes every reorder handle and moves focus from a focused handle to its row's Remove button (
TransferList.tsx:410-430,:580-581), whilespec:AST-085FR1 keeps a disabled control in the tab order and FR4 lets a component move focus only to a destination its own record names; keep each handle as a focusable disabled control, which Core's handle already supports throughListItemisDisabled.
Turning reordering off for a TransferList with default props, which removes its default mouse drag, still waits on an engineering owner's decision. The earlier comments on row heights, the Remove column, handle names with renderOption and the internal reorder subpath are resolved at this head. This pull request lands only after #8106 and #8139 are approved and land.
[Automated review]
| // A disabled list shows no reorder handle. A focused handle unmounts, so | ||
| // focus goes to its row's Remove button, which stays focusable. The focus | ||
| // is read during render: by the time an effect runs, the handle is gone. | ||
| const wasDisabledRef = useRef(isDisabled); | ||
| const handoffRef = useRef<{element: Element; row: Element} | null>(null); | ||
| if (isDisabled && !wasDisabledRef.current) { | ||
| const active = rootRef.current?.ownerDocument.activeElement; | ||
| const row = active?.closest('[data-transfer-list-row]'); | ||
| handoffRef.current = | ||
| active != null && row != null ? {element: active, row} : null; | ||
| } | ||
| wasDisabledRef.current = isDisabled; | ||
| useLayoutEffect(() => { | ||
| const handoff = handoffRef.current; | ||
| handoffRef.current = null; | ||
| if (handoff != null && !handoff.element.isConnected) { | ||
| handoff.row | ||
| .querySelector<HTMLElement>('[data-transfer-list-action="selected"]') | ||
| ?.focus(); | ||
| } | ||
| }, [isDisabled]); |
There was a problem hiding this comment.
A disabled list now unmounts every reorder handle (isOrdering is false at :580-581) and this moves a focused handle's focus to the row's Remove, which also opens its "Owner cannot be moved" tooltip. spec:AST-085 FR1 keeps a disabled control in the tab order ("a tab stop stays a tab stop"), and FR4 lets a component move focus only to a destination its own record names; main keeps each grip focusable and aria-disabled. Core's handle already has that state (ListReorderHandle.tsx:382-393, through ListItem isDisabled), so keeping the reorderable and disabling the rows would conform and remove this render-phase read and Effect.
Stack: #8106 → #8139 → #8143 → #8163
This merges after #8139 and #8106. Review only this PR's change: vjeux/astryx@ee13b42...164d15d
Why
TransferList lets people order the items they picked, such as columns to show. Its grip grabbed every touch at once, so swiping over grips on a phone didn't scroll the panel.
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. TransferList had its own drag code, duplicating core
List's reorder (#8139). The selected panel becomes a coreListwithreorderable: hold to drag, flick to scroll, pinch to zoom, and keyboard reorder mode. Following Cindy's direction (her commits are here),reorderabletakes thecreateReorderableobject, withisReorderable(value)andonReorder({key, to}). TransferListSelector takes it too: in staged mode a move edits the draft, and Cancel drops it.Before:
<TransferList isReorderable />. After:<TransferList reorderable={createReorderable({onReorder})} />.This changes desktop behavior too: reordering is opt-in through the owner's object, so a TransferList without
reorderableno longer reorders with a mouse either (the oldisReorderabledefaulted to true). Owner rule, Nov 4 2026 (Cindy): anything Lab-only gets an OK. Her commits are here, and she asked that TransferList's doc page and this description carry the migration below.To migrate:
TransferList's doc page carries the same example, "Migrating from isReorderable to reorderable".
Core's internal entry
@astryxdesign/core/internal/reordercomes in here with its first user. It adapts the owner's object, so the panel runs the owner's engine with its own moves, and the owner'sstart(key)still finds the panel's list. Like Core's other internal entries, it is undocumented and outside the public API.Options and tradeoffs
Why this choice
Reusing core's reorder means one implementation, and TransferList gets the hold, the keyboard path and the announcements. The cost is the API change; Lab is private, so there's no changeset. An option that can't move shows no grip (
spec:AST-103FR1). Reordering pauses while a search filters the panel, where a position is ambiguous. Cindy'sinternal/reorder.tstakes string or number keys to match her #8592, named here for her review.A disabled TransferList
Main's TransferList
isDisabled(#10192) was written for TransferList's own grip, which this change replaces with core List's handle. The governing decision is #10192: its behavior carries over, with no new shared API and both of its tests. The port is its own commit on this change:The change's own commit carries #8143's files as they were, so on its own it sets #10192's TransferList lines aside until the port restores them; the pull request squash-merges. A last commit splits three comment sentences over 25 words.
How to check it
In Storybook
Lab/TransferList:Gates at the stack's top, 01fcc86 (#8163), which carries this head:
check:knowledge.Bundle cost
Measured with the PR size report's build options on the built packages (the eager chunks, minified, gzip). List and ListItem import 70.0 KB, and 114.8 KB with
createReorderable. TransferList imports 87.8 KB, and 132.7 KB withcreateReorderable(45.0 KB more, 141.3 KB minified). Withoutreorderable, TransferList's import holds no reorder engine, gesture or animator module: the engine comes with the owner's object.Evidence
Before: the grip drags at once, and on iOS 26.2: after, hold, drag and drop, and a pinch zooms.
Pull request size
1,042 lines added and 1,090 removed in 15 files: 231 added of source and 679 of tests. TransferList's own drag code goes.