Repository navigation
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📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsDateInput (@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/DateInput | +28.3KB |
+14B |
| core/DateTimeInput | +27.7KB |
+15B |
| core/PowerSearch | +28.0KB |
+14B |
| core/TimeInput | +27.1KB |
+30B |
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 | 34.6KB | 34.6KB | +21B |
Accessibility Audit
Status: No accessibility violations detected.
Visual Regression
55 of 502 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
![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 draft spec:AST-094 (FR1–FR23 and the amendments it proposes to current records) and on the wheel's new look (32 px pitch, magnified band, plain selected row); an engineering owner records it by commenting /approve-spec ece20fbd147eca75500fb8542994e3257c42ef7c, and the author may do so as an engineering owner. Whatever that decision, please fix the issues below and split the stack so this pull request carries only the Wheel commit, since it currently changes 28,785 authored lines.
- A wheel still coasting when another wheel commits stops between rows and never commits, because a new
optionsarray rerunsmotion.attach(), which stops the motion (Wheel.tsx:413-423,wheelMotion.ts:117-123). - With
presentation="bottom-sheet"and a mouse, dragging a wheel down now dismisses the sheet; the base guarded against this and its tests were removed. - Home and End pressed while a wheel is moving land on the wrong row (
Wheel.tsx:785-791).
[Automated review]
ece20fb to
ee9446d
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23 with DEC-1 and DEC-2, including the amendments it proposes to current records) and on the wheel's new look (the cylinder with a magnified band, the 32 px row pitch and the selected row without accent or semibold); any engineering owner, the author included when they are one, records it by commenting /approve-spec ee9446ddeeb92e94419d9276cf620e19605718dc on this pull request. Whatever that decision, these still need changes:
- A mouse drag on a wheel in the fine-pointer bottom sheet now dismisses the sheet instead of turning the wheel, and the tests that guarded it were deleted.
- Land the stacked predecessors separately so this pull request carries only the wheel commit, then fix Home and End pressed while the wheel coasts, the active row repaired in a layout effect, and the changeset and documentation references the review lists.
[Automated review]
ee9446d to
2def93c
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23 with DEC-1 and DEC-2, including the amendments it proposes to current records) and on three Wheel choices no spec states yet: the new look (cylinder with a magnified band, 32 px pitch, plain selected row), turns that ignore Reduce Motion, and the nearest-enabled rest for bounded wheels. Any engineering owner records that decision by commenting /approve-spec 2def93ccb659fd3f9d6d484b1256c633124e790f on this pull request, and the author may do so as an engineering owner. Whatever the decision, these need changes:
- A mouse drag on a wheel in the forced bottom sheet now dismisses the sheet instead of turning the wheel, and the tests that guarded it were deleted.
@astryxdesign/core/DateInput/tokens.stylexdrops the releasedwheelItemSizeandwheelEdgePaddingkeys in a patch; keep them and movewheelGeometryout of the public module.- Land the stacked predecessors separately so this pull request carries only the Wheel commit, then fix Home and End pressed during a coast, the active row repaired in a layout effect, and the changeset and documentation references noted inline.
[Automated review]
|
|
||
| // Keep the finger: inside a BottomSheet the sheet would otherwise read a | ||
| // downward drag here as swipe-to-dismiss (see useOwnScrollGesture). | ||
| useOwnScrollGesture(columnRef, 'all', {isEnabled: isActive}); |
There was a problem hiding this comment.
This covers touch only. The base also called usePointerDragScroll, which stopped a mouse pointerdown before BottomSheet's body drag could arm and capture the pointer; without it, a downward mouse drag on a wheel in a forced presentation="bottom-sheet" dismisses the sheet instead of turning the wheel. Please keep a press that starts on a wheel away from the sheet's drag again, and restore the deleted mouse-drag tests (DateInputTouch.test.tsx) against the gesture path.
There was a problem hiding this comment.
Resolved at adb185f. A mouse, a fine pointer, gets today's scrolling column, which keeps usePointerDragScroll; the six mouse-drag tests in DateInputTouch.test.tsx run unchanged under a fine pointer with the sheet asked for. Under a finger the column stops pointerdown from reaching the sheet (jsdom case "keeps a finger that lands on it from an enclosing sheet"; Chromium case "a downward drag on a wheel moves its rows and leaves the sheet where it is", where the sheet moved 9.3 px before). PickerWheel.test.tsx also has mouse cases for a mouse on a touch device: a primary-button drag turns the wheel and commits where it rests, a secondary-button drag does nothing.
| case 'Home': | ||
| event.preventDefault(); | ||
| moveBy(-activeIndex); | ||
| moveBy(-restIndex); | ||
| break; | ||
| case 'End': | ||
| event.preventDefault(); | ||
| moveBy(options.length - 1 - activeIndex); | ||
| moveBy(count - 1 - restIndex); |
There was a problem hiding this comment.
moveBy steps from motion.target() (the row the wheel is heading to), but these deltas are measured from restIndex, the last committed row. While a coast, a tap spin or a scroll-wheel snap is running, Home or End lands between the motion's target and the end: Home pressed while a coast from minute 10 heads for 40 commits 30, not 0. Making Home and End absolute (the first and last enabled rows) fixes it; a test that presses them mid-coast would guard it.
There was a problem hiding this comment.
Fixed in adb185f: Home and End go to the first and last enabled row wherever the wheel is heading. New case "takes Home and End to the first and last enabled rows, even mid-coast" fails before the change (Home committed 30 from a coast heading past 40) and passes now.
| useIsomorphicLayoutEffect(() => { | ||
| if (!isActive || dragRef.current != null || motion.isMoving()) { | ||
| return; | ||
| } | ||
| const position = nearestPosition(selectedIndex, motion.offset(), layout); | ||
| if (Math.abs(motion.offset() - position * ROW_HEIGHT) > 0.01) { | ||
| motion.setOffset(position * ROW_HEIGHT); | ||
| } | ||
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- the active row follows the wheel's resting offset, which only exists in the motion engine | ||
| setRestIndex(selectedIndex); |
There was a problem hiding this comment.
A value set from outside first renders the old restIndex into aria-activedescendant, and this layout effect then repairs it, which docs/architecture/react-component-runtime.md INV1 rules out. Adjusting restIndex during render when selectedIndex changes at rest avoids the follow-up update; the effect can keep syncing the motion engine's offset.
There was a problem hiding this comment.
Fixed in adb185f: the restIndex state is gone. The active descendant derives from the committed value during render, and the park effect only syncs the motion engine's offset. New case "points the active row at a value set from outside in the same commit" (React Profiler) saw two commits, the first with the old row, before the change; now one commit with the new row.
| * inlines only literal constants, so the values are repeated here; a test | ||
| * holds them to the table. | ||
| */ | ||
| export const wheelGeometry = stylex.defineConsts({ |
There was a problem hiding this comment.
@astryxdesign/core/DateInput/tokens.stylex is a published subpath, so removing dateInputTouchSizes.wheelItemSize and dateInputTouchGeometry.wheelEdgePadding breaks code that reads them in what the changesets declare a patch, and wheelGeometry becomes public API. Consider keeping the two keys and moving wheelGeometry into a .stylex.ts module outside the exports map; only Wheel.tsx and WheelGroup.tsx read it.
There was a problem hiding this comment.
Fixed in adb185f: tokens.stylex.ts is unchanged from main (both released keys stay; today's column still uses them), and wheelGeometry moved to the unexported DateInput/wheelGeometry.stylex.ts. A new test keeps the published module to its released names (WEEKS_PER_PANE, dateInputTouchGeometry, dateInputTouchSizes).
| * ``` | ||
| * <WheelGroup> | ||
| * <Wheel label="Hour" options={hours} value={hour} onChange={setHour} /> | ||
| * <Wheel label="Minute" options={minutes} value={minute} onChange={setMinute} looping /> |
There was a problem hiding this comment.
The prop is isLooping; looping is not a Wheel prop.
There was a problem hiding this comment.
Fixed: the example uses isLooping.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] The picker wheels in the TimeInput, DateTimeInput and DateInput bottom sheets now look and move like iOS's own picker. The rows sit on a cylinder seen in perspective, with one capsule band across every column and the row inside it magnified. A flick coasts at UIKit's deceleration onto the row UIKit's projection picks. A finger landing on a moving wheel stops it. A tap on a row turns it onto the band. Past the first or last row, a drag rubber-bands and the wheel bounces back. As on iOS, the value commits when the wheel comes to rest, and the coast keeps running while the page's main thread is busy. Arrow keys and clicks on a row still commit at once. |
There was a problem hiding this comment.
Nothing renders WheelGroup at this head, so each column draws its own band; "one capsule band across every column" describes the follow-up that adopts the group, not this change.
There was a problem hiding this comment.
Fixed: the changeset no longer mentions a band across every column (that is #8161's), and it is now tagged [feat] as a new visual direction under AST-017 FR3.
2def93c to
1c1e238
Compare
vjeux
changed the title
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23 with DEC-1 and DEC-2), on three Wheel choices no spec states yet (the new look, turns that ignore Reduce Motion, and the nearest-enabled rest), on the theme motion roles and platform adaptation condition (which also need spec:AST-012 FR2 amended), and on the motion/experimental and gestures/experimental subpaths, which sit outside the experimental boundary spec:AST-017 FR21 names. Any engineering owner, the author included, records that decision by commenting /approve-spec 1c1e2386fe3a81037daa6fa879c4c82bd6349161 on this pull request. Whatever the decision, these need changes:
- The hard-coded 744 px compact width for
presentation="adaptive"contradictsspec:AST-012FR9, which defines compact as width below the theme'smdpoint or height below 500 px. - Home and End pressed while a wheel is still moving commit the wrong row, and
wheelGeometrybecomes a public export of@astryxdesign/core/DateInput/tokens.stylex. - Land the stacked predecessors separately and bring the Wheel commit under 2,000 lines, then fix the layout-effect repair of the active row and the changesets noted inline.
[Automated review]
| case 'Home': | ||
| event.preventDefault(); | ||
| moveBy(-restIndex); | ||
| break; | ||
| case 'End': | ||
| event.preventDefault(); | ||
| moveBy(count - 1 - restIndex); | ||
| break; |
There was a problem hiding this comment.
moveBy steps from motion.target() (the row the wheel is heading to, line 646), but these deltas are measured from restIndex, the last rested row. While a coast, a tap spin or a rest turn runs, the two differ: on the 60-row minute wheel, Home pressed 200 ms into a flick up from 10 commits 23, and End 200 ms into a flick down from 50 commits 36. Make Home and End absolute (the first and last enabled rows) and add a test that presses them mid-motion.
There was a problem hiding this comment.
Fixed at adb185f: Home and End go to the first and last enabled rows wherever the wheel is heading. The case "takes Home and End to the first and last enabled rows, even mid-coast" fails before the change.
| useIsomorphicLayoutEffect(() => { | ||
| if (!isActive || dragRef.current != null || motion.isMoving()) { | ||
| return; | ||
| } | ||
| const position = nearestPosition(selectedIndex, motion.offset(), layout); | ||
| if (Math.abs(motion.offset() - position * ROW_HEIGHT) > 0.01) { | ||
| motion.setOffset(position * ROW_HEIGHT); | ||
| } | ||
| // eslint-disable-next-line @eslint-react/set-state-in-effect -- the active row follows the wheel's resting offset, which only exists in the motion engine | ||
| setRestIndex(selectedIndex); | ||
| }, [isActive, selectedIndex, layout, motion]); |
There was a problem hiding this comment.
A value set from outside first renders the old restIndex into aria-activedescendant (line 757), then this layout effect repairs it, which docs/architecture/react-component-runtime.md INV1 rules out ("MUST NOT render a known-wrong initial state and repair it in a follow-up Effect"). The lint reason names the component's own motion engine rather than an external system. Adjust restIndex during render when selectedIndex changes at rest, and keep the effect for the engine's offset.
There was a problem hiding this comment.
Fixed at adb185f: there is no restIndex state. The active descendant derives from the committed value during render, and the effect only syncs the motion engine's offset. The case "points the active row at a value set from outside in the same commit" uses a React Profiler to check for one commit.
| export const TOUCH_QUERY = '(pointer: coarse)'; | ||
|
|
||
| /** The viewport is compact: layout behaviours. */ | ||
| export const COMPACT_QUERY = '(width < 744px), (height < 500px)'; |
There was a problem hiding this comment.
spec:AST-012 FR9 on main makes compact "viewport width … below the effective md point (FR1) or its viewport height … below 500", with no size numbers of a resolver's own, and architecture:public-component-api INV17 says the same. A hard-coded 744 px edge sends a coarse 744–767 px screen (a 744 px iPad mini in portrait) to the popover where FR9 makes it compact, and ignores a theme that moves md. Build the query from the theme's effective md point (768 by default) and the 500 px height, and have the changeset name presentation as the way to keep a surface.
| * holds them to the table. Outside the iOS idiom the wheels keep the sizes | ||
| * above. | ||
| */ | ||
| export const wheelGeometry = stylex.defineConsts({ |
There was a problem hiding this comment.
@astryxdesign/core/DateInput/tokens.stylex is a published subpath, so this adds a stable public export (spec:AST-017 FR3) although the PR body says there is no API change, and only PickerWheel.tsx and WheelGroup.tsx read it. Moving it to a .stylex.ts module outside the exports map keeps UIKit's picker numbers free to change.
There was a problem hiding this comment.
Fixed at adb185f: tokens.stylex.ts is unchanged from main, and the picker geometry lives in the unexported wheelGeometry.stylex.ts. A test keeps the published module to its released names.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [chore] An internal `usePresentation` / `readPresentation` resolves a component's `presentation` prop into the surface to render and whether its touch input modes apply, keeping an omitted value distinct from an explicit `'popover'` so a caller's choice always wins over a touch default. No component reads it yet; behavior is unchanged. |
There was a problem hiding this comment.
At this head Wheel and WheelGroup read usePresentation, so "No component reads it yet; behavior is unchanged" is no longer true; the same goes for the "no component uses it yet" lines in the gesture, UIKit table and wheel-values changesets.
There was a problem hiding this comment.
Resolved: this stack no longer contains #8298. The wheel reads useTouchPresentation(), so presentation-resolver.md is not in this pull request.
|
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. |
…nt lets go The host's touch-action rules sit in an adopted sheet or a style element per document or shadow root, and were never removed, so a page whose last recognizer detached kept them, against spec:AST-094 FR24 (nothing touch-related remains after the last instance unmounts). Fails until a root's rules go with its last marked element.
…th its last marked element The touch-action writer records which elements it marked in each document or shadow root, and when the last one is unmarked (its last recognizer detached) it removes that root's adopted sheet or style element, so nothing touch-related remains after the last instance unmounts (spec:AST-094 FR24). A later mark writes the rules again; an element marked in another root first lets go of the old one.
… the inline style it reads - The motion registry, the host's engine gate and the README name the presented hit test as an engine workaround (spec:AST-013 FR6) with its review condition (re-measure on each iOS release; remove it once iOS WebKit hit-tests a compositor-moved element where it is drawn) and its regression fixture. - While a curve runs the animator holds the value it starts from inline, so presentation.ts and the README no longer say the inline style holds the end.
…core/internal/touch Core ships the internal entry @astryxdesign/core/internal/touch, which re-exports the gesture primitives for Astryx's own packages, and the Lab-import guard fails a Lab import of core outside core's exports (architecture:react-component-runtime INV13). The README and the barrel's header named internal workspace imports and said nothing here is exported; they now name that entry.
…ill needs the element, and the listeners that cancel The release note, the stream's header, the host's comment and the README said the stream is one passive listener per event type and that a document-capture stopPropagation no longer starves the recognizers. Such a stop never hid a listener on the same node, and some subscriptions cancel. They now say what changed: an app's window-capture stopPropagation, or a document-capture stopImmediatePropagation registered before core, no longer hides a touch; a recognizer that must claim still needs the touch to reach its element; the gesture host's touchend, click and contextmenu cancel while a claimer is attached and the page hold's touchmove while it holds; and the stream stays ahead of an app's window-capture listener only until a type's listener is re-added. The README's list of blocking touchmove listeners gains the claimed touch's target and the document while a claiming element moves.
…default it moves
The {stylexOptions} form of astryxStylex now passes StyleX
treeshakeCompensation: true unless the caller sets it. spec:AST-017 FR3
asks the release note to name the old and new default and the option
that restores the old one; it now does.
…er and for the two stops the input stream fixes - A still finger with sub-pixel jitter over a scroll view keeps its tap and scrolls nothing (spec:AST-094 FR18–FR22). - A tap is still recognized under an app's window-capture stopPropagation, and under a document-capture stopImmediatePropagation registered before core's: the recognizers hear touches on the window's capture phase. Chromium and WebKit pass.
…s once Where nothing that claims is attached, the host's touchend listener is passive and cannot cancel the engine's own click after a retargeted tap clicks what is drawn. The shared click guard swallows that late click whatever is attached; this case pins it, as asked on meta#8293.
…esture host's subscriptions The gesture host subscribes no click: the one cancelling click subscriber is the click guard's, added at a window's first arm and kept. The release note, the stream's header and the gestures README listed click with the host's touchend and contextmenu; they now name the guard's own.
…-nearest-valid
…-nearest-valid
…el-nearest-valid
…e turn - The wheel reads the pointer type on the way down. usePointerDragScroll stops a primary mouse press from bubbling, so after a touch a mouse drag's travel still cross-faded on a fine pointer under reduced motion. - With meta#8483 merged in, the travel waits for a mouse drag's release instead of running whenever the hand pauses; a test covers it here. - DateInput's Owns line claims the shared wheel's mechanics; the TimeInput and DateTimeInput records own their sheets' time wheels. - FR13 and DateTimeInput FR8 say a shown wheel turns onto a value changed from outside, which the code and its tests now cite.
…he turn) into uikit/c21-wheel
…ps its mark after a nudge - A value changed from outside while the wheel is shown (a bound clamping the time after another wheel turned) turns the wheel onto its row, as UIDatePicker does and FR13 now says. The first park when the wheel is shown stays at once. - A motion that ends on the row it began on (a nudge that eases back, a drag off a row and back) gives the band's mark back to that row: the player forgets its last report when a motion starts, so the row it ends on is reported again. - The ctrl-wheel note cites FR43 (a ctrl-wheel is a pinch) and FR10 (with no pinch of its own, the wheel leaves it to the page's zoom).
…sheet on the desktop viewport This file's mouse cases run on a desktop viewport, where the field shows its popover unless it asks for the sheet; the case meta#8483 added now asks for it, as its neighbours do.
The touch column's options lie under its drawn rows, so the proof taps the PM row's place on the screen, where the column's tap recognizer takes it, rather than asking for a tap on the option element.
…nto uikit/c21-wheel-listbox
…e new foundation) into uikit/c21-wheel-proofs
…nto uikit/c21-wheel-proofs
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on what this pull request writes into the draft component:DateInput and component:DateTimeInput records (FR13 with its second sentence, which keeps an out-of-bounds committed value after a drag back onto it, and its new third sentence, which turns a wheel onto a clamped value; DEC-1; DateInput's ownership of the shared wheel; DateTimeInput FR8), and on whether the 300 ms tap spin and the snap rate may stay constants under spec:AST-094 FR4; an engineering owner records that decision by commenting /approve-spec with the full head SHA, and the author may do so as an engineering owner. Whatever that decision, these still need changes:
- Land the unmerged stack first, or merge
mainagain as each part lands: the diff is 33,990 authored lines, 2,983 of them this pull request's own. - A second clamp during the hour wheel's 300 ms turn leaves it resting on the old target (
PickerWheel.tsx:469-475); re-check the committed row when a turn ends and test two clamps inside one turn. - Play the wheel's motion through the shared animator, or have
architecture:react-component-runtimeINV13 amended to admit a component-owned player. - Add browser proofs of a flick's coast through a main-thread block and of a mid-coast catch (Chromium phone emulation and WebKit), plus real-device recordings, as
spec:AST-094FR5, FR6 and Platform support ask. - Correct the Changeset's "keep today's look" and "With a mouse the wheels keep today's column" (
spec:AST-017FR3).
[Automated review]
| if (dragRef.current != null || motion.isMoving()) { | ||
| return; | ||
| } | ||
| const target = selectedIndex * ROW_HEIGHT; | ||
| if (Math.abs(motion.offset() - target) > 0.01) { | ||
| if (parkedRef.current) { | ||
| move(turnCurve(columnRef.current, motion.offset(), target)); |
There was a problem hiding this comment.
This bails while any motion runs and only runs again when selectedIndex changes, and the turn it starts has no onEnd. In TimeInput's "Bottom Sheet With Min Max" story (9 AM to 5 PM, at 10 AM), PM clamps the time to 5 PM and starts this 300 ms turn from 10 to 5; AM within the turn clamps it to 9 AM, this returns, and the wheel rests on 5, painted under the band, while the field holds 9:00 AM. Re-checking the committed row when a turn ends (or retargeting a running park turn) would fix it; a test with two clamps inside one turn would catch it.
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger, on every touch device, the picker wheels in the TimeInput, DateTimeInput and DateInput bottom sheets now move like UIPickerView, and keep today's look: 28 px rows that tip and fade toward the edges of the column, with the row under the band in semibold accent text on its plate. A flick coasts at UIKit's deceleration onto the row UIKit's projection picks. A finger landing on a moving wheel stops it. A tap on a row turns it onto the band. Past the first or last row, a drag rubber-bands and the wheel bounces back. As on iOS, the value commits when the wheel comes to rest, and the coast keeps running while the page's main thread is busy. Arrow keys and clicks on a row still commit at once. Under reduced motion a flick still coasts, as scrolling does, with the rows fading rather than tipping; a tap, a key or a slow release cross-fades onto its row. `presentation="popover"` keeps the previous surface; no value keeps the native-scroll column under a finger. With a mouse the wheels keep today's column. |
There was a problem hiding this comment.
"Keep today's look" holds only where animation-timeline: view() is supported: elsewhere today's column draws a plain, fully legible list (Wheel.tsx:186-196), while the touch column tips and fades on every engine (wheelLook.ts:81-114). No row is marked while the touch column coasts, snaps or turns, where today's highlight follows the scroll, and Wheel picks the column by the primary pointer, so a mouse on a phone or tablet gets the touch column. spec:AST-017 FR3 asks the Changeset to name the old and new appearance.
| } | ||
|
|
||
| /** Play `curve` from the current offset; the rows end at curve.to. */ | ||
| play(curve: WheelCurve, {ticks = false, onEnd}: PlayOptions = {}): void { |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 has a component's own touch logic be "small glue" over the shared modules, the animator among them, and its verification row fails "a component bundling its own copy of a module's primitive". This player bakes, starts, follows, stops and cross-fades curves itself and imports nothing from motion/animator.ts; either a per-row channel in the animator or an INV13 amendment would settle it.
|
Mapped to 161c506:
Open:
|
…nothing else does (#8671) ## Why Every pull request that gives a component a gesture fails `test-node` once merged with main, and the failure names no component: `scripts/touch-support-bundle.test.mjs` requires Core's root and hooks entries to carry no gesture code. At least six open pull requests (#8142, #8190, #8337, #8159, #8098, #8658) fail it this way. ## What led to it This comes from the work to make Astryx's touch components behave as native iOS ones do. Since #8465, a component that uses a gesture imports the touch modules directly (`architecture:react-component-runtime` INV13), and the root entry re-exports every component, so it carries gesture code as soon as one component does. ## Options and tradeoffs - Exempt the root entry: unblocks everyone, but stops catching a component that pulls gesture code it does not use. - Check each public import on its own: catches exactly that, at the cost of a list of the imports allowed to carry gesture code. ## Why this choice The per-import check keeps INV13's promise, a component without gestures ships none of their code, while letting gesture users through by name. It was offered on #8562 and aimed at main's test. ## What changes - Each public import of Core is built the way the size report builds it, and may carry gesture code only if `GESTURE_USERS` names it. A failure lists the import, its gesture modules with their sizes, and the chain of imports that brings them. - The root entry may carry only what a named import carries; the utils entry stays gesture-free. - A strict case keeps Button and TextInput free of gesture code whatever the list says. - `GESTURE_USERS` starts empty, so every import stays as strict as on main. The pull request that gives a component a gesture adds that component's import, and each import that reaches it, with a comment naming the gesture, so the new gesture user shows up in review. ## How to check it 1. Run `pnpm exec vitest run --project node scripts/touch-support-bundle.test.mjs` on main with this branch merged: it passes. 2. On a tree whose component uses a gesture, run it without naming that import in `GESTURE_USERS`: the test fails and names that import and the chain that brings the gesture code. Name it, and the test passes. 3. Import a gesture module from Button: the strict case fails. ## Evidence - With the list empty, the test passes on main: no public import carries gesture code today. - On open trees that give a component a gesture (AppShell's scroll-edge bar, Slider, the List reorder), the per-import case fails naming exactly those imports and their chains, and passes once each is named. - No product code changes. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context.
…wins over touch defaults (#8298) <!-- Template: maintenance.md (an internal resolver; no behavior change until a component reads it). --> Small batch of the merge plan: one commit on main holding only this change. Its paragraph for the gestures README waits for that README, which comes with #8076; the resolver's file header carries the same contract. ## Maintainer impact A component that behaves differently under a finger, such as a menu that tracks the finger or becomes a sheet, must still let a caller pin the desktop behavior. Today components read `presentation ?? 'popover'`, which cannot tell an omitted prop from an explicit `'popover'`, so a component that adopts a touch default would override a caller who asked for the popover. The menu, context menu and picker work that adopts touch modes builds on this resolver (#8104, #8627, #8080, #8125, #8201, #8159, #8162). ## Intended invariant A caller's explicit `presentation` always wins over the touch default (`architecture:public-component-api` INV16), and the surface equals `useAdaptivePresentation`'s for every value on every device, so no caller sees a change: | `presentation` | surface | touch input modes | | ------------------------------- | --------------------------------------------- | ----------------- | | `'popover'` | popover | never | | `'bottom-sheet'` | sheet | under a finger | | `'adaptive'` | sheet on a compact touch screen, else popover | under a finger | | omitted, `whenOmitted: 'touch'` | popover | under a finger | The modes follow the pointer, never the platform. The server and hydrating render never present touch modes, and no hint changes that (`spec:AST-094` FR13). The hook listens only to the queries a request reads (`architecture:react-component-runtime` INV5): none for a pinned `'popover'`, the pointer for `'bottom-sheet'` and the omitted `'touch'` default, and the pointer and compact size for `'adaptive'`. A touch-mode style is applied through `touchModes`, never nested under the iOS-only `@supports` gate. ## Change - `utils/presentation.ts`: `usePresentation` and `readPresentation` (and the pure `resolvePresentation`) return the surface to render and `touchModes`, whether the component's touch input modes apply; `usePresentation` subscribes to each media query only while the request reads it. Internal: no component reads it yet. - `hooks/useAdaptivePresentation.ts`: its header points a component that also switches input modes to `usePresentation`, whose surface is its own. ## Evidence - Failure before the change: an omitted `presentation` and an explicit `'popover'` resolve the same, so a touch default cannot tell them apart; a server render given a touch hint must still answer the popover with no touch modes, a case that failed first. On an iPhone, a resolver that seeded its server snapshot from the live window fails both the server-render case and the `hydrateRoot` case; listening to both queries for every value fails the subscription case. A resolver that gives `'adaptive'` the sheet without the coarse pointer, or reads compact by width only, fails the surface matrix; one that ignores the theme's `md` fails the themed case. - Success after the change: the unit matrix (iPhone, iPad, Android, desktop, Mac Safari, a fine-pointer narrow window, a phone in landscape, and the 767/768 and 499/500 compact edges, plus `'adaptive'` under a theme whose `md` is 1000) covers each row above, a live pointer change and the one-moment read; on an iPhone, the server render and the hydrating render answer the popover with no touch modes, and after hydration the phone's sheet with touch modes; each request holds only the subscriptions it reads. - Product/runtime behavior verified unchanged: every value renders `useAdaptivePresentation`'s surface on every device, and no component reads the resolver yet. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context. ## Testing Local gates at 9f0595f, one commit on main d73908f holding only this change (4 files, +571/−1): - `pnpm test:ui` (core UI): 654 files, 16,459 tests passed. - `pnpm exec vitest run --project node`: 804 files passed; the only failures are main's own story-tree and release-cache reds, tracked in #8509 and #8520. - `tsc --project packages/core/tsconfig.json --noEmit`, strict ESLint and Prettier on every changed file: clean. - `pnpm check:repo`: clean.
Depends on #8526 (on #8156 and #8360) and #8131. Review only this PR's change: vjeux/astryx@5fffebe...161c506 (it also shows #8131's lines).
User need
On a phone, the TimeInput, DateTimeInput and DateInput sheets' wheels are scroll-snap columns; in WebKit a flick stops at a fifth of the iOS picker's distance. This makes them move like the iOS picker under a finger, in today's look (#8480 draws UIKit's); a mouse keeps today's column.
Problem and solution fit
What led to it: Astryx's UIKit-parity work (
spec:AST-094) makes phone presentations behave like iOS. #8156 holds UIPickerView's measured values and math, #8526 plays its curves on the compositor, and this wires them into the wheel under a finger:WheelrendersPickerWheelunder a finger and today's scrolling column under a fine pointer.Current specification
spec:AST-094FR1, FR3–FR6, FR8, FR9, FR13, FR23, FR24, FR43;design:reduced-motionDR2;architecture:component-style-authoringINV7.Public delta and caller burden
No API change. Size (INV6): DateInput +25.2 KB gzip JavaScript (TimeInput +24.2, DateTimeInput +24.5), CSS +15 B.
Design and alternatives
Evidence
iOS 26.2 simulator, Mobile Safari, finger dots shown:
Under a trackpad (#8131's rest, Chromium, ¼ speed):
Android 15 emulator, Chrome 124, a 60 Hz finger:
Scope
Testing
On a phone or the iOS simulator, open Storybook's TimeInput › Bottom Sheet With Min Max:
Automated:
PickerWheel.test.tsxdrives real touch sequences through the gesture host for each step, and the Chromium specsWheelRestandWheelPinchZoomrun them in a browser. The recordings caught two faults, each now tested: a coast that stalled in Mobile Safari through a long task, and a bold row that rode away from the band mid-coast. Not this change: CI's root-entry bundle check fails on main's test, which #8671 fixes. On the Android emulator only, a 1 px grey hairline can show along a tilted row's edge; neither iOS nor Chromium draws it.Since the review on fe003a4 (now 161c506): a value clamped from outside, as when tapping PM past the maximum, turns the wheel instead of jumping, and the band's mark comes back after a motion that ends where it began. Each has a test that fails on the previous head.
Merge order: #8360 and #8156, #8526 and #8131, then this PR (full order in #8156).