feat(duration-input): add experimental duration input#3783
feat(duration-input): add experimental duration input#3783neil-krichi wants to merge 2 commits intomainfrom
Conversation
✅ No New Circular DependenciesNo new circular dependencies detected. Current count: 0 |
🔍 Visual review for your branch is published 🔍Here are the links to: |
📦 Alpha Package Version PublishedUse Use |
Coverage Report for packages/react
File Coverage
|
||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Pull request overview
Adds an experimental DurationInput field to the React package’s experimental Forms Fields, using a minutes-based value API to support Gamma migration.
Changes:
- Introduces
DurationInputcomponent that renders hours/minutes inputs and emits total minutes. - Adds Storybook stories for common states (filled, empty, error, readonly, disabled).
- Adds unit tests covering rendering, normalization, clearing, and controlled updates; exports the field from the experimental Fields entrypoint.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/experimental/Forms/Fields/exports.ts | Re-exports the new experimental field from the Forms Fields entrypoint. |
| packages/react/src/experimental/Forms/Fields/DurationInput/index.tsx | Barrel export for the new field. |
| packages/react/src/experimental/Forms/Fields/DurationInput/DurationInput.tsx | Implements the DurationInput component (hours/minutes UI + minutes-based API). |
| packages/react/src/experimental/Forms/Fields/DurationInput/stories/DurationInput.stories.tsx | Adds Storybook stories for the new field. |
| packages/react/src/experimental/Forms/Fields/DurationInput/tests/DurationInput.test.tsx | Adds focused unit coverage for core behaviors. |
|
|
||
| const nextDuration = splitDuration(value, hideMinutes) | ||
| setHours(nextDuration.hours) | ||
| setMinutes(nextDuration.minutes) | ||
| }, [value, hideMinutes, hours, minutes]) |
There was a problem hiding this comment.
The state-sync effect runs whenever hours/minutes change and always reconciles local state back to value. Because value is optional, using the component without a controlled value (i.e. value === undefined) will cause any user typing to be immediately overwritten back to empty strings. Consider making the component explicitly controlled (require value/onChange) or support uncontrolled usage by skipping reconciliation when value is undefined and only syncing local state when the controlled value actually changes (e.g. via a controllable-state helper).
There was a problem hiding this comment.
Good catch. I updated the component to avoid reconciling local state when value is undefined, so uncontrolled usage no longer resets typed input. I also added a regression test covering uncontrolled typing and normalization.
| | "required" | ||
| | "readonly" | ||
| | "size" | ||
| | "className" |
There was a problem hiding this comment.
This component’s public props currently expose className via InputFieldProps. In packages/react/AGENTS.md it’s a documented convention that public components should not accept className (use a private-prop pattern instead). Consider removing className from the exported props or making it internal-only.
| | "className" |
There was a problem hiding this comment.
Addressed. I removed className from the public props and switched the component to the private-props wrapper pattern so className stays internal-only, matching the package convention.
Summary
DurationInputfield in f0 with a minutes-based public API to ease Gamma migrationclassNameinternal-only to match package conventionsfactorialusing the real packaged F0 buildTesting
pnpm exec vitest run --project=unit src/experimental/Forms/Fields/DurationInput/__tests__/DurationInput.test.tsxpnpm tscpnpm lintpnpm jest src/modules/attendance/components/ClockIn/TabsBreakdown/CompensationPannel/FlexibleCompensationAllocation/AllocationInputs.spec.tsx --runInBandinfactorial/frontend