Feat add char limits bank acc form#1407
Conversation
Zishan-7
commented
Nov 6, 2025
Zishan-7
commented
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds Radix Select dependency and a new BaseSelect component, extends BaseInput with optional right-side content, introduces US/MX state constants, and integrates select, per-field char counts, and debounced BIC/IBAN validation into the DynamicBankAccountForm. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
237-282: Consider adding autocomplete attributes for bank account fields.The renderInput function now supports character count display, which is well-implemented. However, based on learnings, bank account input fields should include appropriate
autocompleteattributes when the recipient type is'us'or'iban'.Based on learnings
Consider adding an
autocompleteparameter torenderInputand applying it to relevant fields:const renderInput = ( name: keyof IBankAccountDetails, placeholder: string, rules: any, type: string = 'text', rightAdornment?: React.ReactNode, onBlur?: (field: any) => Promise<void> | void, showCharCount?: boolean, - maxLength?: number + maxLength?: number, + autoComplete?: string ) => ( <div className="w-full"> <div className="relative"> <Controller name={name} control={control} rules={rules} render={({ field }) => ( <BaseInput {...field} type={type} placeholder={placeholder} + autoComplete={autoComplete} className={twMerge(Then use it for account number fields:
// For US bank account renderInput('accountNumber', 'Account Number', {...}, 'text', undefined, undefined, false, undefined, 'bank-account-number') // For IBAN renderInput('accountNumber', 'IBAN', {...}, 'text', undefined, onBlur, false, undefined, 'bank-account-number')
📜 Review details
Configuration used: CodeRabbit 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 (7)
package.json(1 hunks)src/components/0_Bruddle/BaseInput.tsx(1 hunks)src/components/0_Bruddle/BaseSelect.tsx(1 hunks)src/components/0_Bruddle/index.ts(1 hunks)src/components/AddWithdraw/DynamicBankAccountForm.tsx(7 hunks)src/constants/index.ts(1 hunks)src/constants/stateCodes.consts.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 545
File: src/components/Global/GeneralRecipientInput/index.tsx:118-126
Timestamp: 2024-11-26T12:06:11.603Z
Learning: For bank account input fields, use `autocomplete="bank-account-number"` when the recipient type is `'us'` or `'iban'`.
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/constants/stateCodes.consts.tssrc/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-11-26T12:06:11.603Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 545
File: src/components/Global/GeneralRecipientInput/index.tsx:118-126
Timestamp: 2024-11-26T12:06:11.603Z
Learning: For bank account input fields, use `autocomplete="bank-account-number"` when the recipient type is `'us'` or `'iban'`.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-10-07T10:19:20.077Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1286
File: src/components/MultiStepBankAccountForm/FormInput.tsx:10-10
Timestamp: 2025-10-07T10:19:20.077Z
Learning: The codebase follows a strict policy of avoiding `any` types. Always use proper TypeScript types, particularly from libraries like react-hook-form (RegisterOptions, FieldErrors, UseFormSetValue, etc.) rather than using `any`.
Applied to files:
src/components/0_Bruddle/BaseSelect.tsx
🧬 Code graph analysis (2)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
src/constants/stateCodes.consts.ts (2)
MX_STATES(64-97)US_STATES(5-56)
src/components/0_Bruddle/BaseSelect.tsx (1)
src/components/Global/Icons/Icon.tsx (1)
Icon(209-218)
⏰ 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: Deploy-Preview
🔇 Additional comments (9)
src/components/0_Bruddle/BaseInput.tsx (1)
11-30: LGTM! Clean implementation of rightContent support.The refactor properly wraps the input in a container div to support absolutely-positioned right content, and adds appropriate right padding to prevent text overlap. The pointer-events-none on the rightContent ensures it doesn't interfere with input interaction.
package.json (1)
37-37: LGTM! Appropriate dependency for the new BaseSelect component.The @radix-ui/react-select library is a well-maintained, accessible UI primitive that aligns with modern React patterns.
src/components/0_Bruddle/index.ts (1)
3-3: LGTM! Properly extends the Bruddle component barrel.src/constants/index.ts (1)
10-10: LGTM! Properly extends the constants barrel.src/components/AddWithdraw/DynamicBankAccountForm.tsx (2)
479-494: LGTM! State selection properly implemented with country-specific options.The replacement of the text input with a BaseSelect dropdown for state selection significantly improves UX and data quality by preventing invalid state entries. The integration with MX_STATES and US_STATES constants ensures type-safe, validated state codes.
284-309: LGTM! Clean abstraction for select field rendering.The renderSelect helper properly mirrors the pattern of renderInput while integrating the new BaseSelect component with error handling and validation.
src/components/0_Bruddle/BaseSelect.tsx (2)
24-91: LGTM! Well-implemented accessible select component.The BaseSelect component properly leverages Radix UI's Select primitives with appropriate styling, error state handling, and accessibility features. The onBlur trigger on close and dynamic width matching are particularly good touches.
50-50: Icon names are properly supported.Both 'chevron-down' and 'check' icon names are defined in the IconName type and mapped in the iconComponents registry within src/components/Global/Icons/Icon.tsx. The Icon component has full type safety and both icons are available for use.
src/constants/stateCodes.consts.ts (1)
1-99: I need more detailed information about the actual ISO codes. Let me search for the specific code standards:Based on the search results, the ISO 3166-2:US codes use two letters which is the postal abbreviation of the state. The US state codes in your file (AL, AK, AZ, etc.) match this standard correctly.
However, I was unable to complete verification of the Mexican state codes due to tool limitations. The format of the ISO 3166-2 codes is different for each country and may be alphabetic, numeric, or alphanumeric, and Mexico's three-letter format does require specific verification against the official ISO 3166-2:MX standard to confirm accuracy.
Verify Mexican state codes against ISO 3166-2:MX standard.
The US state codes are correctly formatted as postal abbreviations per ISO 3166-2:US. However, the Mexican state codes (AGU, BCN, BCS, etc.) require manual verification against the official ISO 3166-2:MX registry to ensure they precisely match the current standard, as these codes may vary depending on the standard version being referenced.
|
@coderabbitai review in depth. What are possible bugs introduced by this? Is this breaking any existing functionality? Evaluate code quality, DRY, etc |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 24767 🏁 Script executed: Length of output: 3873 🏁 Script executed: Length of output: 1015 🏁 Script executed: Length of output: 3633 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 27 🏁 Script executed: Length of output: 49 🏁 Script executed: Length of output: 8654 🏁 Script executed: Length of output: 66 🏁 Script executed: Length of output: 755 🏁 Script executed: Length of output: 12622 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 4094 🏁 Script executed: Length of output: 6352 @Hugo0, I've completed an in-depth review. Here's my comprehensive analysis: 🐛 Critical Bugs1. BaseInput Padding Bug (High Severity)The // In BaseInput.tsx
<input ref={ref} className={twMerge(c, 'pr-15 md:pr-18')} {...props} />Issue: The padding classes Fix: <input
ref={ref}
className={twMerge(c, rightContent && 'pr-15 md:pr-18')}
{...props}
/>2. BIC Validation Race Condition (Medium Severity)In validate: async (value: string) => {
if (!value || value.trim().length === 0) return 'BIC is required'
// Only validate if the value matches the debounced value
if (value.trim() !== debouncedBicValue?.trim()) {
return true // Skip validation until debounced value is ready
}
// ...
}Issue: Returning Better approach: Disable the submit button while 3. Icon Dependencies Not Verified (Low-Medium Severity)BaseSelect uses icons that I couldn't verify exist:
Risk: Runtime errors if icons are missing. 🔥 Breaking ChangesState Field Migration (High Impact)The state field changed from a free-text input to a dropdown select: Before: renderInput('state', 'Your State', { required: 'State is required' })After: renderSelect('state', 'Select your state',
isMx ? MX_STATES.map(...) : US_STATES.map(...),
{ required: 'State is required' }
)Impact:
Recommendation: Add a migration function to convert old state names to codes, or clear // In useEffect or before setting default values
if (persistedFormData?.state) {
const stateCode = findStateCodeFromName(persistedFormData.state, country);
if (!stateCode) {
dispatch(bankFormActions.clearField('state'));
}
}📊 Code Quality Issues1. DRY Violation in State SelectionLines 489-501 duplicate the state options mapping: {renderSelect(
'state',
'Select your state',
isMx
? MX_STATES.map((state) => ({
label: state.name,
value: state.code,
}))
: US_STATES.map((state) => ({
label: state.name,
value: state.code,
})),
{ required: 'State is required' }
)}Better: const stateOptions = useMemo(() =>
(isMx ? MX_STATES : US_STATES).map(state => ({
label: state.name,
value: state.code,
})),
[isMx]
);
{renderSelect('state', 'Select your state', stateOptions, { required: 'State is required' })}2. Magic NumbersThe 35-character limit is hardcoded in multiple places (line 477, 480): maxLength: { value: 35, message: 'Street address must be 35 characters or less' }
// ...
true, 35Better: Define as a constant: const STREET_ADDRESS_MAX_LENGTH = 35;3. BaseSelect Error Prop Not UtilizedThe <Select.Root
value={value}
onValueChange={onValueChange}
disabled={disabled}
// error prop not passed here
>Impact: This is actually fine since Radix UI doesn't have an error prop, but the error styling only affects visual appearance. 4. Missing TestsNo tests were added for:
Critical: Bank account forms are sensitive financial features that must have comprehensive tests. 5. Accessibility Concerns in BaseSelectMissing:
Better: <Select.Trigger
ref={ref}
aria-label={placeholder}
aria-invalid={error}
className={...}
>6. Character Count Edge CaseLine 341: While this works, it's more explicit to handle undefined: {(field.value?.length ?? 0)}/{maxLength}7. Inconsistent Error DisplayLines 349-351 show errors conditionally: {errors[name] && touchedFields[name] && <ErrorAlert description={errors[name]?.message ?? ''} />}But the character counter (line 341) doesn't check ✅ Good Practices Found
🎯 Recommendations PriorityHigh Priority:
Medium Priority: Low Priority: Would you like me to create follow-up issues for any of these findings, or would you prefer to address them in this PR? 🧠 Learnings used |
…-char-limits-bank-acc-form
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 (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
429-440: BIC validation may allow invalid codes during rapid submission.The validation returns
trueon line 434 when the input doesn't match the debounced value, which allows the form to validate successfully before the debounced BIC check completes. If a user types an invalid BIC and quickly submits (before the 500ms debounce), the validation passes.Consider one of these approaches:
Option 1: Ensure validation doesn't prematurely return success:
validate: async (value: string) => { if (!value || value.trim().length === 0) return 'BIC is required' // Only validate if the value matches the debounced value (to prevent API calls on every keystroke) if (value.trim() !== debouncedBicValue?.trim()) { - return true // Skip validation until debounced value is ready + // Validation pending - indicate the field is not yet validated + return 'Validating BIC...' } setisCheckingBICValid(true) const isValid = await validateBic(value.trim()) setisCheckingBICValid(false) return isValid || 'Invalid BIC code' },Option 2: Keep submit button disabled while BIC value differs from debounced value (requires additional state tracking).
🧹 Nitpick comments (2)
src/components/0_Bruddle/BaseSelect.tsx (1)
38-63: Consider adding accessibility attributes.The Select is missing
aria-labeloraria-labelledbyfor screen readers, and the error state isn't communicated viaaria-invalid. Consider enhancing accessibility:<Root value={value} onValueChange={onValueChange} disabled={disabled} + aria-label={placeholder} + aria-invalid={error} onOpenChange={(open) => {This helps screen reader users understand the control's purpose and validation state.
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
483-486: Consider memoizing state options mapping.The inline
.map()on lines 483-486 creates a new options array on every render. While not critical, memoizing this computation could improve performance:const stateOptions = useMemo( () => (isMx ? MX_STATES : US_STATES).map((state) => ({ label: state.name, value: state.code, })), [isMx] ) // Then use in renderSelect: {renderSelect( 'state', 'Select your state', stateOptions, { required: 'State is required' } )}
📜 Review details
Configuration used: CodeRabbit 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 (4)
package.json(1 hunks)src/components/0_Bruddle/BaseInput.tsx(1 hunks)src/components/0_Bruddle/BaseSelect.tsx(1 hunks)src/components/AddWithdraw/DynamicBankAccountForm.tsx(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- package.json
- src/components/0_Bruddle/BaseInput.tsx
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1407
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:459-475
Timestamp: 2025-11-06T08:19:05.201Z
Learning: In src/components/AddWithdraw/DynamicBankAccountForm.tsx, the street address field has a 35-character maximum length limit because the backend provider enforces this constraint, applicable to both US and Mexican addresses.
📚 Learning: 2025-11-06T08:19:05.201Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1407
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:459-475
Timestamp: 2025-11-06T08:19:05.201Z
Learning: In src/components/AddWithdraw/DynamicBankAccountForm.tsx, the street address field has a 35-character maximum length limit because the backend provider enforces this constraint, applicable to both US and Mexican addresses.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/0_Bruddle/BaseSelect.tsx
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-12-11T10:13:22.806Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 564
File: src/components/Request/Pay/Views/Initial.view.tsx:430-430
Timestamp: 2024-12-11T10:13:22.806Z
Learning: In the React TypeScript file `src/components/Request/Pay/Views/Initial.view.tsx`, when reviewing the `InitialView` component, do not flag potential issues with using non-null assertion `!` on the `slippagePercentage` variable, as handling undefined values in this context is considered out of scope.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsxsrc/components/0_Bruddle/BaseSelect.tsx
📚 Learning: 2024-11-26T12:06:11.603Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 545
File: src/components/Global/GeneralRecipientInput/index.tsx:118-126
Timestamp: 2024-11-26T12:06:11.603Z
Learning: For bank account input fields, use `autocomplete="bank-account-number"` when the recipient type is `'us'` or `'iban'`.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-12-02T17:19:18.532Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 551
File: src/components/Request/Create/Views/Initial.view.tsx:151-156
Timestamp: 2024-12-02T17:19:18.532Z
Learning: In the `InitialView` component at `src/components/Request/Create/Views/Initial.view.tsx`, when setting the default chain and token in the `useEffect` triggered by `isPeanutWallet`, it's acceptable to omit the setters from the dependency array and not include additional error handling for invalid defaults.
Applied to files:
src/components/0_Bruddle/BaseSelect.tsx
📚 Learning: 2025-09-18T09:30:42.901Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1230
File: src/app/(mobile-ui)/withdraw/page.tsx:92-97
Timestamp: 2025-09-18T09:30:42.901Z
Learning: In src/app/(mobile-ui)/withdraw/page.tsx, the useEffect that calls setShowAllWithdrawMethods(true) when amountFromContext exists is intentionally designed to run only on component mount (empty dependency array), not when amountFromContext changes. This is the correct behavior for the withdraw flow where showing all methods should only happen on initial load when an amount is already present.
Applied to files:
src/components/0_Bruddle/BaseSelect.tsx
📚 Learning: 2025-10-07T10:19:20.077Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 1286
File: src/components/MultiStepBankAccountForm/FormInput.tsx:10-10
Timestamp: 2025-10-07T10:19:20.077Z
Learning: The codebase follows a strict policy of avoiding `any` types. Always use proper TypeScript types, particularly from libraries like react-hook-form (RegisterOptions, FieldErrors, UseFormSetValue, etc.) rather than using `any`.
Applied to files:
src/components/0_Bruddle/BaseSelect.tsx
🧬 Code graph analysis (2)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
src/constants/stateCodes.consts.ts (2)
MX_STATES(64-97)US_STATES(5-56)
src/components/0_Bruddle/BaseSelect.tsx (1)
src/components/Global/Icons/Icon.tsx (1)
Icon(209-218)
⏰ 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: Deploy-Preview
🔇 Additional comments (7)
src/components/0_Bruddle/BaseSelect.tsx (4)
1-17: LGTM: Explicit imports used.The explicit Radix UI Select imports align with the codebase preference noted in past reviews.
19-33: LGTM: Clean TypeScript definitions.The exported
BaseSelectOptioninterface and internalBaseSelectPropsare well-structured with appropriate prop types.
101-103: LGTM: Standard component export pattern.The displayName and default export follow React best practices.
61-61: No action required – icons are properly registered.Both
'chevron-down'and'check'icons are already defined and registered in the Icon component (src/components/Global/Icons/Icon.tsx). The imports and iconComponents mappings confirm their availability, so there is no risk of runtime warnings or missing icons.Likely an incorrect or invalid review comment.
src/components/AddWithdraw/DynamicBankAccountForm.tsx (3)
8-8: LGTM: Necessary imports for new select functionality.The imports for
BaseSelect,twMerge, and state constants support the new dropdown state selection and character count features.Also applies to: 23-24
238-274: LGTM: Character count implementation is functional.The
rightContentprop correctly displays character counts for inputs with length limits. The nullish coalescing (??) on line 271 safely handles undefined values.
285-310: LGTM: renderSelect properly integrates BaseSelect.The function correctly uses
react-hook-form'sControllerand passes error states toBaseSelect. The error display and styling logic mirrorsrenderInputfor consistency.
…n DynamicBankAccountForm
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
429-441: The BIC validator enables premature submission during debounce delay.The validator returns
true(validation passes) on line 434 when the input hasn't yet matched the debounced value. This causes:
isValidbecomestrueimmediatelysetisCheckingBICValid(true)(line 437) never executes for unmatched values- The submit button becomes enabled despite pending validation
- User can submit before the debounced BIC API call completes
The guard at line 138-140 won't prevent this because
isValidatingremains false for synchronous early returns.Fix: Return
falseinstead oftrueon line 434 to keep validation in a failed state until the debounced value is ready:if (value.trim() !== debouncedBicValue?.trim()) { - return true // Skip validation until debounced value is ready + return false // Validation incomplete until debounced value matches }This ensures the button stays disabled (
!isValidcondition) while waiting for debounce.
🧹 Nitpick comments (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
480-490: Consider memoizing the state options mapping.The state options are computed inline on every render (lines 483-486). For better performance, consider memoizing this computation:
+ const stateOptions = useMemo( + () => + (isMx ? MX_STATES : US_STATES).map((state) => ({ + label: state.name, + value: state.code, + })), + [isMx] + ) + {renderSelect( 'state', 'Select your state', - (isMx ? MX_STATES : US_STATES).map((state) => ({ - label: state.name, - value: state.code, - })), + stateOptions, { required: 'State is required', } )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx(8 hunks)
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1407
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:459-475
Timestamp: 2025-11-06T08:19:05.201Z
Learning: In src/components/AddWithdraw/DynamicBankAccountForm.tsx, the street address field has a 35-character maximum length limit because the backend provider enforces this constraint, applicable to both US and Mexican addresses.
📚 Learning: 2025-11-06T08:19:05.201Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1407
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:459-475
Timestamp: 2025-11-06T08:19:05.201Z
Learning: In src/components/AddWithdraw/DynamicBankAccountForm.tsx, the street address field has a 35-character maximum length limit because the backend provider enforces this constraint, applicable to both US and Mexican addresses.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-25T11:33:46.776Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 484
File: src/components/Cashout/Components/Initial.view.tsx:273-274
Timestamp: 2024-10-25T11:33:46.776Z
Learning: In the `InitialCashoutView` component (`src/components/Cashout/Components/Initial.view.tsx`), linked bank accounts should not generate error states, and the `ValidatedInput` component will clear any error messages if needed. Therefore, it's unnecessary to manually clear the error state when selecting or clearing linked bank accounts.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-14T14:42:54.411Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/utils/withdraw.utils.ts:181-191
Timestamp: 2025-08-14T14:42:54.411Z
Learning: The countryCodeMap in src/components/AddMoney/consts/index.ts uses uppercase 3-letter country codes as keys (like 'AUT', 'BEL', 'CZE') that map to 2-letter country codes, requiring input normalization to uppercase for proper lookups.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-08-13T18:22:01.941Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1094
File: src/components/AddWithdraw/DynamicBankAccountForm.tsx:0-0
Timestamp: 2025-08-13T18:22:01.941Z
Learning: In the DynamicBankAccountForm component, the countryName parameter from useParams will always resemble a country title, not a URL slug.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2025-05-22T15:38:48.586Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 869
File: src/app/(mobile-ui)/withdraw/page.tsx:82-88
Timestamp: 2025-05-22T15:38:48.586Z
Learning: The country-specific withdrawal route exists at src/app/(mobile-ui)/withdraw/[...country]/page.tsx and renders the AddWithdrawCountriesList component with flow="withdraw".
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-11-26T12:06:11.603Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 545
File: src/components/Global/GeneralRecipientInput/index.tsx:118-126
Timestamp: 2024-11-26T12:06:11.603Z
Learning: For bank account input fields, use `autocomplete="bank-account-number"` when the recipient type is `'us'` or `'iban'`.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-18T08:54:22.142Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 458
File: src/components/Offramp/Confirm.view.tsx:96-96
Timestamp: 2024-10-18T08:54:22.142Z
Learning: In the `src/components/Offramp/Confirm.view.tsx` file, it's acceptable to include crass or informal language in code comments.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-29T12:20:47.207Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 495
File: src/components/Create/Link/Input.view.tsx:244-248
Timestamp: 2024-10-29T12:20:47.207Z
Learning: In the `TokenAmountInput` component within `src/components/Global/TokenAmountInput/index.tsx`, when `balance` is undefined, the `maxValue` prop should be set to an empty string `''`.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
📚 Learning: 2024-10-23T09:38:04.446Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 469
File: src/app/request/pay/page.tsx:32-64
Timestamp: 2024-10-23T09:38:04.446Z
Learning: Within `src/app/request/pay/page.tsx`, extracting the `getBaseUrl` function does not add significant readability, and the host URL construction code is expected to change soon.
Applied to files:
src/components/AddWithdraw/DynamicBankAccountForm.tsx
🧬 Code graph analysis (1)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (1)
src/constants/stateCodes.consts.ts (2)
MX_STATES(64-97)US_STATES(5-56)
⏰ 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: Deploy-Preview
🔇 Additional comments (5)
src/components/AddWithdraw/DynamicBankAccountForm.tsx (5)
8-8: LGTM!The new imports are properly utilized throughout the component for select functionality, conditional styling, and state code constants.
Also applies to: 23-24
86-86: LGTM!Extracting the street address length limit to a named constant improves maintainability and the comment properly documents the backend constraint.
238-240: LGTM!The additions to
renderInputare well-implemented:
- Parameters are properly typed and optional
- Conditional error styling using
twMergeis clean- Character count display safely handles undefined values with nullish coalescing
Also applies to: 253-256, 268-274
285-310: LGTM!The
renderSelecthelper follows the same pattern asrenderInput, providing consistent error handling, validation integration, and conditional styling.
460-476: LGTM!The street address field correctly uses the
STREET_ADDRESS_MAX_LENGTHconstant consistently in both validation rules and the character counter, ensuring the UI and validation stay synchronized.Based on learnings
2 participants
