Review and refactor formedible package code #39
Conversation
…onents Phase 1 refactoring to eliminate code duplication by using existing utilities: - Updated 22 field components to use useFieldState hook instead of manual field API value extraction and event handler boilerplate - Updated 3 dropdown fields (multi-select, color-picker, phone) to use useDropdown hook instead of manual click-outside detection - Updated 7 option-based fields to use normalizeOptions utility instead of inline normalization logic Benefits: - Eliminated ~215 lines of duplicate code - Single source of truth for common field patterns - Improved maintainability and consistency - All existing functionality preserved exactly Files updated: - text-field, number-field, textarea-field, select-field, radio-field - checkbox-field, switch-field, slider-field, rating-field, date-field - combobox-field, multi-select-field, multicombobox-field - color-picker-field, phone-field, file-upload-field - array-field, autocomplete-field, masked-input-field, duration-picker-field No breaking changes - all fields maintain identical behavior.
Added missing dependencies that are imported by field components: - lucide-react ^0.487.0 (for icons) - clsx ^2.1.1 (for className utility) - tailwind-merge ^3.3.1 (for className merging) - class-variance-authority ^0.7.1 (for component variants) Also fixed TypeScript type issue in slider-field.tsx where raw value was being rendered without casting (line 129). Versions match those used in web app to ensure compatibility. Build now succeeds: - ESM build: ✓ - CJS build: ✓ - DTS build: ✓
Created new utilities to further reduce code duplication: 1. Created useFormValues hook (~40 lines saved) - Extracts form subscription pattern used in date-field and object-field - Provides reactive access to all form values for dynamic field behavior - Location: src/hooks/use-form-values.ts 2. Created getFieldInputClassName utility (~30 lines saved) - Standardizes error styling pattern across 10+ field components - Provides consistent border-destructive styling for validation errors - Added to src/lib/utils.ts 3. Updated field components to use new utilities: - date-field.tsx: Now uses useFormValues for dynamic date restrictions - object-field.tsx: Now uses useFormValues for dynamic text resolution - text-field, number-field, textarea-field, select-field: Use getFieldInputClassName - date-field, combobox-field, phone-field, color-picker-field: Use getFieldInputClassName - multicombobox-field, radio-field: Use getFieldInputClassName 4. Type consolidation analysis completed: - Discovered that *FieldProps and *FieldSpecificProps serve different purposes - *FieldProps used in config/schema definitions - *FieldSpecificProps used in actual components - Both type systems are necessary and actively used - no consolidation needed Total reduction: ~70 lines of duplicate code eliminated Build status: ✓ All builds passing (ESM, CJS, DTS)
WalkthroughThe PR refactors field component state management by introducing new hooks and utilities to standardize how form fields interact with their underlying field API. It replaces direct fieldApi manipulation across 20+ components with a consistent abstraction layer, adds four new UI library dependencies, and centralizes class name composition for error styling. Changes
Sequence DiagramsequenceDiagram
participant FC as Field Component
participant Hook as useFieldState Hook
participant FieldAPI as FieldAPI (form core)
rect rgb(220, 240, 255)
Note over FC,FieldAPI: Old Pattern
FC->>FieldAPI: Direct access fieldApi.state.value
FC->>FieldAPI: Call fieldApi.handleChange(newValue)
FieldAPI->>FC: (no abstraction)
end
rect rgb(240, 255, 220)
Note over FC,FieldAPI: New Pattern
FC->>Hook: useFieldState(fieldApi)
Hook->>FieldAPI: Extract state & subscribe
Hook->>FC: Return { value, onChange, onBlur, ... }
FC->>Hook: Call onChange(newValue)
Hook->>FieldAPI: Delegates to fieldApi.handleChange
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25–30 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/formedible/src/components/formedible/fields/rating-field.tsx (1)
126-126: Use the hook-providedonBlurfor consistency.Line 126 directly calls
fieldApi.handleBlur(), while line 47 uses theonBlurcallback fromuseFieldState. For consistency with the rest of this component and the broader refactor pattern, use the hook-providedonBlurhere as well.Apply this diff:
- onBlur={() => fieldApi.handleBlur()} + onBlur={onBlur}packages/formedible/src/components/formedible/fields/slider-field.tsx (1)
80-80: UseuseMemooruseIdfor stable slider ID generation.The
Math.random()call executes on every render, generating a new ID each time. This causes:
- Accumulation of unused
<style>tags in the DOM (lines 155-160)- Unnecessary re-renders due to className changes
- Performance degradation
Apply this diff to generate a stable ID:
+import React, { useId } from "react"; ... - const sliderId = `slider-${name}-${Math.random().toString(36).substring(2, 9)}`; + const sliderId = `slider-${name}-${useId().replace(/:/g, '-')}`;Alternatively, use
useMemo:+import React, { useMemo } from "react"; ... - const sliderId = `slider-${name}-${Math.random().toString(36).substring(2, 9)}`; + const sliderId = useMemo( + () => `slider-${name}-${Math.random().toString(36).substring(2, 9)}`, + [name] + );
🧹 Nitpick comments (4)
packages/formedible/src/hooks/use-form-values.ts (1)
20-22: Replaceanytype assertion with proper typing.The type assertion to
anyon line 21 bypasses TypeScript's type safety. Since you're already accessingfieldApi.form.state.valueson line 14, the store subscription should use the same type structure.Apply this diff to use proper typing:
const unsubscribe = fieldApi.form.store.subscribe((state) => { - setFormValues((state as any).values); + setFormValues(state.values || {}); });Alternatively, if the store state type is available from
@tanstack/react-form, import and use it explicitly:import type { FormState } from '@tanstack/react-form'; const unsubscribe = fieldApi.form.store.subscribe((state: FormState<any>) => { setFormValues(state.values || {}); });packages/formedible/src/components/formedible/fields/combobox-field.tsx (1)
39-41: Consider adding a fallback for the value cast.The value is cast to
stringwithout a fallback. If the value isnullorundefined, this could cause comparison issues.Consider adding a fallback:
const selectedOption = normalizedOptions.find( - (option) => option.value === (value as string) + (option) => option.value === ((value as string) || "") );packages/formedible/src/components/formedible/fields/array-field.tsx (1)
246-264: Consider usingonBlurfrom the hook for consistency.Line 259 uses
fieldApi.handleBlurdirectly in the mock field API creation. While this works for propagating blur events to the parent field, it would be more consistent with the refactoring pattern to useonBlurfrom theuseFieldStatehook.Apply this diff for consistency:
const createItemFieldApi = useCallback( (index: number) => { return { name: `${name}[${index}]`, state: { value: value[index], meta: { errors: [], isTouched: false, isValidating: false, }, }, handleChange: (newValue: unknown) => updateItem(index, newValue), - handleBlur: () => fieldApi.handleBlur(), + handleBlur: onBlur, form: fieldApi.form, }; }, - [name, value, updateItem, fieldApi] + [name, value, updateItem, onBlur, fieldApi] );packages/formedible/src/components/formedible/fields/select-field.tsx (1)
9-9: Select hook + normalization look solid; minor type/key cleanups possibleThe refactor to
useFieldState,normalizeOptions, andgetFieldInputClassNameis consistent and keeps the component nicely declarative.Two small, optional cleanups:
- You’re manually re‑declaring the option shape in
SelectFieldSpecificProps. SincenormalizeOptionsalready works withNormalizedOption, consider importing that type and usingoptions: (string | NormalizedOption)[]here to avoid divergence if the option shape evolves.key={option.value + index}works, but ifoption.valueis already unique within the options array,key={option.value}is simpler and avoids index usage in keys.Also applies to: 12-12, 28-32, 35-35, 47-47, 59-63
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (24)
packages/formedible/package.json(1 hunks)packages/formedible/src/components/formedible/fields/array-field.tsx(4 hunks)packages/formedible/src/components/formedible/fields/autocomplete-field.tsx(1 hunks)packages/formedible/src/components/formedible/fields/checkbox-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/color-picker-field.tsx(6 hunks)packages/formedible/src/components/formedible/fields/combobox-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/date-field.tsx(4 hunks)packages/formedible/src/components/formedible/fields/duration-picker-field.tsx(3 hunks)packages/formedible/src/components/formedible/fields/file-upload-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/masked-input-field.tsx(3 hunks)packages/formedible/src/components/formedible/fields/multi-select-field.tsx(4 hunks)packages/formedible/src/components/formedible/fields/multicombobox-field.tsx(5 hunks)packages/formedible/src/components/formedible/fields/number-field.tsx(3 hunks)packages/formedible/src/components/formedible/fields/object-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/phone-field.tsx(6 hunks)packages/formedible/src/components/formedible/fields/radio-field.tsx(4 hunks)packages/formedible/src/components/formedible/fields/rating-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/select-field.tsx(4 hunks)packages/formedible/src/components/formedible/fields/slider-field.tsx(6 hunks)packages/formedible/src/components/formedible/fields/switch-field.tsx(2 hunks)packages/formedible/src/components/formedible/fields/text-field.tsx(3 hunks)packages/formedible/src/components/formedible/fields/textarea-field.tsx(3 hunks)packages/formedible/src/hooks/use-form-values.ts(1 hunks)packages/formedible/src/lib/utils.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
packages/formedible/**/*.{ts,tsx}
📄 CodeRabbit inference engine (GEMINI.md)
Use TypeScript for the core Formedible library code
Files:
packages/formedible/src/components/formedible/fields/object-field.tsxpackages/formedible/src/components/formedible/fields/autocomplete-field.tsxpackages/formedible/src/components/formedible/fields/masked-input-field.tsxpackages/formedible/src/components/formedible/fields/date-field.tsxpackages/formedible/src/components/formedible/fields/combobox-field.tsxpackages/formedible/src/components/formedible/fields/duration-picker-field.tsxpackages/formedible/src/components/formedible/fields/file-upload-field.tsxpackages/formedible/src/components/formedible/fields/phone-field.tsxpackages/formedible/src/components/formedible/fields/rating-field.tsxpackages/formedible/src/components/formedible/fields/multicombobox-field.tsxpackages/formedible/src/components/formedible/fields/switch-field.tsxpackages/formedible/src/components/formedible/fields/select-field.tsxpackages/formedible/src/components/formedible/fields/color-picker-field.tsxpackages/formedible/src/components/formedible/fields/textarea-field.tsxpackages/formedible/src/components/formedible/fields/text-field.tsxpackages/formedible/src/components/formedible/fields/array-field.tsxpackages/formedible/src/components/formedible/fields/multi-select-field.tsxpackages/formedible/src/components/formedible/fields/radio-field.tsxpackages/formedible/src/components/formedible/fields/checkbox-field.tsxpackages/formedible/src/hooks/use-form-values.tspackages/formedible/src/components/formedible/fields/number-field.tsxpackages/formedible/src/components/formedible/fields/slider-field.tsxpackages/formedible/src/lib/utils.ts
packages/formedible/**
📄 CodeRabbit inference engine (GEMINI.md)
Place and maintain all core library code under
packages/formedible
Files:
packages/formedible/src/components/formedible/fields/object-field.tsxpackages/formedible/src/components/formedible/fields/autocomplete-field.tsxpackages/formedible/src/components/formedible/fields/masked-input-field.tsxpackages/formedible/src/components/formedible/fields/date-field.tsxpackages/formedible/src/components/formedible/fields/combobox-field.tsxpackages/formedible/src/components/formedible/fields/duration-picker-field.tsxpackages/formedible/src/components/formedible/fields/file-upload-field.tsxpackages/formedible/src/components/formedible/fields/phone-field.tsxpackages/formedible/src/components/formedible/fields/rating-field.tsxpackages/formedible/src/components/formedible/fields/multicombobox-field.tsxpackages/formedible/src/components/formedible/fields/switch-field.tsxpackages/formedible/src/components/formedible/fields/select-field.tsxpackages/formedible/src/components/formedible/fields/color-picker-field.tsxpackages/formedible/src/components/formedible/fields/textarea-field.tsxpackages/formedible/src/components/formedible/fields/text-field.tsxpackages/formedible/src/components/formedible/fields/array-field.tsxpackages/formedible/src/components/formedible/fields/multi-select-field.tsxpackages/formedible/src/components/formedible/fields/radio-field.tsxpackages/formedible/src/components/formedible/fields/checkbox-field.tsxpackages/formedible/src/hooks/use-form-values.tspackages/formedible/src/components/formedible/fields/number-field.tsxpackages/formedible/src/components/formedible/fields/slider-field.tsxpackages/formedible/src/lib/utils.tspackages/formedible/package.json
🧠 Learnings (1)
📚 Learning: 2025-08-22T14:55:03.957Z
Learnt from: CR
Repo: DimitriGilbert/Formedible PR: 0
File: GEMINI.md:0-0
Timestamp: 2025-08-22T14:55:03.957Z
Learning: Applies to packages/formedible/**/*.{ts,tsx} : Use TypeScript for the core Formedible library code
Applied to files:
packages/formedible/src/components/formedible/fields/object-field.tsxpackages/formedible/src/components/formedible/fields/autocomplete-field.tsxpackages/formedible/src/components/formedible/fields/date-field.tsxpackages/formedible/src/components/formedible/fields/combobox-field.tsxpackages/formedible/src/components/formedible/fields/duration-picker-field.tsxpackages/formedible/src/components/formedible/fields/file-upload-field.tsxpackages/formedible/src/components/formedible/fields/phone-field.tsxpackages/formedible/src/components/formedible/fields/rating-field.tsxpackages/formedible/src/components/formedible/fields/multicombobox-field.tsxpackages/formedible/src/components/formedible/fields/select-field.tsxpackages/formedible/src/components/formedible/fields/textarea-field.tsxpackages/formedible/src/components/formedible/fields/text-field.tsxpackages/formedible/src/components/formedible/fields/array-field.tsxpackages/formedible/src/components/formedible/fields/radio-field.tsxpackages/formedible/src/components/formedible/fields/checkbox-field.tsxpackages/formedible/src/hooks/use-form-values.tspackages/formedible/src/components/formedible/fields/number-field.tsxpackages/formedible/package.json
🧬 Code graph analysis (19)
packages/formedible/src/components/formedible/fields/object-field.tsx (1)
packages/formedible/src/hooks/use-form-values.ts (1)
useFormValues(12-28)
packages/formedible/src/components/formedible/fields/masked-input-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/date-field.tsx (3)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)packages/formedible/src/hooks/use-form-values.ts (1)
useFormValues(12-28)packages/formedible/src/lib/utils.ts (2)
cn(4-6)getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/combobox-field.tsx (1)
packages/formedible/src/lib/utils.ts (3)
normalizeOptions(13-19)cn(4-6)getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/duration-picker-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/file-upload-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/phone-field.tsx (2)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)packages/formedible/src/lib/utils.ts (2)
getFieldInputClassName(28-38)cn(4-6)
packages/formedible/src/components/formedible/fields/rating-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/multicombobox-field.tsx (1)
packages/formedible/src/lib/utils.ts (2)
normalizeOptions(13-19)getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/switch-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/select-field.tsx (1)
packages/formedible/src/lib/utils.ts (2)
normalizeOptions(13-19)getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/color-picker-field.tsx (2)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)packages/formedible/src/lib/utils.ts (2)
getFieldInputClassName(28-38)cn(4-6)
packages/formedible/src/components/formedible/fields/textarea-field.tsx (1)
packages/formedible/src/lib/utils.ts (1)
getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/text-field.tsx (1)
packages/formedible/src/lib/utils.ts (1)
getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/array-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/multi-select-field.tsx (2)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)packages/formedible/src/lib/utils.ts (1)
normalizeOptions(13-19)
packages/formedible/src/components/formedible/fields/radio-field.tsx (2)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)packages/formedible/src/lib/utils.ts (2)
normalizeOptions(13-19)getFieldInputClassName(28-38)
packages/formedible/src/components/formedible/fields/checkbox-field.tsx (1)
packages/formedible/src/hooks/use-field-state.ts (1)
useFieldState(13-38)
packages/formedible/src/components/formedible/fields/number-field.tsx (1)
packages/formedible/src/lib/utils.ts (1)
getFieldInputClassName(28-38)
🔇 Additional comments (20)
packages/formedible/src/components/formedible/fields/masked-input-field.tsx (1)
7-7: LGTM - Consistent with hook-based refactor pattern.The introduction of
useFieldStateand the replacement of directfieldApi.handleChangewith the hook-providedonChangealigns with the broader refactor described in the PR.Also applies to: 45-45, 176-176
packages/formedible/src/components/formedible/fields/autocomplete-field.tsx (1)
7-7: LGTM - Good consolidation of shared utility.Replacing the local
normalizeOptionshelper with the imported utility from@/lib/utilseliminates code duplication and centralizes the normalization logic.packages/formedible/src/components/formedible/fields/duration-picker-field.tsx (1)
14-14: LGTM - Consistent with the state management refactor.The migration to
useFieldStatefornameandonChangefollows the established pattern across other field components in this PR.Also applies to: 73-73, 87-87
packages/formedible/src/components/formedible/fields/checkbox-field.tsx (1)
7-7: LGTM - Complete and clean hook migration.The checkbox field demonstrates a thorough migration to
useFieldState, extracting all necessary values (name,value,isDisabled,onChange,onBlur) and eliminating directfieldApihandler calls.Also applies to: 17-21, 39-39
packages/formedible/src/components/formedible/fields/object-field.tsx (1)
8-8: LGTM - Effective consolidation of form value subscriptions.Replacing manual state management and
useEffectsubscriptions with theuseFormValueshook centralizes form value tracking and reduces boilerplate.Also applies to: 22-22
packages/formedible/src/components/formedible/fields/file-upload-field.tsx (1)
8-8: LGTM - Complete hook-based state management.The file upload field comprehensively adopts
useFieldState, replacing all directfieldApihandler calls with hook-provided callbacks for consistent state management.Also applies to: 25-26, 30-31, 35-35, 40-40
packages/formedible/src/components/formedible/fields/rating-field.tsx (1)
7-7: LGTM - Hook migration aligns with refactor pattern.The adoption of
useFieldStateto derivevalue,isDisabled,onChange, andonBluris consistent with the broader state management refactor across field components.Also applies to: 38-39, 46-47, 51-51
packages/formedible/package.json (1)
58-64: All dependencies are valid, up-to-date within their pinned ranges, and free from security vulnerabilities.Verification confirms:
- class-variance-authority (^0.7.1), clsx (^2.1.1), lucide-react (^0.487.0), and tailwind-merge (^3.3.1) all exist on npm and match or are safely within their latest minor/patch versions.
- Zero security advisories across all four packages.
- Caret ranges allow safe updates: lucide-react's range safely excludes the FingerprintPattern breaking change in 0.554.0, and tailwind-merge 3.4.0 introduces no breaking changes.
packages/formedible/src/lib/utils.ts (1)
21-38: LGTM! Clean error styling abstraction.The utility function properly centralizes error styling logic and integrates well with the existing
cnutility. The type signature correctly allowsundefinedvalues in the rest parameters, which will be safely filtered byclsx.packages/formedible/src/components/formedible/fields/switch-field.tsx (1)
7-7: LGTM! Consistent hook-based state management.The migration to
useFieldStateis correctly implemented. The hook-providedonChangeandonBlurhandlers properly replace directfieldApimanipulation, maintaining the component's behavior while aligning with the new state management pattern.Also applies to: 17-21, 39-39
packages/formedible/src/components/formedible/fields/combobox-field.tsx (1)
17-17: LGTM! Well-integrated hook and utility usage.The refactoring successfully migrates to
useFieldStateand shared utilities (normalizeOptions,getFieldInputClassName). All handlers are properly wired and the error styling is correctly centralized.Also applies to: 20-20, 33-33, 45-45, 51-51, 80-80
packages/formedible/src/components/formedible/fields/textarea-field.tsx (1)
3-3: LGTM! Proper hook integration with good practices.The refactoring correctly adopts
useFieldStateandgetFieldInputClassName. The value handling includes a proper fallback ((value as string) || ''), and aliasingonChangetoonFieldChangeavoids naming conflicts with the native event handler.Also applies to: 6-6, 19-25, 39-39
packages/formedible/src/components/formedible/fields/radio-field.tsx (1)
5-5: LGTM! Consistent refactoring with proper error styling.The migration to
useFieldState,normalizeOptions, andgetFieldInputClassNameis correctly implemented. The value handling includes a proper fallback, and the error styling is applied correctly to individual radio items.Also applies to: 8-8, 20-25, 37-37, 56-56
packages/formedible/src/components/formedible/fields/array-field.tsx (1)
8-8: LGTM! Thorough migration to hook-based state management.The refactoring successfully migrates all array operations to use
useFieldState. All add, remove, update, and reorder operations correctly useonChange, andonBluris appropriately called inremoveItem. The callback dependencies are properly updated to reflect the new handlers.Also applies to: 118-118, 174-174, 182-183, 192-192, 237-237
packages/formedible/src/components/formedible/fields/number-field.tsx (1)
4-7: Hook-based number field wiring looks correctUsing
useFieldStateplus a localonChangethat normalizes the value tonumber | string | undefinedand then dispatches viaonFieldChangepreserves expected behavior, andgetFieldInputClassNamecentralizes error styling cleanly. No issues from my side here.Also applies to: 22-23, 35-35, 45-45
packages/formedible/src/components/formedible/fields/phone-field.tsx (1)
5-5: Phone field refactor touseFieldStatelooks goodThe move to
useFieldStatefor value/disabled/errors plus pushing updates viaonChange/onBlurkeeps the phone field consistent with the rest of the library. Formatting logic and country handling remain intact, and error styling viagetFieldInputClassNameis applied in the right places.Also applies to: 9-9, 82-84, 139-140, 156-157, 187-190, 236-236, 240-241
packages/formedible/src/components/formedible/fields/date-field.tsx (1)
4-4: Date field hook integration is solid; confirmCalendar.disabledsemanticsThe refactor to
useFieldState/useFormValuesand thedisableDate(date, formValues)wrapper looks coherent and keeps the behavior extensible while preserving the original(date) => booleanusage. The disabled-date memoization and error styling viagetFieldInputClassNameare also in good shape.One thing to double-check: when
isDisabledis true,disabledMatchersbecomestrueand is passed into<Calendar disabled={disabledMatchers} />. That’s fine as long as yourCalendarcomponent (and underlying date-picker lib) explicitly supportsdisabled={true}as “disable all dates” in addition to arrays/matchers.Please confirm against the
Calendarcomponent’sdisabledprop typing/behavior thattrueis an accepted value for “disable everything”.Also applies to: 15-16, 28-29, 40-60, 61-64, 67-71
packages/formedible/src/components/formedible/fields/multicombobox-field.tsx (1)
18-18: LGTM! Clean refactor to centralized field state management.The migration to
useFieldStateand utility functions is implemented correctly:
- Safe array defaulting on line 40 prevents runtime errors
normalizeOptionsutility replaces manual option processing- Event handlers properly wired through the hook
- Error styling delegated to
getFieldInputClassNameThe refactor maintains all existing functionality while improving consistency.
Also applies to: 22-22, 39-74, 101-101
packages/formedible/src/components/formedible/fields/slider-field.tsx (1)
26-26: LGTM! State management refactor is correctly implemented.The migration to
useFieldStatehook is done correctly:
- Proper destructuring of field state and handlers
- Safe fallback to
minwhen value is not a number- Event handlers correctly use
onChangefrom the hook- All state interactions centralized through the hook
The refactor maintains existing functionality while improving consistency across field components.
Also applies to: 54-54, 70-70, 140-140
packages/formedible/src/components/formedible/fields/text-field.tsx (1)
4-4: LGTM! Refactor correctly implements centralized state management.The migration to
useFieldStateand utility functions is well-executed:
- Smart aliasing to
onFieldChange(line 21) avoids naming conflict with the localonChangewrappergetFieldInputClassNameproperly handles error styling with loading state- Existing datalist functionality preserved
- All event handlers correctly wired through the hook
The refactor improves consistency while maintaining all functionality.
Also applies to: 7-7, 21-21, 90-98
| const { value: fieldValue, isDisabled, hasErrors, onChange, onBlur } = useFieldState(fieldApi); | ||
| const value = (fieldValue as string) || "#000000"; | ||
|
|
||
| const [isOpen, setIsOpen] = useState(false); | ||
| const { isOpen, setIsOpen, containerRef } = useDropdown(); | ||
| const [customInput, setCustomInput] = useState(value); |
There was a problem hiding this comment.
Fix stored color format and restore inputClassName on the main input
Two coupled issues here:
- Stored value vs. display format
valueis treated as a hex color (normalizedValueprefixes#and is used asbackgroundColor), and utilities likehexToRgb/hexToHslassume hex.- In the current handlers you sometimes call
onChange(formattedColor)(rgb/hsl) or arbitraryinputValue. Whenformat !== "hex", this can store non-hex values, makingnormalizedValuebecome things like#rgb(255,0,0), which is not a valid CSS color and breaks the preview as well as the “Ensure value is always a valid hex color” contract.
You can keep the field’s stored value as canonical hex and use formatColor purely for what you render:
@@
- const { value: fieldValue, isDisabled, hasErrors, onChange, onBlur } = useFieldState(fieldApi);
- const value = (fieldValue as string) || "#000000";
+ const { value: fieldValue, isDisabled, hasErrors, onChange, onBlur } = useFieldState(fieldApi);
+ const value = (fieldValue as string) || "#000000";
@@
const handleColorSelect = (color: string) => {
- const formattedColor = formatColor(color, format);
- onChange(formattedColor);
+ // Store hex, format only for display
+ onChange(color);
setCustomInput(color);
setIsOpen(false);
onBlur();
};
@@
const handleCustomInputChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const inputValue = e.target.value;
setCustomInput(inputValue);
- // Validate and update if it's a valid color
- if (inputValue.match(/^#[0-9A-Fa-f]{6}$/)) {
- const formattedColor = formatColor(inputValue, format);
- onChange(formattedColor);
- }
+ // Validate and update only when it's a valid hex color
+ if (isValidColor(inputValue)) {
+ onChange(inputValue);
+ }
};
@@
const handleNativeColorChange = (e: React.ChangeEvent<HTMLInputElement>) => {
const color = e.target.value;
- const formattedColor = formatColor(color, format);
- onChange(formattedColor);
+ // Native color input always yields hex
+ onChange(color);
setCustomInput(color);
};
@@
<Input
value={displayValue}
onChange={(e) => {
const inputValue = e.target.value;
- onChange(inputValue);
- // Try to extract hex value for internal use
- if (inputValue.startsWith("#")) {
- setCustomInput(inputValue);
- }
+ // Allow typing hex directly into the main input; keep storage hex-only
+ setCustomInput(inputValue);
+ if (isValidColor(inputValue)) {
+ onChange(inputValue);
+ }
}}This keeps fieldValue as hex, while displayValue = formatColor(normalizedValue, format) continues to provide rgb/hsl/hex presentation.
inputClassNamenot applied to the main input
The main text input currently ignores inputClassName and only applies error styling:
className={cn(
"flex-1",
getFieldInputClassName(undefined, hasErrors)
)}Every other field component wires caller-supplied inputClassName through getFieldInputClassName. To match that and avoid a regression in styling control:
className={cn(
"flex-1",
- getFieldInputClassName(undefined, hasErrors)
+ getFieldInputClassName(inputClassName, hasErrors)
)}This restores consumer-provided classes on the main input while still centralizing error styling.
Also applies to: 125-131, 133-141, 144-149, 164-167, 181-182, 188-193, 198-203
🤖 Prompt for AI Agents
In packages/formedible/src/components/formedible/fields/color-picker-field.tsx
around lines 114-118 (and related blocks at 125-131, 133-141, 144-149, 164-167,
181-182, 188-193, 198-203), the component sometimes stores non-hex color strings
(rgb/hsl or raw input) into fieldValue and also omits the caller-supplied
inputClassName on the main input; fix by keeping the canonical stored value
always as a hex string (normalize/convert any picked/parsed color to hex before
calling onChange) and use a separate displayValue = formatColor(normalizedHex,
format) for what is shown in the input/preview; update all handlers (picker
change, preset click, custom input confirm/blur) to convert to hex then call
onChange(hex) and only use formatColor for rendering; and restore inputClassName
on the main input by passing the incoming inputClassName into
getFieldInputClassName so error styling and consumer classes are both applied.
| const selectedValues = Array.isArray(fieldApi.state?.value) | ||
| ? fieldApi.state?.value | ||
| : []; | ||
| const { value, onChange, onBlur } = useFieldState(fieldApi); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Complete the migration to useFieldState.
The refactoring is incomplete. While useFieldState is used at line 29, several places still access fieldApi directly:
- Line 109: derives
isDisabledfromfieldApi.form.state.isSubmittinginstead of using the hook - Line 126: checks errors via
fieldApi.state?.meta?.errors.lengthinstead of usinghasErrorsfrom the hook - Line 172: calls
fieldApi.handleBlurinstead of usingonBlurfrom the hook
Apply this diff to complete the migration:
- const { value, onChange, onBlur } = useFieldState(fieldApi);
+ const { name, value, isDisabled, hasErrors, onChange, onBlur } = useFieldState(fieldApi);
const selectedValues = Array.isArray(value) ? value : [];Then update the usages:
- const isDisabled = fieldApi.form.state.isSubmitting;
+ // Remove this line, use isDisabled from hook className={cn(
"min-h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-sm ring-offset-background",
"focus-within:ring-2 focus-within:ring-ring focus-within:ring-offset-2",
- fieldApi.state?.meta?.errors.length ? "border-destructive" : "",
+ hasErrors ? "border-destructive" : "",
isDisabled ? "opacity-50 cursor-not-allowed" : "cursor-text"
)} onFocus={() => setIsOpen(true)}
- onBlur={fieldApi.handleBlur}
+ onBlur={onBlur}
placeholder={selectedValues.length === 0 ? placeholder : ""}Also applies to: 109-109, 126-126, 172-172
| {showRawValue && ( | ||
| <div className="text-xs text-muted-foreground"> | ||
| Raw: {fieldApi.state?.value} | ||
| Raw: {String(value)} |
There was a problem hiding this comment.
Use fieldValue for consistency with the rest of the component.
Line 129 displays the raw value, but line 54 computes fieldValue with a fallback to min when value is not a number. All other references in the component (lines 66, 86, 140, 146, 164) use fieldValue.
For consistency and to ensure the display matches the actual slider behavior, use fieldValue here.
Apply this diff:
- Raw: {String(value)}
+ Raw: {String(fieldValue)}📝 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.
| Raw: {String(value)} | |
| Raw: {String(fieldValue)} |
🤖 Prompt for AI Agents
In packages/formedible/src/components/formedible/fields/slider-field.tsx around
line 129, the component prints the raw value using String(value) while the rest
of the component (and the slider behavior) uses the computed fieldValue (which
falls back to min when value is not a number); replace the displayed raw value
with String(fieldValue) so the shown "Raw:" value is consistent with the slider
state and other references.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.