Repository navigation
refactor(DateInput, DateTimeInput): share typed entry in useTypedDateField - #9837
nynexman4464 wants to merge 41 commits into
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.
|
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
Changes requested. The refactor preserves behavior: the existing DateInput, DateTimeInput and DateRangeInput suites pass unchanged, and every moved condition matches the code it replaces. One repository rule needs a small follow-up before approval.
Blocking
- Removing the comments that cited
spec:AST-085FR1 and FR2 inpackages/core/src/DateInput/DateInput.tsxdrops those cited claims frompnpm specs:for packages/core/src/DateInput/DateInput.tsx, whicharchitecture:knowledge-contractsINV29 allows only after a record links them, so addspec:AST-085/FR1andspec:AST-085/FR2tosystem_specsindocs/specs/components/DateInput/DateInput.spec.md, which makes the before and after output match.
Two non-blocking comment-wording suggestions are inline.
[Automated review]
| if (pendingInput === null) { | ||
| return; | ||
| } | ||
| // Disabled or busy: keep the value, drop the draft (spec:AST-085 FR2). |
There was a problem hiding this comment.
This comment and the one at old line 835 were this file's only spec:AST-085 FR2 and FR1 citations, so pnpm specs:for packages/core/src/DateInput/DateInput.tsx no longer lists those claims (architecture:knowledge-contracts INV29). Add spec:AST-085/FR1 and spec:AST-085/FR2 to system_specs in docs/specs/components/DateInput/DateInput.spec.md to restore them.
| if (pendingInput === null) { | ||
| return; | ||
| } | ||
| // A disabled or busy field keeps its value and drops the draft. |
There was a problem hiding this comment.
Non-blocking (INV29): this reads as a rule for every field, but it holds only when dropsDraftWhenDisabled is true. DateTimeInput still parses its draft. The option's JSDoc already says this, so the comment can go.
| {/* | ||
| Live region announcing rejected typed input to assistive technology, | ||
| with one message for each kind of rejection (WCAG 3.3.1). | ||
| Announces a typed rejection that no shown status carries, with one |
There was a problem hiding this comment.
Non-blocking (INV29): inside an InputGroup the field shows the typed status and this alert still announces it, so "no shown status carries" is not quite accurate. Suggested wording: "Announces a typed rejection that the field status does not announce. The field status announces the rest." The DateTimeInput comment and the alertMessage JSDoc in useTypedDate.ts need the same change.
cb8dd2f to
3cef6a3
Compare
be8e6e7 to
961c512
Compare
There was a problem hiding this comment.
Changes requested. The refactor preserves behavior: the existing DateInput, DateTimeInput and DateRangeInput suites pass unchanged, and every moved condition matches the code it replaces. One repository rule needs a small follow-up before approval.
Blocking
- Removing the comments that cited
spec:AST-085FR1 and FR2 inpackages/core/src/DateInput/DateInput.tsxdrops those cited claims frompnpm specs:for packages/core/src/DateInput/DateInput.tsx, whicharchitecture:knowledge-contractsINV29 allows only after a record links them, so addspec:AST-085/FR1andspec:AST-085/FR2tosystem_specsindocs/specs/components/DateInput/DateInput.spec.md, which makes the before and after output match.
Two non-blocking comment-wording suggestions are inline.
[Automated review]
| if (pendingInput === null) { | ||
| return; | ||
| } | ||
| // Disabled or busy: keep the value, drop the draft (spec:AST-085 FR2). |
There was a problem hiding this comment.
This comment and the one at old line 835 were this file's only spec:AST-085 FR2 and FR1 citations, so pnpm specs:for packages/core/src/DateInput/DateInput.tsx no longer lists those claims (architecture:knowledge-contracts INV29). Add spec:AST-085/FR1 and spec:AST-085/FR2 to system_specs in docs/specs/components/DateInput/DateInput.spec.md to restore them.
| if (pendingInput === null) { | ||
| return; | ||
| } | ||
| // A disabled or busy field keeps its value and drops the draft. |
There was a problem hiding this comment.
Non-blocking (INV29): this reads as a rule for every field, but it holds only when dropsDraftWhenDisabled is true. DateTimeInput still parses its draft. The option's JSDoc already says this, so the comment can go.
| {/* | ||
| Live region announcing rejected typed input to assistive technology, | ||
| with one message for each kind of rejection (WCAG 3.3.1). | ||
| Announces a typed rejection that no shown status carries, with one |
There was a problem hiding this comment.
Non-blocking (INV29): inside an InputGroup the field shows the typed status and this alert still announces it, so "no shown status carries" is not quite accurate. Suggested wording: "Announces a typed rejection that the field status does not announce. The field status announces the rest." The DateTimeInput comment and the alertMessage JSDoc in useTypedDate.ts need the same change.
3cef6a3 to
a26bf47
Compare
961c512 to
d05ad0c
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet. It runs once the pull request is ready for review and targets main.
The refactor keeps typing, commits, rejection messages, the field status and the alert unchanged in both fields. The existing DateInput, DateTimeInput and DateRangeInput suites pass with no test edits, and each moved condition matches the code it replaces. The earlier blocking item is resolved: component:DateInput now links spec:AST-085/FR1 and /FR2, so pnpm specs:for packages/core/src/DateInput/DateInput.tsx lists the same records and cited claims as before.
[Automated review]
a26bf47 to
37c9150
Compare
d05ad0c to
d5dc7bb
Compare
There was a problem hiding this comment.
Approved pending CI: CI has not run on this head yet, and it runs once the pull request is ready for review and targets main.
Typing, commits, rejection messages, the field status and the alert stay unchanged in both fields: the existing DateInput, DateTimeInput, DateRangeInput and Calendar suites pass with no test edits, and the same typed-entry states read identically before and after the move. Two optional comment cleanups are noted inline.
[Automated review]
| const {isDateDisabled} = useCalendarConstraints({min, max, dateConstraints}); | ||
| const typedDate = useTypedDate({ | ||
|
|
||
| // Unified change handler that fires both onChange and changeAction |
There was a problem hiding this comment.
Non-blocking (architecture:knowledge-contracts INV29): this comment restates what fireChange does, so it can go; the name and the body already say it.
| // DateTimeInput fixes its status presentation to the detached message box, | ||
| // so the shared helper suppresses the on-field icon (the message box carries | ||
| // its own leading glyph). Routed through the helper for consistency with the | ||
| // rest of the bordered input family. |
There was a problem hiding this comment.
Non-blocking (architecture:knowledge-contracts INV29): the last sentence narrates the call. Suggested, keeping only the reason:
// The detached message box carries its own glyph, so the helper shows no on-field icon.
37c9150 to
02527f4
Compare
d5dc7bb to
e1cc948
Compare
|
/approve-spec e1cc948 |
02527f4 to
6db495e
Compare
e1cc948 to
1766698
Compare
…he lost native warning as a failure DateInput FR21 states that a pane's grid name keeps the era of its month's middle day. The DateTimeInput FR12 verification row again fails a missing warning for an explicit native.
DateRangeInput takes a calendar prop and otherwise follows the provider's calendar. Its trigger text and popover calendar use the calendar; values stay Gregorian ISO dates.
…d the -u-ca- request The provider's calendar prop docs and record list DateRangeInput. The DateRangeInput record and changeset state that an explicit -u-ca- locale is the application's request, and the record's intent paragraph is rewrapped.
…y their standard names
…regorian dates The DateRangeInput record and the changeset say that a provider locale with a -u-ca- keyword keeps Gregorian dates; set calendar to choose one.
A test asserts that the trigger's accessible name writes the range in the calendar. INV10 names DateRangeInput among the displays that follow the calendar.
…onthOf calendarYearMonthOf now takes a calendar month, so the trigger's same-year check passes the month of each date.
…Field DateInput and DateTimeInput kept two copies of the same typed-entry logic: the typed text, the committed rejection, the date that typing last reported, and the commit on Enter or blur. useTypedDateField in useTypedDate.ts now owns that state and the rejection status and alert for both fields. Each field keeps its own clear and disabled rules: DateInput drops typed text when a disabled or busy field commits, and DateTimeInput reads it. Behavior is unchanged.
… DateInput record The comments that cited these claims left DateInput.tsx in the shared-hook move, so the DateInput record now links them. The live-region comments and the alertMessage doc say which rejections the alert announces.
e56a372 to
669e1b2
Compare
github-actions
Bot
added
the
needs:design-review
|
/approve-spec 669e1b2 |
There was a problem hiding this comment.
This pull request is reviewed as part of its stack; see the review on #9842.
PR Analysis Report🔎 Preview ·
|
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| Calendar · Storybook | 57.7 kB (+214 B) · 9.3 kB CSS (0) | 1,714 (0) | ✗ · ✓ · ✓ |
| PowerSearch · Storybook | 🔴 258.1 kB (+8.4 kB) · 22.3 kB CSS (0) | 7,201 (0) | ✓ · ✓ · ✓ |
| DateInput · Storybook | 🔴 163.0 kB (+8.0 kB) · 15.5 kB CSS (0) | 5,610 (+507) | ✓ · ✓ · ✓ |
6 more components
| Component | Size | LoC | Visual · A11y · RTL |
|---|---|---|---|
| DateTimeInput · Storybook | 🔴 184.8 kB (+7.7 kB) · 17.2 kB CSS (0) | 3,554 (−11) | ✓ · ✓ · ✓ |
| TimeInput | 140.4 kB (0) · 14.5 kB CSS (0) | — | — · — · — |
| Timestamp | 22.9 kB (−8 B) · 8.9 kB CSS (0) | — | — · — · — |
| DateRangeInput · Storybook | 75.7 kB (−103 B) · 11.2 kB CSS (0) | 684 (+17) | ✓ · ✓ · ✓ |
| lab/Comments | 271.5 kB (new) · 2.5 kB CSS (new) | — | — · — · — |
| lab/CommentThread | 266.4 kB (new) · 2.4 kB CSS (new) | — | — · — · — |
Visual changes: 2 snapshots in 1 component
A change is a question for the reviewer, not a failure.
| Component · story | Before | After | Diff |
|---|---|---|---|
| Calendar · Default (neutral, light) |  |
 |
 |
| Calendar · Default (neutral, dark) |  |
 |
 |
Package size: measured; main has no measurement of the merge base to compare yet.
API changes: cli +8 · ~1 · −0 (against the merge base's committed report); core +3 · ~0 · −0
Committed API report behind this build: cli 9 rows, core 3 rows; run pnpm api:report.
12 API changes (advisory)
| Package | Where | Signature | Usage | |
|---|---|---|---|---|
| + | cli |
./api · Types · type SearchResultEntry |
deprecated?: ComponentDeprecation | undefined |
import type {SearchResultEntry} from '@astryxdesign/cli/api'; const x: SearchResultEntry = {deprecated: …} |
| + | cli |
./authoring · Types · interface ComponentDeprecation |
interface ComponentDeprecation |
import type {ComponentDeprecation} from '@astryxdesign/cli/authoring' |
| + | cli |
./authoring · Types · interface MultiComponentDoc |
deprecated?: ComponentDeprecation |
import type {MultiComponentDoc} from '@astryxdesign/cli/authoring'; const x: MultiComponentDoc = {deprecated: …} |
| + | cli |
./authoring · Types · interface SingleComponentDoc |
deprecated?: ComponentDeprecation |
import type {SingleComponentDoc} from '@astryxdesign/cli/authoring'; const x: SingleComponentDoc = {deprecated: …} |
| + | cli |
./doc · Types · interface ComponentBaseDoc |
deprecated?: ComponentDeprecation |
import type {ComponentBaseDoc} from '@astryxdesign/cli/doc'; const x: ComponentBaseDoc = {deprecated: …} |
| + | cli |
./doc · Types · interface SubComponentDoc |
deprecated?: ComponentDeprecation |
import type {SubComponentDoc} from '@astryxdesign/cli/doc'; const x: SubComponentDoc = {deprecated: …} |
| + | cli |
./doc · Also exported from an earlier entry point · ComponentDeprecation |
ComponentDeprecation (./authoring) |
import {ComponentDeprecation} from '@astryxdesign/cli/doc' |
| + | cli |
./template · Also exported from an earlier entry point · ComponentDeprecation |
ComponentDeprecation (./authoring) |
import {ComponentDeprecation} from '@astryxdesign/cli/template' |
| ~ | cli |
./api · Functions · function search |
function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "hook" | "theme" | "doc" | undefined; limit?: number | undefined }) => Promise<SearchResponse> → function search: (query: string, options?: { cwd?: string | undefined; type?: "template" | "component" | "doc" | "theme" | "hook" | undefined; limit?: number | undefined }) => Promise<SearchResponse> |
import {search} from '@astryxdesign/cli/api'; search(…) |
| + | core |
. · Components · component DateInput |
prop calendar?: CalendarSystem = the InternationalizationProvider's calendar, else `'gregory'` |
<DateInput calendar="s calendar, else `" /> |
| + | core |
. · Components · component DateRangeInput |
prop calendar?: CalendarSystem = the InternationalizationProvider's calendar, else `'gregory'` |
<DateRangeInput calendar="s calendar, else `" /> |
| + | core |
. · Components · component DateTimeInput |
prop calendar?: CalendarSystem = the InternationalizationProvider's calendar, else `'gregory'` |
<DateTimeInput calendar="s calendar, else `" /> |
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
…ates in the active calendar (#9643, #9842) Lands the declared stack #9643 → #9842; each part was reviewed on its own pull request. - #9643 (with #10031, #9794, #9653 and #9837 folded in): DateInput and DateTimeInput read and write typed dates in the active calendar; a non-Gregorian calendar uses the Astryx picker instead of the native control; the bottom sheet shows months of the calendar; DateRangeInput shows the range in the calendar; one shared typed-entry hook serves both fields. - #9842: while DateInput or DateTimeInput is disabled or busy, typed text stays without a rejection, and commits or gives way once the field is editable. Fixes #8958.
Stack: #9643 → #10031 → #9794 → #9653 → #9837 → #9842
Maintainer impact
DateInput and DateTimeInput each kept a copy of the same typed-entry logic: the typed text, the committed rejection, the date that typing last reported, the commit on Enter or blur, and the rejection status and alert. A fix to typed entry had to land twice, and the two copies could drift.
Intended invariant
No behavior change. Typing, commits,
onChangereports, rejection messages, the field status, the alert, and the format hint stay as they are in both components.Change
useTypedDateFieldinpackages/core/src/DateInput/useTypedDate.tsnow owns the typed text, the committed rejection, the last reported date, the commit on Enter or blur, the reset when a value comes from outside, the shown text and format hint, the shown status, and the alert message. It builds onuseTypedDate.DateInputandDateTimeInputcall the hook and keep only what differs between them: how a date is reported (DateTimeInputcombines it with the time), how emptied text clears the value, whether the shown status announces itself (anInputGroupDateInputdoes not), and the existing difference on commit in a disabled or busy field (DateInputdrops the typed text;DateTimeInputreads it).component:DateInputnow linksspec:AST-085FR1 and FR2 insystem_specs, because the source comments that cited them moved out ofDateInput.tsx. No requirement text changes.fireChange, and the narration in DateTimeInput's status-icon comment, which keeps only its reason.[chore]changeset records the internal change.Evidence
Scope
Testing
pnpm exec vitest run --maxWorkers=2over every Vitest file inpackages/core/src/DateInputandpackages/core/src/DateTimeInput, plusdocPropLiterals.test.ts: 22 files, 932 tests passed.pnpm exec tsc --noEmit -p packages/core: passes.eslintandprettier --checkon the changed files: clean.pnpm check:knowledgeandpnpm check:changesets: pass.node scripts/pre-review.mjs --base nynexman4464/feat/calendar-presentations: 724 authored lines.Writing rubric results
Record:
component:DateInput(twosystem_specsentries,spec:AST-085/FR1andspec:AST-085/FR2).