Repository navigation
refactor(form-input): add validation & size stories and status text support - #255
Conversation
…upport Add dedicated Storybook stories for ValidationStates and Sizes to show all validation states (default, error, success, warning) and input size variants (sm, md, lg). Update existing Molecules FormInput story to: - expose a new boolean control `showHelpTextIcon` in story args. - add a default state variant and fix duplicated IDs for error/warning variants. - rename several labels for clarity and replace `extraText` usage with `helpText` where appropriate. - change multiple examples to use `helpText` consistently. In the FormInput component: - replace ExtraText/ErrorText imports with a unified StatusText atom. - add `showHelpTextIcon` prop and remove `extraText` prop and related id logic. - simplify rendering of status/help text to rely on StatusText. These changes clarify the component API, fix story inaccuracies, and provide explicit visual documentation of validation and sizing options.
|
Review changes with Changed Files
|
|
You do not have enough credits to review this pull request. Please purchase more credits to continue. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Rate limit exceeded@BleedingDev has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 7 minutes and 36 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughThe form input component has been refactored to consolidate help and status text rendering through a unified StatusText component, replacing separate ErrorText and ExtraText handling. Props have been updated to remove Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Mesa DescriptionTL;DRRefactored What changed?
Description generated by Mesa. Update settings |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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 (4)
libs/ui/src/molecules/form-input.tsx (1)
16-44: Remove unused showHelpTextIcon parameter from FormInputRaw.The
showHelpTextIconparameter is accepted byFormInputRaw(line 21) and passed down fromFormInput(line 71), but it's never used withinFormInputRawitself. The component simply renders{helpText}directly at line 42 without any conditional logic based on this prop.Since the
StatusTextwrapper with the icon is applied in the parentFormInputcomponent (lines 58-66), there's no need forFormInputRawto receive this prop.🔎 Proposed fix
interface FormInputRawProps extends InputProps { id: string label: ReactNode validateStatus?: ValidateStatus helpText?: ReactNode - showHelpTextIcon?: boolean } export function FormInputRaw({ id, label, validateStatus = "default", helpText, - showHelpTextIcon, size = "md", required, disabled, ...props }: FormInputRawProps) {And in FormInput:
id={id} size={size} validateStatus={validateStatus} - showHelpTextIcon={showHelpTextIcon} {...props} />libs/ui/stories/molecules/form-input.stories.tsx (3)
59-74: Fix empty ID attributes.The empty string IDs (
id="") at lines 61, 65, and 72 are invalid. Each input must have a unique ID for proper label association and accessibility.🔎 Proposed fix
<VariantGroup title="Requirement States"> <div className="w-64"> - <FormInput id="" label="Default" placeholder="Enter value" /> + <FormInput id="default-requirement" label="Default" placeholder="Enter value" /> </div> <div className="w-64"> <FormInput - id="" + id="required-requirement" label="Required" placeholder="Enter value" required /> </div> <div className="w-64"> - <FormInput id="" label="Optional" placeholder="Enter value" /> + <FormInput id="optional-requirement" label="Optional" placeholder="Enter value" /> </div> </VariantGroup>
115-194: Fix duplicate "success-input" IDs throughout AllVariants.Multiple FormInput components share the same ID "success-input" (lines 118, 138, 148, 158, 171, 179, 189). This violates HTML standards, breaks label-to-input associations, and causes accessibility issues. Each input must have a unique ID.
🔎 Proposed fix
<VariantGroup title="Supporting Text"> <div className="w-64"> <FormInput - id="success-input" + id="helper-text-example" label="With helper text" placeholder="Enter value" helpText="This is help text below input" /> </div> <div className="w-64"> <FormInput - id="error-input" + id="error-message-example" label="With error message" placeholder="Enter value" helpText="This is error text!" validateStatus="error" /> </div> </VariantGroup> <VariantGroup title="Sizes"> <div className="w-64"> <FormInput - id="success-input" + id="small-size-example" label="Small input" placeholder="Enter value" helpText="This is error text below input" validateStatus="error" size="sm" /> </div> <div className="w-64"> <FormInput - id="success-input" + id="medium-size-example" label="Default input" placeholder="Enter value" helpText="This is error text below input" validateStatus="error" size="md" /> </div> <div className="w-64"> <FormInput - id="success-input" + id="large-size-example" label="Large input" placeholder="Enter value" helpText="This is error text below input" validateStatus="error" size="lg" /> </div> </VariantGroup> <VariantGroup title="Special States"> <div className="w-64"> <FormInput - id="success-input" + id="disabled-example" label="Disabled input" placeholder="Cannot edit" disabled /> </div> <div className="w-64"> <FormInput - id="success-input" + id="readonly-example" label="Read-only input" placeholder="Read only" readOnly value="Fixed value" /> </div> <div className="w-64"> <FormInput - id="success-input" + id="default-value-example" label="With default value" defaultValue="Prefilled value" /> </div> </VariantGroup>
263-373: Fix duplicate IDs in InteractiveValidation and RegistrationForm stories.Both stories use "success-input" as the ID for all FormInput components (lines 289, 322, 329, 338, 346, 355). Each input within a story must have a unique ID.
🔎 Proposed fix
function EmailValidationExample() { // ... state logic ... return ( <div className="w-80"> <h3 className="mb-4 font-medium text-lg">Email Validation</h3> <FormInput - id="success-input" + id="email-validation-input" label="Email" placeholder="your@email.com" required value={email} onChange={(e) => setEmail(e.target.value)} onBlur={() => setTouched(true)} validateStatus={validateStatus} helpText={ showError ? 'Please enter a valid email' : 'Used for login and notifications' } /> // ... </div> ) } // Form usage example export const RegistrationForm: Story = { render: () => { return ( <div className="w-96 rounded-lg border border-gray-200 p-6 shadow-sm"> <h2 className="mb-6 font-semibold text-xl">Account Registration</h2> <div className="space-y-4"> <FormInput - id="success-input" + id="fullname-input" label="Full name" placeholder="John Doe" required /> <FormInput - id="success-input" + id="email-input" label="Email" type="email" placeholder="john@example.com" required helpText="We'll send confirmation to this email" /> <FormInput - id="success-input" + id="username-input" label="Username" placeholder="johndoe" required helpText="Visible to other users" /> <FormInput - id="success-input" + id="password-input" label="Password" type="password" placeholder="••••••••" required helpText="Min 8 characters, 1 number, 1 special character" /> <FormInput - id="success-input" + id="phone-input" label="Phone number" type="tel" placeholder="+1 (XXX) XXX-XXXX" /> </div> // ... </div> ) }, }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/molecules/form-input.tsxlibs/ui/stories/molecules/form-input.stories.tsx
🧰 Additional context used
📓 Path-based instructions (5)
libs/ui/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Implement UI library components using Zag.js for React and Tailwind CSS for styling
Files:
libs/ui/stories/molecules/form-input.stories.tsxlibs/ui/src/molecules/form-input.tsx
libs/ui/stories/**/*.stories.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/stories/**/*.stories.tsx: Storybook stories must exist for components and cover all variants and states
Use Storybook title hierarchy matching the layer (e.g., Atoms/ComponentName)
Files:
libs/ui/stories/molecules/form-input.stories.tsx
libs/ui/src/{atoms,molecules}/**/*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
libs/ui/src/{atoms,molecules}/**/*.tsx: React 19 components must not use forwardRef; pass ref as a prop instead
Do not use useCallback in components
Component implementations should use tv() with token-based classes (no raw Tailwind arbitrary values)
Use data attributes for state-based styling (e.g., data-[state=open], data-[disabled])
Do not use arbitrary Tailwind values in class names (e.g., bg-[#ff0000], bg-[var(--...)]); use token classes instead
Do not use direct semantic tokens like bg-primary; use component-specific token classes (e.g., bg-button-bg-primary)
Exported React components should be PascalCase (e.g., ComponentName)
Do not use abbreviations in component names (e.g., btn, pc)
When using Zag.js, derive styles from data attributes (e.g., data-state) rather than imperative styling
Files:
libs/ui/src/molecules/form-input.tsx
libs/ui/src/molecules/**/*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Interactive components must use Zag.js patterns (machine, useMachine, normalizeProps, connect)
Files:
libs/ui/src/molecules/form-input.tsx
libs/ui/src/{atoms,molecules}/[a-z0-9-]*.tsx
📄 CodeRabbit inference engine (libs/ui/CLAUDE.md)
Component files must be placed under src/atoms or src/molecules and named in kebab-case (component-name.tsx)
Files:
libs/ui/src/molecules/form-input.tsx
🧠 Learnings (18)
📓 Common learnings
Learnt from: KaiUweCZE
Repo: NMIT-WR/new-engine PR: 151
File: libs/ui/stories/molecules/product-card.stories.tsx:214-227
Timestamp: 2025-10-03T08:38:48.431Z
Learning: Story files in libs/ui/stories/**/*.tsx can use semantic tokens (e.g., bg-accent, text-accent-fg, hover:bg-accent-hover) for demonstration purposes, unlike component implementation files which should use component-specific token classes.
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/stories/**/*.stories.tsx : Storybook stories must exist for components and cover all variants and states
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsx
📚 Learning: 2025-10-03T08:38:48.431Z
Learnt from: KaiUweCZE
Repo: NMIT-WR/new-engine PR: 151
File: libs/ui/stories/molecules/product-card.stories.tsx:214-227
Timestamp: 2025-10-03T08:38:48.431Z
Learning: Story files in libs/ui/stories/**/*.tsx can use semantic tokens (e.g., bg-accent, text-accent-fg, hover:bg-accent-hover) for demonstration purposes, unlike component implementation files which should use component-specific token classes.
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/stories/**/*.stories.tsx : Use Storybook title hierarchy matching the layer (e.g., Atoms/ComponentName)
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Use data attributes for state-based styling (e.g., data-[state=open], data-[disabled])
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsxlibs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Component implementations should use tv() with token-based classes (no raw Tailwind arbitrary values)
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsxlibs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Use tv() with slots for component styles; only add variants/sizes when necessary
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsxlibs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-12-16T19:45:17.746Z
Learnt from: BleedingDev
Repo: NMIT-WR/new-engine PR: 207
File: libs/ui/src/molecules/select.tsx:50-50
Timestamp: 2025-12-16T19:45:17.746Z
Learning: When reviewing Tailwind classes in TSX/TS files, prefer using square brackets for arbitrary CSS values and complex expressions. Specifically: - Do not use the parentheses syntax (z-(--z-index)) for anything beyond simple CSS variable references; this syntax auto-wraps in var() and cannot handle calc or complex functions. - Use the square brackets syntax (e.g., h-[calc(var(--available-height)-var(--spacing-content))]) for calc expressions, var with calc, and any complex CSS expressions. This rule applies broadly to Tailwind v4 usage in TSX code across the project.
Applied to files:
libs/ui/stories/molecules/form-input.stories.tsxlibs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use direct semantic tokens like bg-primary; use component-specific token classes (e.g., bg-button-bg-primary)
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : React 19 components must not use forwardRef; pass ref as a prop instead
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/molecules/**/*.tsx : Interactive components must use Zag.js patterns (machine, useMachine, normalizeProps, connect)
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Do not hardcode colors or spacing in JSX/TSX; use design tokens and/or tv()
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use arbitrary Tailwind values in class names (e.g., bg-[#ff0000], bg-[var(--...)]); use token classes instead
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Do not use abbreviations in component names (e.g., btn, pc)
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Define component props with TypeScript interfaces
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : In each component TSX file, import its token CSS (e.g., import './_component-name.css')
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:30:50.058Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: libs/ui/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:30:50.058Z
Learning: Applies to libs/ui/src/{atoms,molecules}/**/*.tsx : Exported React components should be PascalCase (e.g., ComponentName)
Applied to files:
libs/ui/src/molecules/form-input.tsx
📚 Learning: 2025-09-07T21:29:55.558Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: apps/frontend-demo/CLAUDE.md:0-0
Timestamp: 2025-09-07T21:29:55.558Z
Learning: Applies to apps/frontend-demo/src/components/@(atoms|molecules|organisms)/*.tsx : Use functional components and hooks for implementation
Applied to files:
libs/ui/src/molecules/form-input.tsx
🧬 Code graph analysis (1)
libs/ui/stories/molecules/form-input.stories.tsx (1)
libs/ui/src/molecules/form-input.tsx (1)
FormInput(47-75)
🪛 GitHub Actions: CI
libs/ui/stories/molecules/form-input.stories.tsx
[error] pnpm build failed. See logs for details: error in declaration generation (ELIFECYCLE).
libs/ui/src/molecules/form-input.tsx
[error] 4-4: Cannot find module '../atoms/status-text' or its corresponding type declarations.
[error] TypeScript declaration generation failed during build: Failed to generate declaration files.
[error] pnpm build failed. See logs for details: error in declaration generation (ELIFECYCLE).
🔇 Additional comments (3)
libs/ui/stories/molecules/form-input.stories.tsx (3)
36-39: Good addition of showHelpTextIcon control.The new
showHelpTextIconcontrol in argTypes properly documents the new prop and allows interactive testing in Storybook.
198-232: Excellent addition of ValidationStates story.The dedicated
ValidationStatesstory provides clear visual documentation of all four validation states in one place. This improves the Storybook documentation and makes it easier for developers to understand and test validation behaviour.
234-261: Excellent addition of Sizes story.The dedicated
Sizesstory clearly demonstrates all three size variants (sm, md, lg) in one place, improving documentation and making it easier to visualise the size options.
|
@coderabbitai This PR depends on #249 which introduces the StatusText component. Review assuming #249 is merged first. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1627 🏁 Script executed: Length of output: 9894 🏁 Script executed: Length of output: 471 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 43 🏁 Script executed: Length of output: 1416 🏁 Script executed: Length of output: 15923 🏁 Script executed: Length of output: 132 @KaiUweCZE, thank you for the context! I've reviewed this PR assuming #249 is merged. The refactoring looks solid overall—consolidating status text rendering into the Issues Found1. Dead Code in FormInputRawFile: The export function FormInputRaw({
id,
label,
validateStatus = "default",
helpText,
showHelpTextIcon, // ← Declared but never used
size = "md",
required,
disabled,
...props
}: FormInputRawProps) {
return (
<div className="flex flex-col gap-form-field-gap">
<Label disabled={disabled} htmlFor={id} required={required} size={size}>
{label}
</Label>
<Input
disabled={disabled}
id={id}
required={required}
size={size}
variant={validateStatus}
{...props}
className="p-input-sm md:p-input-md"
/>
{helpText} {/* ← helpText is already wrapped in StatusText by FormInput */}
</div>
)
}Since Recommendation: Remove interface FormInputRawProps extends InputProps {
id: string
label: ReactNode
validateStatus?: ValidateStatus
helpText?: ReactNode
// Remove: showHelpTextIcon?: boolean
}
export function FormInput({
helpText,
id,
validateStatus = "default",
showHelpTextIcon = validateStatus === "default" ? false : true,
size = "md",
...props
}: FormInputRawProps) {
return (
<FormInputRaw
helpText={
helpText && (
<StatusText
status={validateStatus}
showIcon={showHelpTextIcon}
size={size}
>
{helpText}
</StatusText>
)
}
id={id}
size={size}
validateStatus={validateStatus}
// Remove: showHelpTextIcon={showHelpTextIcon}
{...props}
/>
)
}2. Duplicate IDs in StoriesFile: Multiple FormInput instances share the same // Lines in AllVariants, Sizes, Special States, InteractiveValidation, RegistrationForm
<FormInput id="success-input" ... />
<FormInput id="success-input" ... /> {/* duplicate */}
<FormInput id="success-input" ... /> {/* duplicate */}While this works in isolated Storybook stories, it violates HTML accessibility standards (IDs must be unique within a document). Recommendation: Use unique, descriptive IDs for each input: // Validation States
<FormInput id="default-validation" ... />
<FormInput id="success-validation" ... />
<FormInput id="error-validation" ... />
<FormInput id="warning-validation" ... />
// Sizes
<FormInput id="size-sm" ... />
<FormInput id="size-md" ... />
<FormInput id="size-lg" ... />
// etc.Positive Observations✅ The conditional default for showHelpTextIcon = validateStatus === "default" ? false : trueThis implements appropriate business logic (show icons for validation states) rather than duplicating StatusText's defaults. ✅ The new ✅ Proper use of conventional commit format in the PR title. Would you like me to review the changes once these issues are addressed? 🧠 Learnings used |
Add dedicated Storybook stories for ValidationStates and Sizes to show
all validation states (default, error, success, warning) and input size
variants (sm, md, lg). Update existing Molecules FormInput story to:
showHelpTextIconin story args.variants.
extraTextusage withhelpTextwhere appropriate.helpTextconsistently.In the FormInput component:
showHelpTextIconprop and removeextraTextprop and related idlogic.
These changes clarify the component API, fix story inaccuracies, and
provide explicit visual documentation of validation and sizing options.
Summary by CodeRabbit
Release Notes
New Features
Documentation
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.