Repository navigation
reafactor(ui/select): replace helper/error text with unified status help text - #251
Conversation
…elp text Replace separate helperText and errorText props with a unified helpText and validateStatus API. Render a single StatusText component for all auxiliary messaging (default/error/success/warning) and control whether an icon is shown via showHelpTextIcon. Map validateStatus === "error" to the existing invalid flag to preserve previous behavior for form integration. Adjust Select stories to use validateStatus, helpText and showHelpTextIcon, update examples to demonstrate error/success/warning states, and rename WithHelperText story to WithHelpText. Also tidy layout in the States story (stack items in a column). Add responsive text sizing for status messages based on select size.
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.
|
WalkthroughThe Select component's validation and help text API is refactored from separate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Mesa DescriptionTL;DRRefactored the What changed?
Description generated by Mesa. Update settings |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
libs/ui/src/molecules/select.tsxlibs/ui/stories/molecules/select.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/src/molecules/select.tsxlibs/ui/stories/molecules/select.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/select.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/select.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/select.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/select.stories.tsx
🧠 Learnings (19)
📓 Common learnings
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)
📚 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/select.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/src/molecules/select.tsxlibs/ui/stories/molecules/select.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 : 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/select.tsxlibs/ui/stories/molecules/select.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 : When using Zag.js, derive styles from data attributes (e.g., data-state) rather than imperative styling
Applied to files:
libs/ui/src/molecules/select.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/src/molecules/select.tsx
📚 Learning: 2025-09-07T21:28:54.320Z
Learnt from: CR
Repo: NMIT-WR/new-engine PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-07T21:28:54.320Z
Learning: Applies to libs/ui/**/*.{ts,tsx} : Implement UI library components using Zag.js for React and Tailwind CSS for styling
Applied to files:
libs/ui/src/molecules/select.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/select.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/src/molecules/select.tsxlibs/ui/stories/molecules/select.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 : 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/select.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/select.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/src/molecules/select.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 Tailwind classes mapped to tokens (e.g., bg-button-bg, p-button-sm); never use arbitrary CSS variable values like bg-[var(--...)] or p-[var(--...)]
Applied to files:
libs/ui/src/molecules/select.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/select.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/select.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/select.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/src/molecules/select.tsxlibs/ui/stories/molecules/select.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 : Storybook stories must exist for components and cover all variants and states
Applied to files:
libs/ui/stories/molecules/select.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/select.stories.tsx
🧬 Code graph analysis (1)
libs/ui/stories/molecules/select.stories.tsx (1)
libs/ui/src/molecules/select.tsx (1)
Select(109-267)
🪛 GitHub Actions: CI
libs/ui/src/molecules/select.tsx
[error] 8-8: TS2307: Cannot find module '../atoms/status-text' or its corresponding type declarations.
[error] 1-1: Failed to generate declaration files. The build process for libs/ui failed due to the above TypeScript error.
libs/ui/stories/molecules/select.stories.tsx
[error] 1-1: Failed to generate declaration files. The build process for libs/ui failed due to the above TypeScript error.
🔇 Additional comments (8)
libs/ui/src/molecules/select.tsx (3)
101-103: Well-structured API refactoring.The new props
validateStatus,helpText, andshowHelpTextIconprovide a cleaner, more unified interface compared to the previoushelperTextanderrorTextapproach. The status-driven design aligns with common form validation patterns.
152-152: Correct mapping of validateStatus to invalid flag.The mapping
invalid: validateStatus === "error"properly preserves backward compatibility with Zag.js's internal invalid state whilst supporting the new validateStatus API.
255-263: Clean conditional rendering of StatusText.The StatusText is only rendered when
helpTextis provided, with appropriate props passed for status indication. This approach is clean and consistent with the unified status API design.libs/ui/stories/molecules/select.stories.tsx (5)
114-126: ArgTypes properly updated for new API.The argTypes correctly reflect the new
validateStatus,helpText, andshowHelpTextIconprops with appropriate control types and descriptions. This ensures Storybook users can interactively explore the new status-driven API.
183-199: Excellent coverage of validation states.The States story now comprehensively demonstrates all validation states (error, success, warning) with appropriate help text for each. This provides clear examples for developers implementing forms with the Select component.
224-239: Consistent story rename and API usage.The story rename from
WithHelperTexttoWithHelpTextand the updated prop usage align perfectly with the refactored API. The description accurately reflects the new help text functionality.
350-355: Smart conditional validation in form.The conditional
validateStatusandhelpTextlogic effectively demonstrates real-world form validation patterns, showing error state when the required country field is empty and clearing the message once a selection is made.
179-179: Thew-mdclass is a valid custom token in your design system. It maps to the--container-md: 28rem;token defined inlibs/ui/src/tokens/_layout.cssand is used consistently throughout the Storybook stories as part of your semantic sizing scale (xs, sm, md, lg, etc.). No action required.
|
@coderabbitai This PR depends on #249 which introduces the StatusText component. Review assuming #249 is merged first. |
|
@KaiUweCZE, thank you for providing that context! I appreciate you mentioning the dependency on #249 upfront—this aligns with what I've learnt from previous PRs about checking for dependencies before flagging missing components. I'll review this PR assuming the 🧠 Learnings used✅ Actions performedReview triggered.
|
Replace separate helperText and errorText props with a unified helpText
and validateStatus API. Render a single StatusText component for all
auxiliary messaging (default/error/success/warning) and control whether
an icon is shown via showHelpTextIcon. Map validateStatus === "error"
to the existing invalid flag to preserve previous behavior for form
integration.
Adjust Select stories to use validateStatus, helpText and showHelpTextIcon,
update examples to demonstrate error/success/warning states, and rename
WithHelperText story to WithHelpText. Also tidy layout in the States
story (stack items in a column). Add responsive text sizing for status
messages based on select size.
Summary by CodeRabbit
helperTextanderrorTextprops with a status-driven approach. New props includevalidateStatus(supporting default, error, success, and warning states),helpTextfor custom text display, andshowHelpTextIconfor icon control. This provides enhanced flexibility in validation state management and user feedback presentation.✏️ Tip: You can customize this high-level summary in your review settings.