-
Notifications
You must be signed in to change notification settings - Fork 147
feat: Add InputGroup primitive and use it for Input based components #658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a composable InputGroup system (addons, buttons, text, textarea), new Label and Textarea primitives, refactors many input components to use InputGroup/addon APIs, introduces FieldTextarea with stories and tests, updates form label usage, tweaks theme tokens, and adds three new package dependencies. Changes
Sequence Diagram(s)(Skipped — changes are broad component additions and refactors without a single clear sequential runtime flow to visualize.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Possibly related PRs
Suggested labels
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
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/ui/checkbox.tsx (1)
56-71: Critical: aria-labelledby references non-existent ID when noLabel=true.When
noLabelistrue, the component renders without a label wrapper (usingFragment), butaria-labelledby={id}is still applied to theCheckboxPrimitive.Root. This creates an invalid ARIA reference because no element in the DOM has the referencedid, violating WCAG guidelines and breaking screen reader functionality.🔧 Proposed fix
return ( <Comp {...compProps}> <CheckboxPrimitive.Root className={cn(checkboxVariants({ size }), className)} {...props} - aria-labelledby={id} + aria-labelledby={noLabel ? undefined : id} >src/components/ui/number-input.tsx (1)
67-79: Add accessible names (aria-label) to icon-only increment/decrement buttons.The
NumberField.IncrementandNumberField.Decrementcomponents don't provide default accessible names in Base UI v1.0.0. Without explicitaria-labelattributes, screen readers will not announce these icon-only buttons properly, creating a WCAG 2.1 Level A accessibility violation.Pass
aria-labelto each button control:
- Line 70 (Decrement): Add
aria-labelto the Button component- Lines 101–106 (Increment/Decrement in classic mode): Add
aria-labelto NumberField.Increment and NumberField.Decrement- Line 114 (Increment): Add
aria-labelto the Button componentConsider using translation keys for internationalization (e.g., using
useTranslation()which is already imported).Also applies to: 92-108, 111-123
🤖 Fix all issues with AI agents
In @src/components/ui/input-group.stories.tsx:
- Line 187: The exported function is misspelled as TexareaExample; rename the
function and its export to TextareaExample to correct the typo and ensure
consumers/imports match the intended name (update the function declaration and
any references/imports that use TexareaExample).
In @src/components/ui/input-group.tsx:
- Around line 87-92: The click-to-focus handler in the onClick callback only
queries for an input element, so clicking the addon won't focus a textarea used
by InputGroupTextarea; update the selector used in the handler (the
querySelector call inside the onClick defined around the
currentTarget.parentElement) to include textarea (e.g., query for "input,
textarea") or a broader selector that matches both input and textarea (and
optionally contenteditable) and then call focus() on the found element; ensure
the found element's type supports focus before invoking it.
- Line 153: The CSS utility 'field-sizing-content' used in the input class
string is not supported in Firefox/Safari; update the InputGroup component to
provide a fallback or document the requirement: wrap the use of
'field-sizing-content' in a feature-query-based approach (apply
'field-sizing-content' only when supported and otherwise fall back to a stable
utility like 'min-w-0' or 'w-full' / 'max-w-full'), or add a project note that
the component requires Chrome 123+; target the class string containing
'field-sizing-content' in the InputGroup (or whichever component renders that
class) and implement the fallback/feature-query or add the browser-requirement
comment in the component docs accordingly.
In @src/components/ui/input.stories.tsx:
- Around line 30-33: The MailIcon used as a purely decorative addon in the Input
stories should be marked aria-hidden to avoid redundant accessible name
announcements; update the story instances that use startAddon/endAddon with
MailIcon (e.g., the Input components and their InputGroup usage) to set
aria-hidden on the MailIcon and ensure the Input examples include or reference
an accessible name in real usage (e.g., via an associated label) so
accessibility is preserved across the examples shown between the existing Input
instances and the similar blocks at lines 36-69.
In @src/components/ui/label.tsx:
- Around line 7-9: In the labelVariants cva call, replace the incorrect Tailwind
utility "align-baseline" with the flex alignment utility "items-baseline" so the
class string becomes 'flex gap-1.5 items-baseline text-sm
peer-disabled:cursor-not-allowed peer-disabled:opacity-70' (update the
labelVariants definition to use items-baseline).
In @src/components/ui/radio-group.tsx:
- Around line 66-82: The RadioPrimitive.Root currently always receives
aria-labelledby={id} even when noLabel is true, causing an invalid ARIA
reference; update the render logic so that aria-labelledby is only passed when a
real label element exists (i.e., when noLabel is false or labelProps/id are
present). Locate the noLabel, Comp, id (from useId()), compProps, and the
RadioPrimitive.Root usage and conditionally include aria-labelledby (or switch
to aria-label/aria-labelledby only when labelProps.id or compId is rendered) so
the RadioPrimitive.Root does not reference a non-existent ID when noLabel ===
true.
In @src/components/ui/select.tsx:
- Line 12: The import in src/components/ui/select.tsx uses non-existent names
ChevronDownIcon and XIcon from lucide-react; update the import statement to use
the correct exported names ChevronDown and X (remove the "Icon" suffix) so
components that reference ChevronDown/X render correctly.
In @src/styles/app.css:
- Line 91: The dark-mode value for the CSS custom property --muted-foreground
produces insufficient contrast; update the dark-theme override that sets
--muted-foreground to use a lighter neutral token (for example
var(--color-neutral-300) or var(--color-neutral-200)) instead of
var(--color-neutral-400) so the muted text on neutral-900 meets WCAG AA contrast
ratios; locate the dark-theme rule that defines --muted-foreground and replace
the value accordingly, test contrast to confirm it exceeds 3:1 (large text) and
preferably 4.5:1 (normal text).
🧹 Nitpick comments (7)
package.json (1)
46-46: Consider consistent version pinning for new dependencies.The migration from
@base-ui-components/react(beta) to@base-ui/react(stable v1.0.0) looks good. However, there's an inconsistency in version pinning:
- Line 58:
@radix-ui/react-labeluses exact version2.1.8- Line 59:
@radix-ui/react-slotuses caret^1.2.4- Line 97:
react-textarea-autosizeuses exact version8.5.9Most dependencies in this project use exact versions for predictability.
📌 Suggested fix for consistent version pinning
- "@radix-ui/react-slot": "^1.2.4", + "@radix-ui/react-slot": "1.2.4",Also applies to: 58-59, 97-97
src/components/ui/textarea.tsx (1)
24-24: Consider using React.forwardRef for better ref handling.While React 19 allows
refas a regular prop, usingReact.forwardRefis still the recommended pattern for component APIs that expose refs. This provides better type inference and clearer intent.♻️ Proposed refactor using forwardRef
-type TextareaProps = Pick< +type TextareaProps = Omit< + Pick< React.ComponentProps<'textarea'>, | 'className' | 'placeholder' | 'id' | 'value' | 'defaultValue' | 'disabled' | 'readOnly' | 'aria-invalid' | 'aria-describedby' | 'autoFocus' | 'autoCapitalize' | 'autoComplete' | 'autoCorrect' | 'onBlur' | 'onChange' | 'onKeyDown' | 'inputMode' -> & - Pick<React.ComponentProps<typeof InputGroup>, 'size'> & { - ref?: React.Ref<HTMLTextAreaElement | null>; - }; + >, + 'ref' +> & + Pick<React.ComponentProps<typeof InputGroup>, 'size'>; -function Textarea({ ref, size, ...props }: TextareaProps) { +const Textarea = React.forwardRef<HTMLTextAreaElement, TextareaProps>( + ({ size, ...props }, ref) => { - return ( + return ( <InputGroup size={size}> <InputGroupTextarea {...props} ref={ref} data-slot="input" /> </InputGroup> - ); -} + ); + } +); + +Textarea.displayName = 'Textarea';Also applies to: 27-27
src/components/ui/label.tsx (1)
11-22: Consider tighteningreftyping (and/or usingforwardRef) for consistency with Radix + TS.
Right nowrefis treated as a regular prop; depending on your React/TS setup, you may wantComponentPropsWithRef<typeof LabelPrimitive.Root>(orforwardRef) to make ref typing/usage unambiguous.Proposed ref/props typing tweak
-const Label = ({ - ref, - className, - ...props -}: React.ComponentProps<typeof LabelPrimitive.Root> & - VariantProps<typeof labelVariants>) => ( +const Label = ({ + ref, + className, + ...props +}: React.ComponentPropsWithRef<typeof LabelPrimitive.Root> & + VariantProps<typeof labelVariants>) => ( <LabelPrimitive.Root ref={ref} className={cn(labelVariants(), className)} {...props} /> );src/components/ui/number-input.tsx (1)
18-28:inputPropstyping is too broad; it can leak non-DOMInputprops onto a native<input>.
inputPropsis derived fromComponentProps<typeof Input>but is spread ontoInputGroupInput(native input) (Line 88), so any customInputprops could become invalid DOM attrs.Proposed direction (type `inputProps` as native input props)
-import type { Input } from '@/components/ui/input'; +import type { InputHTMLAttributes } from 'react'; -type InputProps = ComponentProps<typeof Input>; -type InputPropsRoot = Pick<InputProps, 'placeholder' | 'size' | 'aria-invalid'>; +type NativeInputProps = InputHTMLAttributes<HTMLInputElement>; +type InputPropsRoot = Pick<NativeInputProps, 'placeholder' | 'aria-invalid'> & { + size?: 'sm' | 'default' | 'lg'; +}; type NumberInputProps = ComponentProps<typeof NumberField.Root> & InputPropsRoot & { - inputProps?: Omit<RemoveFromType<InputProps, InputPropsRoot>, 'endElement'>; + inputProps?: Omit<NativeInputProps, keyof InputPropsRoot>; buttons?: 'classic' | 'mobile'; inCents?: boolean; } & { ref?: React.Ref<HTMLInputElement | null>; };Also applies to: 80-91
src/components/ui/input-group.stories.tsx (2)
43-45: Inconsistent export styles: mix of function declarations and arrow functions.
TextExampleuses an arrow function while all other exports use function declarations. Consider using a consistent style throughout the file.Proposed fix
-export const TextExample = () => { +export function TextExample() {Also applies to: 148-148
257-262: Duplicateidattribute value "email" may cause accessibility issues in Storybook.Line 258 uses
id="email"and line 264 usesid="email-2", which is fine. However, if multiple stories are rendered on the same page (e.g., in Storybook docs), theid="email"could conflict with other stories. Consider using more unique IDs prefixed with the story name.src/components/ui/input-group.tsx (1)
46-51: Nestedrole="group"may cause accessibility confusion.Both
InputGroup(line 48) andInputGroupAddon(line 83) userole="group". Nested group roles can be semantically unclear for screen readers. Consider removingrole="group"fromInputGroupAddonsince it's a presentational container within the already-grouped parent.Proposed fix
return ( <div - role="group" data-slot="input-group-addon" data-align={align}Also applies to: 81-86
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (25)
package.jsonsrc/components/form/docs.stories.tsxsrc/components/form/field-number/index.tsxsrc/components/form/field-text/docs.stories.tsxsrc/components/form/form-field-label.tsxsrc/components/ui/base-select.tsxsrc/components/ui/checkbox-group.tsxsrc/components/ui/checkbox.tsxsrc/components/ui/date-input.stories.tsxsrc/components/ui/date-picker.tsxsrc/components/ui/input-group.stories.tsxsrc/components/ui/input-group.tsxsrc/components/ui/input.stories.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/number-input.stories.tsxsrc/components/ui/number-input.tsxsrc/components/ui/radio-group.tsxsrc/components/ui/search-input.tsxsrc/components/ui/select.stories.tsxsrc/components/ui/select.tsxsrc/components/ui/sidebar.tsxsrc/components/ui/textarea.stories.tsxsrc/components/ui/textarea.tsxsrc/styles/app.css
💤 Files with no reviewable changes (1)
- src/components/form/field-text/docs.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (17)
src/components/ui/sidebar.tsx (1)
src/layout/manager/nav-sidebar.tsx (2)
SidebarMenuButton(108-115)SidebarMenuButton(80-87)
src/components/ui/select.stories.tsx (1)
src/components/ui/input-group.tsx (1)
InputGroupButton(166-166)
src/components/ui/radio-group.tsx (1)
src/components/form/field-radio-group/index.tsx (1)
field(59-114)
src/components/ui/checkbox.tsx (2)
src/components/form/field-checkbox/index.tsx (1)
field(50-75)src/components/form/field-checkbox-group/index.tsx (2)
TFieldValues(32-104)field(59-101)
src/components/ui/checkbox-group.tsx (2)
src/components/form/field-checkbox-group/index.tsx (2)
field(59-101)TFieldValues(32-104)src/components/form/field-checkbox-group/docs.stories.tsx (4)
form(119-142)form(60-87)form(89-117)form(144-172)
src/components/ui/search-input.tsx (1)
src/components/ui/input-group.tsx (4)
InputGroupButton(166-166)InputGroup(164-164)InputGroupInput(167-167)InputGroupAddon(165-165)
src/components/form/field-number/index.tsx (1)
src/components/ui/date-input.tsx (1)
props(85-123)
src/components/ui/textarea.tsx (1)
src/components/ui/input-group.tsx (2)
InputGroup(164-164)InputGroupTextarea(169-169)
src/components/ui/number-input.stories.tsx (1)
src/components/ui/number-input.tsx (1)
NumberInput(30-127)
src/components/form/docs.stories.tsx (1)
src/components/ui/input-group.tsx (4)
InputGroup(164-164)InputGroupAddon(165-165)InputGroupText(168-168)InputGroupInput(167-167)
src/components/ui/textarea.stories.tsx (3)
src/components/ui/textarea.tsx (1)
Textarea(35-35)src/components/ui/input.stories.tsx (5)
Default(11-13)Invalid(15-17)Disabled(19-21)ReadOnly(23-25)Sizes(27-35).storybook/preview.tsx (1)
story(85-99)
src/components/ui/date-picker.tsx (2)
src/components/ui/input-group.tsx (1)
InputGroupButton(166-166)src/components/ui/date-input.tsx (3)
props(85-123)e(44-51)inputValue(53-80)
src/components/ui/input-group.stories.tsx (2)
src/components/ui/input-group.tsx (6)
InputGroup(164-164)InputGroupInput(167-167)InputGroupAddon(165-165)InputGroupText(168-168)InputGroupButton(166-166)InputGroupTextarea(169-169)src/components/ui/label.tsx (1)
Label(25-25)
src/components/ui/number-input.tsx (1)
src/components/ui/input-group.tsx (3)
InputGroup(164-164)InputGroupInput(167-167)InputGroupAddon(165-165)
src/components/form/form-field-label.tsx (5)
src/components/ui/label.tsx (1)
Label(25-25)src/features/account/change-name-drawer.tsx (1)
props(34-113)src/components/form/field-checkbox/index.tsx (1)
field(50-75)src/components/form/field-text/index.tsx (3)
TFieldValues(26-85)e(71-74)div(52-82)src/components/form/form-field-controller.tsx (1)
props(86-118)
src/components/ui/input.stories.tsx (2)
src/components/ui/input.tsx (1)
Input(57-57)src/components/ui/input-group.tsx (1)
InputGroupText(168-168)
src/components/ui/input.tsx (1)
src/components/ui/input-group.tsx (3)
InputGroup(164-164)InputGroupAddon(165-165)InputGroupInput(167-167)
🔇 Additional comments (23)
src/components/ui/sidebar.tsx (1)
502-502: LGTM! Improved semantic token usage.The change from
text-muted-foregroundtotext-sidebar-foregroundis semantically more appropriate for sidebar text and aligns with the theme token updates insrc/styles/app.css(lines 106, 143).src/components/ui/checkbox-group.tsx (1)
1-1: LGTM! Library migration to stable release.The import path correctly migrates from
@base-ui-components/react(beta) to@base-ui/react(stable v1.0.0), as reflected inpackage.jsonline 46. No functional changes to the component.src/components/form/field-number/index.tsx (1)
87-90: LGTM! Improved event handling pattern.The updated
onValueChangecallback signature now accepts(value, event)and properly forwards both parameters torest.onValueChange. This allows consumers to access event metadata while maintaining backward compatibility (the event parameter is optional).The change is consistent with similar patterns in other input components (e.g.,
src/components/ui/date-input.tsxlines 84-122).src/components/ui/number-input.stories.tsx (1)
19-19: LGTM! Accessibility improvement with aria-invalid.The update from
invalidprop toaria-invalid={true}anddata-invalidaligns with ARIA standards and improves accessibility. This change is consistent with the NumberInput component's updated API.src/components/ui/date-input.stories.tsx (2)
9-9: LGTM! Import updated for InputGroup refactor.The import change from
ButtontoInputGroupButtonaligns with the new InputGroup primitive system introduced in this PR.
46-54: LGTM! Consistent with InputGroup refactor.The prop name change from
endElementtoendAddonand the component change fromButtontoInputGroupButtonare consistent with the new InputGroup composition pattern. Thesize="icon-xs"is appropriate for the calendar icon trigger.src/components/ui/radio-group.tsx (1)
1-5: LGTM! Library imports updated.The import paths have been correctly updated from
@base-ui-components/reactto@base-ui/react, and the necessary React hooks (Fragment,useId) have been added for the accessibility enhancements.src/components/ui/date-picker.tsx (2)
7-7: LGTM! Import updated for InputGroup refactor.The import change from
ButtontoInputGroupButtonaligns with the new InputGroup primitive system.
32-41: LGTM! Consistent with InputGroup composition pattern.The transition from
endElementtoendAddonand fromButtontoInputGroupButtonis consistent with the broader InputGroup refactor. The calendar trigger is appropriately sized asicon-xs.src/components/ui/checkbox.tsx (1)
1-4: LGTM! Library imports updated.The import path has been correctly updated to
@base-ui/react, anduseIdhas been added for ID generation to support the accessibility enhancements.src/components/form/docs.stories.tsx (2)
18-23: LGTM! Clean migration to InputGroup composition.The import updates correctly reflect the new InputGroup-based pattern.
61-66: LGTM! Consistent InputGroup usage across stories.The InputGroup composition with addons is correctly implemented in both stories. The nullish coalescing (
field.value ?? '') properly handles the nullish value type from the form schema.Also applies to: 111-116
src/components/ui/select.tsx (1)
149-170: LGTM! Clean refactor to InputGroup composition.The migration from
endElementtoendAddonwithInputGroupButtoncorrectly maintains the existing functionality while adopting the new InputGroup pattern. The clear button and dropdown toggle are properly structured.src/components/ui/select.stories.tsx (1)
7-7: LGTM! Story updated to match new InputGroup API.The Customization story correctly demonstrates the new
endAddonprop andInputGroupButtonusage, replacing the previousendElementpattern. The prop name change frominputClassNametoclassNamealigns with the updated component API.Also applies to: 213-218
src/components/ui/search-input.tsx (3)
15-20: LGTM! Correct additions for InputGroup support.The new imports and props (
className,size) enable proper integration with the InputGroup composition pattern.Also applies to: 38-38, 45-45
91-101: LGTM! Clean migration to InputGroupButton.The clear button correctly migrates from
ButtontoInputGroupButton. The margin adjustment from-mr-1.5tomr-0.5is intentional for the new InputGroup layout.
107-118: LGTM! Well-structured InputGroup composition.The refactor from standalone
InputwithendElementtoInputGroupcomposition withInputGroupInputandInputGroupAddoncorrectly maintains all functionality while adopting the new pattern. Thealign="inline-end"properly positions the end element.src/components/ui/textarea.stories.tsx (1)
1-33: LGTM — solid baseline story coverage (states + sizes).src/components/ui/number-input.tsx (1)
1-1: No changes required. Base UI's render prop accepts both plain JSX elements and render functions, and it automatically merges props (className, style, event handlers) and refs into rendered elements. The code correctly uses plain JSX elements with Base UI's built-in merging behavior, and the explicitmergeRefs([ref, inputRef])at line 85 properly combines the consumer ref with the internal input ref. This implementation is sound and follows Base UI's intended patterns.Likely an incorrect or invalid review comment.
src/components/ui/input.tsx (2)
49-49: Verify:data-slot="input"overrides the internaldata-slot="input-group-control".
InputGroupInputinternally setsdata-slot="input-group-control", but this component passesdata-slot="input". Confirm this override is intentional for form integration or styling purposes.
36-54: LGTM! Clean refactor using the new InputGroup composition.The component now leverages the new InputGroup primitives effectively. The addon pattern is cleaner than the previous element-based approach.
src/components/ui/input-group.tsx (2)
9-39: Well-structured variant definitions with comprehensive state handling.The CVA variants handle size, alignment, focus, error, and disabled states comprehensively. The use of
has-*selectors for conditional styling based on child content is a good pattern.
98-113: LGTM! Good default fortype="button".Setting
type="button"by default prevents accidental form submissions, which is the correct behavior for buttons inside input groups.
| type FormFieldLabelProps = ComponentProps<'label'>; | ||
|
|
||
| export const FormFieldLabel = ({ | ||
| className, | ||
| ...props | ||
| }: FormFieldLabelProps) => { | ||
| export const FormFieldLabel = (props: FormFieldLabelProps) => { | ||
| const ctx = useFormField(); | ||
| return ( | ||
| <label | ||
| id={ctx.labelId} | ||
| htmlFor={ctx.id} | ||
| className={cn('flex gap-1.5 align-baseline text-sm', className)} | ||
| {...props} | ||
| /> | ||
| ); | ||
| return <Label id={ctx.labelId} htmlFor={ctx.id} {...props} />; | ||
| }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don’t allow overriding id / htmlFor from FormFieldLabel props (currently possible).
Because {...props} is last (Line 11), consumers can accidentally override id/htmlFor, breaking the form-field labeling contract.
Proposed fix (prop order + optional type guard)
type FormFieldLabelProps = ComponentProps<'label'>;
export const FormFieldLabel = (props: FormFieldLabelProps) => {
const ctx = useFormField();
- return <Label id={ctx.labelId} htmlFor={ctx.id} {...props} />;
+ return <Label {...props} id={ctx.labelId} htmlFor={ctx.id} />;
};Optionally prevent passing them entirely:
-type FormFieldLabelProps = ComponentProps<'label'>;
+type FormFieldLabelProps = Omit<ComponentProps<'label'>, 'id' | 'htmlFor'>;📝 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.
| type FormFieldLabelProps = ComponentProps<'label'>; | |
| export const FormFieldLabel = ({ | |
| className, | |
| ...props | |
| }: FormFieldLabelProps) => { | |
| export const FormFieldLabel = (props: FormFieldLabelProps) => { | |
| const ctx = useFormField(); | |
| return ( | |
| <label | |
| id={ctx.labelId} | |
| htmlFor={ctx.id} | |
| className={cn('flex gap-1.5 align-baseline text-sm', className)} | |
| {...props} | |
| /> | |
| ); | |
| return <Label id={ctx.labelId} htmlFor={ctx.id} {...props} />; | |
| }; | |
| type FormFieldLabelProps = ComponentProps<'label'>; | |
| export const FormFieldLabel = (props: FormFieldLabelProps) => { | |
| const ctx = useFormField(); | |
| return <Label {...props} id={ctx.labelId} htmlFor={ctx.id} />; | |
| }; |
| ); | ||
| }; | ||
|
|
||
| export function TexareaExample() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo in function name: TexareaExample should be TextareaExample.
Proposed fix
-export function TexareaExample() {
+export function TextareaExample() {📝 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.
| export function TexareaExample() { | |
| export function TextareaExample() { |
🤖 Prompt for AI Agents
In @src/components/ui/input-group.stories.tsx at line 187, The exported function
is misspelled as TexareaExample; rename the function and its export to
TextareaExample to correct the typo and ensure consumers/imports match the
intended name (update the function declaration and any references/imports that
use TexareaExample).
| <TextareaAutosize | ||
| data-slot="input-group-control" | ||
| className={cn( | ||
| 'flex field-sizing-content w-full flex-1 resize-none rounded-none border-0 bg-transparent shadow-none outline-none focus-visible:ring-0 dark:bg-transparent', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
field-sizing-content CSS browser support 2025 compatibility
💡 Result:
Short answer: field-sizing: content is implemented in Chromium-based browsers (Chrome desktop & Android, Edge) since Chrome 123 (Mar 2024). Support in Firefox and Safari was not broadly shipped (listed as limited/unavailable) as of 2025 — treat it as “limited availability” and use fallbacks (min/max-width, JS polyfill) for cross‑browser compatibility. [1][2][3]
Sources:
- Chrome / Chrome Developers docs (field-sizing). [1]
- Web Platform / feature status (browser matrix: Chrome 123+, Firefox/Safari not shipped). [2]
- MDN reference (docs + compatibility note: limited availability). [3]
🏁 Script executed:
# Check for browser support documentation, package.json, or browserslist
fd -t f "(browserslist|.browserslistrc|package.json)" | head -20
# Search for any fallback or CSS handling around this component
rg -A 5 -B 5 "field-sizing" --type tsx --type ts --type cssRepository: BearStudio/start-ui-web
Length of output: 124
🏁 Script executed:
# Search for field-sizing references
rg "field-sizing" .
# Check package.json for browser support targets
cat package.json | grep -A 10 -E "(browserslist|targets|engines|support)"Repository: BearStudio/start-ui-web
Length of output: 901
🏁 Script executed:
# Check for browserslist configuration
cat package.json | grep -A 20 "browserslist"
# Look for tailwind config
fd -name "tailwind.config.*" -o -name "tailwind.config.js"
# Check if there's a .browserslistrc file
cat .browserslistrc 2>/dev/null || echo "No .browserslistrc found"
# Check the actual input-group.tsx file to see context
cat -n src/components/ui/input-group.tsx | sed -n '145,160p'Repository: BearStudio/start-ui-web
Length of output: 940
🏁 Script executed:
# Check tailwind config for browser support targets
fd "tailwind.config" . -type f
# Check for vite config which might indicate target browsers
fd "vite.config" . -type f | head -1 | xargs cat 2>/dev/null | head -30
# Look for any documented browser support or compatibility notes in README
find . -name "README*" -type f | head -1 | xargs cat 2>/dev/null | grep -i "browser\|support\|compat" | head -10Repository: BearStudio/start-ui-web
Length of output: 408
🏁 Script executed:
# Find tailwind and vite config files
find . -maxdepth 2 -name "tailwind.config.*" -o -name "vite.config.*" 2>/dev/null
# Look in common locations
ls -la | grep -E "tailwind|vite"
# Check if there's a tsconfig or similar that might indicate browser targets
find . -maxdepth 2 -name "tsconfig.json" 2>/dev/null | head -1 | xargs cat 2>/dev/null | grep -i "target\|lib" | head -10Repository: BearStudio/start-ui-web
Length of output: 339
🏁 Script executed:
# Check vite.config.ts to see browser targets
cat vite.config.ts | head -50
# Search for any polyfills or feature detection related to field-sizing
rg "polyfill|feature|detect|field" . --max-count 20
# Check if TextareaAutosize component or any wrapper handles this
rg "TextareaAutosize" . -A 3 | head -30Repository: BearStudio/start-ui-web
Length of output: 27264
Add fallback or document browser requirement for field-sizing-content.
field-sizing-content is only supported in Chrome/Edge (Chrome 123+) as of 2025. Firefox and Safari lack support. Either provide a fallback for unsupported browsers or document this as a browser requirement, especially since the project uses polyfills for other features (resize-observer-polyfill).
🤖 Prompt for AI Agents
In @src/components/ui/input-group.tsx at line 153, The CSS utility
'field-sizing-content' used in the input class string is not supported in
Firefox/Safari; update the InputGroup component to provide a fallback or
document the requirement: wrap the use of 'field-sizing-content' in a
feature-query-based approach (apply 'field-sizing-content' only when supported
and otherwise fall back to a stable utility like 'min-w-0' or 'w-full' /
'max-w-full'), or add a project note that the component requires Chrome 123+;
target the class string containing 'field-sizing-content' in the InputGroup (or
whichever component renders that class) and implement the fallback/feature-query
or add the browser-requirement comment in the component docs accordingly.
| <Input size="sm" placeholder="Small" /> | ||
| <Input placeholder="Default" /> | ||
| <Input size="lg" placeholder="Large" /> | ||
| </div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Story updates to startAddon/endAddon look consistent with the new InputGroup model.
Minor: if MailIcon is purely decorative, consider marking it aria-hidden (and ensure the input itself has an accessible name via label in real usage).
Also applies to: 36-69
🤖 Prompt for AI Agents
In @src/components/ui/input.stories.tsx around lines 30 - 33, The MailIcon used
as a purely decorative addon in the Input stories should be marked aria-hidden
to avoid redundant accessible name announcements; update the story instances
that use startAddon/endAddon with MailIcon (e.g., the Input components and their
InputGroup usage) to set aria-hidden on the MailIcon and ensure the Input
examples include or reference an accessible name in real usage (e.g., via an
associated label) so accessibility is preserved across the examples shown
between the existing Input instances and the similar blocks at lines 36-69.
| const labelVariants = cva( | ||
| 'flex gap-1.5 align-baseline text-sm peer-disabled:cursor-not-allowed peer-disabled:opacity-70' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tailwind class likely wrong for flex baseline alignment (align-baseline vs items-baseline).
If the intent is baseline-align children in a flex row, items-baseline is the flex alignment utility; align-baseline is for vertical-align on inline/table-cell contexts.
🤖 Prompt for AI Agents
In @src/components/ui/label.tsx around lines 7 - 9, In the labelVariants cva
call, replace the incorrect Tailwind utility "align-baseline" with the flex
alignment utility "items-baseline" so the class string becomes 'flex gap-1.5
items-baseline text-sm peer-disabled:cursor-not-allowed
peer-disabled:opacity-70' (update the labelVariants definition to use
items-baseline).
| const Comp = noLabel ? Fragment : 'label'; | ||
| const _compId = useId(); | ||
| const id = labelProps?.id ?? _compId; | ||
| const compProps = noLabel | ||
| ? {} | ||
| : { | ||
| ...labelProps, | ||
| id, | ||
| className: cn(labelVariants({ size }), labelProps?.className), | ||
| }; | ||
|
|
||
| return ( | ||
| <Comp {...compProps}> | ||
| <RadioPrimitive.Root | ||
| className={cn(radioVariants({ size }), className)} | ||
| aria-labelledby={id} | ||
| {...rest} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical: aria-labelledby references non-existent ID when noLabel=true.
When noLabel is true, the component renders without a label wrapper (using Fragment), but aria-labelledby={id} is still applied to the RadioPrimitive.Root. This creates an invalid ARIA reference because no element in the DOM has the referenced id, violating WCAG guidelines and breaking screen reader functionality.
🔧 Proposed fix
return (
<Comp {...compProps}>
<RadioPrimitive.Root
className={cn(radioVariants({ size }), className)}
- aria-labelledby={id}
+ aria-labelledby={noLabel ? undefined : id}
{...rest}
>📝 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.
| const Comp = noLabel ? Fragment : 'label'; | |
| const _compId = useId(); | |
| const id = labelProps?.id ?? _compId; | |
| const compProps = noLabel | |
| ? {} | |
| : { | |
| ...labelProps, | |
| id, | |
| className: cn(labelVariants({ size }), labelProps?.className), | |
| }; | |
| return ( | |
| <Comp {...compProps}> | |
| <RadioPrimitive.Root | |
| className={cn(radioVariants({ size }), className)} | |
| aria-labelledby={id} | |
| {...rest} | |
| const Comp = noLabel ? Fragment : 'label'; | |
| const _compId = useId(); | |
| const id = labelProps?.id ?? _compId; | |
| const compProps = noLabel | |
| ? {} | |
| : { | |
| ...labelProps, | |
| id, | |
| className: cn(labelVariants({ size }), labelProps?.className), | |
| }; | |
| return ( | |
| <Comp {...compProps}> | |
| <RadioPrimitive.Root | |
| className={cn(radioVariants({ size }), className)} | |
| aria-labelledby={noLabel ? undefined : id} | |
| {...rest} |
🤖 Prompt for AI Agents
In @src/components/ui/radio-group.tsx around lines 66 - 82, The
RadioPrimitive.Root currently always receives aria-labelledby={id} even when
noLabel is true, causing an invalid ARIA reference; update the render logic so
that aria-labelledby is only passed when a real label element exists (i.e., when
noLabel is false or labelProps/id are present). Locate the noLabel, Comp, id
(from useId()), compProps, and the RadioPrimitive.Root usage and conditionally
include aria-labelledby (or switch to aria-label/aria-labelledby only when
labelProps.id or compId is rendered) so the RadioPrimitive.Root does not
reference a non-existent ID when noLabel === true.
| ComboboxProps, | ||
| } from '@headlessui/react'; | ||
| import { ChevronDown, X } from 'lucide-react'; | ||
| import { ChevronDownIcon, XIcon } from 'lucide-react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
find . -name "package.json" -o -name "package-lock.json" -o -name "yarn.lock" | head -5Repository: BearStudio/start-ui-web
Length of output: 81
🌐 Web query:
lucide-react 0.545.0 ChevronDownIcon XIcon icon exports documentation
💡 Result:
Short answer: lucide-react (v0.545.0) exports icons as named React components (no "Icon" suffix). For example import { ChevronDown } and { X } from 'lucide-react' and use them as and . See the package guide and the specific icon pages for usage and props (size, color, strokeWidth). [1][2][3]
References
- Lucide React guide (usage / props). [1]
- ChevronDown icon (shows import as
ChevronDown). [2] - X icon (shows import as
X). [3]
🏁 Script executed:
cat package.json | grep -A 2 -B 2 lucide-reactRepository: BearStudio/start-ui-web
Length of output: 265
Correct icon import names for lucide-react 0.545.0.
The icon names ChevronDownIcon and XIcon do not exist in lucide-react 0.545.0. These imports will fail. The correct names are ChevronDown and X (without the "Icon" suffix).
Change:
import { ChevronDown, X } from 'lucide-react';
🤖 Prompt for AI Agents
In @src/components/ui/select.tsx at line 12, The import in
src/components/ui/select.tsx uses non-existent names ChevronDownIcon and XIcon
from lucide-react; update the import statement to use the correct exported names
ChevronDown and X (remove the "Icon" suffix) so components that reference
ChevronDown/X render correctly.
4811903 to
08fc060
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/components/ui/input-group.stories.tsx:
- Line 194: The className string contains a misplaced trailing exclamation mark
("min-h-[200px]!") which is invalid Tailwind syntax; update the JSX prop (the
element with className="min-h-[200px]!") to either include the important
modifier at the start ("!min-h-[200px]") or remove the modifier entirely
("min-h-[200px]") so the className is valid.
In @src/components/ui/input-group.tsx:
- Around line 87-92: The InputGroup click handler currently queries only
'input', so clicking the addon won't focus textarea-based inputs (e.g.,
InputGroupTextarea); update the handler in the onClick for InputGroup so it
queries both input and textarea (or, preferably, select by the component data
attribute like [data-slot="input"]) and call focus() on the found element;
reference the onClick arrow function in src/components/ui/input-group.tsx and
replace querySelector('input') with a selector that includes textarea or the
data-slot selector to ensure both input and textarea elements are focused.
🧹 Nitpick comments (1)
src/components/ui/input-group.stories.tsx (1)
58-58: Consider removing!importantoverrides.Multiple stories use
!importantmodifiers in classNames (lines 58, 94, 164, 319). This suggests potential specificity conflicts with the base InputGroup styles. While acceptable in story files for demonstration purposes, consider whether the base component styles need adjustment to avoid requiring!importantin production code.Also applies to: 94-94, 164-164, 319-319
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (20)
package.jsonsrc/components/form/docs.stories.tsxsrc/components/form/field-text/docs.stories.tsxsrc/components/form/form-field-label.tsxsrc/components/ui/base-select.tsxsrc/components/ui/date-input.stories.tsxsrc/components/ui/date-picker.tsxsrc/components/ui/input-group.stories.tsxsrc/components/ui/input-group.tsxsrc/components/ui/input.stories.tsxsrc/components/ui/input.tsxsrc/components/ui/label.tsxsrc/components/ui/number-input.tsxsrc/components/ui/search-input.tsxsrc/components/ui/select.stories.tsxsrc/components/ui/select.tsxsrc/components/ui/sidebar.tsxsrc/components/ui/textarea.stories.tsxsrc/components/ui/textarea.tsxsrc/styles/app.css
💤 Files with no reviewable changes (1)
- src/components/form/field-text/docs.stories.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- src/components/ui/select.stories.tsx
- src/components/ui/input.tsx
- src/components/ui/textarea.stories.tsx
- src/components/form/form-field-label.tsx
- src/components/ui/select.tsx
- src/components/ui/textarea.tsx
- package.json
- src/components/form/docs.stories.tsx
- src/components/ui/label.tsx
🧰 Additional context used
🧬 Code graph analysis (7)
src/components/ui/date-input.stories.tsx (2)
src/components/ui/input-group.tsx (1)
InputGroupButton(166-166)src/components/ui/date-input.tsx (2)
props(85-123)() => undefined(99-99)
src/components/ui/input-group.stories.tsx (2)
src/components/ui/input-group.tsx (6)
InputGroup(164-164)InputGroupInput(167-167)InputGroupAddon(165-165)InputGroupText(168-168)InputGroupButton(166-166)InputGroupTextarea(169-169)src/components/ui/label.tsx (1)
Label(25-25)
src/components/ui/date-picker.tsx (1)
src/components/ui/input-group.tsx (1)
InputGroupButton(166-166)
src/components/ui/input-group.tsx (3)
src/components/ui/date-input.tsx (1)
props(85-123)src/components/ui/number-input.stories.tsx (3)
div(42-50)div(32-40)div(132-139)src/components/ui/radio-group.tsx (1)
RadioGroup(40-47)
src/components/ui/number-input.tsx (1)
src/components/ui/input-group.tsx (3)
InputGroup(164-164)InputGroupInput(167-167)InputGroupAddon(165-165)
src/components/ui/input.stories.tsx (1)
src/components/ui/input-group.tsx (1)
InputGroupText(168-168)
src/components/ui/sidebar.tsx (1)
src/layout/manager/nav-sidebar.tsx (3)
SidebarMenuButton(108-115)SidebarMenuButton(80-87)SidebarMenuButton(68-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (16)
src/components/ui/sidebar.tsx (1)
501-502: LGTM! Semantic token improvement.The change from
text-muted-foregroundtotext-sidebar-foregroundimproves semantic consistency by ensuring sidebar buttons use sidebar-specific color tokens rather than generic muted tokens. This aligns with the CSS theme token adjustments insrc/styles/app.css.src/components/ui/date-input.stories.tsx (2)
9-9: LGTM! Import updated for InputGroup pattern.The import change from
ButtontoInputGroupButtonaligns with the new InputGroup composition system introduced in this PR.
46-54: LGTM! Consistent migration to InputGroup composition.The changes correctly migrate the story to the new InputGroup pattern:
endElement→endAddon(consistent prop naming)Button→InputGroupButton(appropriate wrapper for input addons)The composition structure remains logical with the Popover trigger functioning as an input addon.
src/styles/app.css (2)
91-91: LGTM! Refined theme token values for light mode.The adjustments to
--muted-foreground(neutral-600 → neutral-500) and--sidebar-foreground(neutral-950 → neutral-600) create more consistent, muted foreground colors in light mode. These changes align with the broader token standardization across the UI components.Also applies to: 106-106
129-129: LGTM! Refined theme token values for dark mode.The adjustments to
--muted-foreground(neutral-300 → neutral-400) and--sidebar-foreground(neutral-50 → neutral-300) create more consistent, muted foreground colors in dark mode, maintaining appropriate contrast while improving visual hierarchy.Also applies to: 143-143
src/components/ui/date-picker.tsx (2)
7-7: LGTM! Import updated for InputGroup pattern.The import change from
ButtontoInputGroupButtonaligns with the new InputGroup composition system.
32-41: LGTM! Correct migration to InputGroup composition.The changes properly migrate the DatePicker to use the new InputGroup pattern:
endElement→endAddon(consistent with the new API)Button→InputGroupButton(appropriate for input addon buttons)The conditional rendering and Popover integration remain intact and functional.
src/components/ui/search-input.tsx (4)
15-20: LGTM! Imports added for InputGroup composition.The new imports correctly bring in the InputGroup family of components needed for the refactored composition pattern.
38-38: LGTM! Props exposed for InputGroup composition.The addition of
classNameandsizeprops enables proper forwarding to theInputGroupwrapper, maintaining flexibility in component styling and sizing.Also applies to: 45-45
91-101: LGTM! Clear button migrated to InputGroupButton.The clear button correctly uses
InputGroupButtoninstead ofButton. The margin adjustment from-mr-1.5tomr-0.5likely accommodates the new InputGroup layout constraints and spacing.
107-118: LGTM! Successful refactor to InputGroup composition.The refactor properly implements the InputGroup pattern:
Inputwrapped asInputGroupInputwith props correctly spread- End element (search icon/clear button/spinner) rendered via
InputGroupAddonwithalign="inline-end"sizeandclassNameforwarded toInputGroupfor consistent styling- Ref forwarding preserved to
InputGroupInputThe composition structure maintains all previous functionality while adopting the new composable InputGroup system.
src/components/ui/number-input.tsx (1)
11-16: LGTM! Clean refactor to InputGroup composition.The migration from direct
Inputusage to the newInputGroupcomposition pattern is well-executed. The accessibility prop (aria-invalid) is correctly forwarded, refs are properly merged, and the layout logic for classic vs. mobile button variants is preserved.Also applies to: 80-109
src/components/ui/input.stories.tsx (1)
5-5: LGTM! Clear migration to the new addon API.The story updates properly demonstrate the new
startAddon/endAddonprops and the use ofInputGroupTextfor text content. The added note pointing users to theInputGroupstory for advanced use cases is a helpful touch.Also applies to: 36-70
src/components/ui/input-group.stories.tsx (1)
1-333: Excellent comprehensive examples for InputGroup.The story file provides thorough coverage of InputGroup capabilities across different use cases: icons, text addons, textareas, spinners, labels, and dropdowns. The examples effectively demonstrate the flexibility of the composition pattern.
src/components/ui/input-group.tsx (2)
1-170: Well-architected InputGroup composition system.The module provides a clean, composable API for building complex input groups. Key strengths:
- Consistent use of
data-slotattributes for styling hooks- Proper
role="group"for accessibility- CVA-based variants for responsive sizing
- Flexible alignment system (inline-start/end, block-start/end)
- Good integration with existing Button and TextareaAutosize components
153-153: No action needed.field-sizing-contentis a valid Tailwind CSS v4 utility that maps to the CSS propertyfield-sizing: content;. It is officially documented and supported in the framework.Likely an incorrect or invalid review comment.
| <InputGroupTextarea | ||
| id="textarea-code-32" | ||
| placeholder="console.log('Hello, world!');" | ||
| className="min-h-[200px]!" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix className syntax error.
The exclamation mark is misplaced in the className. It should be "!min-h-[200px]" or "min-h-[200px]" (without the important modifier).
🐛 Proposed fix
- className="min-h-[200px]!"
+ className="!min-h-[200px]"📝 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.
| className="min-h-[200px]!" | |
| className="!min-h-[200px]" |
🤖 Prompt for AI Agents
In @src/components/ui/input-group.stories.tsx at line 194, The className string
contains a misplaced trailing exclamation mark ("min-h-[200px]!") which is
invalid Tailwind syntax; update the JSX prop (the element with
className="min-h-[200px]!") to either include the important modifier at the
start ("!min-h-[200px]") or remove the modifier entirely ("min-h-[200px]") so
the className is valid.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/components/ui/input-group.tsx (1)
77-97: Consider adding keyboard accessibility for the click-to-focus behavior.The click handler focuses the input when clicking on the addon (excluding buttons), which is good UX. However, the logic could be improved:
- The
querySelector('input')won't find a textarea - consider also querying fortextarea.- The type assertion
as HTMLElementshould be safe here but could benefit from a null check.♻️ Suggested improvement
onClick={(e) => { if ((e.target as HTMLElement).closest('button')) { return; } - e.currentTarget.parentElement?.querySelector('input')?.focus(); + const parent = e.currentTarget.parentElement; + const focusable = parent?.querySelector('input') ?? parent?.querySelector('textarea'); + focusable?.focus(); }}src/components/form/field-textarea/field-text.browser.spec.tsx (3)
1-1: File naming inconsistency.The file is named
field-text.browser.spec.tsxbut it tests theFieldTextareacomponent. Consider renaming tofield-textarea.browser.spec.tsxfor consistency and discoverability.
32-32: Incorrect type assertion for textarea element.The element is a
<textarea>, not an<input>. UsingHTMLInputElementis technically incorrect, though it works because both have a.valueproperty. Consider usingHTMLTextAreaElementfor type accuracy.♻️ Suggested fix
- const input = page.getByLabelText('Name').element() as HTMLInputElement; + const input = page.getByLabelText('Name').element() as HTMLTextAreaElement;Also applies to: 65-65
95-99: Consider using explicit assertions instead of try/catch.The try/catch pattern for testing disabled/readOnly behavior is fragile. Consider asserting the element's state directly and using a more robust testing approach.
♻️ Suggested improvement for disabled test
const input = page.getByLabelText('Name'); - try { - await user.type(input, 'another value'); - } catch { - // Expected to fail since input is disabled - } + expect(input.element()).toBeDisabled(); await user.click(page.getByRole('button', { name: 'Submit' }));♻️ Suggested improvement for readOnly test
const input = page.getByLabelText('Name'); - try { - await user.type(input, 'another value'); - } catch { - // Expected to fail since input is readOnly - } + expect(input.element()).toHaveAttribute('readonly'); await user.click(page.getByRole('button', { name: 'Submit' }));Also applies to: 128-132
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/form/docs.stories.tsxsrc/components/form/field-textarea/docs.stories.tsxsrc/components/form/field-textarea/field-text.browser.spec.tsxsrc/components/form/field-textarea/index.tsxsrc/components/form/form-field-controller.tsxsrc/components/ui/input-group.tsxsrc/components/ui/textarea.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/ui/textarea.tsx
🧰 Additional context used
🧬 Code graph analysis (3)
src/components/form/form-field-controller.tsx (2)
src/components/form/field-textarea/index.tsx (2)
FieldTextareaProps(12-24)FieldTextarea(26-84)src/components/form/field-text/index.tsx (3)
TFieldValues(26-85)div(52-82)e(71-74)
src/components/form/field-textarea/docs.stories.tsx (4)
src/lib/zod/zod-utils.ts (1)
zu(12-42)src/components/form/form.tsx (1)
Form(25-54)src/components/form/form-field-label.tsx (1)
FormFieldLabel(9-12)src/components/form/form-field-controller.tsx (1)
FormFieldController(77-148)
src/components/form/docs.stories.tsx (3)
src/components/form/form-field-controller.tsx (1)
FormFieldController(77-148)src/components/ui/input-group.tsx (4)
InputGroup(165-165)InputGroupAddon(166-166)InputGroupText(169-169)InputGroupInput(168-168)src/components/form/field-text/index.tsx (3)
TFieldValues(26-85)e(71-74)div(52-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (17)
src/components/ui/input-group.tsx (5)
9-40: Well-structured variant definitions.The CVA configuration for input group variants is comprehensive, covering size variants, alignment-based layout changes, disabled states, focus states, and error states with proper dark mode support.
41-54: LGTM!The InputGroup component correctly applies variants, passes through props, and uses appropriate data-slot and role attributes for composition and accessibility.
99-114: LGTM!InputGroupButton provides sensible defaults for type, size, and variant while allowing full customization through prop spreading.
128-144: LGTM!InputGroupInput correctly strips native input styling and integrates with the parent InputGroup via data-slot attributes. The read-only and disabled cursor states are properly handled.
146-162: LGTM!InputGroupTextarea follows the same pattern as InputGroupInput, using TextareaAutosize for auto-growing behavior. The styling is consistent with the input variant.
src/components/form/form-field-controller.tsx (3)
18-21: LGTM!The import follows the established pattern used by other field types in this file.
71-71: LGTM!The type union is correctly extended to include FieldTextareaProps.
101-103: LGTM!The textarea case follows the established switch pattern and correctly renders FieldTextarea.
src/components/form/docs.stories.tsx (3)
18-23: LGTM!The imports for InputGroup components are correctly structured and align with the new primitives introduced in this PR.
50-54: Intentional change to demonstrate textarea functionality.The switch from
type="text"totype="textarea"for the Name field appears intentional to showcase the new textarea support. This is appropriate for documentation/stories.Also applies to: 100-104
65-70: LGTM!The InputGroup composition correctly demonstrates the new primitive usage with an addon containing text, showing the "https://" prefix pattern. The
value={field.value ?? ''}handles the nullable field type appropriately.Also applies to: 115-120
src/components/form/field-textarea/docs.stories.tsx (3)
17-28: LGTM!The schema and form options are well-structured. The Zod 4
errorparameter syntax for custom error messages is correctly used.
30-52: LGTM!The Default story provides a clean example of basic textarea usage with proper form integration.
54-81: LGTM!The DefaultValue, Disabled, and ReadOnly stories properly demonstrate the different states and configurations of the textarea field, providing comprehensive documentation coverage.
Also applies to: 83-111, 113-141
src/components/form/field-textarea/index.tsx (3)
12-24: LGTM!The type definition follows the established pattern for field props, correctly extending FieldProps with textarea-specific configuration.
33-42: Thetypevariable is correctly omitted from rest props.The destructuring of
type(line 35) ensures it doesn't get passed to the Textarea component via...rest. This is intentional and correct, even though the variable appears unused.
60-78: LGTM!The Textarea rendering correctly:
- Sets accessibility attributes (aria-invalid, aria-describedby)
- Chains onChange/onBlur handlers to support both react-hook-form and custom handlers
- Follows the same pattern as FieldText from the relevant snippets
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @src/components/ui/input-group.tsx:
- Around line 77-101: InputGroupAddon currently overwrites any user-provided
onClick because it spreads props after setting its own handler; pull onClick out
of props (e.g., const { onClick, ...rest } = props), use rest in the JSX spread,
and call the user onClick inside the component’s click handler (e.g., invoke
onClick(e) before/after the internal focus logic), preserving the original event
and return behavior; reference InputGroupAddon and the onClick prop when
locating and updating the code.
- Around line 150-166: InputGroupTextarea relies on the non‑portable utility
class 'field-sizing-content' which maps to CSS field-sizing: content; add a
browser fallback: update the component's class usage or global stylesheet so
that when field-sizing is unsupported (use @supports not (field-sizing:
content)) you apply an alternative sizing rule (e.g., set inline-size/width or
use flex basis/min-width rules) that mimics the intended behavior; change or add
a fallback utility class name referenced by InputGroupTextarea (and keep
className spreading) so TextareaAutosize gets consistent sizing across
Firefox/Safari.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/ui/input-group.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
src/components/ui/input-group.tsx (2)
src/components/ui/input.tsx (2)
Input(66-114)className(58-64)src/components/ui/number-input.tsx (1)
invalid(25-115)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (8)
src/components/ui/input-group.tsx (8)
1-7: LGTM!The imports are well-structured and necessary for the InputGroup implementation.
41-54: LGTM!The InputGroup component is well-structured with proper accessibility attributes and variant handling.
56-75: LGTM!The addon variants provide flexible alignment options with appropriate styling hooks.
103-118: LGTM!The InputGroupButton wrapper provides sensible defaults and properly extends the Button component.
120-130: LGTM!The InputGroupText component provides a clean wrapper for inline text/content with proper SVG handling.
132-148: LGTM!The InputGroupInput component correctly strips default input styling for seamless composition within the InputGroup.
168-175: LGTM!All components are properly exported for external usage.
9-40: No action needed. The:has()pseudo-class is widely supported in all modern browsers (Safari 15.4+, Chrome/Edge 105+, Firefox 121+), and this modern React 19 project targets contemporary browsers by default. There is no browser compatibility concern with the current implementation.
| function InputGroupAddon({ | ||
| className, | ||
| align = 'inline-start', | ||
| ...props | ||
| }: React.ComponentProps<'div'> & VariantProps<typeof inputGroupAddonVariants>) { | ||
| return ( | ||
| <div | ||
| role="group" | ||
| data-slot="input-group-addon" | ||
| data-align={align} | ||
| className={cn(inputGroupAddonVariants({ align }), className)} | ||
| onClick={(e) => { | ||
| if ((e.target as HTMLElement).closest('button')) { | ||
| return; | ||
| } | ||
| e.currentTarget.parentElement | ||
| ?.querySelector< | ||
| HTMLInputElement | HTMLTextAreaElement | ||
| >('input, textarea') | ||
| ?.focus(); | ||
| }} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve user-provided onClick handler.
The current implementation replaces any onClick prop passed by the user. This could break expected behavior when consumers want to attach custom click handlers.
🔧 Proposed fix to preserve the original onClick
function InputGroupAddon({
className,
align = 'inline-start',
+ onClick,
...props
}: React.ComponentProps<'div'> & VariantProps<typeof inputGroupAddonVariants>) {
return (
<div
role="group"
data-slot="input-group-addon"
data-align={align}
className={cn(inputGroupAddonVariants({ align }), className)}
onClick={(e) => {
if ((e.target as HTMLElement).closest('button')) {
return;
}
e.currentTarget.parentElement
?.querySelector<
HTMLInputElement | HTMLTextAreaElement
>('input, textarea')
?.focus();
+ onClick?.(e);
}}
{...props}
/>
);
}🤖 Prompt for AI Agents
In @src/components/ui/input-group.tsx around lines 77 - 101, InputGroupAddon
currently overwrites any user-provided onClick because it spreads props after
setting its own handler; pull onClick out of props (e.g., const { onClick,
...rest } = props), use rest in the JSX spread, and call the user onClick inside
the component’s click handler (e.g., invoke onClick(e) before/after the internal
focus logic), preserving the original event and return behavior; reference
InputGroupAddon and the onClick prop when locating and updating the code.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/components/ui/input-group.tsx:
- Around line 174-190: The class string in InputGroupTextarea uses the
non‑cross‑browser utility 'field-sizing-content' which breaks Firefox; remove
'field-sizing-content' from the className and replace it with a Firefox-friendly
fallback such as adding 'min-w-0' (and keep existing 'w-full flex-1') so the
textarea can shrink correctly in flex containers; update the class list in the
TextareaAutosize call inside the InputGroupTextarea component accordingly.
🧹 Nitpick comments (4)
src/components/ui/textarea.stories.tsx (1)
35-71: Consider exposing height constraints as component props instead of relying on internal structure.The MinHeight, FixedHeight, and MaxHeight stories use
[&>textarea]:selectors (e.g.,className="[&>textarea]:min-h-24") to target the internal textarea element. This approach is fragile because:
- It tightly couples stories to the component's internal DOM structure
- If the Textarea component's structure changes, these selectors will silently fail
- It conflicts with the default
max-h-64set ininput-group.tsxline 18, requiring overridesConsider adding explicit props to the Textarea component for height control (e.g.,
minHeight,maxHeight,fixedHeight) to make the API more robust and discoverable.Alternative approach with explicit props
If the Textarea component exposed height props, the stories could be written more clearly:
export const MinHeight = () => { return ( <div className="flex flex-col gap-4"> <Textarea placeholder="Placeholder..." minHeight={0} /> <Textarea placeholder="Placeholder..." minHeight="24" /> </div> ); };This would make the API self-documenting and prevent breakage from internal refactoring.
src/components/ui/input-group.tsx (3)
9-64: High complexity in variant selectors may impact maintainability.The
inputGroupVariantsdefinition uses deeply nested CSS selectors withhas-[]and[&>element]:patterns that are complex to parse and maintain. While functionally correct, consider:
- Line 18: The default
[&>textarea]:max-h-64is overridden in stories (seetextarea.stories.tsxlines 38-68), suggesting this default may be too restrictive- Lines 37-57: The size variants contain extensive selector chains (e.g.,
has-[>[data-align=block-end]]:[&>input]:pt-3) that couple styling tightly to the component structure- Line 32: The error state selector
has-[[data-slot][aria-invalid=true]]is difficult to readThese patterns work but create maintenance burden. If the component structure needs to change, many selectors must be updated simultaneously.
Consider documenting selector patterns
Add inline comments explaining the selector patterns to help future maintainers:
const inputGroupVariants = cva( cn( // ... base styles ... // Textarea: Set max height to prevent excessive growth // Note: Can be overridden via className prop 'has-[>textarea]:h-auto [&>textarea]:max-h-64 [&>textarea]:md:text-sm', // ... rest of styles ... ), // ... );
101-125: DOM traversal in onClick handler may be fragile.The InputGroupAddon's onClick handler (lines 112-121) uses
e.currentTarget.parentElement?.querySelector()to find and focus the input/textarea. This approach:
- Assumes the addon is always a direct child of the InputGroup
- Will break if the component hierarchy changes
- Uses a broad querySelector that could match unintended elements in complex layouts
Consider passing the input ref through context or using a more robust identification mechanism.
More robust approach using React Context
// Create a context for the input ref const InputGroupContext = React.createContext<{ inputRef?: React.RefObject<HTMLInputElement | HTMLTextAreaElement>; }>({}); function InputGroup({ ... }) { const inputRef = React.useRef<HTMLInputElement | HTMLTextAreaElement>(null); return ( <InputGroupContext.Provider value={{ inputRef }}> <div ... /> </InputGroupContext.Provider> ); } function InputGroupAddon({ ... }) { const { inputRef } = React.useContext(InputGroupContext); return ( <div onClick={(e) => { if ((e.target as HTMLElement).closest('button')) return; inputRef?.current?.focus(); }} ... /> ); }This approach is more reliable and doesn't depend on DOM structure.
127-142: Clarify the purpose of setting bothdata-sizeandsizeattributes.Line 136 sets both
data-size={size}andsize={size}on the Button component. Ifdata-sizeis used for CSS selectors or testing, consider documenting this pattern. If it's redundant, remove it to avoid confusion.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/ui/input-group.tsxsrc/components/ui/textarea.stories.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/ui/input-group.tsx (3)
src/components/ui/input.tsx (2)
Input(66-114)className(58-64)src/components/ui/date-input.tsx (1)
props(85-123)src/components/ui/radio-group.tsx (1)
RadioGroup(40-47)
src/components/ui/textarea.stories.tsx (1)
src/components/ui/input.stories.tsx (5)
Default(11-13)Invalid(15-17)Disabled(19-21)ReadOnly(23-25)Sizes(27-35)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Playwright E2E Tests
🔇 Additional comments (2)
src/components/ui/textarea.stories.tsx (1)
1-33: LGTM! Standard Storybook stories are well-structured.The Default, Invalid, Disabled, ReadOnly, and Sizes stories follow standard patterns and provide good coverage of the Textarea component's basic states and variations.
src/components/ui/input-group.tsx (1)
1-199: Well-structured composable InputGroup system with comprehensive subcomponents.The InputGroup primitive provides a flexible composition model that addresses the PR objectives of supporting more complex components. The variant system using CVA is appropriate, and the data-slot attributes provide good styling hooks. The exports are clean and the component APIs are consistent.
| function InputGroupTextarea({ | ||
| className, | ||
| ...props | ||
| }: React.ComponentProps<typeof TextareaAutosize>) { | ||
| return ( | ||
| <TextareaAutosize | ||
| data-slot="input-group-control" | ||
| className={cn( | ||
| 'flex field-sizing-content w-full flex-1 resize-none rounded-none border-0 bg-transparent shadow-none outline-none focus-visible:ring-0 dark:bg-transparent', | ||
| 'read-only:cursor-not-allowed disabled:cursor-not-allowed', | ||
|
|
||
| className | ||
| )} | ||
| {...props} | ||
| /> | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
field-sizing-content CSS property browser support
💡 Result:
Short answer: field-sizing: content is supported in Chromium-based browsers (Chrome/Edge/Opera) starting around Chrome/Edge 123+ and in Safari (WebKit) from Safari 26.2+, but Firefox does not support it (no support as of current data). See compatibility tables/details. (caniuse.com)
Citations:
Address missing Firefox support for field-sizing-content CSS property.
Line 182 uses field-sizing-content, which is not supported in Firefox (no support in current versions). While supported in Chromium-based browsers (Chrome/Edge v123+) and Safari (v26.2+), this property will cause layout issues or fallback behavior in Firefox. Consider providing a fallback solution or alternative approach that works across all target browsers, or remove the property if browser compatibility is a requirement.
🤖 Prompt for AI Agents
In @src/components/ui/input-group.tsx around lines 174 - 190, The class string
in InputGroupTextarea uses the non‑cross‑browser utility 'field-sizing-content'
which breaks Firefox; remove 'field-sizing-content' from the className and
replace it with a Firefox-friendly fallback such as adding 'min-w-0' (and keep
existing 'w-full flex-1') so the textarea can shrink correctly in flex
containers; update the class list in the TextareaAutosize call inside the
InputGroupTextarea component accordingly.


This will allow for better composition for more complex components such as MultiSelect, Autocomplete, ...
Summary by CodeRabbit
New Features
Changes
Documentation & Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.