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.
This pull request is on hold for a spec decision: the picker wheel's new look and motion, and the requirements the stack adds in draft spec:AST-094 (FR1–FR23, DEC-1 and DEC-2, including its proposed amendments to current records), have no approved spec yet, so an engineering owner needs to approve, change or reject them. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so as an engineering owner. Independently of that decision, these changes are needed:
- The compare against
mainis 29,707 authored lines across the 31-commit stack; land the predecessor pull requests first so this one holds only its own proof commit. @astryxdesign/core/DateInput/tokens.stylexis a published subpath and 0.7.4 shipsdateInputTouchSizes.wheelItemSizeanddateInputTouchGeometry.wheelEdgePadding; keep them (deprecated) or follow the breaking-change process, and keep the newwheelGeometryout of the exported module.- The deleted "a mouse can drag a wheel" tests have no replacement; add mouse-drag cases to
Wheel.test.tsx. - The description's scope checklist and the WheelRest comment in
.github/workflows/ci.ymlstill describe the previous scope and the native-scrolling wheel.
[Automated review]
| * column of values, not as a stack of buttons with air between them. At | ||
| * this height ~9 options are on screen. | ||
| */ | ||
| wheelItemSize: '28px', |
There was a problem hiding this comment.
dateInputTouchSizes.wheelItemSize ships in @astryxdesign/core@0.7.4 through the published ./DateInput/tokens.stylex subpath, so removing it breaks importers on upgrade (architecture:public-component-api INV9, INV12). Keep it, marked @deprecated, or follow the spec:AST-017 breaking-change lifecycle; the same applies to dateInputTouchGeometry.wheelEdgePadding, and the new wheelGeometry would be better declared outside this exported module.
| # A picker wheel dragged onto a row outside its bounds must come to rest | ||
| # on the nearest row inside them. The stop comes from native scrolling, | ||
| # CSS snapping and the browser's end-of-scroll signal, none of which | ||
| # jsdom has. |
There was a problem hiding this comment.
At this head the wheel no longer stops through native scrolling, CSS snapping or the end-of-scroll signal: it rests through its pan recognizer and compositor curves (packages/core/src/DateInput/Wheel.tsx header). Please reword so the comment matches what this contract exercises.
There was a problem hiding this comment.
Reworded: the step proves the rest on the touch column (gesture system and motion engine) and on today's column under a mouse (native scrolling, CSS snapping and the end-of-scroll signal).
| * It matters on desktop specifically because that is where this surface is | ||
| * reviewed, themed and screenshotted. | ||
| */ | ||
| describe('DateInput — a mouse can drag a wheel', () => { |
There was a problem hiding this comment.
These six tests were the regression evidence for dragging a wheel with a mouse. Mouse drags now go through the gesture host, but no Wheel.test.tsx case drags with a mouse; please add one (a primary-button drag turns the wheel and commits at rest, a secondary-button press does nothing) so the case stays protected (architecture:component-test-sufficiency INV3).
There was a problem hiding this comment.
The six mouse-drag tests are back in DateInputTouch.test.tsx unchanged at this head: a mouse gets today's column. PickerWheel.test.tsx adds mouse cases for the touch column too (a primary-button drag turns and commits; a secondary-button drag does nothing).
e9eae38 to
8d6847c
Compare
There was a problem hiding this comment.
This pull request can't be approved yet: the picker wheels' new look and motion rest on the draft spec:AST-094 (FR1-FR23, DEC-1 and DEC-2), which no engineering owner has approved, including its proposed changes to architecture:theme-tokens and spec:AST-025 FR14. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request, and the author may do so if they are an engineering owner. Whatever that decision, these changes are still needed:
- Keep
dateInputTouchSizes.wheelItemSizeanddateInputTouchGeometry.wheelEdgePaddingin the released@astryxdesign/core/DateInput/tokens.stylexexport (or deprecate them underspec:AST-017), and keepwheelGeometryout of the package exports. - Restore wheel-level mouse-drag tests for the six deleted cases.
- Reduce the compare to this pull request's own change (it currently carries 29,775 authored lines of the unmerged stack), correct the stale wheel-rest comment in
ci.ymland the scope checklist, and getlintgreen.
[Automated review]
| * column of values, not as a stack of buttons with air between them. At | ||
| * this height ~9 options are on screen. | ||
| */ | ||
| wheelItemSize: '28px', |
There was a problem hiding this comment.
wheelItemSize is part of the released @astryxdesign/core/DateInput/tokens.stylex export (0.7.4 declares it, with wheelEdgePadding). Removing both breaks callers without the deprecation and migration spec:AST-017 asks for (architecture:public-component-api INV9): please keep them, or deprecate them with a migration changeset.
| # A picker wheel dragged onto a row outside its bounds must come to rest | ||
| # on the nearest row inside them. The stop comes from native scrolling, | ||
| # CSS snapping and the browser's end-of-scroll signal, none of which | ||
| # jsdom has. |
There was a problem hiding this comment.
This comment describes the old wheel: the head's wheel no longer uses native scrolling, CSS snapping or the end-of-scroll signal, and its stop comes from the motion engine's release plan. Please update it to say what this contract step guards now.
There was a problem hiding this comment.
Reworded: the step proves the rest on the touch column (gesture system and motion engine) and on today's column under a mouse (native scrolling, CSS snapping and the end-of-scroll signal).
| * It matters on desktop specifically because that is where this surface is | ||
| * reviewed, themed and screenshotted. | ||
| */ | ||
| describe('DateInput — a mouse can drag a wheel', () => { |
There was a problem hiding this comment.
These six cases are the only regression evidence for mouse drags on the wheels, and Wheel.test.tsx has no mouse or pointerType case. architecture:component-test-sufficiency INV3 keeps such evidence unless something equivalent replaces it: please add wheel-level cases driven by Pointer Events with pointerType: 'mouse'.
There was a problem hiding this comment.
At this head the six cases are present again (today's column, which a mouse always gets), and PickerWheel.test.tsx adds Pointer Events cases with pointerType: 'mouse' for the touch column.
8d6847c to
6440cde
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23, DEC-1/DEC-2, and its proposed amendments to the theme-tokens, user-states, buttons, interaction-modality and AST-025 FR14 contracts), recorded 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, three fixes are needed:
- A mouse drag on a picker wheel now also drags the bottom sheet and dismisses it on release, so keep the press from the sheet and restore the deleted mouse-drag tests.
wheelItemSizeandwheelEdgePaddingdisappear from the releasedDateInput/tokens.stylexunder patch changesets whilewheelGeometrybecomes public there, so keep the old keys (or declare a breaking change with a migration) and move the new constants to an unexported module.- Against
mainthis pull request carries its whole stack, about 29,600 authored lines, so landing the stack in order would let each pull request be reviewed as its own commits.
[Automated review]
| const pan = useGestureRecognizer( | ||
| columnRef, | ||
| () => | ||
| new PanGestureRecognizer({ | ||
| name: 'wheel-pan', | ||
| direction: 'vertical', | ||
| shouldReceiveTouch: (_pan, touch) => { | ||
| onTouchDownRef.current(touch.time); | ||
| return true; | ||
| }, |
There was a problem hiding this comment.
This recognizer sets no allowedPointerTypes, so it takes mouse presses too, and nothing keeps that press from the sheet's handleBodyPointerDown (useSheetGestures.ts:1271). A 150 px mouse drag on the TimeInput minute wheel slides the whole sheet down and dismisses it on release; on the base the wheel turned and the sheet stayed. Keep the press from the sheet as the base did (or have the sheet skip a press a recognizer owns), and restore the mouse-drag tests.
There was a problem hiding this comment.
Resolved: a fine pointer never renders this column (it draws only under a coarse primary pointer), and under a finger the column stops pointerdown from reaching the sheet, so a press on the wheel never arms the sheet's pull-down (Chromium case: the sheet's top stays put through a downward drag; it moved 9.3 px before). A mouse on a touch device drags the column through its pan without arming the sheet (PickerWheel.test.tsx mouse cases).
| * 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.
wheelGeometry becomes public through @astryxdesign/core/DateInput/tokens.stylex, and this file also drops the released wheelItemSize and wheelEdgePadding (both in the 0.7.4 declarations) under patch changesets. Keep those keys, deprecated, or declare a [breaking] change with a migration, and move these constants to a module no package path exports.
| # A picker wheel dragged onto a row outside its bounds must come to rest | ||
| # on the nearest row inside them. The stop comes from native scrolling, | ||
| # CSS snapping and the browser's end-of-scroll signal, none of which | ||
| # jsdom has. |
There was a problem hiding this comment.
This comment describes native scrolling, CSS snapping and the browser's end-of-scroll signal, none of which the rebuilt wheel uses; it now rests through the gesture recognizer and compositor curves.
There was a problem hiding this comment.
Reworded: the step proves the rest on the touch column (gesture system and motion engine) and on today's column under a mouse (native scrolling, CSS snapping and the end-of-scroll signal).
| // A finger or a mouse dragging a wheel goes through the gesture system: see | ||
| // Wheel.test.tsx. |
There was a problem hiding this comment.
Wheel.test.tsx drives touch only, so the deleted mouse-drag cases (including "keeps the press away from the sheet, which would capture the pointer") have no equivalent, and that regression is live at this head.
There was a problem hiding this comment.
The mouse-drag cases are all present at this head, including "keeps the press away from the sheet", since a mouse gets today's column; the touch column has its own mouse cases and a sheet-stays case.
6440cde to
0c38621
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23, DEC-1/DEC-2 and its proposed amendments to the theme-tokens, user-states, buttons, interaction-modality and AST-025 FR14 contracts), which any engineering owner, the author included, records by commenting /approve-spec <full head SHA> on this pull request. Whatever that decision, these fixes are needed:
- A mouse or pen drag on a picker wheel now also drags the bottom sheet and dismisses it on release, so keep the press from the sheet and restore wheel-level mouse-drag tests.
wheelItemSizeandwheelEdgePaddingdisappear from the releasedDateInput/tokens.stylexunder patch changesets whilewheelGeometrybecomes public there, so keep the old keys (deprecated) or declare a breaking change with a migration, and move the new constants to an unexported module.- Against
mainthis pull request carries its whole stack, about 29,700 authored lines where its own commit is 571, so landing the predecessors in order would let it be reviewed as its own commit. - The wheel-rest workflow comment, the mouse-drag test comment, nine changesets that say no component uses the code yet, and AST-094's citation of a missing AST-090 need correcting.
[Automated review]
0c38621 to
2407960
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on draft spec:AST-094 (FR1–FR23, DEC-1/DEC-2 and its proposed amendments to the theme-tokens, user-states, buttons, interaction-modality and AST-025 FR14 contracts) and on three additions no current spec covers: theme motion roles (which AST-094 FR4 and DEC-1 reject), the platform adaptation condition (outside spec:AST-012 FR2) and the motion/experimental and gestures/experimental subpaths (spec:AST-017 FR21 names only a component's own subpath). Any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request, and keeping the three additions also needs those records amended. Whatever that decision, these fixes are needed:
presentation="adaptive"switches at 744 px (utils/touchPresentation.ts:75-79), butspec:AST-012FR9 andarchitecture:public-component-apiINV17 make every screen belowmd(768) or under 500 px tall compact, so a 744–767 px touch screen gets the popover instead of the sheet; use that definition and update the test, docs, stories and changeset.- Outside the iOS idiom, DateTimeInput's time columns lose their 8 px gap (
TouchDateTimeField.tsx:376-382), although the changeset says those sheets are unchanged. wheelGeometrybecomes public throughDateInput/tokens.stylex, ten changesets say no component uses the code, and AST-094 cites aspec:AST-090that does not exist.- Against
mainthis pull request carries its whole stack, 34,248 authored lines where its own commit is 629, so landing the predecessors in order would let it be reviewed as its own commit.
[Automated review]
| 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.
This hard-codes 744 px. Current spec:AST-012 FR9 (and architecture:public-component-api INV17) makes a screen compact below the theme's effective md point (768 by default) or under 500 px tall, says no resolver keeps size numbers of its own, and DEC-6 rejects approximating platform size classes. As written, a 744–767 px coarse screen such as an iPad mini in portrait gets the popover where FR9 gives it the sheet; please derive compact from the theme's breakpoint and update touchPresentation.test.ts:341-342, the docs, the stories and the changeset.
| inlineSize: '100%', | ||
| minInlineSize: 0, | ||
| blockSize: dateInputTouchGeometry.paneBlockSize, | ||
| gap: spacingVars['--spacing-2'], |
There was a problem hiding this comment.
Removing this gap changes today's columns too: outside the iOS idiom WheelGroup renders its children as a fragment, so the hour, minute, second and AM/PM columns now sit 0 px apart and their full-width band plates run together. TimeInput and the month/year wheels keep their gap, and the wheel-group-inputs changeset says these sheets are unchanged on Android and with a mouse; please keep it (the iOS group is a single flex child).
There was a problem hiding this comment.
Right: #8161 had dropped the gutter, and under a fine pointer WheelGroup renders its columns as a fragment, so today's columns touched. Fixed in ee5538b (re-picked into this branch). The time row's style is main's again, and a new Chromium check under a mouse requires every gap between adjacent columns to be at least 8 px. It read 0 px on the previous head.
| * 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.
./DateInput/tokens.stylex is a supported package path (packages/core/package.json:94), so these nine constants become public API (architecture:public-component-api INV12) without a changeset saying so. Please move them to a .stylex.ts module beside the wheel that no package path exports.
| '@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.
Wheel and WheelGroup read usePresentation in this compare, and the shared query moves the adaptive line of five components, so "No component reads it yet; behavior is unchanged" misdescribes the release (spec:AST-017 FR7). Nine other changesets say no component uses the gesture or motion code while the iOS wheel does.
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.
| measures (0.55 for a scroll view and a sheet), and a release MUST pick its | ||
| rest point by projecting its velocity the way its UIKit counterpart does: | ||
| with a deceleration rate, or with a measured projection time. | ||
| - **FR4 — Physics are not themable in touch presentations.** A touch |
There was a problem hiding this comment.
FR4 (with DEC-1) says touch physics are not themable, while this stack adds theme motion roles (defineTheme({motion: {springs, durations, curves}})) that retune the wheel's tap spin and bounce. An owner can approve one or the other as written; please reconcile them.
| follows. | ||
| - **FR7 — Exits animate as UIKit's do.** A touch presentation MUST play the | ||
| exit its UIKit counterpart plays. **Proposed; pending owner review:** this | ||
| departs from the draft `spec:AST-090` FR1 (exits off by default) and FR10 |
There was a problem hiding this comment.
Neither main nor this head contains a spec:AST-090, so the departure FR7 asks owners to rule on cannot be checked; please cite the record that holds those exit rules, or drop the departure (the current-state impact repeats the citation).
2407960 to
d21bd19
Compare
…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.
…e new foundation) into uikit/c21-wheel-proofs
… FR5 names The coast's proof now blocks the main thread for 1 s, the length spec:AST-094 FR5's verification names, ending while the coast is still over a row from rest so every frame moves a pixel or more. It follows the row that crosses the band halfway through, which stays in sight the whole second. The WebKit skip says what covers Mobile Safari instead: an iOS simulator capture through a 530 ms block.
…against a compositor-only control On a two-core runner the software compositor and the screencast share the core the block spins on, so a compositor animation presents fewer frames there than a display-rate bound allows, though it moves on every frame the compositor presents: pinned to two cores, Lightbox's double-tap zoom proof failed its bound in 7 of 10 runs. startBusyControl shows a small box (a manual popover, in the top layer) whose transform moves it for as long as the page runs; tracked from the same screencast and passed to busyWindowProof as control, it is what the compositor presented in that window. The component passes when it moved on every frame the control moved and stalled no longer than the control; a window where the control moved on fewer than four frames fails. A main-thread animation still fails. No tolerance: both are read from the same screencast frames, and over 140 proofs of a correct compositor animation (Chromium, Chromium pinned to two cores and WebKit, at full and 1-in-6 screencast rates) the component missed none of the control's frames and never stalled longer, while the rAF mutant moved on none of the 39 to 47 frames the control moved in a full window. Red first: five unit cases (a stall the compositor made for both, which the verdict without the control fails; a frozen component; one the control outpaces; one that misses a single control frame; a control that did not move) fail on the old busy.ts.
…ver pooled runs A fade that lasts a fraction of a second leaves one blocked window only a few frames: in WebKit, or on a loaded machine, the control can present fewer than the four one window needs, and the proof fails as too little to judge, saying nothing about the animation. pooledBusyProof(runs) judges several runs of the same short animation as one long window is judged against the control: the control must move on at least four frames over all the runs, the component on every one of them, and in each run its longest stall may be no longer than that run's control's, since a stall belongs to its run. Red first: a 30 ms judged window holds at most three frames and one run fails as too little to judge. Pooled over six runs, a compositor animation passes (no miss in 70 pools, in Chromium, Chromium pinned to two cores and WebKit) and a main-thread one fails (it moved on at most 55% of the control's frames); a pool whose component misses a single control frame fails.
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-proofs
…nto uikit/c21-wheel-listbox
…nto uikit/c21-wheel-listbox
…tep a frame from the change that ends it Against the control, a frame with no new component position was a miss. On an easing tail the component moves under one cast step a frame, which the screencast cannot show on every frame: Lightbox's dismiss-cancel proof in WebKit missed one of the control's 12 to 14 frames in 7 of 10 runs, each time at its spring's last plateau (210, 210, then 208 on WebKit's 2 px grid). Such a run of unchanged frames now counts as moved when the change that ends it is a single cast step. A freeze ends in a catch-up of more than one step and stays a miss, as does a run that no change ends, and the component must still show a new position on at least minJudgedFrames of the control's frames, so a frame-counted animation that stands still through the block and resumes one step a frame fails too. The cast step is read from the tracked value's changes in the judged part rather than from its positions: the photo's position once the block ended (207) sat off WebKit's 2 px grid and made the step read as 1 px. Red first: the tail, pooled tail, off-grid commit and frame-counted freeze cases fail on the previous busy.ts; the single-miss, slow freeze and stop-for-good cases still fail. Lightbox's five busy proofs with this judgment: 30 of 30 in Chromium, 39 of 40 in WebKit (the other presented 3 frames in its judged part, under the 4 a window needs). The harness's own proofs still pass a compositor animation with no miss and fail the rAF one (0 of 39 to 47 control frames) in both engines.
There was a problem hiding this comment.
Approval waits on an engineering owner's decision about the nearest-enabled-row rest and the new turn sentence that this stack writes into the draft component:DateInput record (FR13, DEC-1 and its Owns entry for the shared wheel) and the draft component:DateTimeInput record (FR8); any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request. Whatever that decision, these still need changes:
- The compare from
mainis 35,110 authored lines for 1,120 lines of proofs, so land or retarget onto the stack beneath, and move the@astryxdesign/buildfix to its own pull request. wheelMotion.tsis a second compositor player beside the shared animator, whicharchitecture:react-component-runtimeINV13 rules out without an owner-approved exception.- The tap spin plays a literal 300 ms (
PickerWheel.tsx:225) instead of a motion role, againstspec:AST-094FR4. GestureHost.tsattaches only under a coarse primary pointer, whilespec:AST-094FR19, FR43 and DEC-21 now ask for every pointer.- The WebKit leg of the busy-main-thread proof skips itself (
Wheel.interaction.browser.spec.ts:318), and FR5's verification asks for it.
[Automated review]
| | FR10 | Standalone mode MUST compose Field; admitted InputGroup mode MUST remove competing Field chrome while preserving name, description, value, keyboard, focus, and editing semantics. | `family:input-fields/FR3`; current source and InputGroup tests | Settled shared rule and verified adoption | | ||
| | FR11 | Clear MUST emit `undefined` through the ordinary change pipeline. The pointer and bottom-sheet surfaces restore field focus using their shipped timing; the native surface MUST NOT reclaim focus because doing so reopens the OS picker. Implementation gap: when the field becomes disabled or busy while its clear control holds focus, the control unmounts and focus falls to the document body; `spec:AST-085/FR4` requires focus to stay or move to a destination this record names. | Current source and focused clear tests; `spec:AST-085/FR4` | Verified shipped behavior | | ||
| | FR12 | No surface renders native `disabled`. A disabled field MUST stay focusable with `aria-disabled`, block typing, picking, clearing, and opening, describe its disabled reason when one is given, and take disabled paint. While busy (`isLoading` or a pending Action) the pointer and bottom-sheet surfaces expose `aria-disabled` and `aria-busy`, stay focusable, and block new commits; the native surface does the same for `isLoading`, and during a pending Action stays editable so the OS picker remains attached but rejects additional commits until the Action settles. A busy field that is not disabled keeps its ordinary paint and Spinner, shows no hover ring while commits are blocked, and keeps its keyboard focus ring. | `spec:AST-085/FR1–FR6`; `family:input-fields/FR4–FR6`; `family:buttons/FR4`; current source and focused tests | Settled shared rule and verified adoption | | ||
| | FR13 | A bottom-sheet picker wheel that comes to rest on a disabled row (outside `min`/`max`) MUST continue to the nearest enabled row, a tie going toward the committed row, and select it there. A wheel resting on its committed row MUST NOT select or commit, even when that row is outside the bounds, so opening a bounded sheet never changes an untouched value: a committed value outside the bounds stays, and an empty field stays empty. A shown wheel whose value changes from outside, as when a bound clamps the time after another wheel turned, MUST turn onto the new row rather than jump to it. | DEC-1; UIDatePicker on the iOS 26.2 simulator; wheel unit and real-browser tests | Settled by DEC-1 | |
There was a problem hiding this comment.
FR13 (with DEC-1, the Owns entry and component:DateTimeInput FR8) is new normative text in a draft record, including the new turn sentence, so it waits on an engineering owner's exact-head approval: any engineering owner, the author included, records it with /approve-spec <full head SHA>.
| return spinCurve( | ||
| from, | ||
| to, | ||
| picker.tapSpin.durationMs / 1000, |
There was a problem hiding this comment.
spec:AST-094 FR4: "A component MUST read touch motion through these roles and MUST NOT play a literal." The tap spin's 300 ms comes straight from picker.tapSpin.durationMs, which motion/uikit/picker.ts says no role registers, so a theme's tempo never reaches it. Registering it as an internal role, as timing.curves.move and scroll.bounce are, and reading it through resolveMotion would conform.
| this.strip.style.willChange = 'transform'; | ||
| promoted.push(this.strip); | ||
| animations.push( | ||
| this.strip.animate(keyframes, {duration: totalMs, easing: 'linear'}), |
There was a problem hiding this comment.
architecture:react-component-runtime INV13 names one animator for Core's touch behavior and fails "a component bundling its own copy of a module's primitive". This player bakes and plays its own Web Animations beside the shared animator; driving the rows through the shared animator, or an owner-approved INV13 exception, would resolve it.
| if (options.perEventPointer === true) { | ||
| this.perEventRecognizers.add(recognizer); | ||
| } else if (!this.isCoarse) { | ||
| this.warnFinePointer(); |
There was a problem hiding this comment.
On main, spec:AST-094 FR19 now reads "Recognizers attach for every pointer, with FR43's exceptions", DEC-21 rejects "keeping gestures and press tracking touch-only", and INV13 says "The gesture host attaches for every pointer, with no touch-only query". Here an attach under a fine pointer installs nothing and warns, with perEventPointer as the opt-in.
| page, | ||
| }, testInfo) => { | ||
| const {driver, minutes, strip, start} = await openWheel(page); | ||
| test.skip(!driver.scriptedTimestamps, SCRIPTED_RELEASE_SKIP); |
There was a problem hiding this comment.
FR5's verification row names "Chromium phone emulation; WebKit", and AST-094's Platform support asks for "the busy-main-thread proof in WebKit". This skip leaves the wheel's own player without that proof; a reproducible release velocity (or a programmatic coast of the same length) with a 1 s block inside it would cover WebKit.
…ng curve only when it is at most a frame longer than the run before it astracat found that the sub-step credit let pooledBusyProof pass a rAF loop that steps a fixed 1 or 2 px a frame: in each 70 ms block the cast still shows the loop's last steps, then it stands still and goes on one step a frame without catching up, so its run ended in a single step. With the previous busy.ts the self-test's frame-counted loop passed the pooled proof in 5 of 5 Chromium runs. A slowing curve lengthens its runs a frame at a time, so a run now counts as moved only when it is at most one frame longer than the component's run before it; a stall is longer than the runs before it. A judged part with no change reads the finest cast step, 0.5 px, so a larger jump never passes for one step. The self-test adds the frame-counted loop, at 1 and 2 px a frame, to the pooled proof's mutants: it fails every pool (Chromium 10 of 10, moved on 2 to 8 of 7 to 12 control frames; WebKit 10 of 10, 0 or 1 of 11 or 12). The timed rAF mutant can read as moved on one control frame, presented as the block starts, so its assertion takes 0 or 1 and also requires its stall to exceed the control's (Chromium showed 1 of 40 once). Red first: the frame-counted stall, the pooled frame-counted loop and the finest-step cases fail on the previous busy.ts. Lightbox's five busy proofs with this judgment: 30 of 30 in Chromium; 33 of 40 in WebKit with no miss, the other 7 presenting too few control frames to judge.
… long a frame takes The proof's guard compared the longest frame gap of two openings under CPU throttling, and the load on the machine running it moved that gap more than the wheel did: at one head the 200-year opening's worst gap ran from 266 to 516 ms, against 183 to 233 ms for two years, so the guard failed now and then on a busy machine. It now counts the year wheel's elements the opening writes or adds, from the tap until the wheel rests: 1 for two years and 19 for 200, the same on every run. A cost per year shows as 200 or more. Counting needs no CPU throttling, so the proof runs in WebKit too. It waits for the wheel to rest instead of a fixed 1.5 s, and the sheet's wait no longer swallows a failure.
…il cancelled, and its compositor run is blocked only once on the compositor C23 saw the control self-test fail under load at eight workers: "660 ms judged at 16.6 ms/frame: 28 frames presented, 0/28 moved". Its animations ran once, for 1,600 ms from before the screencast started, so on a loaded runner, where starting the cast takes long, the compositor's animation had ended before the block and nothing moved in the judged part. Both animations now go back and forth until cancelled, as the pooled self-test's do, and the compositor's block uses `during`, so it starts only once that animation runs on the compositor. Red first: with the setup stretched to 1.8 s, as a loaded runner takes, the previous test fails with exactly that read (0/39 moved) and this one passes. Normal runs at eight workers: 10 of 10 in Chromium and 10 of 10 in WebKit.
… control) into uikit/c21-wheel-proofs
…nst a compositor-only control [skip ci] The 1 s busy proof marked one row and asked that every frame the compositor presented show it moved. Two things made it fail on a busy machine. Late in the block the coast moves about a pixel a frame, under the screencast's step, so a frame delivered early shows no change; and over a whole second the coast travels further than the column shows, so the marked row was out of sight at one end, turned and faded at the other. It now follows markers that ride the strip itself, every three rows in three colours, placed before the flick so the block meets them already on screen. They move with the coast and never turn or fade, and each is judged while within reach of the band; together they cover the block. The judgment is busyWindowProof's against a compositor-only control recorded in the same frames (meta#8906): the coast moves on every frame the control moves, a run under one step that a single step ends counts as moved, and its longest stall is no longer than the control's.
|
Mapped to e075b06:
Open: the tap spin's duration as a role (FR4 names no picker role, and |
… into uikit/c21-wheel-uikit-look # Conflicts: # packages/core/src/DateInput/__tests__/Wheel.interaction.browser.spec.ts # packages/core/src/DateInput/__tests__/WheelActivation.interaction.browser.spec.ts
There was a problem hiding this comment.
Approval waits on an engineering owner’s decision about the nearest-enabled-row rest and the turn onto a clamped value that this stack writes into the draft component:DateInput record (FR13, DEC-1 and its Owns entry for the shared wheel) and the draft component:DateTimeInput record (FR8); any engineering owner, the author included, records that decision by commenting /approve-spec <full head SHA> on this pull request, and FR13’s turn should also say how it behaves under Reduce Motion. Whatever that decision, these still need changes:
- The compare from
mainis 36,204 authored lines for about 1,150 lines of proofs, so land or retarget onto the stack and #8906 beneath, and move the@astryxdesign/buildfix to its own pull request. wheelMotion.tsis a second compositor player beside the shared animator, whicharchitecture:react-component-runtimeINV13 rules out without an owner-approved exception.- The tap spin plays a literal 300 ms (
PickerWheel.tsx:225) instead of a motion role, againstspec:AST-094FR4. GestureHost.tsattaches only under a coarse primary pointer, whilespec:AST-094FR19, FR43 and DEC-21 ask for every pointer.- The WebKit leg of the busy-main-thread proof skips itself (
Wheel.interaction.browser.spec.ts:335), and FR5’s verification asks for it.
[Automated review]
| | FR10 | Standalone mode MUST compose Field; admitted InputGroup mode MUST remove competing Field chrome while preserving name, description, value, keyboard, focus, and editing semantics. | `family:input-fields/FR3`; current source and InputGroup tests | Settled shared rule and verified adoption | | ||
| | FR11 | Clear MUST emit `undefined` through the ordinary change pipeline. The pointer and bottom-sheet surfaces restore field focus using their shipped timing; the native surface MUST NOT reclaim focus because doing so reopens the OS picker. Implementation gap: when the field becomes disabled or busy while its clear control holds focus, the control unmounts and focus falls to the document body; `spec:AST-085/FR4` requires focus to stay or move to a destination this record names. | Current source and focused clear tests; `spec:AST-085/FR4` | Verified shipped behavior | | ||
| | FR12 | No surface renders native `disabled`. A disabled field MUST stay focusable with `aria-disabled`, block typing, picking, clearing, and opening, describe its disabled reason when one is given, and take disabled paint. While busy (`isLoading` or a pending Action) the pointer and bottom-sheet surfaces expose `aria-disabled` and `aria-busy`, stay focusable, and block new commits; the native surface does the same for `isLoading`, and during a pending Action stays editable so the OS picker remains attached but rejects additional commits until the Action settles. A busy field that is not disabled keeps its ordinary paint and Spinner, shows no hover ring while commits are blocked, and keeps its keyboard focus ring. | `spec:AST-085/FR1–FR6`; `family:input-fields/FR4–FR6`; `family:buttons/FR4`; current source and focused tests | Settled shared rule and verified adoption | | ||
| | FR13 | A bottom-sheet picker wheel that comes to rest on a disabled row (outside `min`/`max`) MUST continue to the nearest enabled row, a tie going toward the committed row, and select it there. A wheel resting on its committed row MUST NOT select or commit, even when that row is outside the bounds, so opening a bounded sheet never changes an untouched value: a committed value outside the bounds stays, and an empty field stays empty. A shown wheel whose value changes from outside, as when a bound clamps the time after another wheel turned, MUST turn onto the new row rather than jump to it. | DEC-1; UIDatePicker on the iOS 26.2 simulator; wheel unit and real-browser tests | Settled by DEC-1 | |
There was a problem hiding this comment.
FR13’s last sentence says a shown wheel MUST turn onto a value changed from outside rather than jump, and names no Reduce Motion form. design:reduced-motion DR3 asks a fine-pointer change to complete at once, and DR2 a coarse-pointer one to cross-fade in 200 ms or less; the code already does both through reducedMotionFade (PickerWheel.tsx:330-359, Wheel.tsx:344), so the requirement could say the turn follows DR2 and DR3 under Reduce Motion.
…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 #8159. Review only this PR's change: vjeux/astryx@fe003a4...276a6e9
Maintainer impact
The touch wheel (#8159) moves through a gesture recognizer and compositor animations, and the unit tests' jsdom has neither. Without browser proofs, CI would pass a wheel that freezes when the page is busy, drifts from UIKit's curve, can't be caught mid-coast, takes the wrong Reduce Motion path, or shows the touch wheel under a mouse.
Evidence
A flick's first 400 ms of coast, one frame per 16.7 ms (Chromium, touch):
A flick in Mobile Safari (iOS 26.2 simulator) through a 530 ms block, and on Android 15 (Chrome 124):
Opening the month and year wheels (Chromium, 4× CPU, 390 × 844, worst frame after the tap, 8 runs):
Intended invariant
What led to it: Astryx's UIKit-parity work (
spec:AST-094) makes the wheel behave like iOS, and these proofs keep it that way: motion runs on the compositor through a busy main thread (FR5), a finger catches any motion (FR6), and under Reduce Motion a flick's coast plays while every other turn cross-fades (FR8,design:reduced-motionDR2). Each year is drawn once (architecture:component-style-authoringINV7).Change
Options: jsdom tests alone are fast but can't see the compositor; screenshot diffs catch looks, not timing. Chosen: browser specs with real touch input in Chromium and WebKit, judging the compositor's own frames. The cost is about a minute of CI, and some cases run only in Chromium, which alone exposes the rendering trace.
Wheel.interaction.browser.spec.tsdrives a 60-row minute wheel (theInternal/Wheelstory): a flick coasts onto its row; the coast advances every frame through a block, with no layout or paint per frame; a finger stops it; Reduce Motion fades a tap in 200 ms or less and lands it through a 700 ms block; a mouse drag scrolls today's column.WheelActivation.interaction.browser.spec.ts, on DateInput's 2- and 200-year stories: every year listed once, and the long range opens about as fast as the short one at 4× CPU throttling.Scope
Testing
pnpm exec playwright test --config playwright.interaction.config.tson the two specs. In Chromium all 11 pass; in WebKit 6 pass and 5 skip (the Chromium-only trace and block cases, and the two mouse cases).Merge order: #8156, #8526, #8159, then this PR (full order in #8156).