Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
github-actions
Bot
added
the
needs:design-review
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
This review is on hold for a spec decision: whether the month and year wheels in the DateInput and DateTimeInput bottom sheets should change the chosen date, as iOS's inline date picker does, has no approved spec yet (component:DateInput and component:DateTimeInput are draft, and the pull request marks it as pending owner review), and the new draft spec:AST-094 would also amend the current spec:AST-025 FR14 and design:user-states DR1. 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. Independently of that decision, these changes are needed before approval:
- Reduce this pull request's diff to its own commit: it currently carries the whole stack, about 36,700 changed lines, so land the predecessors first or base the branch on #8206.
- In DateTimeInput, the stepped time wheels round 23:59 to 12:00 AM without moving the date, so a turn of the minute wheel commits a time about 23 hours earlier on the same day.
spec:AST-094FR7 citesspec:AST-090, which does not exist in the repository, and the newdateConstraintsrefusal on the wheel path has no test.
[Automated review]
| } | ||
| const minutes = time.hour * 60 + time.minute + time.second / 60; | ||
| const rounded = Math.round(minutes / interval) * interval; | ||
| return partsOf(rounded * 60); |
There was a problem hiding this comment.
In DateTimeInput this wrap drops a day: with timeIncrement={15}, a value of 2026-03-21T23:59 shows 12:00 AM, and turning the minute wheel to :45 commits 2026-03-21T00:45, because commitWheelTime keeps the date. Rounding down near midnight in the DateTimeInput sheet, or carrying the rounding into the date, avoids it; a test at 23:59 with a 15-minute step would pin it.
| 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.
spec:AST-090 does not exist at this head or on main, so the departure this requirement asks owners to accept cannot be checked. Drop the reference, or cite the record where it lives.
There was a problem hiding this comment.
This branch no longer edits the AST-094 record (#8378 on main carries it).
| min != null ? plainDateFromISO(min) : null, | ||
| max != null ? plainDateFromISO(max) : null, | ||
| ); | ||
| if (!plainDateIsEqual(moved, selectedDate) && !isDateDisabled(moved)) { |
There was a problem hiding this comment.
This refusal branch has no test in either sheet (TouchDateTimeField.tsx:834 too). A test where dateConstraints refuses the moved date, asserting that onChange is not called while the calendar still moves, would protect it.
There was a problem hiding this comment.
Added: both sheets test a refused date (May 21); head 0c6ff10.
ba61d2a to
e8f8953
Compare
There was a problem hiding this comment.
This review is on hold for a spec decision: whether the month and year wheels in the DateInput and DateTimeInput bottom sheets should change the chosen date, as iOS's inline date picker does, has no approved spec yet (component:DateInput and component:DateTimeInput are draft), and the new draft spec:AST-094 would also amend the current spec:AST-025 FR14 and design:user-states DR1. 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. Independently of that decision, these changes are needed before approval:
- Reduce this pull request's diff to its own commit: it still carries the whole stack, about 36,500 changed lines, so land the predecessors first or base the branch on #8206.
- In DateTimeInput, the stepped time wheels round 23:59 to 12:00 AM without moving the date, so a turn of the minute wheel commits a time about 23 hours earlier on the same day.
spec:AST-094FR7 citesspec:AST-090, which does not exist in the repository, and the newdateConstraintsrefusal on the wheel path has no test.
[Automated review]
| } | ||
| const minutes = time.hour * 60 + time.minute + time.second / 60; | ||
| const rounded = Math.round(minutes / interval) * interval; | ||
| return partsOf(rounded * 60); |
There was a problem hiding this comment.
In DateTimeInput this wrap drops a day: with timeIncrement={15}, a value of 2026-03-21T23:59 shows 12:00 AM, and turning the minute wheel to :45 commits 2026-03-21T00:45, because commitWheelTime keeps the date. Rounding down near midnight in the DateTimeInput sheet, or carrying the rounding into the date, avoids it; a test at 23:59 with a 15-minute step would pin it.
| 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.
spec:AST-090 does not exist at this head or on main, so the departure this requirement asks owners to accept cannot be checked. Drop the reference, or cite the record where it lives.
There was a problem hiding this comment.
This branch no longer edits the AST-094 record.
| min != null ? plainDateFromISO(min) : null, | ||
| max != null ? plainDateFromISO(max) : null, | ||
| ); | ||
| if (!plainDateIsEqual(moved, selectedDate) && !isDateDisabled(moved)) { |
There was a problem hiding this comment.
This refusal branch has no test in either sheet (TouchDateTimeField.tsx:834 too). A test where dateConstraints refuses the moved date, asserting that onChange is not called while the calendar still moves, would protect it.
There was a problem hiding this comment.
Added: both sheets test a refused date; head 0c6ff10.
|
|
||
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); |
There was a problem hiding this comment.
goTo reports synchronously through the caller's onIndexChange, so this Effect echoes a controlled move back to the owner, the Effect-driven callback the React runtime rules forbid, and its comment names no system it synchronizes. No caller passes index yet; skipping the report for a controlled move, or dropping the unused path, keeps it from biting later.
e8f8953 to
4f517fd
Compare
4f517fd to
a05d1b1
Compare
There was a problem hiding this comment.
This review is on hold for a spec decision: whether the month and year wheels in the DateInput and DateTimeInput bottom sheets should change the chosen date, as iOS's inline date picker does, has no approved spec yet (component:DateInput and component:DateTimeInput are draft), and draft spec:AST-094 in this range would also amend the current spec:AST-025 FR14 and design:user-states DR1. 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. Independently of that decision, these changes are needed before approval:
- Reduce this pull request's diff to its own commit: it still carries the whole stack, about 37,200 changed lines, so land the predecessors first or base the branch on #8206.
- In DateTimeInput, the stepped time wheels round 23:59 to 12:00 AM without moving the date, so a turn of the minute wheel commits a time about 23 hours earlier on the same day.
spec:AST-094FR7 citesspec:AST-090, which does not exist in the repository, and thedateConstraintsrefusal on the new wheel path has no test in either sheet.
[Automated review]
| return hasSeconds ? time : {...time, second: 0}; | ||
| } | ||
| const minutes = time.hour * 60 + time.minute + time.second / 60; | ||
| const rounded = Math.round(minutes / interval) * interval; |
There was a problem hiding this comment.
In DateTimeInput this wrap drops a day: with timeIncrement={15}, a value of 2026-03-21T23:59 shows 12:00 AM, and turning the minute wheel to :45 commits 2026-03-21T00:45, because restOn turns from the shown time and commitWheelTime keeps the date (TouchDateTimeField.tsx:832-844). Rounding down at the end of the day, or carrying the rounding into the date, avoids it; a test at 23:59 with a 15-minute step would pin it.
| 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.
spec:AST-090 does not exist at this head or on main, so the departure this requirement asks owners to accept cannot be checked. Drop the reference, or cite the record where it lives once it lands (the current-state impact at lines 234-236 names it too).
| min != null ? plainDateFromISO(min) : null, | ||
| max != null ? plainDateFromISO(max) : null, | ||
| ); | ||
| if (!plainDateIsEqual(moved, selectedDate) && !isDateDisabled(moved)) { |
There was a problem hiding this comment.
This refusal branch has no test in either sheet (TouchDateTimeField.tsx:825 too). A test where dateConstraints refuses the moved date, asserting that onChange is not called while the calendar still moves, would cover the promise that a refused date never reaches the caller.
There was a problem hiding this comment.
Added in 278a61a: both sheets have a test where dateConstraints refuses the moved date (May 21). The date stays unchosen while the calendar moves.
| useEffect(() => { | ||
| if (index != null && index !== pager.index) { | ||
| pager.goTo(index, {reason: 'controlled'}); | ||
| } | ||
| }, [pager, index]); |
There was a problem hiding this comment.
goTo reports synchronously through the caller's onIndexChange, so this Effect echoes a controlled move back to the owner, the Effect-driven callback the React component runtime contract rules out, and its comment names no external system. Naming the pager engine and skipping the report for a controlled move, or dropping the unused controlled path, would fix it.
a05d1b1 to
246ee17
Compare
There was a problem hiding this comment.
This pull request is on hold for a spec decision: no approved spec says whether the month and year wheels in the DateInput and DateTimeInput bottom sheets should change the chosen date (component:DateInput and component:DateTimeInput are draft), and the draft spec:AST-094 carried in this range would also amend current scrolling and press-state specs. 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. Independently of that decision, these changes are needed before approval:
- Reduce the diff to this pull request's own commit: it still carries the whole stack, about 37,500 changed lines, so land the predecessors first or base the branch on #8206.
- In the DateTimeInput sheet, a stepped minute wheel rounds 23:59 to 12:00 AM without moving the date, so turning it commits a time about 23 hours earlier on the same day (
timeWheelRules.ts,TouchDateTimeField.tsx). spec:AST-094FR7 citesspec:AST-090, which does not exist; thedateConstraintsrefusal on the new wheel path has no test in either sheet; andusePager's controlled-index Effect callsonIndexChangefrom an Effect.
[Automated review]
246ee17 to
278a61a
Compare
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision on two touch behaviors that no spec states, and it needs changes either way.
Owner decision needed: under a finger, the sheet wheels now let Arrow, Page, Home and End skip disabled rows (on TimeInput's 12-hour hour wheel with bounds 12:00 to 17:00, ArrowDown from 5 PM commits 12:00 PM, where a mouse and main commit nothing), and the band's accent mark leaves every row while a wheel coasts, snaps or turns. component:DateInput owns how a sheet wheel steps and looks, so an engineering owner settles this by answering, or by having that record state the rule and commenting /approve-spec <full head SHA>; the author may do so as an engineering owner.
Required changes:
- The range against
mainis 9,674 authored lines, over the 8,000-line batch ceiling, so land the rest-rule, UIPickerView, UIDatePicker-table and AM/PM commits separately and rebase onto the 610-line month/year change. timeWheelRules.ts(overlaps,minuteRows,secondRows,meridiemRows) copies the row and bounds rules thatTouchTimeField.tsxandTouchDateTimeField.tsxalready implement for the mouse columns, so build those options from one shared helper and delete therangeOverlapscopies.snapCurveinwheelPhysics.tsplayspicker.snapRate, which no motion role carries, againstspec:AST-094FR4, so make it a role or amend FR4.- The wheel vibrates as rows reach the band on drags and coasts (
wheelStrip.ts), but no record states the iOS Safari gap thatspec:AST-094FR23 asks the component to record. component:DateInputDEC-2 now says the finger and mouse columns look the same and tip back, then says the finger rows do not tip, and its non-goal line still says they are the same (it also names a branch, "main's"), so state the two looks once.PickerWheel.tsxattaches the strip to the animator from a layout Effect; a callback ref on the strip owns attach, detach and replacement (architecture:react-component-runtimeINV6).
The month/year change itself works as described: on a phone, one tap on February moves January 25 to February 25 while a mouse still moves only the calendar.
[Automated review]
| } | ||
|
|
||
| /** Whether [start, end] (seconds) overlaps the bounds. */ | ||
| function overlaps(start: number, end: number, bounds: TimeBounds): boolean { |
There was a problem hiding this comment.
This bounds overlap, and the minute, second and AM/PM row rules below (minuteRows, secondRows, meridiemRows), repeat what TouchTimeField.tsx (rangeOverlaps and its option memos) and TouchDateTimeField.tsx already compute for the mouse columns. A change to which rows a bound disables would then reach only one pointer's users. Have the mouse columns build their minute, second and AM/PM options from these functions, and delete the two rangeOverlaps copies.
| */ | ||
| export function snapCurve(from: number, to: number, scale = 1): WheelCurve { | ||
| const distance = from - to; | ||
| const rate = picker.snapRate * 1000; |
There was a problem hiding this comment.
The snap at rest plays picker.snapRate directly, and no motion role carries it (registerPickerTable registers only tapSpin.durationMs). spec:AST-094 FR4 says a component "MUST read touch motion through these roles and MUST NOT play a literal". Express the snap as a role, or amend FR4 on main to put the picker snap on its fixed list.
| return; | ||
| } | ||
| lastTick = time; | ||
| playHaptic('selection'); |
There was a problem hiding this comment.
This selection tick plays on drag frames and during the coast. spec:AST-094 FR23 says such a mid-drag moment has no haptic in iOS Safari "and the component records that gap". No DateInput, TimeInput or DateTimeInput record mentions it yet. One Current-state line in component:DateInput would cover it.
There was a problem hiding this comment.
| Every sheet wheel in DateInput, TimeInput, and DateTimeInput looks the same under | ||
| a finger as under a mouse: one flat scrolling column whose rows tip back and fade | ||
| away from the committed row. | ||
| away from the committed row. Under a finger the rows fade without tipping or |
There was a problem hiding this comment.
This sentence contradicts the one before it ("looks the same under a finger as under a mouse: one flat scrolling column whose rows tip back") and the non-goal at lines 122-123, which still says the finger column is the same as the mouse one. "main's" also names a branch. Suggest stating the two looks once: under a finger the rows fade without tipping; under a mouse they also tip back on the scroll timeline.
| ); | ||
|
|
||
| // Hand the strip to the animator, and the rows' layout to the motion. | ||
| useIsomorphicLayoutEffect(() => { |
There was a problem hiding this comment.
Handing the strip to the animator is work caused by the element attaching. architecture:react-component-runtime INV6 gives that to a callback ref, which also handles null detachment and replacement. The animator expects React's replacement order, attach(null) and then the new node (motion/animator.ts:3075-3088); this Effect never detaches and re-attaches on every layout change. A callback ref on the strip that calls motion.attach(node) would do it, and setLayout can stay keyed on layout.
| isDisabled?: boolean; | ||
| } | ||
|
|
||
| /** Options for a programmatic turn. */ |
There was a problem hiding this comment.
Non-blocking: this doc comment has nothing under it (the next line is WheelHandle's own comment). Suggest deleting it.
d3bc5f0 to
4832d6e
Compare
|
Owner decision. The spec owners ruled on both touch behaviors (Nov 2):
#8159 records both in Size. This pull request's own change is 580 lines (vjeux/astryx@ed7552d...4832d6e). It stacks on #8179 in the B4 order and rebases onto |
There was a problem hiding this comment.
This pull request is on hold for an engineering owner's decision on the wheel records, and it needs changes either way.
Owner decision needed: component:DateInput FR17 steps a 12-hour hour wheel in time order (at a 12:00–17:00 bound, ArrowDown from 5 PM commits nothing and from 12 PM commits 1 PM), while the Nov 2 ruling quoted on the pull request says a key moves to "the next enabled row in the key's direction", which from 5 PM is the 12 row. DEC-2's flat rows under a finger, component:TimeInput FR7 and the other record edits were approved only at an earlier head. An engineering owner records the decision by commenting /approve-spec <full head SHA> or by amending the records on main; the author may do so as an engineering owner.
Required changes:
- The range against
mainis 10,674 authored lines around this pull request's own 610-line change, over the owner-directed batch ceiling of 8,000, and it conflicts withmaininmotion-roles.mdandmotion/uikit/index.ts, so land the stacked wheel commits first and rebase onto the two own commits. snapCurveinwheelPhysics.tsplayspicker.snapRate, a literal rather than a motion role, againstspec:AST-094FR4, so register the snap as a role or have FR4 record it as a gap, as it does for the pager.- The 12-hour conversion now lives in three copies (
hour12Ofand the inline 12-to-24 mapping intimeWheelRules.ts, andhour12From24/hour24From12inTouchTimeField.tsxandTouchDateTimeField.tsx), so export one pair fromtimeWheelRules.tsand call it from both sheets.
The month/year change itself works as described: on a phone, a tap on February moves January 25 to February 25, and a mouse still moves only the calendar.
[Automated review]
| */ | ||
| export function snapCurve(from: number, to: number, scale = 1): WheelCurve { | ||
| const distance = from - to; | ||
| const rate = picker.snapRate * 1000; |
There was a problem hiding this comment.
picker.snapRate is a literal, not a motion role: spec:AST-094 FR4 says a component "MUST read touch motion through these roles and MUST NOT play a literal", so a theme's motion knobs never reach this snap. Register it as a role, or have FR4 name the picker snap in its implementation gaps, as it does for the pager.
| } | ||
|
|
||
| /** The 12-hour clock's label for a 24-hour hour: 0 → 12, 13 → 1. */ | ||
| export function hour12Of(hour: number): number { |
There was a problem hiding this comment.
hour12Of is the same conversion as hour12From24 in TouchTimeField.tsx:153 and TouchDateTimeField.tsx:420, and :228 and :306 repeat their hour24From12. Export one pair from here and call it from both sheets' composeWheelTime, so a fix reaches the mouse columns too.
| const RUBBER_BAND_DIMENSION = picker.geometry.rubberBandDimension * UIKIT_SCALE; | ||
|
|
||
| const styles = stylex.create({ | ||
| // Today's column (Wheel.tsx): the same box, band and rows, moved by the |
There was a problem hiding this comment.
Non-blocking: "Today's column" names the scrolling column by a date. Suggested: "The scrolling column (Wheel.tsx): the same box, band and rows, moved by the gesture system and the compositor." The same applies at :95 and useWheelListbox.ts:44.
|
/approve-spec 0a1769a |
|
This head. Rebased onto Size (INV6), main cae1f66 → this head: DateInput 103.0 → 144.3 KB, DateTimeInput 114.9 → 156.4 KB, PowerSearch 183.6 → 225.1 KB, MobilePowerSearch 168.5 → 209.7 KB, TimeInput 90.1 → 130.5 KB, almost all of it #8159's shared touch runtime. This pull request's own part, #8179 → this head: +0.1 to +0.2 KB for DateInput, DateTimeInput, PowerSearch and MobilePowerSearch; CSS unchanged. Lines. This pull request's own change is 580 authored lines, 462 of them tests (vjeux/astryx@986b0bb...0a1769a). It merges after #8159, #8178 and #8179 in B4's order. |
There was a problem hiding this comment.
Changes requested. The month/year wheel behaviour itself is owner-approved and its tests pass; two things need fixing before this can merge.
- This range carries 10,037 authored lines from 17 commits around this pull request's own 610-line change; please land #8159 and #8179 first and rebase so this pull request contains only its two commits.
- Two changesets present behaviour that already shipped as new:
.changeset/wheel-uikit-picker.mdsays a value changed from outside "turns the wheel onto its row instead of jumping there" (the shown wheel already turned onto such a value), and the second bullet of.changeset/time-wheels-follow-uidatepicker.mdlists the rest pastmin/max, the other columns turning, and opening never changing the value, all of which already worked; please reword them to describe only what changes.
Non-blocking: .changeset/wheel-interaction-proofs.md calls the mouse column "today's column"; "the scrolling column" reads better in release notes.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a coarse primary pointer, as on a phone or a tablet, the picker wheels in the TimeInput, DateTimeInput and DateInput bottom sheets now move like UIPickerView. Their rows keep the 28 px pitch and lie flat, fading toward the edges of the column in every browser; the scrolling column under a mouse tilts and fades its rows only where the browser supports scroll-driven animations, and shows a plain list elsewhere. The row under the band is in semibold accent text on its plate while the wheel rests and while a finger drags it; no row is marked while the wheel coasts, snaps or turns. 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, and a value changed from outside while the sheet is open, as when a bound clamps the time, turns the wheel onto its row instead of jumping there. 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; 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. Each row that reaches the band ticks with a short vibration where the browser supports it, as on Android; iOS Safari has no vibration. Under a fine primary pointer, as with a mouse on a desktop, the wheels keep the scrolling column. |
There was a problem hiding this comment.
A shown wheel already turns onto a value changed from outside on main (Wheel.tsx's park effect smooth-scrolls to it, as component:DateInput FR14 requires). Please drop "instead of jumping there" so the release note does not announce a fix for something that worked.
| [fix] Under a finger, the time wheels in the TimeInput and DateTimeInput bottom sheets now follow iOS's date picker: | ||
|
|
||
| - The AM/PM column follows the hour. Turning the hour from 11 to 12 now gives 12 PM, not 12 AM, and the AM/PM column turns onto PM. From 12 PM to 1 it stays PM. | ||
| - A wheel that comes to rest past `min` or `max` goes on to the nearest time inside them and commits it there, and the other columns turn onto it. Opening the sheet never changes the value: a time outside the bounds stays as it is, and an empty field stays empty. |
There was a problem hiding this comment.
Everything in this bullet already worked under a finger on main: the rest goes on to the nearest enabled row, the composed time is clamped into the bounds, the other columns turn onto it (FR14), and opening commits nothing (FR13). Please keep only the AM/PM bullet as the change, or mark this one "as before".
There was a problem hiding this comment.
Approval waits on an engineering owner's decision on this head's spec edits, and the size of the range needs fixing either way.
Owner decision needed: the edits to component:DateInput, component:DateTimeInput, component:TimeInput and spec:AST-094 were approved only at the previous head, and a new commit cancels that approval. This head also narrows DateInput FR14's tap-spin sentence and FR18 to "Under a finger", and changes what the exported clampTime returns for a bound between whole minutes, which no record covers for direct callers. An engineering owner records the decision by commenting /approve-spec <full head SHA> on the pull request (the author may do so as an engineering owner), or by amending the records.
Blocking:
- The pull request carries 10,109 authored lines, above the 8,000-line batch ceiling, around its own 612-line change; land #8159, #8178 and #8179, then rebase so this pull request is its own two commits.
The two changesets flagged last time now describe only what changes. Four new comments restate the code under them; see the inline notes (non-blocking).
[Automated review]
| (next: number) => { | ||
| setMonthIndex(next); | ||
| scrollerHandleRef.current?.scrollToMonth(next, 'auto'); | ||
| // Only the finger's wheels move the date; the scrolling ones move the calendar. |
There was a problem hiding this comment.
Non-blocking: the block comment above already says a mouse moves only the calendar, and this line restates the if below it. Suggest dropping it.
| (next: number) => { | ||
| setMonthIndex(next); | ||
| scrollerHandleRef.current?.scrollToMonth(next, 'auto'); | ||
| // Only the finger's wheels move the date; the scrolling ones move the calendar. |
There was a problem hiding this comment.
Non-blocking: same as in TouchDateField.tsx; the block comment above covers it. Suggest dropping this line.
| ); | ||
| const [activePanel, setActivePanel] = useState<TouchDateTimePanel>('date'); | ||
| const [isWheelOpen, setIsWheelOpen] = useState(false); | ||
| // A finger gets UIDatePicker's time wheels; a mouse gets the scrolling columns. |
There was a problem hiding this comment.
Non-blocking: this restates the isTouch ternary below. Suggest dropping it, or keeping only why the two pointers get different wheels.
| const mergedInputRef = useMergedRefs(ref, inputRef); | ||
| const inputGroup = useInputGroup(); | ||
| const [isSheetOpen, setIsSheetOpen] = useState(false); | ||
| // A finger gets UIDatePicker's wheels; a mouse gets the scrolling columns. |
There was a problem hiding this comment.
Non-blocking: this restates the isTouch ternary below. Suggest dropping it, or keeping only why the two pointers get different wheels.
|
Size (INV6), main cae1f66 → c5f6df1, gzip. JavaScript: DateInput 103.0 → 144.3 KB, DateTimeInput 114.9 → 156.4 KB, PowerSearch 183.6 → 225.1 KB, MobilePowerSearch 168.5 → 209.6 KB, TimeInput 90.1 → 130.5 KB. CSS: TimeInput +73 B, DateInput +51 B, DateTimeInput +49 B, PowerSearch +47 B, MobilePowerSearch +0 B, all from #8159's wheel styles. Almost all of it is #8159's shared touch runtime, which a page pays once (spec owners, Nov 2). This pull request's own part, #8179 → this head: +0.1 to +0.2 KB for DateInput, DateTimeInput, PowerSearch and MobilePowerSearch; CSS unchanged. Lines. After #8159, #8178 and #8179 land, this pull request is its own two commits. |
Under a coarse primary pointer, the TimeInput, DateTimeInput and DateInput sheet wheels move like UIPickerView: a flick coasts at UIKit's deceleration onto the row UIKit's projection picks, a finger landing on a moving wheel stops it where it is drawn, a tap on a row turns it onto the band, and past the first or last row a drag rubber-bands and bounces back. The value commits when the wheel comes to rest, by the rest rule (component:DateInput FR13), and the coast keeps running while the main thread is busy (spec:AST-094 FR5): one strip moves the rows on the shared animator, with no Layout or Paint per frame. A value changed from outside while the sheet is open turns the wheel onto its row instead of jumping there (component:DateInput FR14); a second change during that turn turns on from where the wheel is drawn. The rows lie flat (component:DateInput DEC-2): today's 28 px pitch, fading toward the column's edges without tipping or shrinking. Under a mouse the scrolling column and its tilt are unchanged. Under Reduce Motion a flick still coasts, as scrolling does; a tap, a key or a slow release cross-fades onto its row (design:reduced-motion DR2). useOwnScrollGesture now also keeps the pointer a touch lands with, for a scroller that pans through the gesture system, so the wheel adds no listener of its own for the sheet's pull-down (architecture:react-component-runtime INV16). The touch-support bundle check lists DateInput, TimeInput and DateTimeInput as gesture users, with PowerSearch and Table, whose value editors reach TimeInput's wheels (architecture:react-component-runtime INV13). The wheel's proof stories sit under Core/Wheel. The tap spin's 300 ms is an internal motion role, `picker.tapSpin.durationMs` (spec:AST-094 FR4), which every turn reads under its column; the snap at rest keeps UIKit's measured rate as a constant, because a motion role is a spring, a duration or a curve. The record: DEC-2 states that under a finger the rows fade without tipping or shrinking; FR14 names the tap spin's role and FR13 records the snap rate's FR4 gap; the FR13 and FR14 verification rows cite the touch wheel's cases; FR15's gap sentence holds for both pointers. The generated motion role table lists the new role. Combines the wheel's values and math, its browser proofs, one listbox hook for both presentations, and the owner's rest rule and spin; looping stays out until a record covers it.
The flick proof fitted the coast against UIKit's plan for velocities around the scripted 900 px/s. In WebKit the page reads the release velocity from the timestamps of the inputs it receives, and WebKit stamps each input as it dispatches it, in whole milliseconds. Under load the intervals of the 120 Hz script jitter from 3 to 17 ms while each step stays 7 or 8 px, so one interval reads anywhere from 580 to 1,750 px/s, and a short last interval lifts the release past twice the script. The coast then followed UIKit's plan for that faster release, but no fit in the scripted range matched it. The proof now records the inputs a finger's pan reads (its touchstart, one sample per touch pointermove, and its touchend) on the host's clock, holds each time at the latest as the gesture arena does, and runs the pan's own velocity tracker over them. The whole coast must follow UIKit's plan for that velocity, within the same 0.75 px. In Chromium, which replays the script's timestamps, the measured velocity must also be within 5% of the script.
…ch turns it one row component:DateInput FR15: under a coarse primary pointer, the sheet wheels' pan takes a trackpad's two-finger scroll, as spec:AST-094 FR43 allows a record to grant (the pan allows `continuous` scroll input): the rows follow the fingers, coast on at the lift with their last speed, and rest on a row by the rest rule (FR13). The platform's momentum after the lift never moves the wheel. A mouse wheel's notch turns the wheel one row, as an arrow key does, with no coast. The gesture host leaves a mouse wheel's notches to what is under the pointer; the wheel steps on a wheel the host reads as a notch (`scrollTypeOf`, now exported, so the reading has one owner) and leaves a wheel the pan took, a ctrl-wheel (the page's zoom) and a sideways scroll alone. The wheel's own pixel scroll handling goes. The record's FR15 loses its implementation and evidence gaps and its current-state line; its verification row cites the unit cases and the browser proofs (notches in Chromium; the trackpad flick and slow stopped drag in WebKit, whose driver sends a trackpad's precise deltas, on the touch column opened alone in a desktop context). Red first: the notch, three-notch, line-mode and trackpad flick cases fail on the wheel's previous pixel scrolling; the flick rests 80 px past the lift there, 426 px with this change.
…, which nothing here calls `spinTo` on a Wheel's ref (`WheelHandle`) and the `resolveRest` prop have no caller in this change: the time wheels that turn a column from script land with TimeInput's sheet, and no column passes a rest rule. Both go, with their tests and Changeset; the touch wheel always rests on the nearest enabled row (`component:DateInput` FR13), and today's column is Wheel's fine-pointer branch again, with no handle around it.
…eet wheels look like and tick The strip reaches the animator from a callback ref: attaching is work the element's attaching causes, and the ref lets the animator go as the strip detaches or is replaced (architecture:react-component-runtime INV6). The motion's layout still follows the rows in an Effect. component:DateInput DEC-2 now says one thing: under a finger the rows lie flat at the scrolling column's pitch and fade toward its edges without tipping or shrinking, and under a mouse the scrolling column tips and fades its rows on a scroll timeline; its title and the cylinder non-goal match. Current-state impact records that the selection tick vibrates where the browser has the Vibration API, as on Android, and is silent in iOS Safari (spec:AST-094 FR23). The Changeset names the vibration and says the wheels keep today's column under a fine primary pointer; the wide-range story no longer says rows tip.
… one, under a finger and a mouse The spec owners' rulings, recorded in component:DateInput: - FR17: in a sheet wheel, Arrow, Page, Home and End move to the next enabled row in the key's direction and stop at the last enabled row; a key with no enabled row in that direction commits nothing, under every presentation. The touch wheel already did; the scrolling column under a mouse stopped at a disabled row instead. Both now share one rule (`steppedIndex` and `endIndex` in wheelOptions.ts). The new cases at a 12:00–17:00 bound and across a disabled row fail for the mouse column before this change. - FR18: while a wheel coasts, snaps or turns, no row carries the selection band's accent mark; the mark returns on the row that comes to rest, as UIPickerView does (spec:AST-094 FR3). A new case covers the snap and a key's turn beside the fling's coast.
…M comes before 1 PM The spec owners' FR17 for component:DateInput, verbatim: in a sheet wheel, Arrow, Page, Home and End move through the wheel's enabled values in time order, skipping disabled ones, and stop at the first or last enabled value; a key with no enabled value in its direction commits nothing, under every presentation. On a 12-hour hour wheel, time order follows the full time (12 PM comes before 1 PM), not the column's row order. A wheel option may now say where it falls when a key steps the wheel (`order`); without it, rows step in list order. The 12-hour hour columns of TimeInput's and DateTimeInput's sheets set the full hour, so at a 12:00–17:00 bound ArrowDown and End from 5 PM commit nothing, and ArrowDown from 12 PM commits 1 PM, under a finger and a mouse. Before this change both walked row order: ArrowDown from 5 PM skipped 6 to 11 and committed 12 PM. The field-level cases and the Wheel cases on a 12-hour column fail without it. The mouse column's change is a defect fix: a key could not reach 12 PM there, against WCAG 2.1.1.
Comments, the story and two Changesets said "today's column" for the snap-scrolling column under a mouse, which names nothing once this lands. They now say the scrolling column. The listbox hook's Changeset no longer claims that no behavior changes: this pull request changes how keys step.
…h on its row Under a finger, a key stopped the wheel's motion before it asked where to go. With no allowed row in its direction, as a second ArrowDown during the turn onto the last allowed row finds, the key then did nothing more: the wheel stayed between rows while the field showed the committed value. The key now finds its row first and leaves the turn alone when there is none. The new case fails before this change, 21.5 px off its row.
… shows The column faded its rows with a mask on itself, the focusable listbox. A mask clips everything the element paints, its outline included, so a wheel focused from the keyboard under a finger matched :focus-visible and showed no ring (architecture:interaction-modality INV1). The fade moves to an inner layer around the band and the strip; the rows fade as before. A jsdom case checks that layer, and a Chromium case checks the focused column's computed mask and outline and the fade layer's gradient. Two comments still said "Today's column" and "Today's selection band"; they now name the scrolling column and the selection band.
…ementation gap The spec owners' ruling on the sheet picker wheel's snap rate: it is an implementation gap of spec:AST-094 FR4, in the record's own words. "The sheet picker wheel's snap rate (UIKit's measured picker deceleration) plays from an internal constant and is not yet a named role; a role kind for rates comes with a later change." The gap now lives only in FR4, so component:DateInput FR13 and its Current-state line no longer state it.
The date picker domain of the UIKit constants table, each rule sourced: the month/year wheels change the date, keeping the day; the AM/PM column follows the hour; a column that comes to rest past a bound turns back onto it; and a day the month lacks clamps to its last day. The date and time touch sheets follow these rules. Nothing is exported from the package entry point.
…heel The mouse column turns by the browser's smooth scroll and marks each row as it scrolls, so FR14's tap-spin sentence and FR18 start with "Under a finger". The record's scope line lists FR17 and FR18. Two comments that restated the function below them are gone, and two changesets no longer present behavior that already shipped as new.
The time wheels turn a column by themselves when another column's change moves its value, so the Wheel offers that turn on its ref: `spinTo(index)` (`WheelHandle`). Under a finger the wheel turns onto the row on the tap spin, then commits it as a rest would; under a fine pointer the scrolling column commits the row at once, and the new value scrolls it there. Each reports whether the row was committed: a disabled row or an index outside the rows resolves false at once, and a finger, another turn or the wheel's unmounting cutting the turn short resolves false.
the hour, as UIDatePicker's does Under a finger the TimeInput and DateTimeInput sheets render TimeWheels, through the pickers' touch layer (datePickerTouch.ts). The half of the day changes only when the hour's move passes between 11 and 12: 11 AM turned to 12 gives 12 PM, and the AM/PM column turns onto the new half on the wheel's own spin (component:DateInput FR14). The hour column does not loop, so 12 counts as next to 1: 12 PM turned to 1 stays 1 PM, and 1 PM turned to 9 stays 9 PM. A column that comes to rest past min or max goes on to the nearest time inside them and commits it there, the other columns turning onto it; an untouched time outside the bounds stays as it is. A turn the wheels start by themselves is not a person's rest: its arrival commits nothing, even when the owner kept the old time. With a mouse the sheets keep today's independent columns. The columns do not loop, and the minute wheel lists every minute.
…column TimeInput FR7 records the rule the spec owners asked for at the B4 head. On the non-looping hour column, 12 counts as next to 1. Moving between 12 and 1–6 keeps the half of the day; any other move flips it when it passes between 11 and 12. It cites DateInput FR14's turn-onto rule, and the FR7 verification row names the tests.
…ove the date like UIDatePicker's Under a finger, turning the month or year wheel behind the sheet calendar's title moves a chosen date to the wheels' month and keeps its day: October 15 becomes May 15, and a day the month lacks becomes its last day (January 31 becomes February 28), inside min and max; DateTimeInput keeps the time. A date dateConstraints refuses is not chosen, and with no date chosen the wheels move only the calendar. With a mouse the wheels move the calendar only, as today. The month wheel does not loop.
DateInput FR16 records that under a coarse primary pointer the sheet calendar's month and year wheels move a chosen date to their month and keep its day. A day the month lacks becomes its last day (January 31 becomes February 28), and the date stays within min and max. DateTimeInput FR10 does the same and keeps the time. Each has a verification row.
|
/approve-spec d060a1c |
There was a problem hiding this comment.
Changes requested. The month and year wheel behaviour and every record edit are approved by an engineering owner at this head, and the unit tests pass; the range has to shrink before it can merge.
- This range carries 10,104 authored lines from 18 commits around this pull request's own 610-line change, above the 8,000-line limit for an owner-directed batch; the 16 predecessor commits already have their own pull requests (#8159, #8178, #8179), so land those and rebase onto
mainso this pull request is its own two commits.
Non-blocking: four comments in PickerWheel.tsx and Wheel.tsx restate the line below them (inline notes).
[Automated review]
| [motion], | ||
| ); | ||
|
|
||
| // The motion's layout follows the rows. |
There was a problem hiding this comment.
Non-blocking (INV29): this restates motion.setLayout(layout) below. Suggest dropping it.
| } | ||
| }, [isActive, motion]); | ||
|
|
||
| // A finger lands: a moving wheel stops where it is on screen. |
There was a problem hiding this comment.
Non-blocking (INV29): this restates what onTouchDown does. Suggest dropping it.
| return () => column.removeEventListener('wheel', onWheel); | ||
| }, [isActive, moveBy]); | ||
|
|
||
| // Home and End: the first and the last enabled row. |
There was a problem hiding this comment.
Non-blocking (INV29): moveToEnd and endIndex already say this. Suggest dropping it.
| [moveTo, options, activeIndex], | ||
| ); | ||
|
|
||
| // Home and End: the first and the last enabled row. |
There was a problem hiding this comment.
Non-blocking (INV29): same as in PickerWheel.tsx; toEnd and endIndex already say this.
Batch: B4 · Wheel / picker (DateInput, TimeInput, DateTimeInput)
Depends on #8179. Review only this pull request's change: vjeux/astryx@12a0c16...d060a1c
User impact
Under a finger, turning the month or year wheel behind the DateInput or DateTimeInput sheet calendar's title moves only the calendar. A chosen date stays in its old month, where UIDatePicker moves the date with the wheels.
Problem and solution fit
dateInMonth). A day the month lacks becomes its last day (January 31 becomes February 28), and the date stays withinminandmax. DateTimeInput keeps the time.dateConstraintsrefuses is not chosen, and the calendar still moves. With no date chosen, the wheels move only the calendar. Opening the wheels changes nothing.Expected behavior and authority
component:DateInputFR16 andcomponent:DateTimeInputFR10, as recorded here, with a verification row each. Each cites DateInput FR14's turn-onto rule. The spec owners asked for it at the B4 head and approve it there with/approve-spec.family:input-fieldsFR6 foronChange, thenchangeAction, and the optimistic value.Smallest restoration
TouchDateField.tsxandTouchDateTimeField.tsx(the wheels' change handler),monthGeometry.ts(dateInMonth), and the two records' rows.Writing rubric results
Run over DateInput FR16, DateTimeInput FR10 and their verification rows (prettier re-pads the two tables):
Evidence
MonthYearWheels.interaction.browser.spec.ts"a turn of the month wheel moves the chosen date, keeping its day", under a real touch.DateInputTouch.test.tsx"under a finger: moves the chosen date with the wheels, keeping its day" and "under a finger: clamps a day the new month lacks to its last day", andDateTimeInputTouch.test.tsx"under a finger: moves the chosen date with the month wheel, keeping the day and the time".Scope
Earlier review threads: each is answered on its thread. The looping ones, and those on the old record files beside the source, are moot.
Testing
Local gates at d060a1c, the top of this stack, on #8159's 5c8919c (main ce667be). Results still running say so, and a comment posts them when they land.
pnpm -F @astryxdesign/core typecheck, the Storybook typecheck,api:report --check,sync-exports --check,check:repo,check:knowledge,check:i18n-runtime,check:motion-roles,check:changesets, strict ESLint and Prettier on the touched files: running.pnpm test:uiandpnpm test:node, full: running.DateInputAudit,WheelPinchZoom,WheelRest.a11y.chromium.spec.ts): running.TimeWheels.interactionandMonthYearWheels.interaction, Chromium ×3, Chromium on two pinned cores ×3, WebKit in the CI container ×10: running.analyze-pr.js), gzip, main ce667be → this head, and this pull request's own part: running.