feat: TimeInput size variants, addons, and locale-aware hourCycle#693
Draft
feat: TimeInput size variants, addons, and locale-aware hourCycle#693
Conversation
- Add size variants (sm/default/lg) to TimeInput via InputGroup integration - Add startAddon/endAddon props matching the Input component pattern - Derive hourCycle from app locale (i18n.language) using Intl.DateTimeFormat - Extend InputGroup to support non-native controls via data-slot selectors - Add @internationalized/date as direct dependency for Time values in stories - Add doc stories: Default, Sizes, Invalid, Disabled, ReadOnly, StartEndAddons, SizesWithAddons, WithInputGroup - Fix typo in DateSegment focused state class (missing opening bracket)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new TimeInput component (with DateInput and DateSegment), Storybook stories, input-group styling adjustments for non-native controls, and two new dependencies: Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Consumer Component
participant TimeInput as TimeInput wrapper
participant I18n as Locale/i18n
participant Aria as react-aria-components (AriaTimeField)
participant InputGroup as InputGroup / Addons
Consumer->>TimeInput: mount with props (value, size, startAddon, endAddon, hourCycle?)
TimeInput->>I18n: resolve hourCycle (prop or locale fallback)
TimeInput->>Aria: render AriaTimeField with resolved hourCycle and props
TimeInput->>InputGroup: optionally render startAddon / endAddon around AriaTimeField
Aria-->>TimeInput: provide segment render props (isInvalid, isDisabled, isFocused)
TimeInput->>Consumer: render DateSegment(s) with appropriate classNames/state
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/input-group.tsx (1)
123-132:⚠️ Potential issue | 🟡 MinorClicking an addon doesn't focus
DateInput— thequerySelectormisses non-native controls.
querySelector('input, textarea')returnsnullwhen aTimeInput(whoseDateInputrenders as adiv) is used, so the focus call is a no-op. Users who click a start/end addon icon expect focus to land on the time input.🐛 Proposed fix
onClick={(e) => { if ((e.target as HTMLElement).closest('button')) { return; } - e.currentTarget.parentElement - ?.querySelector< - HTMLInputElement | HTMLTextAreaElement - >('input, textarea') - ?.focus(); + const parent = e.currentTarget.parentElement; + ( + parent?.querySelector<HTMLInputElement | HTMLTextAreaElement>( + 'input, textarea' + ) ?? + // Fallback for non-native controls (e.g., react-aria DateInput segments) + parent?.querySelector<HTMLElement>('[role="spinbutton"]') + )?.focus(); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/input-group.tsx` around lines 123 - 132, The click handler in InputGroup's onClick currently queries only 'input, textarea' so custom controls like TimeInput (which render a div) are missed; update the onClick in the InputGroup component to find the first focusable child rather than only native inputs: query for a broader selector such as 'input, textarea, [tabindex]:not([tabindex="-1"]), [contenteditable="true"], [role="textbox"]', then if an element is found cast to HTMLElement and call .focus(); ensure you still ignore clicks inside actual buttons by keeping the existing closest('button') guard. Target the onClick arrow function in src/components/ui/input-group.tsx when making this change.
🧹 Nitpick comments (1)
src/components/ui/time-input.tsx (1)
86-87: Optional: memoizeresolvedHourCycleto avoid allocatingIntl.DateTimeFormaton every render.When
hourCycleisundefined(the default),getHourCycleconstructs a newIntl.DateTimeFormaton every render. The value only changes wheni18n.languageor the explicithourCycleprop changes, so auseMemois appropriate.♻️ Proposed refactor
+import { useMemo } from 'react'; ... - const resolvedHourCycle = hourCycle ?? getHourCycle(i18n.language); + const resolvedHourCycle = useMemo( + () => hourCycle ?? getHourCycle(i18n.language), + [hourCycle, i18n.language] + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/time-input.tsx` around lines 86 - 87, The resolvedHourCycle value is recomputed on every render when hourCycle is undefined because getHourCycle allocates a new Intl.DateTimeFormat; wrap the computation in useMemo so resolvedHourCycle is memoized: compute resolvedHourCycle via useMemo(() => hourCycle ?? getHourCycle(i18n.language), [hourCycle, i18n.language]) (keep using useTranslation and the same getHourCycle helper) so it only recalculates when hourCycle or i18n.language changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 52: The package.json contains caret-pinned dependencies
"@internationalized/date" and "react-aria-components" which is inconsistent with
the rest of the file; change both entries to exact versions (remove the leading
"^" and replace with the specific version number to match the lockstyle used
elsewhere) so dependency versions are fully pinned and deterministic (update the
values for the "@internationalized/date" and "react-aria-components" entries).
In `@src/components/ui/time-input.stories.tsx`:
- Around line 12-99: The stories render TimeInput/TimeField instances without an
accessible name; add an aria-label to at least the Default story (e.g., pass
aria-label="Select time" to the TimeInput in the Default export) and reuse the
same aria-label prop on other story variants (WithDefaultValue, Sizes, Invalid,
Disabled, ReadOnly, StartEndAddons, SizesWithAddons, WithInputGroup) so the
TimeInput/TimeField has an accessible name for a11y and Storybook a11y checks.
---
Outside diff comments:
In `@src/components/ui/input-group.tsx`:
- Around line 123-132: The click handler in InputGroup's onClick currently
queries only 'input, textarea' so custom controls like TimeInput (which render a
div) are missed; update the onClick in the InputGroup component to find the
first focusable child rather than only native inputs: query for a broader
selector such as 'input, textarea, [tabindex]:not([tabindex="-1"]),
[contenteditable="true"], [role="textbox"]', then if an element is found cast to
HTMLElement and call .focus(); ensure you still ignore clicks inside actual
buttons by keeping the existing closest('button') guard. Target the onClick
arrow function in src/components/ui/input-group.tsx when making this change.
---
Nitpick comments:
In `@src/components/ui/time-input.tsx`:
- Around line 86-87: The resolvedHourCycle value is recomputed on every render
when hourCycle is undefined because getHourCycle allocates a new
Intl.DateTimeFormat; wrap the computation in useMemo so resolvedHourCycle is
memoized: compute resolvedHourCycle via useMemo(() => hourCycle ??
getHourCycle(i18n.language), [hourCycle, i18n.language]) (keep using
useTranslation and the same getHourCycle helper) so it only recalculates when
hourCycle or i18n.language changes.
| "@better-upload/server": "3.0.12", | ||
| "@fontsource-variable/inter": "5.2.8", | ||
| "@hookform/resolvers": "5.2.2", | ||
| "@internationalized/date": "^3.11.0", |
Contributor
There was a problem hiding this comment.
Pin new dependencies to exact versions for consistency.
Every other dependency in package.json uses exact version pinning (no ^ or ~). Using caret ranges on @internationalized/date and react-aria-components is inconsistent and can allow unintended minor/patch updates during pnpm update runs.
📦 Suggested fix
- "@internationalized/date": "^3.11.0",
+ "@internationalized/date": "3.11.0",- "react-aria-components": "^1.15.1",
+ "react-aria-components": "1.15.1",Also applies to: 88-88
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 52, The package.json contains caret-pinned dependencies
"@internationalized/date" and "react-aria-components" which is inconsistent with
the rest of the file; change both entries to exact versions (remove the leading
"^" and replace with the specific version number to match the lockstyle used
elsewhere) so dependency versions are fully pinned and deterministic (update the
values for the "@internationalized/date" and "react-aria-components" entries).
Comment on lines
12
to
99
| export const Default = () => { | ||
| return <TimeInput />; | ||
| }; | ||
|
|
||
| export const WithDefaultValue = () => { | ||
| return <TimeInput defaultValue={new Time(14, 30)} />; | ||
| }; | ||
|
|
||
| export const Sizes = () => { | ||
| return ( | ||
| <div className="flex flex-col gap-4"> | ||
| <TimeInput size="sm" defaultValue={new Time(9, 0)} /> | ||
| <TimeInput defaultValue={new Time(14, 30)} /> | ||
| <TimeInput size="lg" defaultValue={new Time(18, 45)} /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export const Invalid = () => { | ||
| return <TimeInput isInvalid defaultValue={new Time(14, 30)} />; | ||
| }; | ||
|
|
||
| export const Disabled = () => { | ||
| return <TimeInput isDisabled defaultValue={new Time(14, 30)} />; | ||
| }; | ||
|
|
||
| export const ReadOnly = () => { | ||
| return <TimeInput isReadOnly defaultValue={new Time(14, 30)} />; | ||
| }; | ||
|
|
||
| export const StartEndAddons = () => { | ||
| return ( | ||
| <div className="flex flex-col gap-4"> | ||
| <p className="text-sm"> | ||
| See <strong>InputGroup</strong> for more advanced use cases | ||
| </p> | ||
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(9, 0)} /> | ||
| <TimeInput | ||
| endAddon={<InputGroupText>UTC</InputGroupText>} | ||
| defaultValue={new Time(14, 30)} | ||
| /> | ||
| <TimeInput | ||
| startAddon={<ClockIcon />} | ||
| endAddon={<InputGroupText>UTC</InputGroupText>} | ||
| defaultValue={new Time(18, 45)} | ||
| /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export const SizesWithAddons = () => { | ||
| return ( | ||
| <div className="flex flex-col gap-4"> | ||
| <TimeInput | ||
| size="sm" | ||
| startAddon={<ClockIcon />} | ||
| defaultValue={new Time(9, 0)} | ||
| /> | ||
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(14, 30)} /> | ||
| <TimeInput | ||
| size="lg" | ||
| startAddon={<ClockIcon />} | ||
| defaultValue={new Time(18, 45)} | ||
| /> | ||
| </div> | ||
| ); | ||
| }; | ||
|
|
||
| export const WithInputGroup = () => { | ||
| return ( | ||
| <div className="flex flex-col gap-4"> | ||
| <TimeInput | ||
| startAddon={<ClockIcon />} | ||
| endAddon={ | ||
| <InputGroupButton size="icon-xs"> | ||
| <ClockIcon /> | ||
| </InputGroupButton> | ||
| } | ||
| defaultValue={new Time(14, 30)} | ||
| /> | ||
| <TimeInput | ||
| startAddon={<InputGroupText>Start</InputGroupText>} | ||
| endAddon={<InputGroupText>hrs</InputGroupText>} | ||
| defaultValue={new Time(9, 0)} | ||
| /> | ||
| </div> | ||
| ); | ||
| }; |
Contributor
There was a problem hiding this comment.
Consider adding aria-label to at least one story for a11y demo coverage.
None of the stories supply aria-label or label, leaving every rendered TimeField without an accessible name. The a11y Storybook addon will flag violations. Adding an aria-label to at least the Default story (and ideally reusing it across others) would demonstrate the expected usage and keep the a11y panel clean.
💡 Example
export const Default = () => {
- return <TimeInput />;
+ return <TimeInput aria-label="Meeting time" />;
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const Default = () => { | |
| return <TimeInput />; | |
| }; | |
| export const WithDefaultValue = () => { | |
| return <TimeInput defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const Sizes = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput size="sm" defaultValue={new Time(9, 0)} /> | |
| <TimeInput defaultValue={new Time(14, 30)} /> | |
| <TimeInput size="lg" defaultValue={new Time(18, 45)} /> | |
| </div> | |
| ); | |
| }; | |
| export const Invalid = () => { | |
| return <TimeInput isInvalid defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const Disabled = () => { | |
| return <TimeInput isDisabled defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const ReadOnly = () => { | |
| return <TimeInput isReadOnly defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const StartEndAddons = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <p className="text-sm"> | |
| See <strong>InputGroup</strong> for more advanced use cases | |
| </p> | |
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(9, 0)} /> | |
| <TimeInput | |
| endAddon={<InputGroupText>UTC</InputGroupText>} | |
| defaultValue={new Time(14, 30)} | |
| /> | |
| <TimeInput | |
| startAddon={<ClockIcon />} | |
| endAddon={<InputGroupText>UTC</InputGroupText>} | |
| defaultValue={new Time(18, 45)} | |
| /> | |
| </div> | |
| ); | |
| }; | |
| export const SizesWithAddons = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput | |
| size="sm" | |
| startAddon={<ClockIcon />} | |
| defaultValue={new Time(9, 0)} | |
| /> | |
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(14, 30)} /> | |
| <TimeInput | |
| size="lg" | |
| startAddon={<ClockIcon />} | |
| defaultValue={new Time(18, 45)} | |
| /> | |
| </div> | |
| ); | |
| }; | |
| export const WithInputGroup = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput | |
| startAddon={<ClockIcon />} | |
| endAddon={ | |
| <InputGroupButton size="icon-xs"> | |
| <ClockIcon /> | |
| </InputGroupButton> | |
| } | |
| defaultValue={new Time(14, 30)} | |
| /> | |
| <TimeInput | |
| startAddon={<InputGroupText>Start</InputGroupText>} | |
| endAddon={<InputGroupText>hrs</InputGroupText>} | |
| defaultValue={new Time(9, 0)} | |
| /> | |
| </div> | |
| ); | |
| }; | |
| export const Default = () => { | |
| return <TimeInput aria-label="Meeting time" />; | |
| }; | |
| export const WithDefaultValue = () => { | |
| return <TimeInput defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const Sizes = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput size="sm" defaultValue={new Time(9, 0)} /> | |
| <TimeInput defaultValue={new Time(14, 30)} /> | |
| <TimeInput size="lg" defaultValue={new Time(18, 45)} /> | |
| </div> | |
| ); | |
| }; | |
| export const Invalid = () => { | |
| return <TimeInput isInvalid defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const Disabled = () => { | |
| return <TimeInput isDisabled defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const ReadOnly = () => { | |
| return <TimeInput isReadOnly defaultValue={new Time(14, 30)} />; | |
| }; | |
| export const StartEndAddons = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <p className="text-sm"> | |
| See <strong>InputGroup</strong> for more advanced use cases | |
| </p> | |
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(9, 0)} /> | |
| <TimeInput | |
| endAddon={<InputGroupText>UTC</InputGroupText>} | |
| defaultValue={new Time(14, 30)} | |
| /> | |
| <TimeInput | |
| startAddon={<ClockIcon />} | |
| endAddon={<InputGroupText>UTC</InputGroupText>} | |
| defaultValue={new Time(18, 45)} | |
| /> | |
| </div> | |
| ); | |
| }; | |
| export const SizesWithAddons = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput | |
| size="sm" | |
| startAddon={<ClockIcon />} | |
| defaultValue={new Time(9, 0)} | |
| /> | |
| <TimeInput startAddon={<ClockIcon />} defaultValue={new Time(14, 30)} /> | |
| <TimeInput | |
| size="lg" | |
| startAddon={<ClockIcon />} | |
| defaultValue={new Time(18, 45)} | |
| /> | |
| </div> | |
| ); | |
| }; | |
| export const WithInputGroup = () => { | |
| return ( | |
| <div className="flex flex-col gap-4"> | |
| <TimeInput | |
| startAddon={<ClockIcon />} | |
| endAddon={ | |
| <InputGroupButton size="icon-xs"> | |
| <ClockIcon /> | |
| </InputGroupButton> | |
| } | |
| defaultValue={new Time(14, 30)} | |
| /> | |
| <TimeInput | |
| startAddon={<InputGroupText>Start</InputGroupText>} | |
| endAddon={<InputGroupText>hrs</InputGroupText>} | |
| defaultValue={new Time(9, 0)} | |
| /> | |
| </div> | |
| ); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ui/time-input.stories.tsx` around lines 12 - 99, The stories
render TimeInput/TimeField instances without an accessible name; add an
aria-label to at least the Default story (e.g., pass aria-label="Select time" to
the TimeInput in the Default export) and reuse the same aria-label prop on other
story variants (WithDefaultValue, Sizes, Invalid, Disabled, ReadOnly,
StartEndAddons, SizesWithAddons, WithInputGroup) so the TimeInput/TimeField has
an accessible name for a11y and Storybook a11y checks.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/ui/input-group.tsx (1)
123-132:⚠️ Potential issue | 🟠 Major
InputGroupAddonclick-to-focus won't work for React AriaDateInputinTimeInputThe
onClickhandler at lines 127–131 queries only'input, textarea'. React Aria'sDateInputrenders as a div-based component without a native input, so clicking astartAddon/endAddonon aTimeInputwill silently find no match and fail to focus. The stories demonstrateTimeInputwith addons, but the focus UX is broken.React Aria's
DateInputlacks ref forwarding to handle.focus()calls on the wrapper. Consider extending the click handler to target non-native controls or updatingDateInputto forward focus to its first focusable segment.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/input-group.tsx` around lines 123 - 132, The onClick in InputGroupAddon currently only queries 'input, textarea' so React Aria's DateInput (used by TimeInput) which renders as a non-native wrapper never gets focused; update the onClick handler in InputGroupAddon to first try the existing query ('input, textarea') and if that returns null, fall back to locating the first focusable descendant (e.g. querySelector for focusable selectors like '[tabindex]:not([tabindex="-1"]), button, a, input, textarea, select, [role="group"], [data-react-aria]') and call .focus() on it; alternatively (or additionally) implement ref forwarding and a focus() proxy on DateInput so DateInput.forwardRef can expose a focus method that focuses its first segment, then ensure TimeInput uses that forwarded ref so calling .focus() on the addon will correctly focus DateInput.
🧹 Nitpick comments (1)
src/components/ui/input-group.tsx (1)
17-18: Verbose repeated selector for non-native controlsThe selector
[&>[data-slot=input-group-control]:not(input):not(textarea)]is repeated verbatim four times (base + 3 size variants). In Tailwind v4, a@custom-variantor a CSS custom property alias could eliminate the repetition. At minimum, a local constant would keep the CVA strings readable and reduce the risk of a typo in one copy diverging from the others.♻️ Suggested approach — extract the repeated selector prefix
+ const CONTROL_SLOT = '[&>[data-slot=input-group-control]:not(input):not(textarea)]'; const inputGroupVariants = cva( cn( // ... - '[&>[data-slot=input-group-control]:not(input):not(textarea)]:md:text-sm', + `${CONTROL_SLOT}:md:text-sm`, // ... ), { variants: { size: { default: cn( // ... - '[&>[data-slot=input-group-control]:not(input):not(textarea)]:px-2.5 has-[>[data-align=inline-end]]:[&>[data-slot=input-group-control]:not(input):not(textarea)]:pr-1.5 has-[>[data-align=inline-start]]:[&>[data-slot=input-group-control]:not(input):not(textarea)]:pl-1.5', + `${CONTROL_SLOT}:px-2.5 has-[>[data-align=inline-end]]:${CONTROL_SLOT}:pr-1.5 has-[>[data-align=inline-start]]:${CONTROL_SLOT}:pl-1.5`,Note: Tailwind's class scanner statically extracts class strings; if you move the selector into a JS template literal, Tailwind may not detect the generated class names automatically. Verify with your build output before committing, or keep the strings literal and add a comment linking the copies.
Also applies to: 45-46, 54-55, 62-64
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ui/input-group.tsx` around lines 17 - 18, Extract the repeated selector string "[&>[data-slot=input-group-control]:not(input):not(textarea)]" used in the input-group CVA/class strings into a single constant (e.g., INPUT_GROUP_CONTROL_SELECTOR) and replace all four verbatim occurrences in src/components/ui/input-group.tsx with that constant; ensure you update every size variant occurrence (the copies at the base and the three size variants) so they stay identical, and either keep the selector as a literal comment next to each usage for Tailwind's scanner or verify your build/plugin config will pick up the generated class names if you use a template literal or custom variant.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/ui/input-group.tsx`:
- Line 30: Replace the permissive selector 'data-[disabled]:cursor-not-allowed'
in input-group.tsx with the explicit-value form
'data-[disabled=true]:cursor-not-allowed' so it matches the existing group
selector (group-data-[disabled=true]/input-group:opacity-50); update the
InputGroup class definition that contains this selector and ensure TimeInput
usage (which sets data-disabled={isDisabled || undefined}) continues to render
"true" when disabled so both cursor and opacity rules stay consistent.
---
Outside diff comments:
In `@src/components/ui/input-group.tsx`:
- Around line 123-132: The onClick in InputGroupAddon currently only queries
'input, textarea' so React Aria's DateInput (used by TimeInput) which renders as
a non-native wrapper never gets focused; update the onClick handler in
InputGroupAddon to first try the existing query ('input, textarea') and if that
returns null, fall back to locating the first focusable descendant (e.g.
querySelector for focusable selectors like '[tabindex]:not([tabindex="-1"]),
button, a, input, textarea, select, [role="group"], [data-react-aria]') and call
.focus() on it; alternatively (or additionally) implement ref forwarding and a
focus() proxy on DateInput so DateInput.forwardRef can expose a focus method
that focuses its first segment, then ensure TimeInput uses that forwarded ref so
calling .focus() on the addon will correctly focus DateInput.
---
Nitpick comments:
In `@src/components/ui/input-group.tsx`:
- Around line 17-18: Extract the repeated selector string
"[&>[data-slot=input-group-control]:not(input):not(textarea)]" used in the
input-group CVA/class strings into a single constant (e.g.,
INPUT_GROUP_CONTROL_SELECTOR) and replace all four verbatim occurrences in
src/components/ui/input-group.tsx with that constant; ensure you update every
size variant occurrence (the copies at the base and the three size variants) so
they stay identical, and either keep the selector as a literal comment next to
each usage for Tailwind's scanner or verify your build/plugin config will pick
up the generated class names if you use a template literal or custom variant.
… CSS conflict" This reverts commit 7708c74.
Contributor
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/components/ui/input-group.tsx`:
- Line 27: The selector for the cursor state is too permissive: replace the
loose 'data-disabled:cursor-not-allowed' usage with the strict form used
elsewhere so both selectors require data-disabled="true" (i.e., update the
class/string 'data-disabled:cursor-not-allowed' to
'data-[disabled=true]:cursor-not-allowed' so it matches the
'group-data-[disabled=true]/input-group:opacity-50' convention used in this
component).
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
TimeInputnow supportssize="sm",size="default"(default), andsize="lg"matching all other input componentsstartAddonandendAddonprops, integrated withInputGroupfollowing the same pattern asInputi18n.language) viaIntl.DateTimeFormat, with thehourCycleprop still available for explicit overrideInputGroupwithdata-slot=input-group-controlCSS selectors to correctly apply padding to non-native-input controls (e.g. React Aria'sDateInputdiv)Default,Sizes,Invalid,Disabled,ReadOnly,StartEndAddons,SizesWithAddons,WithInputGroupstories[in theDateSegmentfocused state classTest plan
TimeInputrenders correctly in all three sizes (sm, default, lg)fr→ 24h,en→ 12h)hourCycleprop overrides the locale defaultSummary by CodeRabbit
New Features
Documentation
Style
Chores