Repository navigation
fix(DateInput, DateTimeInput): under a finger the month/year wheels move the date like UIDatePicker's - #8209
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🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DateInput · Storybook | 180.0 kB (+93 B) · 16.7 kB CSS (0) | 5,679 (+43) | ✓ · ✓ · ✓ |
| DateTimeInput · Storybook | 202.2 kB (+71 B) · 18.4 kB CSS (0) | 3,595 (+28) | ✓ · ✓ · ✓ |
| PowerSearch | 279.4 kB (+63 B) · 23.5 kB CSS (0) | — | — · — · — |
Package size: @astryxdesign/core 6.88 MB packed (+1.4 kB); no new dependencies.
API changes: none in this pull request
Committed API report behind this build: cli 19 rows, core 25 rows; run pnpm api:report.
Touched components checked; add ci:run-visual, ci:run-a11y or ci:run-rtl to run the full suites.
Generated by PR Enrichment workflow | 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 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]
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]
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]
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.
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]
|
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. |
|
/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]
|
/approve-spec 2331365 |
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 affected unit tests and the core typecheck pass; the range has to shrink before it can merge.
- This range carries 10,145 authored lines from 23 commits around this pull request's own 645-line change, above the 8,000-line limit for an owner-directed batch; the 21 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 restate the line below them (PickerWheel.tsx:361, :443, :687 and Wheel.tsx:540); dropping them keeps the source to its reasons.
[Automated review]
|
/approve-spec 5c5f34d |
There was a problem hiding this comment.
Changes requested. An engineering owner approved every record edit at this head, and the date move, the AM/PM rule and the clampTime fix each have passing tests; one scope item blocks.
- The diff against
mainis 3,836 authored lines around this pull request's own 590-line change, because it also carries #8178 and #8179, and no size exemption applies: the stack exemption needs the sameStack:line in every listed pull request, but #8200 and #8206 list #8159 first, and the batch exemption covers only changes a current spec already settled, which FR19 and the other record edits were not; make theStack:line identical in #8178, #8179, #8209, #8200 and #8206, or land #8178 and #8179 first so this pull request is its own commit.
Non-blocking: the comments above handleWheelChange in TouchDateField.tsx and TouchDateTimeField.tsx restate the handler; keep only the reason (see the inline note).
[Automated review]
|
/approve-spec 038a612 |
There was a problem hiding this comment.
Changes requested. Every record edit (DateInput FR20, DateTimeInput FR11, TimeInput FR7) is approved by an engineering owner at this head, and the month/year date move, the AM/PM rule and the whole-minute clamp each have passing unit tests; one scope item blocks.
- The diff against
mainis 3,748 authored lines around this pull request's own 615-line change because it also carries #8179's three commits, and no size exemption applies, since the stack exemption needs one identicalStack:line in every listed pull request while this one reads#8179 → #8209 → #8200 → #8206 (#8178 merged)and #8179, #8200 and #8206 still list #8178 first: make the line identical in all four, or land #8179 first so this pull request is its own commit.
Non-blocking: the comments above handleWheelChange in TouchDateField.tsx and TouchDateTimeField.tsx restate the handler, so keep only the reason (see the inline notes). No record says whether a date the wheels move is announced to a screen reader; a sentence in DateInput FR20 would settle it.
[Automated review]
|
/approve-spec aae9f16 |
There was a problem hiding this comment.
Changes requested. An engineering owner approved the new DateInput FR20 and DateTimeInput FR11 rows at this head, and the date move under a finger has passing unit tests; one shared-code item blocks.
- Miss:
dateInMonthinpackages/core/src/DateInput/monthGeometry.ts(lines 109–114) repeats the day clamp ofplainDateAddMonthsinpackages/core/src/utils/plainDate.ts, which Calendar already calls for the same keep-the-day rule, so build the moved date withplainDateAddMonths(date, monthIndex - monthIndexOf(date))and keep only themin/maxclamp.
Non-blocking: the comments above handleWheelChange in both sheets restate the handler (see the earlier inline notes), and DateInput's Compatibility row and class do not yet mention FR20's new commit path. The Stack: line here differs from the one in #8200 and #8206, which still list #8179, so this pull request was reviewed on its own.
[Automated review]
|
/approve-spec f937fb6 |
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet; it starts once the pull request leaves draft and a maintainer adds the ci:run label.
An engineering owner approved DateInput FR20 and DateTimeInput FR11 at this head, and dateInMonth now reuses plainDateAddMonths, which resolves the earlier blocking item.
- Non-blocking: DateInput's Compatibility row and class do not yet mention FR20's new commit path.
- The
Stack:line here differs from the one in #8200 and #8206, which still list #8179, so this pull request was reviewed on its own.
[Automated review]
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #8206.
…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, by the months of the calendar the sheet shows; 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 FR22 records the rule under a coarse primary pointer, and DateTimeInput FR14 does the same and keeps the time. Each has a verification row.
…n end, not on still frames before a deadline
`openWheels` waited for the sheet, and then for the wheels, with the
harness's `waitForRest`: three still frames within 5 s. On a loaded
runner the page can present no frame for 6 s right after the sheet
opens, so the wait ran out with nothing moving ("still moving", while
the page held no animation). The proof now checks the sheet's open
transform, and then the wheels' full opacity, once a frame with no
deadline of its own, and waits on each animation's `finished` promise.
A starved runner only makes it later.
The comments above both sheets' handleWheelChange keep only the reason: UIDatePicker moves a chosen date with the wheels. The day and bound clamps read from dateInMonth itself, which moves the date by the calendar's own months through calendarShiftMonths.
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #8206.
|
/approve-spec a98a089 |
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.
|
/approve-spec 6c7cecb |
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #8200.
Main gained meta#10231, which adds DateInput FR22 for the touch sheet's month arrows. This pull request's month/year wheel requirement moves from FR22 to FR23, with its verification row, the Bottom-sheet calendar row, the scope line and DateTimeInput FR14's two citations. Only the numbers change; this pull request's own changes are otherwise unchanged.
|
/approve-spec 5911468 |
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #8200.
Batch: B4 · Wheel / picker (DateInput, TimeInput, DateTimeInput)
Stack: #8209 → #8200
This pull request is three commits on
main9c80ad1. In the Gregorian calendar its product diff matches d060a1c's, where the phone check passed.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; with no date chosen, the wheels move only the calendar. Opening the wheels changes nothing.Expected behavior and authority
component:DateInputFR23 andcomponent:DateTimeInputFR14, as recorded here, with a verification row each. Each cites DateInput FR14's turn-onto rule./approve-specat this head.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
Over DateInput FR23, DateTimeInput FR14 and their verification rows (prettier re-pads DateInput's design table): R1 narration, R2 research, R4 rejected alternatives, R5 change references, R6 open questions: none. R3 evidence: test names only.
Scope
Testing
This head, 5911468, merges
main24d845b into 6c7cecb. #10231 took DateInput FR22, so this pull request's wheel rule moves to FR23, and DateTimeInput FR14 cites FR23 in its sentence and its source cell; the rule's text is unchanged. At this head:check:knowledge0;pnpm -F @astryxdesign/core typecheck0; the DateInput, DateTimeInput and Calendar unit tests: 1,276 passed.The pre-merge head a98a089 has three commits on
main9c80ad1. They are the earlier head f937fb6's three, rebased past #9643/#9842 (see "Changed by the rebase" below). The first moves the date by the active calendar's months. The second, the wheels' spec wait, is unchanged. The third keeps only its comment trims, because the month move now goes throughcalendarShiftMonths.pnpm install --frozen-lockfile0;check:knowledge0;check:changesets0;pnpm -F @astryxdesign/core typecheck0;CI=true pnpm lint0 errors; the DateInput, TimeInput, DateTimeInput, Calendar, utils and hooks unit tests: 3,472 passed, including the Persian and Hebrew month-move case.MonthYearWheelsandTimeWheelsinteraction specs against Storybook built at this head: Chromium 8 passed; WebKit (--cpus=4,LP_NUM_THREADS=4), twice: 14 passed, 2 skipped (the blocked-main-thread case, by design).pnpm review:pre0.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" andDateTimeInputTouch.test.tsx"under a finger: moves the chosen date with the month wheel, keeping the day and the time".Changed by the rebase onto #9643/#9842
Main now shows the sheet in the active calendar (#9643/#9842), so this head moves the date by that calendar's months.
dateInMonth(model, date, monthIndex, min, max)callscalendarShiftMonthsfrom the month model, where it called GregorianplainDateAddMonths, and both sheets pass theirmonthModel. In a Gregorian-aligned calendar the result is byte-for-byte the same.DateInputTouch.calendar.test.tsx"moves a date by calendar months, keeping its day of the month": Persian 15 and 31 Shahrivar become 15 and 30 Mehr; Hebrew 30 Adar I 5784 becomes 29 Adar 5785.