Repository navigation
Conversation
vjeux
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
October 24, 2026 07:31
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
Contributor
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
github-actions
Bot
requested review from
cvkxx,
ernestt,
kentonquatman and
rubyycheung
October 24, 2026 07:32
This was referenced Oct 24, 2026
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
This needs an owner decision before it can be approved. The reorder values and model rest only on draft spec:AST-094 FR1–FR3, which this branch adds without owner approval, and draft design:ordered-collection-reordering DR3 asks for surrounding rows to stay put until commit, while offsetsFor slides them during the drag. An engineering owner can settle it by commenting /approve-spec <full head SHA> on this pull request to approve FR1–FR3 for these values (the author can do this as an engineering owner), or by naming the record that should govern instead. Whatever that decision, these still need changes:
- Land or retarget the stacked predecessors (#8045, #8062, #8106) so this pull request carries only the reorder commit; today it carries 18,141 authored lines.
proposedSlotproposes a new slot at pickup for any row more than twice the height of the row below it (a 100 pt row over 44 pt rows proposes slot 2 at rest), so count only the boundaries crossed since pickup and add a tall-row test.- The
lintjob runs out of memory on this head but passes on the base.
[Automated review]
| } | ||
| const top = index < dragged ? rows[index].top : rows[index].top - shift; | ||
| const boundary = top + rows[index].height + gap / 2; | ||
| if (boundary < center) { |
Contributor
There was a problem hiding this comment.
proposedSlot counts every boundary above the centre, so a row more than twice the height of the row below it starts past that row's boundary. For rows [44, 100, 44, 44], row 1 at its resting centre (94) returns slot 2, and offsetsFor slides row 2 up 100 pt before the finger moves. Counting only the boundaries crossed since pickup (start from dragged and step each time the centre passes the next boundary in the direction it moved) keeps every measured 44 pt case. A tall-row case in reorderModel.test.ts would pin it.
4 tasks done
Contributor
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsItem (@astryxdesign/core) · View in Storybook
TreeList (@astryxdesign/core) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included.
|
| Component | JavaScript | CSS |
|---|---|---|
| core/CheckboxList | +7.7KB |
+22B |
| core/CheckboxListItem | +3.8KB |
+611B |
| core/ContextMenu | +12.1KB |
+626B |
| core/List | +7.9KB |
+40B |
| core/ListItem | +3.8KB |
+611B |
| core/Markdown | +12.0KB |
+620B |
| core/PowerSearch | +11.8KB |
+355B |
| core/Table | +12.0KB |
+618B |
| core/TableCell | +12.0KB |
+620B |
| core/TableHeaderCell | +12.1KB |
+611B |
| core/TreeList | +11.0KB |
+384B |
Bundle Size Summary
Gzip figures measure each package's entry file, not the code an import pulls in.
| Package | Entry (ESM) | Entry gzip before | Entry gzip after | Delta |
|---|---|---|---|---|
| @astryxdesign/core | 4.9KB | 1.2KB | 1.2KB | 0B |
| Package stylesheet | Gzip before | Gzip after | Delta |
|---|---|---|---|
| @astryxdesign/core | 35.4KB | 36.0KB | +648B |
Accessibility Audit
Status: 2 accessibility violation(s) found — 2 serious.
Radio group pattern - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 3/11 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
RadioList - 1 issue(s)
- 🟠 serious: Ensure the contrast between foreground and background colors meets WCAG 2 AA minimum contrast ratio thresholds
- Rule:
color-contrast· Affects 3/14 stories · Learn more - WCAG: 1.4.3 (Level AA)
- Rule:
Visual Regression
55 of 504 shot(s) changed.
A change here is a question, not a failure: check whether the after is the
picture you intended. Record that review on the PR. Baseline maintenance is an
explicit dispatch of CI; this report never rewrites the baseline or adds a release gate.
| component | story | theme | mode | pixels |
|---|---|---|---|---|
| Selector | Indicator Space Bottom Sheet Narrow End RTL | neutral | light | 753,933 |
| Selector | Indicator Space Bottom Sheet Wide Start | neutral | light | 740,072 |
| RichTextEditor | Markdown parity: side by side | probe | dark | 558,840 |
| RichTextEditor | Markdown parity: side by side | probe | light | 540,756 |
| Chat | Full AI Chat | probe | light | 230,823 |
| Chat | Full AI Chat | probe | dark | 229,805 |
| AppShell | With Mobile Nav | probe | light | 217,010 |
| AppShell | With Mobile Nav | probe | dark | 216,960 |
| Collapsible | Dividers | probe | dark | 66,445 |
| Collapsible | Dividers | probe | light | 66,445 |
| AppShell | Playground | neutral | dark | 65,198 |
| AppShell | Playground | neutral | light | 63,788 |
| Chat | Default | neutral | dark | 36,304 |
| Chat | Default | neutral | light | 35,767 |
| Chat | Empty State | probe | light | 23,531 |
| Chat | Empty State | probe | dark | 22,622 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | dark | 15,378 |
| ChatAutoScroll | Scroll Behavior Comparison | neutral | light | 14,107 |
| Chat | With Attachments | probe | light | 13,406 |
| Chat | With Attachments | probe | dark | 11,232 |
and 35 more.
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report
Contributor
Author
|
On the predecessor stack: this pull request is stacked, as its first line says, and only its last commit (a0df534, 7 files, +606 −2) is its own. It rebases onto main as each predecessor lands, and the diff then shrinks to that commit. On |
Contributor
|
Posted by Cindy's agent on her behalf. Parts of this PR re-implement behavior that already exists, or is being specified, as a shared part of core. Converging keeps one implementation per behavior:
This is about where the behavior lives, not about the PR's intent. |
Contributor
Author
|
Agreed: one implementation.
This pull request's own commit (the reorder values and model) reads no pointer, writing direction or motion preference. The List reorder that uses it keys its motion on the resolver's touch presentation ( |
vjeux
force-pushed
the
uikit/c13-reorder-values
branch
from
October 25, 2026 01:16
a0df534 to
bf440cf
Compare
Contributor
Author
|
Pushed: bf440cf, on #8106 at 4d54dbc, which carries all three.
|
vjeux
force-pushed
the
uikit/c13-reorder-values
branch
from
October 25, 2026 01:46
bf440cf to
21aab26
Compare
Contributor
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: the reorder values and model rest only on draft spec:AST-094 FR1–FR3, which this branch adds without owner approval, while draft design:ordered-collection-reordering DR3 asks for surrounding rows to stay put until commit and AST-094 FR4 says touch motion is not themable, which the new reorder.slide.spring role contradicts. An engineering owner can settle it by commenting /approve-spec <full head SHA> on this pull request to approve AST-094 for these values (the author can do this as an engineering owner), or by naming the record that should govern them instead; that approval reads the record against the ## Writing rubric results the description does not yet carry. Whatever that decision, these still need changes:
- Land or retarget the stacked predecessors so this pull request carries only the reorder commit; it now carries 39,053 authored lines.
- The carried
platformadaptation condition inthemeAdaptations.tscontradicts the closedwhenvocabulary ofspec:AST-012FR2. uikit.reordernow reaches@astryxdesign/core/motion/experimentalwithout@experimentalor an[experimental]changeset; drop the barrel re-export or declare it.
[Automated review]
| * is everything else. Pair `ios` with `pointer: 'coarse'` for a touch | ||
| * profile, such as touch motion tuned per platform. | ||
| */ | ||
| platform?: 'ios' | 'other'; |
Contributor
There was a problem hiding this comment.
platform widens the adaptation when vocabulary that spec:AST-012 FR2 closes: "A when object may contain width, height, pointer, contrast, and motion". Drop it here, or have an engineering owner amend FR2 before it ships.
| } from './gestures'; | ||
| export {list} from './list'; | ||
| export {navigation} from './navigation'; | ||
| export {reorder} from './reorder'; |
Contributor
There was a problem hiding this comment.
This re-export puts reorder on the experimental @astryxdesign/core/motion/experimental subpath as uikit.reorder (motion/experimental.ts:71), but reorder carries no @experimental and .changeset/reorder-values.md is a [chore] that says it is not exported (spec:AST-017 FR21–FR22). registerTables.ts and reorderModel.ts import ./reorder by path, so dropping this line keeps the table internal; otherwise tag it @experimental and make the changeset [experimental].
Comment on lines
+14
to
+20
| * UIKit's rules (iOS 26.2, measured): the proposed slot changes when the | ||
| * dragged row's centre crosses a row boundary of the layout WITHOUT the | ||
| * dragged row, with no hysteresis; the rows between the dragged row's place | ||
| * and its slot each move by the dragged row's height; the drop goes to the | ||
| * current slot, from rest, on a spring whose duration scales with the | ||
| * remaining distance; autoscroll moves a whole number of points per tick, | ||
| * more the deeper the dragged row's edge is past the scroll view's edge. |
Contributor
There was a problem hiding this comment.
This still states the measured rule as a crossing of "the layout WITHOUT the dragged row". proposedSlot (lines 102–108) follows that only while the row below is at least as tall, or the row above at most as tall, as the dragged row; otherwise it clamps to the neighbour's near edge, and no measurement covers those cases (a 44 pt row under an 88 pt row now changes places after 66 pt, where this rule gives 22 pt). Stating the implemented rule here and marking the clamp cases unmeasured would keep the header honest.
Comment on lines
+171
to
+182
| '::after': { | ||
| content: { | ||
| default: null, | ||
| '[data-astryx-travelling]:not([aria-selected="true"],[aria-current="true"])': | ||
| '""', | ||
| }, | ||
| position: 'absolute', | ||
| inset: 0, | ||
| zIndex: -2, | ||
| borderRadius: 'inherit', | ||
| pointerEvents: 'none', | ||
| backgroundColor: colorVars['--color-background-surface'], |
Contributor
There was a problem hiding this comment.
These ::after properties apply to every row, and the caller's xstyle is merged after them, so an ::after decoration passed through xstyle inherits position: absolute, inset: 0, z-index: -2 and the surface background unless it sets each one. .changeset/list-row-press-compositor.md tells callers to move row decoration to ::after. Scoping these properties to [data-astryx-travelling], or pointing the changeset at a child element for ListItem, avoids the trap.
vjeux
force-pushed
the
uikit/c13-reorder-values
branch
from
October 25, 2026 02:40
21aab26 to
96480fc
Compare
…never needed From review on meta#8077: the build-css press-paint check filtered out a `--_press-paint: transparent` declaration, with a note, that the row layer never writes (only the overlay declares the press paint), and interactionOverlay.cell.test.ts defined a `hostRules` helper no case uses. Both go; the stylesheet check is as on the line below again.
…ers a row with an end control From review on meta#8085: - `isTouchClick` and its comment sat between `Item`'s docblock and `export function Item`, so the docblock attached to the helper and `Item` lost its description in editors and Storybook. The helper now sits above the docblock. - component:List FR10's verification names a row with an end control. A new List story puts a compact row whose end content is a switch between two interactive rows, and the browser spec checks that the interactive rows grow only to half the gap to the switch, that the browser hit-tests the switch at its center, and that a real tap toggles it with no tap reaching the rows beside it. With the hit area's neighbour clamp removed, the case fails (a band of 5.5 px where half the gap is 2 px).
…meta#8890 and meta#8360's current head) into the tree expansion change # Conflicts: # packages/core/src/motion/uikit/list.ts
…ast its divider From review on meta#8085, two ways a tap in an interactive row's band missed the row: - The band is the row's ::after, and the rows after it are positioned with no z-index, so a later row that takes no taps (a static row, such as the switch row in the "With end controls" story) painted and hit-tested over the band. The layer now has z-index: 1 on itself; its even-odd clip still leaves the row's own box to the row's content, and the row still makes no stacking context. - The layer is offset from the row's padding box, while the insets are measured from its border box, so below a divided row the band was the divider's width short. A host that draws a border now declares it (--_hit-border-{top,right,bottom,left}), and a divided ListItem declares its divider's, so the band starts at the divider's outer edge. Above everything at z-index auto, the band still takes no tap from a control near the list: the hit area's neighbour clamp measures every interactive element on the page, not only rows. The "With end controls" story gains a button in its header and one 4 px after the list, and a browser case hit-tests both; with the clamp removed it fails (a 6 px band where half the gap is 2 px). Red first: the browser spec now reads the band past the border box and hit-tests points inside Network's band. On the previous CSS, the end-control case read a 1 px band where half the gap is 2 px and a compact divided row's band read -1 px; with z-index removed, a point 0.5 px below Network hit the Wi-Fi row.
…nd the list bounds what rises From review on meta#8085 and meta#8106: - A ListItem with swipeActions is a stacking context at rest (Item isolates and translates its swipe root), so its touch band's z-index ordered only inside it and a following static row still took taps in the band. Under a finger, a swipeable interactive row now raises itself (z-index: 1 on the row; with a mouse nothing changes). The "With end controls" story's Network row is now swipeable, and the browser case checks that the row isolates and that taps in its band are its own. - The list's ul now isolates (isolation: isolate), so what rises inside it (a row's band, a swipeable row, and later a crossing row) orders above the rows after it but stays below a page-level sticky header at the same z-index (spec:AST-027 DEC-1, DEC-2). Red first: with Network swipeable, a point 0.5 px below it hit the Wi-Fi row; the unit cases for the list's isolation and the swipeable row's raise failed on the previous CSS.
From review on meta#8085: ListItem raised every interactive row with a swipeActions prop, while Item makes a row a stacking context (serves its swipe actions) only when a side has an entry, and not for a disabled row, a link root, a role that forbids interactive descendants, or a row whose press goes to a control inside it (interactiveRef). Rows with empty swipe sides or a delegate control were raised for nothing. Item's condition is now one function, servesSwipeActions, which Item and ListItem both use. Red first: in List.test.tsx, a row with empty swipe sides and a row with a delegate control both carried the raise on the change before this one.
From review on meta#8106: a swipeable interactive row's raise (meta#8085) and the travelling raise both set the row's zIndex, and StyleX keeps one per element. The swipe raise came later, so under a finger every swipeable row sat at 1, the moved row included, and the moved row (first in the document) slid beneath the rows it crossed; with a mouse the swipe raise's default dropped the travelling raise altogether. The swipe raise now carries the travelling raise above its own (2), with a mouse as under a finger. The swipe story gains a "Move the last row first" button, and a browser case freezes that reorder halfway and hit-tests where the moved row overlaps a row it crosses, under a finger and with a mouse. On the change before this one it hit Inbox; it now hits Archive.
…e rows it crosses From review on meta#8106: the travelling raise was z-index 1, the same as a swipeable row's raise under a finger, so a moved row without swipe actions (first in the document) slid beneath the swipeable rows it crossed. The travelling raise is now 2, above the swipe raise. The swipe story gains a row without swipe actions (Notes) and two move buttons, "Move Archive first" (a swipeable row) and "Move Notes first". A new browser case freezes Notes' move halfway and hit-tests where it overlaps a row it crosses, under a finger and with a mouse: on the change before this one it hit Inbox under a finger; it now hits Notes. The swipeable-row case moves Archive with its own button.
ListItem declares --_hit-border-bottom (the divider's width, 0 on the last row) for the touch hit layer, and List.doc.mjs did not list it, so derivedVarRegistry.test.ts failed: "List has undocumented CSS vars in source: --_hit-border-bottom". It is now in List.doc.mjs theming.vars[] as a private var, and in VARS_WITHOUT_DERIVED_MAPPING with its reason: it is the ::after layer's geometry, which follows --border-width, not a property a theme author sets (as with --_thumbnail-hit-inset and --_input-clear-hit-inset). Red first: "List: all source vars are in doc file" failed on the change before this one; it passes now (122 of 122).
…head Main's merges of meta#8045 and meta#8046 (the frame-evidence harness and the coarse press) conflict with the older foundation heads this branch carries, in the interaction harness files. meta#8106's current head carries that main and the foundation it now rests on, so taking it leaves this branch clean against main with nothing resolved by hand. A clean merge.
Contributor
There was a problem hiding this comment.
This needs an engineering owner's decision before it can be approved: proposedSlot keeps UIKit's crossing rule, which departs from spec:AST-103 FR1 (a drag reports the slot whose centre is nearest the dragged item's centre) wherever neighbouring rows differ in height, so past the 88 pt row of the measured table it moves to slot 6 at a centre of 308 pt where FR1 does at 286 pt, and the measured 306.8 pt sample lands on different slots. An engineering owner settles it by updating FR1 so a list without positions() follows UIKit's rule and recording that by commenting /approve-spec <full head SHA> on this pull request (the author may do so as an engineering owner), or by keeping FR1, in which case proposedSlot and its replay test conform to it. Whatever is decided, the diff still carries #8106's current head, 35,537 authored lines against this pull request's own 510, so land #8106 first and this pull request shrinks to the seven reorder files.
[Automated review]
vjeux
marked this pull request as draft
October 29, 2026 22:31
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Nov 3, 2026
…affd96): meta#8139's base Wave 2: F2's meta#8336 (9affd96, main-clean and lint-clean) under meta#8138 with the list stack's re-fold (meta#8106 d37a9d4). One conflict, in pressFeedback.ts's header: its @output line keeps the list row's press profile (CELL_PRESS_PROFILE, cellPressableProps), which the list stack adds and F2's line does not touch; the code merged on its own.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Nov 3, 2026
9affd96 and meta#8076's guard) under the List reorder Wave 2: the base re-folded onto main. Two resolutions: - utils/hitArea.ts and its test take the base's copy (meta#8635 at bdab881, through the list stack's meta#8085), which observes nothing under a fine pointer and listens for presses once, on the document. - ListItem.tsx keeps both sides: the list stack's shared touch hit area for an interactive row (useTouchHitArea, its fixed touch row heights gone) and this PR's trailing reorder handle, its row style and the handle-less row's name.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Nov 3, 2026
…eeping its velocity spec:AST-094 FR4: every spring a touch presentation plays is a motion role, and FR6 keeps a moving element's velocity. A drag cancelled (or a drop refused) while its row still glided from an earlier commit sent that row home on settleSpring, the reorder table's distance-scaled literal, from rest. It now goes home on springs.settle from where it is shown, at the speed it has, as every other row already did. The touch layer's slide (touchMotion.slide, slideBase), which nothing called since the rows stay still during a drag, goes with the reorder table's slide value (meta#8138); so does the test that pinned the slide's spring as an internal constant. Failing-first: a jsdom test commits a key move, picks the row up again while it glides and cancels the drag: home ran on a 0.085 s literal from rest; now on springs.settle (0.5 s, bounce 0) keeping its velocity.
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Nov 3, 2026
vjeux
added a commit
to vjeux/astryx
that referenced
this pull request
Nov 3, 2026
…66faa7f) under the List reorder The model proposes drop positions and moves no row, and only fixed rows that end a list bound a drag. Three conflicts, all this PR's vocabulary (its slot is the drop position): reorderModel.ts, its test and reorder.ts take meta#8138's new text with dropPosition for proposedSlot, and the headers name this PR's readers (listReorderEngine.tsx, ListReorderTouch.ts).
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #8106, and carries its current head. Review only this PR's change: vjeux/astryx@781c381...26a8bca (only merges since: main, then #8106's current head).
Why
A reorderable list on a phone should feel like the iOS one people know: the same pickup delay, autoscroll speed and drop rules. Those numbers must come from measuring iOS, not guessing.
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. The List reorder (#8139) needs its values and rules first. This PR adds them, unused until then:
motion/uikit/reorder.tsholds values measured on iOS 26.2. A still finger picks a row up after 150 ms, so a flick scrolls instead. Autoscroll moves 2 to 10 points every 20 ms, depending on how deep the finger is into the edge.List/reorderModel.tsholds the rules: where a dragged row lands, which rows it may pass (only pinned rows at the ends stop it), and keeping it inside the list.MEASUREMENTS.mdlists the three values that are still guesses.Options and tradeoffs
spec:AST-103FR1's slot whose centre is nearest the dragged centre. They agree for rows of one height and differ past a row of another (past the 88 pt row: 308 pt against 275 to 286 pt).Why this choice
iOS 26.2 shows no lift, so neither does this.
design:ordered-collection-reorderingchose still rows, so UIKit's measured slide stays in the file header only. The 8 pt inset ships as a guess, marked inMEASUREMENTS.md, rather than holding the reorder back.Owner decision: the model follows UIKit's measured landing rule, which the replay test pins. I propose that FR1's nearest-centre rule cover
positions()and attached containers, with a list withoutpositions()following UIKit's rule. The alternative is conforming the model to FR1 for lists too, which gives up iOS's measured landing points past rows of different heights.How to check it
Nothing is visible yet; #8139 uses these values. I ran
reorderModel.test.ts, and it passes: it replays a drag measured on iOS, a 44 pt row past an 88 pt row and back, and all 8 landing positions match. It also checks that a row at rest never moves and that a pinned row in the middle doesn't stop a drag; that one failed before its fix.Evidence
The model's tests.
Stack (merge in this order): the motion foundation and the list stack through #8106 → this → the List reorder.