Repository navigation
Conversation
The hint was a fixed English string. Resolve it through @astryx.timeInput.formatHint with the sample time in the field's display format; TimeInput FR5 drops the closed gap.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:spec-owner-review
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewModified ComponentsTimeInput (@astryxdesign/core) · View in Storybook
Component Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
|
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.
Approval waits on an engineering owner's decision about the clause this PR adds to component:TimeInput FR5, "its sample time uses the field's display format", which the current spec does not state: an engineering owner either approves it, as is or reworded, by commenting /approve-spec <full head SHA> on the final head (the author may do so as an engineering owner), or asks for it to be dropped. The catalog-backed hint itself works, and three fixes are needed either way:
- Move
FORMAT_HINT_SAMPLE_TIMEabove theTimeInputdoc comment so the exported component keeps its JSDoc, which states the handler order the input-fields family contract requires (TimeInput.tsxlines 932-945). - Update the record's other mentions of the closed gap: the Compatibility/readiness row (line 46), the Format hint bullet under Current-state impact (line 240), and the FR3–FR5 evidence gap in the Verification map (line 208).
- Make the new FR5 wording and the code agree for
hasSecondsfields, whose value shows seconds while the hint does not.
[Automated review]
| /** The time the focused-empty format hint shows (2:30 PM, or 14:30). */ | ||
| const FORMAT_HINT_SAMPLE_TIME = '14:30' as ISOTimeString; |
There was a problem hiding this comment.
This constant now sits between TimeInput's doc comment and export function TimeInput, so TypeScript attaches that JSDoc to the constant and the exported component loses it in editor hover and in the emitted declaration. The input-fields family contract (FR10) asks for the handler-order statement in the component's JSDoc. Moving the constant up next to TOUCH_POINTER_QUERY keeps both comments where they belong.
| | FR2 | Every surface reports changes as `onChange(isoTime \| undefined)` and then, when supplied, `changeAction` in a transition; while the Action is pending the field shows the proposed value; while `isLoading` or while the shown value differs from `value`, it sets `aria-busy`, shows a decorative Spinner, and announces the busy start once; once the Action settles the field shows `value`. | `family:input-fields` FR5, FR6; `spec:AST-086` FR8; current source | | ||
| | FR3 | Typed field: a native text input with a leading clock icon. Typing keeps a draft and commits as soon as the draft parses to an in-range time different from `value`. On blur an empty draft commits `undefined` (when a value exists), a valid draft commits if changed, and an invalid draft reverts. An invalid draft sets `aria-invalid` and fills a persistent assertive region with the localized invalid-time message. Typed times MUST parse in the active locale, and a commit MUST reflect the whole typed time (`architecture:internationalization` INV8). Implementation gap: the parser takes no locale. It recognizes only an English `a`/`p` day period, a `:` separator, and ASCII digits, and it commits as soon as a prefix parses: typing "2.30pm" commits 02:00 from "2" and keeps it on blur; `fi-FI`'s "14.30" keeps 14:00; under `es-ES` "2:30 p. m." commits 02:30 with no warning; and `ja-JP` "午後2:30" and `ar-SA` digits are rejected. | `spec:AST-086` FR3; current tests; `architecture:internationalization` INV8 | | ||
| | FR4 | Typed field: ArrowUp and ArrowDown step by `increment` minutes from `value`, or from the current time when empty, commit only when the result is in range, and announce the new time politely (in FR5's display text for the field's hour format, including FR5's digit, separator, and day-period gap). Arrow keys that belong to an IME composition do nothing. The shown text MUST match the committed and announced time. Implementation gap: stepping while a typed draft is pending commits and announces the stepped time but keeps showing the draft, and blur then commits the draft when it parses. | Current tests | | ||
| | FR5 | Typed field: the value shows as 12-hour or 24-hour text, with seconds when `hasSeconds`. While focused, enabled, and empty, the placeholder becomes a format hint for the hour format ("e.g., 2:30 PM" or "e.g., 14:30"); otherwise it is `placeholder` or the localized default. The format hint MUST be catalog text (`architecture:internationalization` INV1a); its sample time uses the field's display format. Implementation gap: a field that becomes disabled while focused keeps showing the hint until blur. On every surface, time text MUST use the locale's digits, separator, and (for twelve-hour text) day-period words (`architecture:internationalization` INV1a, INV8). Implementation gap: every surface's field text and the stepped announcement use ASCII digits, a `:` separator, and a fixed English "AM"/"PM" whatever the locale, and the sheet's hour, minute, and second wheel options use ASCII digits; the sheet's AM/PM wheel uses the catalog's day-period words. | `architecture:internationalization`; current tests | |
There was a problem hiding this comment.
"its sample time uses the field's display format" includes seconds when hasSeconds is set (the value then shows 2:30:00 PM), but the hint is built without seconds (TimeInput.tsx line 592), so the wording or the code needs to change. The record also still calls the hint fixed English in the Compatibility/readiness row (line 46) and under Current-state impact (line 240), and the Verification map still says no test asserts the format hint (line 208).
There was a problem hiding this comment.
Approval still waits on an engineering owner's decision about the clause this PR adds to component:TimeInput FR5, "its sample time uses the field's display format", which the current spec does not state: an engineering owner keeps it, rewords it or asks for it to be dropped, then records approval by commenting /approve-spec <full head SHA> on the final head (the author may do so as an engineering owner). The changeset is now tagged correctly, and three fixes from the last review are still needed either way:
- Move
FORMAT_HINT_SAMPLE_TIMEabove theTimeInputdoc comment so the exported component keeps its JSDoc, which states the handler order the input-fields family contract requires (TimeInput.tsxlines 932-945). - Update the record's other mentions of the closed gap: the Compatibility/readiness row (line 46), the FR3–FR5 evidence gap in the Verification map (line 208) and the Format hint bullet under Current-state impact (line 240).
- Make the FR5 wording, the changeset's last sentence and the code agree for
hasSecondsfields, whose value shows seconds while the hint does not.
[Automated review]
There was a problem hiding this comment.
Changes requested: the catalog-backed hint works and now matches component:TimeInput FR5, but two fixes are still needed before this can merge.
- Move
FORMAT_HINT_SAMPLE_TIMEabove theTimeInputdoc comment (TimeInput.tsxlines 932-945); as placed, the exported component ships without its JSDoc, which carries the handler-order statement the input-fields family contract (FR10) requires. - Update the record's other mentions of the closed gap: the Compatibility/readiness row (line 46), the FR3–FR5 evidence gap in the Verification map (line 208) and the Format hint bullet under Current-state impact (line 240).
[Automated review]
…d hint gap everywhere The sample-time constant sat between TimeInput's JSDoc and its declaration, detaching the doc from the published typings. Move it above. The record's readiness row, verification row and Current-state impact also stop calling the hint fixed English and untested.
There was a problem hiding this comment.
Approval recommended: the focused, empty hint is now one catalog message (@astryx.timeInput.formatHint) whose sample comes from the field's own formatter, as component:TimeInput FR5 requires, and English output is unchanged. The new override test fails before the change and passes after it, and the catalog, runtime-catalog and changeset checks pass. Before merge, the record edit still needs the repository's spec-owner approval on the final head, which an engineering owner (the author included) records by commenting /approve-spec <full head SHA>. Shipped locales will show a translated hint once translations of the new message land.
[Automated review]
|
/approve-spec a55a41d |
github-actions
Bot
removed
the
needs:spec-owner-review
Part of #8955 (TimeInput). Slider is #9023; TextArea follows separately; the Table resize handle is fixed in its own PR.
User impact
People typing a time in a non-English app see the focused, empty
TimeInputhint in English ("e.g., 2:30 PM"), right where the field tells them how to type. The rest of the field's text is translated.Problem and solution fit
TimeInput.tsx.TimeInputFR5 requires the hint to be catalog text, and it listed this as an implementation gap.The fix makes the hint one catalog message,
e.g., {time}, whose sample time comes from the field's own display formatter, so translators control the wording and order.Expected behavior and authority
component:TimeInput(current) FR5: "The format hint MUST be catalog text (architecture:internationalizationINV1a). Implementation gap: it is a fixed English string." This PR closes that gap and removes it from the record. FR5's separate gap about locale digits, separators and day-period words in all time text is unchanged; the sample follows the display formatter, so it will pick that fix up too.Smallest restoration
TimeInput.tsx: the focused-empty placeholder ist('@astryx.timeInput.formatHint', {time}), withtime= the sample 14:30 in the field's display format (2:30 PM or 14:30).locales/en.json:@astryx.timeInput.formatHint=e.g., {time}, with a translator description.TimeInput.spec.md: FR5 drops the closed gap sentence; nothing else in the record changes (the table only re-aligns).Evidence
e.g., 2:30 PM,e.g., 14:30); the TimeInput and DateTimeInput suites pass (378).TimeInput.test.tsx"TimeInput format hint (FR5)".Writing rubric results
Scope
Testing
pnpm vitest run packages/core/src/TimeInput packages/core/src/DateTimeInput;node scripts/check-i18n-catalog.mjs;node scripts/generate-i18n-runtime.mjs --check;pnpm check:knowledge.