Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
Generated by PR Enrichment workflow | View current CI run |
There was a problem hiding this comment.
Approval waits on an engineering owner's decision. The calendar toggle's timing moves from the theme's motion tokens to UIKit constants under draft AST-094 (FR1, FR4), which would narrow the current theme-tokens architecture (INV3) for touch presentations, and the reading-direction chevron that turns down has no governing spec yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the records say what is approved; the author may do so as an engineering owner. Whatever is decided, these need changing:
- Land the stacked predecessors first, so this pull request's compare holds its own 371-line commit instead of 36,310 lines.
- Make the
lintcheck pass: ESLint runs out of memory on this head but passes onmain. - Add real-browser evidence for DateTimeInput's new cross-fade and chevron turn and for the RTL chevron (AST-013 FR10), since the browser spec covers only the DateInput story in LTR.
- Tag
.changeset/calendar-toggle-uidatepicker-timing.md[feat]rather than[fix]and name the old chevron, because it changes visual direction (AST-017 FR3).
[Automated review]
856179f to
93d2ed7
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the calendar toggle's timing moves from the theme's motion tokens to UIKit constants under draft AST-094 (FR1, FR4), which would narrow the current theme-tokens architecture (INV3) for touch presentations, and the reading-direction chevron that turns down has no governing spec yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the records say what is approved; the author may do so as an engineering owner. Whatever is decided, these still need changing:
- Land the stacked predecessors first, so this pull request's compare holds its own 371-line commit instead of 36,158 lines.
- Tag
.changeset/calendar-toggle-uidatepicker-timing.md[feat]rather than[fix]and name the old chevron, because it changes visual direction (AST-017 FR3). - AST-094 FR7 cites AST-090, which only #8003 proposes, so land that first or qualify the sentence.
[Automated review]
93d2ed7 to
b2a1fa4
Compare
b2a1fa4 to
f80dc69
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the calendar toggle's timing moves from the theme's motion tokens to UIKit constants under draft AST-094 (FR1, FR4), which would narrow the current theme-tokens architecture (INV3) for touch presentations, while the reading-direction chevron and keeping the full 250 ms fade and 90° turn under Reduce Motion (draft FR8) have no governing spec yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the records say what is approved; the author may do so as an engineering owner. Whatever is decided, these still need changing:
- Land the stacked predecessors first, so this pull request's compare holds its own 671-line commit instead of 36,863 lines.
- Tag
.changeset/calendar-toggle-uidatepicker-timing.md[feat]rather than[fix]and name the old chevron and the old 0.01 s Reduce Motion timing, because it changes visual direction (AST-017 FR3). - AST-094 FR7 cites AST-090, which only #8003 proposes, so land that first or qualify the sentence.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: the calendar toggle's timing moves from the theme's motion tokens to UIKit constants under draft AST-094 (FR1, FR4), which would narrow the current theme-tokens architecture (INV3) for touch presentations, while the reading-direction chevron and keeping the full 250 ms fade and 90° turn under Reduce Motion (draft FR8) have no governing spec yet. An engineering owner records that decision by commenting /approve-spec <full head SHA> on this pull request once the records say what is approved; the author may do so as an engineering owner. Whatever is decided, these still need changing:
- Land the stacked predecessors first, so this pull request's compare holds its own 1,001 lines instead of 37,193.
- Attach an iOS simulator recording of the CSS fade jumping after a busy commit, or keep the fades as CSS transitions, because moving them into an Effect needs that Mobile Safari evidence (component-style-authoring INV2, AST-013 FR10).
- Tag
.changeset/calendar-toggle-uidatepicker-timing.md[feat]rather than[fix]and name the old chevron and the old 0.01 s Reduce Motion timing (AST-017 FR3). - AST-094 FR7 cites AST-090, which only #8003 proposes, so land that first or qualify the sentence.
[Automated review]
c4eeaa3 to
0fbbb07
Compare
0fbbb07 to
09bc4ee
Compare
The proof read the sheet and the grid with two evaluates per touch move. On a loaded runner that slowed the script, and the fast flick lost the speed that pages it. The page now samples both itself, every frame.
…ads the pager's page `spec:AST-094` FR45, now on main, has motion a tap starts play the theme's duration and easing tokens. An arrow's step plays the shared pager's animated scroll, UIKit's 300 ms sinusoid, which AST-094 FR4 already records as the pager's gap. DateInput's record now names that gap, and the changeset, the story and the browser spec stop promising 0.3 s. The arrow's proof checks that the step travels one month, one way, without fitting the curve. The title read the nearest month from MonthPager's own pane width. It now reads the pager's own page geometry through `nearestPage`.
…nner's timing WebKit stamps an injected touch when it arrives, so on a loaded runner the moves after a stalled dispatch arrive back to back and read as a much faster finger, and a late lift reads as a resting one. Four proofs failed this way. "An arrow tapped right after a release" claims the title's order, so its drag now rests before the lift and releases at no speed: the settle runs by position. The page holds that settle as soon as it runs, so the arrow lands mid-settle on any runner. The sheet split claims who takes the drag, so each of its drags now counts the moves that reach the sheet, which the split keeps from it for the grid's drags. Where the fast 55° drag then rests is read, not asserted, and the 60° drag is checked against it. "The arrow names the month at the tap" holds the step's motion to sample it. Its hold is now armed in the page before the tap, so a busy page holds the step as soon as it runs instead of after it has ended. "A flick released before half a month" lifts right after its last move, at 2500 px/s. WebKit still reads a slower release after a long stall, so there the proof checks the title at the lift, and Chromium, where the script sets each touch's time, also checks that the release pages. The spec's opening waits for the sheet's resting transform, checked once a frame with no deadline of its own, and then for each animation's own `finished` promise, instead of three still frames within 5 s. The two storyboards film a step and a settle. Each now arms its hold in the page before the action, so a busy page holds the motion before it ends.
The grid's PageUp and PageDown move by the active calendar's months, and with Shift by its years. Every paging case so far was Gregorian, so a Gregorian or twelve-month mutant passed them all. Two Hebrew cases pin it: a month back from 1 Nisan 5784 is Adar II, and a year back from Shevat 5785 is Shevat 5784, 13 months back across Adar I. Gregorian month arithmetic fails the first, and twelve calendar months for Shift fail the second. The AR8 verification row cites the case.
The two trackpad cases stepped their wheels on real timers, 8 ms and 40 ms apart. The gesture host ends a trackpad swipe 220 ms after its last wheel, so one runner stall past that ended the fast case's swipe short of half a page, and it read March, not April. Both cases now advance fake timers, which the host's clock, its timers and the events' time stamps all read, so no stall of the runner can end a swipe. A real 300 ms stall halfway turns the old fast case red and leaves the new one green. 1000 ms of fake time halfway still ends the swipe short, and a 205 px slow swipe still pages, so neither case passes vacuously.
"the month/year toggle leaves the calendar's days alone" spied on `useCalendarDays` through `../Calendar`, but `MonthPane` imports it from `../Calendar/hooks/useCalendarDays`, so the spy saw no pane and the case could not fail. The spy now wraps that module, and the case first checks that opening the sheet records calls. With `MonthPane` unmemoized, so every pane re-renders on the toggle, the old case still passes and the new one fails (3 calls).
…llow UIDatePicker
…nd the chevron dips on the shared dip Done handed focus to the first focusable day of the calendar, a month mounted off-screen, and in Chromium the calendar was still hidden then, so focus fell to the page. It now takes the shown month's tab stop, retrying each frame until the calendar can take focus. Under Reduced Motion the chevron's dip is playReducedMotionDip, and its turn lands in the dip's landing callback instead of on a CSS delay.
…nd-off DateInput FR23 records the title chevron's direction, its quarter turn and its Reduced Motion dip under a finger. DateTimeInput FR15 follows it, and records that Done hands focus to the shown month's tab stop.
On a loaded runner the 0.25 s fade could end before the test's hold reached the page. The page now arms the hold before the tap (armHold), and the storyboard films what it held.
The mouse calendar gives every mounted month a tab stop, so Done focused the first mounted month's day, off-screen. The hand-off now takes the tab stop inside the panel's box. Under a finger the pager has one tab stop, so nothing changes there. The hand-off uses the shared focusable selector.
…so Done's hand-off costs no render Done hands focus from the wheels to the shown month's tab stop. The day's focus handler recorded that day, so React rendered once more after the toggle's commit. The tab stop already names the day that a recorded focus would name. So while no day is recorded, its focus now records nothing. The test "Done hands focus from the wheels to the calendar in the toggle's commit" checks it. It fails at the parent (2 commits against 1) and passes here. Main's useTooltip change (meta#9756) removed an extra render that hid this one.
…s tokens `spec:AST-094` FR45 has motion a tap starts play the theme duration token nearest its platform measurement in the Storybook theme, with the easing for its kind, and FR1 keeps measured values out of it. Under a finger the toggle's fades and the chevron's quarter turn played UIKit's 0.25 s ease-in-ease-out through the `timing.stateChangeMs` and `timing.curves.move` roles. The toggle now plays `--duration-medium-min`, 225 ms in the Storybook theme against the measured 250 ms. The wheels and the header's actions arrive on `--ease-entrance` and leave on `--ease-exit`: a transition takes the curve of the state it goes to, so the hidden styles carry the exit curve. The chevron's turn takes `--ease-move`. A unit test holds the token to the nearest of neutral's durations, and the browser specs read the tokens from the page. DateInput FR23, two browser test names and the changeset follow; Reduced Motion's cross-fade and the chevron's dip stay.
…ds its cross-fade The panels swap `inert` when the cross-fade ends, on either panel's own transition end. Once the sheet settles, the wheels' warm-up turns their `visibility` on, on the toggle's duration. A toggle soon after it saw that transition end mid-fade: the shield came off and the panels swapped `inert` while they still faded. The hook now counts only the transitions that run after the toggle (`transitionrun`), and their ends end the fade. A cancel whose property runs again at once is a replacement, not an end, so the replacement's own end ends the fade.
…warm-up to end The toggle spec's opening waited for the sheet and the warmed-up wheels with `waitForRest`, which needs three still frames within 5 s. A starved runner can present one frame in seconds, so the wait failed with "still moving" while nothing moved. It now waits for each one's resting value, checked once a frame with no deadline of its own, and then for each animation's own `finished` promise, as the month and year wheels' spec does.
… keeps the fade, in any presentation The owner ruled on the day-tap fade per pointer: a finger's tapped day selects at once, as UIDatePicker's does and as the owner felt it on a device; a mouse or pen selection keeps main's fade, in any presentation. In either presentation the last press's pointer decides the fade, so a trackpad click or a stylus tap in the pager keeps it and a finger's tap in the scroller drops it. Before any press, as for a keyboard pick, the presentation's own default holds. DateInput FR9 records the rule. Unit cases press with each pointer type in both presentations, and browser cases tap with a finger, pick with a pen in the touch sheet and click with a mouse.
DateInput FR24 states what the month/year toggle does under a finger: no tap reaches either panel while it cross-fades, the actions that leave with the toggle stop taking taps at once, and the incoming ones take taps when the outgoing panel's fade ends. It also states the focus hand-off: when the title or Done toggles while focus is inside the outgoing panel, focus moves into the incoming one. DateTimeInput FR15 points its toggle at FR24. Two tests complete FR24's evidence: DateInput's Done and DateTimeInput's title hand focus to the incoming panel in the toggle's commit. Both fail on the parent pull request's head. The changeset scopes the duration token to the neutral theme, where it is the token nearest iOS's 0.25 s.
|
/approve-spec 664ed1a |
There was a problem hiding this comment.
Changes requested on this stack: #8209 is approved, and #8200 and #8206 each need the phone evidence the touch-presentation spec requires. A pull request lands only when it and every pull request below it are approved.
#8209
Approved; no blocking findings.
#8200
Changes needed:
- The month pager's touch behaviour is shown only on the iOS simulator and in Android emulator runs from earlier heads, but
spec:AST-094Platform support and DEC-27 require real-device iOS Safari evidence and Chrome with Gboard in the Android emulator at this head, with animations at normal scale and removed, plus a slow-device profile.
#8206
Changes needed:
- Miss: the toggle's Mobile Safari claims (
.changeset/calendar-toggle-uidatepicker-timing.mdline 8, "the fade shows from near its start", and the description's first-frame table) rest on the iOS simulator at an earlier head, whilespec:AST-094requires real-device iOS Safari evidence and Android emulator runs at this head, which the description notes are still requested.
Whole stack
Fails on the same missing phone evidence. Otherwise the three pull requests are consistent, every spec edit has an engineering owner's approval at its exact head, and no code defect was found.
[Automated review]
Main gained meta#10285, which adds a `getOnlyButton` import beside the testing-library import this pull request widens in DateTimeInputTouch.test.tsx. The merge keeps both imports; this pull request's own changes are unchanged.
Main gained meta#10285, which adds a `getOnlyButton` import beside the testing-library import this pull request widens in DateTimeInputTouch.test.tsx. The merge keeps both imports; this pull request's own changes are unchanged.
Main gained meta#10285, which adds a `getOnlyButton` import beside the testing-library import this pull request widens in DateTimeInputTouch.test.tsx. The merge keeps both imports; this pull request's own changes are unchanged.
This pull request now sits on meta#8209's merge with main, so the stack is tested and merged together. Its own changes are unchanged.
This pull request now sits on meta#8200's merge with main, so the stack is tested and merged together. Its own changes are unchanged.
|
/approve-spec a712d60 |
There was a problem hiding this comment.
Changes requested. The spec edits carry an engineering owner's approval at this head; three things still need fixing:
- Regression: #8209's and #8200's bodies declare
Stack: #8209 → #8200while this one declaresStack: #8209 → #8200 → #8206, so this pull request is reviewed on its own and its 7,673-line compare has no size exemption; put the identical three-PRStack:line in #8209's and #8200's bodies. - Miss: the toggle's Mobile Safari claim (
.changeset/calendar-toggle-uidatepicker-timing.mdline 8) needs the real-device iOS result the description only names as "card 36" (device, head and outcome) and Android emulator runs at this head (animations at normal scale and removed, plus a slow-device profile), which the touch-presentation spec requires and the description defers to the final head. - Miss: the pager's touch claims (
.changeset/calendar-pages-like-uidatepicker.md) have the owner's real-device iOS check but no Android emulator runs at this head either.
CI has not run on this draft head. Optional: the comment "Under a finger the fade plays the toggle's tokens" repeats above four style blocks in TouchDateField.tsx; one copy beside TOGGLE_DURATION keeps the reason.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [fix] Under a finger, the calendar in the DateInput and DateTimeInput bottom sheets now pages its months like the iOS date picker: |
There was a problem hiding this comment.
Miss: this tells consumers the touch calendar pages like the iOS date picker (one month per swipe, the 10 px sheet split, the trackpad pan), but the description shows no real-device iOS Safari evidence and no Android emulator run at this head. spec:AST-094 Platform support and DEC-27 ask for real-device evidence for every iOS Safari claim and, for Android Chrome, Chrome with Gboard in the Android emulator at the exact head, with animations at normal scale and removed, plus a slow-device profile.
There was a problem hiding this comment.
The note said the warm-up makes the fade show from near its start. On the iOS Simulator at this head it does not: the toggle's commit holds the main thread for about 45 ms, as it does on main, and Mobile Safari clocks the fade from the style change, so its first frame shows it 54-82 % done. The note now says only what the warm-up does: the wheels are laid out once the sheet settles, so a toggle after that only changes their opacity.
|
/approve-spec d84ec6e |
There was a problem hiding this comment.
Changes requested. The spec edits carry an engineering owner's approval at this head; three things still need fixing:
- Regression: #8209's and #8200's bodies declare
Stack: #8209 → #8200while this one declaresStack: #8209 → #8200 → #8206, so this pull request is reviewed on its own and its 7,673-line compare has no size exemption; put the identical three-PRStack:line in #8209's and #8200's bodies. - Miss: the toggle still lacks the phone evidence
spec:AST-094asks for (Platform support, DEC-27): the description's iOS table (9–23 % of the fade lost before its first frame) contradicts the 54–82 % the newest commit measures at this head, card 36's real-device result is not stated, and there are no Android emulator runs at this head (animations at normal scale and removed, plus a slow-device profile). - Miss: the pager's touch claims (
.changeset/calendar-pages-like-uidatepicker.md) have the owner's real-device iOS check but no Android emulator runs at this head either.
CI has not run on this draft head. Optional: the comment "Under a finger the fade plays the toggle's tokens" repeats above four style blocks in TouchDateField.tsx, and DateInput's wheels panel still flips pointer-events on each toggle, so the new "only changes their opacity" line is broader than the code.
[Automated review]
| '@astryxdesign/core': patch | ||
| --- | ||
|
|
||
| [feat] Under a finger, the calendar in the DateInput and DateTimeInput bottom sheets now toggles to its month and year wheels like the iOS date picker: |
There was a problem hiding this comment.
Miss: these touch claims still have no Android emulator run at this head, and the description's iOS table (9–23 % of the fade lost before its first frame) contradicts the 54–82 % the newest commit measures here, with card 36's real-device result unstated. spec:AST-094 Platform support and DEC-27 ask for real-device iOS evidence and, for Android Chrome, Chrome with Gboard in the emulator at the exact head, with animations at normal scale and removed, plus a slow-device profile.
| [feat] Under a finger, the calendar in the DateInput and DateTimeInput bottom sheets now toggles to its month and year wheels like the iOS date picker: | ||
|
|
||
| - The month/year toggle cross-fades over `--duration-medium-min` (225 ms in the neutral theme, the token nearest iOS's 0.25 s there), where DateInput used a quick linear fade and DateTimeInput an instant swap. The wheels arrive on the theme's entrance curve and leave on its exit curve. As in UIKit, the header's month arrows and Reset and the footer's buttons take no tap during the fade, and a second tap on the title turns it around. | ||
| - The hidden month/year wheels are laid out once the sheet settles, so a toggle after that only changes their opacity. |
There was a problem hiding this comment.
Non-blocking: in DateInput the warm wheels panel keeps pointer-events: none from panelOverlayHidden (TouchDateField.tsx:656; panelOverlayWarm restores only visibility), so the toggle also flips pointer-events. Dropping the property (inert covers hit testing) or saying "only changes their opacity and pointer-events" would match the code.
Batch: B4 · DateInput and DateTimeInput calendar toggle and selection
Stack: #8209 → #8200 → #8206
Depends on #8200. Review only this pull request's change: vjeux/astryx@c6b74ea...d84ec6e
User impact
Under a finger, DateInput's month/year toggle fades linearly with a 180° flip, and DateTimeInput's swaps at once. Now both cross-fade on
--duration-medium-min(225 ms in the neutral theme), and the chevron points along the reading direction and turns a quarter. A finger's tap selects a day at once. A mouse keeps the current toggle.Visual authority
spec:AST-094FR45: "In a component whose main interaction is a tap", a tap-started motion with a measured duration "plays the theme duration token whose value in the Storybook theme (neutral) is nearest that measurement, with the easing for its kind (entrance, exit or move)"; "an in-place toggle cross-fade is not an exit". The toggle plays--duration-medium-min(225 ms in neutral; UIKit's 250 ms stays the reference), arriving on--ease-entrance, leaving on--ease-exit; the chevron turns on--ease-move. The day-tap rule is a specs decision (FR9).For the owner:
component:DateInputFR23 records the chevron's reading direction, its quarter turn and its Reduce Motion dip, and now its timing under FR45 with the 250 ms reference.component:DateTimeInputFR15 follows it and records Done's focus hand-off; its verification row now also cites the mouse case. FR9 now records the day-tap rule. FR24 records the toggle's tap shield and focus hand-off; DateTimeInput FR15 cites it. All need/approve-specat this head.Commit 7, the tab-stop fix, stays here: it changes #8200's
MonthPane.tsxand turns this pull request's FR15 test green.State matrix
playReducedMotionDip) whose turn lands mid-dip; with a mouse, at once.analyze-pr.js): added here when the build lands.Writing rubric results
Over
component:DateInputFR9, FR23 and FR24,component:DateTimeInputFR15 and their verification rows: R1 narration, R2 research, R4 rejected alternatives, R5 change references, R6 open questions: none. R3 evidence: test names only.Scope
Testing
This head, d84ec6e, sits on #8200's head c6b74ea. It holds the earlier head 664ed1a's twelve commits, rebased past #9643/#9842 (see "Changed by the rebase onto #9643/#9842" below), then two merges with
main(one hand edit:DateTimeInputTouch.test.tsxkeeps this pull request's testing-library import besidemain'sgetOnlyButtonimport), then one changeset commit.The changeset commit narrows one line: the wheels' warm-up lays them out once the sheet settles, so a toggle after that only changes their opacity. It no longer says the fade shows from near its start; see "The fade on iOS" below.
At this head's merge, b1fb87b (the same tree as a712d60):
pnpm -F @astryxdesign/core typecheck0;CI=true pnpm lint0 errors; Prettier clean over the stack's files; the DateInput and DateTimeInput unit tests: 1,024 passed. At this head:check:changesets0.Measured on this head's preview (Chromium, a touch pointer, a tap on the title, the neutral theme at its default tempo): every toggle transition runs 225 ms; start to end 217.5–217.7 ms in DateInput and 221.2–221.3 ms in DateTimeInput.
MonthYearWheels,MonthPager,MonthYearToggle,MonthScrollerandMonthScrollerStepsagainst Storybook built at de6c245, this product code on main e657e2e: Chromium 44 passed, 2 skipped; WebKit (--cpus=4,LP_NUM_THREADS=4) 43 passed, 3 skipped.The FR45 timing,
useFadeInert's red-first case, the toggle spec's waits, the per-pointer day-tap rule and the focus hand-off's two red-first tests are as in their commits; the rebase changed none of them.Before and after
real-device iOS: card 36 at this head; Android: Code Quality at the final head
At this head, on the iOS 26.2 Simulator (iPhone 17 Pro, Mobile Safari), against
mainddedd4c; a red ring marks the finger:mainRecorded on the iOS 26.2 simulator and the Android emulator at the earlier head c4d5c04, with UIKit's 0.25 s ease-in-ease-out; this head plays the theme's tokens below.
The fade on iOS. Mobile Safari clocks a transition from the style change, so the toggle's commit flips no inherited property on the panels. Their
inertswaps when the fade ends,aria-hiddenand focus move with the toggle, and a shield takes touches meanwhile. The hidden wheels are laid out once the sheet settles. The toggle's commit still holds the main thread for about 45 ms on the iOS Simulator, as onmain:BottomSheetPanelreadswindow.innerHeightin the commit, which forces a layout. So the fade's first frame shows it partly done. Share of the change shown in the first frame, on the Simulator at low load, 3 takes each, againstmain:mainFocus. The toggle's own handler decides the focus hand-off (
useFadeInert'sfollowFocus), so focus inside the outgoing panel moves in the toggle's commit; no Effect decides it afterwards. Done hands it to the shown month's tab stop, retrying each frame until the calendar can take focus.Changed by the rebase onto #9643/#9842
Main took DateInput FR20 and FR21 and DateTimeInput FR11–FR13 (#9643/#9842), so this pull request's records renumber, with their text unchanged. The memoized month/year wheels also take main's
monthModelandkeptDay.