fix: address book baby #10803
fix: address book baby #10803
Conversation
📝 WalkthroughWalkthroughAdds an Address Book feature flag and implementation: new persisted Redux slice with migrations, types, selectors and utils; UI components and modals for save/select/delete flows; wiring into the send modal and inputs; translation keys; and removal/simplification of responsive send routing. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User
participant SendModal
participant AddressBookUI
participant ModalProvider
participant Redux
Note over SendModal: Address selection / save / delete flows (feature-flag gated)
User->>SendModal: Open Send -> Address step
SendModal->>AddressBookUI: Request entries (chainId)
AddressBookUI->>SendModal: Render list
User->>AddressBookUI: Click entry
AddressBookUI->>SendModal: Populate address input
SendModal->>SendModal: Navigate -> AmountDetails
Note over SendModal,ModalProvider: Save new address flow
User->>SendModal: Enter/paste address -> click Save
SendModal->>ModalProvider: Open AddressBookSaveModal
User->>ModalProvider: Submit label
ModalProvider->>Redux: dispatch addAddress(entry)
Redux->>Redux: isDuplicateEntry (EVM-aware)
alt Not duplicate
Redux-->>ModalProvider: saved
else Duplicate
Redux-->>ModalProvider: rejected
end
ModalProvider-->>SendModal: close modal -> input updated
Note over SendModal,AddressBookUI: Delete flow
User->>AddressBookUI: Click delete on entry
AddressBookUI->>ModalProvider: Open ConfirmDelete
User->>ModalProvider: Confirm delete
ModalProvider->>Redux: dispatch deleteAddress(entry)
Redux-->>AddressBookUI: removed -> UI updates
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (3 warnings)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (5)**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
Files:
**/*.tsx📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
Files:
**/*📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
Files:
**/*.{tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
Files:
🧠 Learnings (43)📓 Common learnings📚 Learning: 2025-10-21T17:11:18.087ZApplied to files:
📚 Learning: 2025-08-29T07:07:49.332ZApplied to files:
📚 Learning: 2025-08-03T22:09:37.542ZApplied to files:
📚 Learning: 2025-09-12T10:44:46.723ZApplied to files:
📚 Learning: 2025-08-03T22:09:37.542ZApplied to files:
📚 Learning: 2025-08-08T11:41:36.971ZApplied to files:
📚 Learning: 2025-09-08T15:53:09.362ZApplied to files:
📚 Learning: 2025-08-15T07:51:16.374ZApplied to files:
📚 Learning: 2025-07-24T09:43:11.699ZApplied to files:
📚 Learning: 2025-08-10T21:09:25.643ZApplied to files:
📚 Learning: 2025-08-15T07:51:16.374ZApplied to files:
📚 Learning: 2025-08-08T15:00:49.887ZApplied to files:
📚 Learning: 2025-08-15T07:51:16.374ZApplied to files:
📚 Learning: 2025-08-22T12:58:36.070ZApplied to files:
📚 Learning: 2025-08-26T19:04:38.672ZApplied to files:
📚 Learning: 2025-09-04T17:29:59.479ZApplied to files:
📚 Learning: 2025-10-23T14:27:19.073ZApplied to files:
📚 Learning: 2025-08-27T09:51:36.901ZApplied to files:
📚 Learning: 2025-08-27T09:47:06.275ZApplied to files:
📚 Learning: 2025-09-17T22:40:30.149ZApplied to files:
📚 Learning: 2025-08-25T12:59:43.842ZApplied to files:
📚 Learning: 2025-08-22T13:00:44.879ZApplied to files:
📚 Learning: 2025-09-02T12:38:46.940ZApplied to files:
📚 Learning: 2025-08-08T15:00:22.321ZApplied to files:
📚 Learning: 2025-09-02T08:34:08.157ZApplied to files:
📚 Learning: 2025-08-22T12:58:26.590ZApplied to files:
📚 Learning: 2025-09-13T16:45:17.166ZApplied to files:
📚 Learning: 2025-08-25T23:32:13.876ZApplied to files:
📚 Learning: 2025-09-12T11:56:19.437ZApplied to files:
📚 Learning: 2025-09-10T15:35:46.223ZApplied to files:
📚 Learning: 2025-09-02T00:02:44.889ZApplied to files:
📚 Learning: 2025-09-08T22:00:48.005ZApplied to files:
📚 Learning: 2025-09-12T12:00:33.924ZApplied to files:
📚 Learning: 2025-09-12T13:16:27.004ZApplied to files:
📚 Learning: 2025-09-12T12:00:33.924ZApplied to files:
📚 Learning: 2025-10-07T03:44:27.350ZApplied to files:
📚 Learning: 2025-09-04T10:18:34.140ZApplied to files:
📚 Learning: 2025-08-05T22:41:35.473ZApplied to files:
📚 Learning: 2025-08-14T17:54:32.563ZApplied to files:
📚 Learning: 2025-08-27T09:47:59.394ZApplied to files:
📚 Learning: 2025-10-21T23:21:22.304ZApplied to files:
📚 Learning: 2025-07-24T21:05:13.642ZApplied to files:
🧬 Code graph analysis (1)src/components/Modals/Send/views/Address.tsx (3)
⏰ 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)
🔇 Additional comments (3)
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 |
NeOMakinG
force-pushed
the
address-book
branch
from
60698bc to
92c20c1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Modals/Send/views/SendAmountDetails.tsx (1)
199-229: Harden async address validation; ensure setIsValidating is always clearedIf parseAddressInputWithChainId throws, setIsValidating stays true and derived fields may be stale. Wrap in try/catch/finally.
Apply this diff:
@@ validate: { validateAddress: async (rawInput: string) => { if (!asset) return if (rawInput === '') return @@ - setIsValidating(true) - setValue(SendFormFields.To, '') - setValue(SendFormFields.VanityAddress, '') - const { assetId, chainId } = asset - const parseAddressInputWithChainIdArgs = { - assetId, - chainId, - urlOrAddress, - disableUrlParsing: true, - } - const { address, vanityAddress } = await parseAddressInputWithChainId( - parseAddressInputWithChainIdArgs, - ) - setIsValidating(false) - setValue(SendFormFields.To, address) - setValue(SendFormFields.VanityAddress, vanityAddress) - const invalidMessage = 'common.invalidAddress' - return address ? true : invalidMessage + setIsValidating(true) + setValue(SendFormFields.To, '') + setValue(SendFormFields.VanityAddress, '') + const { assetId, chainId } = asset + const args = { assetId, chainId, urlOrAddress, disableUrlParsing: true } + try { + const { address, vanityAddress } = await parseAddressInputWithChainId(args) + setValue(SendFormFields.To, address) + setValue(SendFormFields.VanityAddress, vanityAddress) + return address ? true : 'common.invalidAddress' + } catch (e) { + // Optionally log with structured logger if available + setValue(SendFormFields.To, '') + setValue(SendFormFields.VanityAddress, '') + return 'common.invalidAddress' + } finally { + setIsValidating(false) + } }, },As per coding guidelines (async ops must be guarded).
🧹 Nitpick comments (21)
src/components/Modals/Send/components/QRCodeIcon.tsx (2)
8-16: Simplify sizing; drop useMemo+sx in favor of Icon.boxSize.Use Chakra’s boxSize on Icon and remove the memoized sx object.
- const iconSx = useMemo( - () => ({ - svg: { - width: '24px', - height: '24px', - }, - }), - [], - )
19-29: Use Icon.boxSize and mark decorative icon as hidden from AT.This avoids extra sx and improves a11y if the icon is purely visual.
- <Flex - bg={qrBackground} - borderRadius='full' - color='text.primary' - boxSize='44px' - alignItems='center' - justifyContent='center' - sx={iconSx} - > - <Icon as={TbScan} /> + <Flex + bg={qrBackground} + borderRadius='full' + color='text.primary' + boxSize='44px' + alignItems='center' + justifyContent='center' + > + <Icon as={TbScan} boxSize='24px' aria-hidden='true' focusable='false' /> </Flex>src/state/migrations/clearAddressBook.ts (1)
1-9: Clear migration is minimal and correct.Resetting to
initialStateis fine for v0. Consider importingPersistPartialfrom the public entry (redux-persist) for future-proofing, unless other migrations already use thees/path.src/vite-env.d.ts (1)
119-120: Place the new feature flag with the other feature flags.
VITE_FEATURE_ADDRESS_BOOKcurrently sits in the “URLs and API keys” section. Move it to the top “Feature flags” block to keep flags grouped and discoverable.src/components/Modals/Send/Form.tsx (1)
270-276: Avoidanyin catch; useunknownwith a type guard.Prevents unsafe access and aligns with TS guidelines.
Apply this diff:
- } catch (e: any) { - setAddressError(e.message) + } catch (e: unknown) { + const message = e instanceof Error ? e.message : 'Unknown error' + setAddressError(message) }src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
22-25: Make delete confirm resilient to async/slow operations.If
onDeleteever becomes async, add a loading state and disable buttons to prevent double submits; type it to allowPromise<void>. Example:-import { useCallback } from 'react' +import { useCallback, useState } from 'react' @@ -type ConfirmDeleteProps = { +type ConfirmDeleteProps = { entryName: string - onDelete: () => void + onDelete: () => Promise<void> | void onClose: () => void isOpen: boolean } @@ -export const ConfirmDelete = ({ entryName, onDelete, onClose, isOpen }: ConfirmDeleteProps) => { +export const ConfirmDelete = ({ entryName, onDelete, onClose, isOpen }: ConfirmDeleteProps) => { + const [isDeleting, setIsDeleting] = useState(false) @@ - const handleConfirm = useCallback(() => { - onDelete() - onClose() - }, [onDelete, onClose]) + const handleConfirm = useCallback(async () => { + if (isDeleting) return + setIsDeleting(true) + await onDelete() + setIsDeleting(false) + onClose() + }, [isDeleting, onDelete, onClose]) @@ - <Button flex={1} onClick={onClose}> + <Button flex={1} onClick={onClose} isDisabled={isDeleting}> <Text translation='common.cancel' /> </Button> - <Button flex={1} colorScheme='red' onClick={handleConfirm}> + <Button flex={1} colorScheme='red' onClick={handleConfirm} isLoading={isDeleting}> <Text translation='common.delete' /> </Button>Ensure translation keys
modals.send.confirmDelete.titleand.messageexist in all supported locales.Also applies to: 41-46, 1-3, 12-17
src/components/Modals/Send/AddAddressModal.tsx (2)
96-105: Trim input at source to avoid whitespace-only names.Add
setValueAsto normalize the name:- <Input - {...register('name', { + <Input + {...register('name', { required: translate('modals.send.addContact.nameRequired'), minLength: { value: 1, message: translate('modals.send.addContact.nameRequired'), }, + setValueAs: (v: string) => v?.trim() ?? '', })}
70-83: Guard against duplicate contacts (same chainId + address).Before dispatching, check if an entry already exists and short‑circuit with a toast.
- const onSubmit = useCallback( - (data: FormData) => { + const onSubmit = useCallback( + (data: FormData) => { + // Prevent duplicates for same chain/address + // If you prefer namespace-level dedupe, adapt the selector accordingly. + const exists = + addressBookSlice.selectors + .selectAddressBookEntriesByChainId( + // @ts-expect-error typed selector usage at call-site + (window as any).__reduxStore__?.getState?.() ?? {}, + chainId, + ) + ?.some(e => e.address.toLowerCase() === address.toLowerCase()) ?? false + if (exists) { + // Optionally use useErrorToast for UX + return + } dispatch( addressBookSlice.actions.addAddress({ name: data.name, address, chainId, }), ) onSuccess?.() handleClose() },Alternatively, compute
existsvia a localuseAppSelectorand close over it. I can wire this with a proper toast if desired.src/components/Modals/Send/views/Confirm.tsx (2)
120-123: Normalize address compare to avoid case-mismatch misses.EVM addresses are case-insensitive (checksums vary). Normalize both sides:
- const selectedAddressBookEntry = useMemo(() => { - return addressBookEntries.find(entry => entry.address === to) - }, [addressBookEntries, to]) + const selectedAddressBookEntry = useMemo(() => { + const toNorm = (to ?? '').toLowerCase() + return addressBookEntries.find(entry => entry.address.toLowerCase() === toNorm) + }, [addressBookEntries, to])
83-91: Minor naming nit: avoid confusion with react-hook-form’s handleSubmit.Prop
handleSubmitcan be mistaken for RHF’shandleSubmit. ConsideronConfirmoronSubmitTxfor clarity. No functional change.Also applies to: 195-217
src/components/Modals/Send/views/Address.tsx (2)
93-98: Normalize address comparison when detecting “custom address”.Avoid false positives due to checksum/case differences:
- const isCustomAddress = useMemo(() => { - if (!input || !address || !asset?.chainId) return false - const existsInAddressBook = addressBookEntries.some(entry => entry.address === address) - return !existsInAddressBook - }, [input, address, addressBookEntries, asset?.chainId]) + const isCustomAddress = useMemo(() => { + if (!input || !address || !asset?.chainId) return false + const addrNorm = address.toLowerCase() + const existsInAddressBook = addressBookEntries.some( + entry => entry.address.toLowerCase() === addrNorm, + ) + return !existsInAddressBook + }, [input, address, addressBookEntries, asset?.chainId])
48-49: Naming nit (optional).
showSaveButtoncould beisSaveButtonVisiblefor boolean clarity. Skip if you prefer current naming.src/components/Modals/Send/AddressBook/AddressBook.tsx (3)
29-33: Avoid redefining AddressBookEntry; import the slice type or Pick<> what you needLocal AddressBookEntry shadows the slice’s AddressBookEntry (adds chainId, createdAt). This invites type drift.
Apply this diff:
@@ -import { addressBookSlice } from '@/state/slices/addressBookSlice/addressBookSlice' +import type { AddressBookEntry as AddressBookEntryModel } from '@/state/slices/addressBookSlice/addressBookSlice' +import { addressBookSlice } from '@/state/slices/addressBookSlice/addressBookSlice' @@ -export type AddressBookEntry = { - id: string - name: string - address: string -} +type AddressBookEntry = Pick<AddressBookEntryModel, 'id' | 'name' | 'address'>Update local usages if needed (e.g., selectedDeleteEntry) to this alias. Based on learnings.
128-132: Type the form context with SendInput for safer accessuseFormContext is untyped here; add the SendInput generic for proper errors/value typing.
Apply this diff:
@@ - const { - control, - formState: { errors }, - } = useFormContext() + const { + control, + formState: { errors }, + } = useFormContext<SendInput>()And import the type:
@@ -import { SendFormFields } from '../SendCommon' +import { SendFormFields } from '../SendCommon' +import type { SendInput } from '@/components/Modals/Send/Form'As per coding guidelines.
216-223: Non-null assertions for selectedDeleteEntry props and reset selection on closeInside the conditional render, selectedDeleteEntry is guaranteed; remove optional chaining to satisfy types. Also clear selectedDeleteEntry on close to avoid stale state.
Apply this diff:
@@ - {selectedDeleteEntry && ( + {selectedDeleteEntry && ( <ConfirmDelete isOpen={isOpen} - onDelete={handleDelete(selectedDeleteEntry?.id)} - onClose={onClose} - entryName={selectedDeleteEntry?.name} + onDelete={handleDelete(selectedDeleteEntry!.id)} + onClose={handleCloseConfirm} + entryName={selectedDeleteEntry!.name} /> )} @@ + const handleCloseConfirm = useCallback(() => { + onClose() + setSelectedDeleteEntry(null) + }, [onClose])src/components/Modals/Send/views/SendAmountDetails.tsx (2)
252-279: Avoid any; strongly type Controller render for amount fieldsUse ControllerRenderProps typing for the two possible amount fields.
Apply this diff:
@@ - const renderController = useCallback( - ({ field: { onChange, value } }: { field: any }) => { + type AmountField = SendFormFields.AmountCryptoPrecision | SendFormFields.FiatAmount + const renderController = useCallback( + ({ + field: { onChange, value }, + }: { + field: ControllerRenderProps<FieldValues, AmountField> + }) => { return ( <NumberFormatAs per coding guidelines.
350-361: Remove nested click targets; put onClick on the ButtonAttaching onClick to HStack and nesting a Button is redundant and can confuse a11y/analytics.
Apply this diff:
- <HStack justify='center' mt={2} spacing={2} onClick={toggleIsFiat} cursor='pointer'> - <Button variant='ghost' size='sm' p={1} minW='auto' h='auto' px={2}> + <HStack justify='center' mt={2} spacing={2}> + <Button variant='ghost' size='sm' p={1} minW='auto' h='auto' px={2} onClick={toggleIsFiat}> <ChakraText fontSize='sm' color='text.subtle' me={2}>src/state/slices/addressBookSlice/addressBookSlice.ts (1)
27-38: Prevent duplicate entries for same address/chainIdCurrently addAddress will add duplicates if invoked repeatedly with the same address/chainId. Consider guarding and updating name instead.
Example adjustment:
@@ - addAddress: create.reducer( + addAddress: create.reducer( (state, action: PayloadAction<Omit<AddressBookEntry, 'id' | 'createdAt'>>) => { - const id = `${action.payload.chainId}_${action.payload.address}_${Date.now()}` + const existingId = state.ids.find( + id => state.byId[id].address === action.payload.address && state.byId[id].chainId === action.payload.chainId, + ) + if (existingId) { + state.byId[existingId].name = action.payload.name + return + } + const id = `${action.payload.chainId}_${action.payload.address}_${Date.now()}` const entry: AddressBookEntry = { ...action.payload, id, createdAt: Date.now(), } state.byId[id] = entry state.ids.push(id) }, ),src/components/Modals/Send/AddressInput/AddressInput.tsx (3)
234-249: Include onClick in deps to avoid stale prop capturerenderController closes over props.onClick but excludes it from deps. Destructure onClick and add it to deps to keep behavior correct without over-rendering.
Apply this diff:
@@ -export const AddressInput = ({ +export const AddressInput = ({ rules, placeholder, enableQr = false, isReadOnly = false, resolvedAddress, chainId, onSaveContact, onFocus, onBlur, onPaste, ...props }: AddressInputProps) => { @@ - onClick={props.onClick ?? handleDisplayClick} + onClick={onClick ?? handleDisplayClick} @@ - // eslint-disable-next-line react-hooks/exhaustive-deps - [ + // eslint-disable-next-line react-hooks/exhaustive-deps + [ placeholder, isInvalid, isReadOnly, isFocused, isDirty, translate, onFocus, onBlur, onPaste, resolvedAddress, addressBookEntry, onSaveContact, + onClick, ],Add the destructure:
@@ -}: AddressInputProps) => { +}: AddressInputProps) => { const navigate = useNavigate() const translate = useTranslate() + const { onClick } = propsAlso applies to: 175-186, 180-181
73-76: Ref type and translation key nits
- The input renders as a textarea; type the ref accordingly.
- Use a consistent translation key for “To” (align with modals.send.sendForm.to used elsewhere).
Apply this diff:
@@ - const inputRef = useRef<HTMLInputElement>(null) + const inputRef = useRef<HTMLTextAreaElement | HTMLInputElement>(null) @@ - <Text color='text.subtle' w='full' pl={4} fontSize='sm'> - {translate('trade.to')} + <Text color='text.subtle' w='full' pl={4} fontSize='sm'> + {translate('modals.send.sendForm.to')} </Text>Also applies to: 148-167
34-43: Remove unused prop pe or wire it throughpe is declared but never used; drop it or pass it down to the rendered input/container for consistency.
Example removal:
- pe?: SpaceProps['pe']Or forward via {...props} where appropriate.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (28)
.env.development(1 hunks).env.production(1 hunks)src/assets/translations/en/main.json(3 hunks)src/components/Modals/QrCode/QrCode.tsx(2 hunks)src/components/Modals/Send/AddAddressModal.tsx(1 hunks)src/components/Modals/Send/AddressBook/AddressBook.tsx(1 hunks)src/components/Modals/Send/AddressBook/ConfirmDelete.tsx(1 hunks)src/components/Modals/Send/AddressInput/AddressInput.tsx(3 hunks)src/components/Modals/Send/Form.tsx(2 hunks)src/components/Modals/Send/Send.tsx(2 hunks)src/components/Modals/Send/SendCommon.tsx(1 hunks)src/components/Modals/Send/components/AddressInputWithDropdown.tsx(0 hunks)src/components/Modals/Send/components/QRCodeIcon.tsx(1 hunks)src/components/Modals/Send/views/Address.tsx(9 hunks)src/components/Modals/Send/views/Confirm.tsx(6 hunks)src/components/Modals/Send/views/SendAmountDetails.tsx(7 hunks)src/config.ts(1 hunks)src/context/ModalProvider/ModalContainer.tsx(2 hunks)src/context/ModalProvider/types.ts(2 hunks)src/state/migrations/clearAddressBook.ts(1 hunks)src/state/migrations/index.ts(2 hunks)src/state/reducer.ts(5 hunks)src/state/slices/addressBookSlice/addressBookSlice.ts(1 hunks)src/state/slices/addressBookSlice/selectors.ts(1 hunks)src/state/slices/preferencesSlice/preferencesSlice.ts(2 hunks)src/state/store.ts(1 hunks)src/test/mocks/store.ts(2 hunks)src/vite-env.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Modals/Send/components/AddressInputWithDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/context/ModalProvider/ModalContainer.tsxsrc/state/migrations/clearAddressBook.tssrc/state/store.tssrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/SendCommon.tsxsrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/state/slices/addressBookSlice/selectors.tssrc/config.tssrc/components/Modals/Send/components/QRCodeIcon.tsxsrc/components/Modals/Send/AddressBook/ConfirmDelete.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/state/migrations/index.tssrc/vite-env.d.tssrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/state/reducer.tssrc/components/Modals/Send/Send.tsxsrc/test/mocks/store.tssrc/components/Modals/Send/AddAddressModal.tsxsrc/state/slices/addressBookSlice/addressBookSlice.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/context/ModalProvider/ModalContainer.tsxsrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/components/QRCodeIcon.tsxsrc/components/Modals/Send/AddressBook/ConfirmDelete.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/Send.tsxsrc/components/Modals/Send/AddAddressModal.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/context/ModalProvider/ModalContainer.tsxsrc/state/migrations/clearAddressBook.tssrc/state/store.tssrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/SendCommon.tsxsrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/state/slices/addressBookSlice/selectors.tssrc/assets/translations/en/main.jsonsrc/config.tssrc/components/Modals/Send/components/QRCodeIcon.tsxsrc/components/Modals/Send/AddressBook/ConfirmDelete.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/state/migrations/index.tssrc/vite-env.d.tssrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/state/reducer.tssrc/components/Modals/Send/Send.tsxsrc/test/mocks/store.tssrc/components/Modals/Send/AddAddressModal.tsxsrc/state/slices/addressBookSlice/addressBookSlice.ts
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/context/ModalProvider/ModalContainer.tsxsrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/components/QRCodeIcon.tsxsrc/components/Modals/Send/AddressBook/ConfirmDelete.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/Send.tsxsrc/components/Modals/Send/AddAddressModal.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/context/ModalProvider/ModalContainer.tsxsrc/state/migrations/clearAddressBook.tssrc/state/store.tssrc/components/Modals/Send/Form.tsxsrc/components/Modals/Send/SendCommon.tsxsrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/state/slices/addressBookSlice/selectors.tssrc/config.tssrc/components/Modals/Send/components/QRCodeIcon.tsxsrc/components/Modals/Send/AddressBook/ConfirmDelete.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/state/migrations/index.tssrc/vite-env.d.tssrc/components/Modals/Send/views/Confirm.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/state/reducer.tssrc/components/Modals/Send/Send.tsxsrc/test/mocks/store.tssrc/components/Modals/Send/AddAddressModal.tsxsrc/state/slices/addressBookSlice/addressBookSlice.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
📚 Learning: 2025-09-04T10:18:34.140Z
Learnt from: gomesalexandre
PR: shapeshift/web#10427
File: src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx:40-40
Timestamp: 2025-09-04T10:18:34.140Z
Learning: In the shapeshift/web codebase, src/state/slices/selectors.ts uses wildcard exports (`export * from './[sliceName]/selectors'`) to re-export all selectors from individual slice selector files, making them available through the barrel import. This means selectors like selectTxByFilter from txHistorySlice/selectors are properly accessible via '@/state/slices/selectors' even though they don't appear in explicit named exports.
Applied to files:
src/state/slices/addressBookSlice/selectors.ts
📚 Learning: 2025-08-27T13:49:48.668Z
Learnt from: NeOMakinG
PR: shapeshift/web#10375
File: src/state/migrations/index.ts:214-215
Timestamp: 2025-08-27T13:49:48.668Z
Learning: The ShapeShift web app uses an automated versioning system for Redux Persist migrations. The version is calculated as `Math.max(...Object.keys(clearAssetsMigrations).map(Number))`, which automatically uses the highest migration number as the version. This eliminates the need to manually update persistConfig versions when adding new migrations - the system automatically bumps the version when new migration numbers are added to the migration objects.
Applied to files:
src/state/reducer.ts
🧬 Code graph analysis (18)
src/context/ModalProvider/ModalContainer.tsx (2)
src/components/Modals/Send/AddAddressModal.tsx (1)
AddAddressModal(46-134)src/utils/makeSuspenseful.tsx (1)
makeSuspenseful(42-62)
src/state/migrations/clearAddressBook.ts (1)
src/state/slices/addressBookSlice/addressBookSlice.ts (2)
AddressBookState(13-16)initialState(18-21)
src/state/store.ts (1)
src/state/reducer.ts (1)
slices(53-68)
src/context/ModalProvider/types.ts (1)
src/components/Modals/Send/AddAddressModal.tsx (1)
AddAddressModalProps(27-31)
src/components/Modals/Send/views/SendAmountDetails.tsx (2)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
AddressInput(53-273)
src/state/slices/addressBookSlice/selectors.ts (3)
src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(23-57)src/state/reducer.ts (1)
ReduxState(207-207)src/state/selectors.ts (1)
selectSearchQueryFromFilter(96-96)
src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (4)
src/components/Modal/components/Dialog.tsx (1)
Dialog(149-149)src/components/Modal/components/DialogHeader.tsx (2)
DialogHeader(15-48)DialogHeaderMiddle(54-58)src/components/Modal/components/DialogFooter.tsx (1)
DialogFooter(9-23)src/components/Text/Text.tsx (1)
Text(19-83)
src/components/Modals/Send/views/Address.tsx (5)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (1)
selectAddressBookEntriesByChainNamespace(16-33)packages/caip/src/constants.ts (1)
ethChainId(60-60)src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
AddressBook(121-226)
src/components/Modals/Send/AddressInput/AddressInput.tsx (4)
src/components/Modals/Send/Form.tsx (1)
SendInput(46-64)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (1)
selectAddressBookEntriesByChainNamespace(16-33)src/components/Text/Text.tsx (1)
Text(19-83)
src/state/migrations/index.ts (1)
src/state/migrations/clearAddressBook.ts (1)
clearAddressBook(6-9)
src/components/Modals/Send/views/Confirm.tsx (2)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (1)
selectAddressBookEntriesByChainNamespace(16-33)
src/components/Modals/QrCode/QrCode.tsx (1)
src/components/Modals/Send/SendCommon.tsx (1)
sendRoutes(11-18)
src/components/Modals/Send/AddressBook/AddressBook.tsx (5)
src/state/slices/addressBookSlice/addressBookSlice.ts (2)
AddressBookEntry(5-11)addressBookSlice(23-57)src/state/store.ts (2)
useAppDispatch(151-151)useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectAddressBookEntriesByChainNamespace(16-33)selectAddressBookEntriesBySearchQuery(43-62)src/components/Text/Text.tsx (1)
Text(19-83)src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
ConfirmDelete(19-51)
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
src/config.ts (1)
getConfig(209-211)
src/state/reducer.ts (2)
src/state/slices/addressBookSlice/addressBookSlice.ts (2)
addressBookSlice(23-57)AddressBookState(13-16)src/state/migrations/index.ts (1)
clearAddressBookMigrations(265-267)
src/components/Modals/Send/Send.tsx (1)
src/components/Modals/Send/SendCommon.tsx (1)
sendRoutes(11-18)
src/components/Modals/Send/AddAddressModal.tsx (3)
src/state/store.ts (1)
useAppDispatch(151-151)src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(23-57)
src/state/slices/addressBookSlice/addressBookSlice.ts (1)
src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
AddressBookEntry(29-33)
🪛 dotenv-linter (4.0.0)
.env.production
[warning] 4-4: [UnorderedKey] The VITE_FEATURE_ADDRESS_BOOK key should go before the VITE_FEATURE_MIXPANEL key
(UnorderedKey)
.env.development
[warning] 9-9: [UnorderedKey] The VITE_FEATURE_ADDRESS_BOOK key should go before the VITE_FEATURE_THORCHAIN_TCY_ACTIVITY key
(UnorderedKey)
⏰ 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: Install and Cache
🔇 Additional comments (17)
src/components/Modals/Send/components/QRCodeIcon.tsx (1)
5-31: Ensure this renders under an error boundary.Confirm a parent boundary wraps this component within the Send modal stack. As per coding guidelines.
src/state/slices/addressBookSlice/selectors.ts (1)
16-33: LGTM on namespace-scoped filtering.Using chainNamespace as the cache key matches the selector’s semantics.
src/state/store.ts (1)
57-57: LGTM — clearing addressBook in clearState is consistent with other slices.src/state/migrations/index.ts (1)
265-267: Migration wiring verified and correct.The
clearAddressBookMigrationsis properly imported (line 15) and integrated intoaddressBookPersistConfigwith version calculation (line 147) and migration function creation (line 148). The config is applied to the addressBook reducer (line 173).src/state/reducer.ts (4)
15-24: Address book slice and migrations wiring look consistent with existing patterns.Imports align with other “clear*” migrations and slice types. No issues.
Also applies to: 27-29
67-68: Registering the slice inslicesmap is correct.Keeps parity with other slices; good for dev tooling/tests relying on
slices.
144-150: Persist config follows the auto-versioning migration pattern.Using
Math.max(Object.keys(clearAddressBookMigrations))+createMigratematches our migration strategy. LGTM.
173-174: Reducer registration is correct.Type param
AddressBookState+persistReducerusage is sound.src/test/mocks/store.ts (2)
167-168: Feature flag added to mocks is aligned.
preferences.featureFlags.AddressBookpresence matches the new env flag and selectors.
363-370: Mock state foraddressBookmatches slice shape.Includes
_persist,byId, andids. This should prevent test rehydration issues.src/components/Modals/Send/Form.tsx (1)
208-214: Unified navigation to Address route looks good.Replacing responsive branching with
navigate(SendRoutes.Address, { replace: true })simplifies flow. Verify desktop parity as per issue #10694.src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
27-50: LGTM — dialog composition and copy look solid.Structure, callbacks, and i18n usage are clean.
src/components/Modals/Send/AddAddressModal.tsx (2)
85-134: LGTM — modal wiring, validation, and dispatch look good.Avatar blockies, reset-on-close, and disabled submit on invalid are correct.
46-50: Error boundary coverage.Confirm this modal is rendered within an ErrorBoundary per guidelines; if not, wrap it or ensure the Modal system provides one.
src/components/Modals/Send/views/Confirm.tsx (1)
270-307: LGTM — integration with address book and avatar rendering.Tooltip, MiddleEllipsis, name rendering, and avatar source logic look good.
Also applies to: 140-147, 273-283
src/components/Modals/Send/views/Address.tsx (1)
210-219: LGTM — feature flag gating, save-contact flow, and QR button UX.Gating via
useFeatureFlag('AddressBook'), save flow using modal, and empty-change handling are well integrated.Also applies to: 243-246, 249-266
src/components/Modals/Send/SendCommon.tsx (1)
8-18: Review comment is incorrect. SendRoutes.Amount is unused and should not be included in sendRoutes.The
sendRoutesarray is correctly defined with only the routes actively used in the Send modal flow. Verification shows:
SendRoutes.Amountis defined in the enum but never referenced anywhere in the codebase- No navigation logic targets
/send/amountor attempts to navigate toSendRoutes.Amount- The
sendRoutesarray serves asinitialEntriesforMemoryRouterin Send.tsx and QrCode.tsx- Other similar modals in the codebase follow the same pattern—their route arrays only include routes they actually use (e.g.,
Trade.tsx,ClaimRoutes.tsx)Including unused routes in
initialEntriesadds unnecessary bloat without functional benefit. The current implementation is correct.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/components/Modals/Send/views/Confirm.tsx (1)
422-425: Guard fromAccountId against undefined accountId (prevents crash).fromAccountId('') will throw when accountId is falsy. Short‑circuit before calling.
- <InlineCopyButton - isDisabled={!accountId || isUtxoAccountId(accountId)} - value={fromAccountId(accountId ?? '').account} - > + <InlineCopyButton + isDisabled={!accountId || isUtxoAccountId(accountId)} + value={accountId ? fromAccountId(accountId).account : ''} + >src/components/Modals/Send/views/SendAmountDetails.tsx (1)
192-195: Fix characters remaining when memo is empty (NaN bug).Number(memo?.length) yields NaN when undefined, causing 0 remaining instead of 256.
- const remainingMemoChars = useMemo(() => bnOrZero(MAX_MEMO_LENGTH - Number(memo?.length)), [memo]) + const remainingMemoChars = useMemo( + () => bnOrZero(MAX_MEMO_LENGTH - (memo?.length ?? 0)), + [memo], + )
🧹 Nitpick comments (11)
src/components/Modals/Send/AddressBook/AddressBookEntryButton.tsx (1)
77-79: Add accessibility attributes to the delete button.The delete button lacks an
aria-labelattribute, which is important for screen readers to announce the button's purpose.Apply this diff:
- <Button size='sm' onClick={handleDelete(entryKey)} sx={deleteButtonSx} flexShrink={0}> + <Button + size='sm' + onClick={handleDelete(entryKey)} + sx={deleteButtonSx} + flexShrink={0} + aria-label={`Delete ${label}`} + > <Icon as={FaTrash} boxSize={4} /> </Button>src/state/slices/addressBookSlice/utils.ts (1)
14-16: Consider using Result pattern for error handling.The function throws an error for invalid entry types. Per coding guidelines, error handling should use the Result pattern with
Ok()andErr()from@sniptt/monads. However, if this is an internal utility where invalid types represent a programming error (not a runtime condition), throwing may be acceptable.As per coding guidelines.
src/components/Modals/Send/views/Confirm.tsx (1)
85-86: Nit: Name reflects ms, not seconds.Constant is in milliseconds; rename for clarity.
-const LONG_PRESS_SECONDS_THRESHOLD = 2000 +const LONG_PRESS_MS_THRESHOLD = 2000 ... - threshold: LONG_PRESS_SECONDS_THRESHOLD, + threshold: LONG_PRESS_MS_THRESHOLD,src/components/Modals/Send/AddressBookSaveModal.tsx (2)
145-147: Disable “Save” when address maps to an internal account.Prevents silent no‑op on submit and aligns UI with behavior.
- <Button type='submit' colorScheme='blue' width='full' size='lg' isDisabled={!isValid}> + <Button + type='submit' + colorScheme='blue' + width='full' + size='lg' + isDisabled={!isValid || !!internalAccountId} + >
118-118: Add accessible name to Avatar.Improves a11y for screen readers with a meaningful fallback.
- <Avatar src={avatarUrl} size='lg' /> + <Avatar src={avatarUrl} size='lg' name={address} />src/components/Modals/Send/AddressBook/AddressBook.tsx (2)
61-63: Normalize address casing when matching selected entry.Case‑insensitive match avoids missed hits on EVM checksummed vs lowercased inputs.
- const selectedEntry = useMemo(() => { - if (!input) return undefined - return addressBookEntries?.find(entry => entry.isExternal && entry.address === input) - }, [addressBookEntries, input]) + const selectedEntry = useMemo(() => { + if (!input) return undefined + const normalized = input.toLowerCase() + return addressBookEntries?.find( + entry => entry.isExternal && entry.address.toLowerCase() === normalized, + ) + }, [addressBookEntries, input])
90-94: Optional: simplify entries selection logic.This is equivalent and removes the selectedEntry dependency.
- const entries = useMemo(() => { - if (selectedEntry || !input || (input && !addressError)) return addressBookEntries - return addressBookSearchEntries - }, [selectedEntry, addressBookEntries, addressBookSearchEntries, input, addressError]) + const entries = useMemo(() => { + // Show all when no input or when input is a valid full address; otherwise show search + return !input || !addressError ? addressBookEntries : addressBookSearchEntries + }, [addressBookEntries, addressBookSearchEntries, input, addressError])src/components/Modals/Send/views/SendAmountDetails.tsx (1)
82-90: Replace any with explicit types for better TS safety.Tighten typings for AmountInput props and Controller render param. As per coding guidelines.
-const AmountInput = (props: any) => { +type AmountInputProps = React.ComponentProps<typeof Input> & { value?: string | number } +const AmountInput = (props: AmountInputProps) => {- const renderController = useCallback( - ({ field: { onChange, value } }: { field: any }) => { + const renderController = useCallback( + ({ + field: { onChange, value }, + }: { + field: + | ControllerRenderProps<FieldValues, SendFormFields.AmountCryptoPrecision> + | ControllerRenderProps<FieldValues, SendFormFields.FiatAmount> + }) => {Also applies to: 253-271
src/components/Modals/Send/views/Address.tsx (1)
213-217: Consider inlining the empty handler.
handleEmptyChangeis only called once at line 240. Inlining it would reduce indirection without sacrificing clarity:- const handleEmptyChange = useCallback(() => { - setValue(SendFormFields.Input, '', { shouldValidate: true }) - setValue(SendFormFields.To, '') - setValue(SendFormFields.VanityAddress, '') - }, [setValue]) + const handleEmptyChange = useCallback(() => { + setValue(SendFormFields.Input, '', { shouldValidate: true }) + setValue(SendFormFields.To, '') + setValue(SendFormFields.VanityAddress, '') + }, [setValue])Or inline directly at the call site if preferred.
src/components/Modals/Send/AddressInput/AddressInput.tsx (2)
175-184: Document the paste workaround more explicitly.The
setTimeoutworkaround on lines 179-180 is necessary but fragile. If the paste handler timing changes (e.g., due to React updates or form library changes), this could break.Consider adding a more explicit comment linking to any related issues or documenting the exact race condition being avoided:
const handlePaste = useCallback(() => { if (isFocused && inputRef.current) { - // Add this at the end of the callstack so we leave space for the paste event to be handled - // by the onChange event handler of the form before blurring which would cancel the paste event + // WORKAROUND: Delay blur to avoid canceling paste event. + // The onChange handler must process the pasted value before blur resets the input mode. + // If this breaks, check if react-hook-form's paste handling timing changed. setTimeout(() => { inputRef.current?.blur() setIsFocused(false) }, 0) } }, [isFocused])
210-344: Consider extracting rendering modes into separate components.The
renderControllercallback spans 134 lines with three distinct rendering modes (editable input, address book entry, compact display). This violates the Single Responsibility Principle and makes the component harder to maintain and test.Consider extracting each mode into a separate component:
EditableAddressInputAddressBookEntryDisplayCompactAddressDisplayThis would improve testability, readability, and allow better isolation of concerns. However, given the project preference for minimal changes (per learnings), this refactor may be deferred unless the component becomes harder to maintain.
Based on learnings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (21)
src/components/Modals/QrCode/QrCode.tsx(1 hunks)src/components/Modals/Send/AddressBook/AddressBook.tsx(1 hunks)src/components/Modals/Send/AddressBook/AddressBookEntryButton.tsx(1 hunks)src/components/Modals/Send/AddressBookSaveModal.tsx(1 hunks)src/components/Modals/Send/AddressInput/AddressInput.tsx(3 hunks)src/components/Modals/Send/Send.tsx(1 hunks)src/components/Modals/Send/SendCommon.tsx(1 hunks)src/components/Modals/Send/views/Address.tsx(9 hunks)src/components/Modals/Send/views/Confirm.tsx(6 hunks)src/components/Modals/Send/views/SendAmountDetails.tsx(7 hunks)src/context/ModalProvider/ModalContainer.tsx(2 hunks)src/context/ModalProvider/types.ts(2 hunks)src/index.tsx(1 hunks)src/state/selectors.ts(1 hunks)src/state/slices/addressBookSlice/addressBookSlice.ts(1 hunks)src/state/slices/addressBookSlice/selectors.ts(1 hunks)src/state/slices/addressBookSlice/types.ts(1 hunks)src/state/slices/addressBookSlice/utils.ts(1 hunks)src/state/slices/assetsSlice/types.ts(1 hunks)src/state/slices/common-selectors.ts(1 hunks)src/test/mocks/store.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/mocks/store.ts
- src/components/Modals/QrCode/QrCode.tsx
- src/components/Modals/Send/Send.tsx
- src/context/ModalProvider/ModalContainer.tsx
- src/state/slices/addressBookSlice/selectors.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/state/selectors.tssrc/state/slices/addressBookSlice/types.tssrc/state/slices/assetsSlice/types.tssrc/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/AddressBookSaveModal.tsxsrc/components/Modals/Send/AddressBook/AddressBookEntryButton.tsxsrc/state/slices/addressBookSlice/addressBookSlice.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Address.tsxsrc/state/slices/common-selectors.tssrc/index.tsxsrc/state/slices/addressBookSlice/utils.tssrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/views/Confirm.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/state/selectors.tssrc/state/slices/addressBookSlice/types.tssrc/state/slices/assetsSlice/types.tssrc/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/AddressBookSaveModal.tsxsrc/components/Modals/Send/AddressBook/AddressBookEntryButton.tsxsrc/state/slices/addressBookSlice/addressBookSlice.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Address.tsxsrc/state/slices/common-selectors.tssrc/index.tsxsrc/state/slices/addressBookSlice/utils.tssrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/views/Confirm.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/state/selectors.tssrc/state/slices/addressBookSlice/types.tssrc/state/slices/assetsSlice/types.tssrc/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/AddressBookSaveModal.tsxsrc/components/Modals/Send/AddressBook/AddressBookEntryButton.tsxsrc/state/slices/addressBookSlice/addressBookSlice.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Address.tsxsrc/state/slices/common-selectors.tssrc/index.tsxsrc/state/slices/addressBookSlice/utils.tssrc/context/ModalProvider/types.tssrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/views/Confirm.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/AddressBookSaveModal.tsxsrc/components/Modals/Send/AddressBook/AddressBookEntryButton.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Address.tsxsrc/index.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/views/Confirm.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/AddressBookSaveModal.tsxsrc/components/Modals/Send/AddressBook/AddressBookEntryButton.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/views/Address.tsxsrc/index.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/views/Confirm.tsx
🧠 Learnings (8)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/state/selectors.ts
📚 Learning: 2025-08-05T23:36:13.214Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/state/slices/preferencesSlice/selectors.ts:21-25
Timestamp: 2025-08-05T23:36:13.214Z
Learning: The AssetId type from 'shapeshiftoss/caip' package is a string type alias, so it can be used directly as a return type for cache key resolvers in re-reselect selectors without needing explicit string conversion.
Applied to files:
src/state/slices/assetsSlice/types.ts
📚 Learning: 2025-09-04T17:35:57.584Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/state/slices/common-selectors.ts:164-178
Timestamp: 2025-09-04T17:35:57.584Z
Learning: In shapeshift/web, while the Asset type allows relatedAssetKey to be AssetId | null | undefined, the actual asset generation and processing system ensures relatedAssetKey is never undefined in practice. All asset creation utilities explicitly set it to either a specific AssetId (for grouped assets) or null (for primary/unique assets). The only undefined assignments occur during intermediate processing phases and get resolved during the final asset generation step.
Applied to files:
src/state/slices/assetsSlice/types.ts
📚 Learning: 2025-09-04T17:35:57.584Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/state/slices/common-selectors.ts:164-178
Timestamp: 2025-09-04T17:35:57.584Z
Learning: In shapeshift/web, while the Asset type allows relatedAssetKey to be AssetId | null | undefined, the actual asset generation and processing system ensures relatedAssetKey is never undefined in practice. All asset creation utilities explicitly set it to either a specific AssetId (for grouped assets) or null (for primary/unique assets). The only exception is temporary undefined assignment during portal asset processing, which gets resolved later in the pipeline.
Applied to files:
src/state/slices/assetsSlice/types.ts
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
PR: shapeshift/web#10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/Modals/Send/SendCommon.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
📚 Learning: 2025-08-08T15:00:22.321Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
Applied to files:
src/state/slices/addressBookSlice/addressBookSlice.ts
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
PR: shapeshift/web#10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/components/Modals/Send/views/Address.tsx
🧬 Code graph analysis (10)
src/components/Modals/Send/AddressBookSaveModal.tsx (5)
src/state/store.ts (2)
useAppDispatch(151-151)useAppSelector(143-143)src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/slices/addressBookSlice/selectors.ts (1)
selectInternalAccountIdByAddress(89-122)src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(28-97)
src/state/slices/addressBookSlice/addressBookSlice.ts (4)
src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)src/lib/utils/evm/index.ts (1)
getSupportedEvmChainIds(196-198)packages/caip/src/constants.ts (1)
CHAIN_NAMESPACE(77-82)src/state/slices/addressBookSlice/utils.ts (1)
isDuplicateEntry(5-37)
src/components/Modals/Send/AddressInput/AddressInput.tsx (6)
src/components/Modals/Send/Form.tsx (1)
SendInput(46-64)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectExternalAddressBookEntryByAddress(128-143)selectInternalAccountIdByAddress(89-122)src/state/slices/portfolioSlice/selectors.ts (1)
selectPortfolioAccountMetadata(109-117)src/lib/utils/utxo.ts (1)
isUtxoAccountId(24-25)src/state/slices/portfolioSlice/utils/index.ts (1)
accountIdToLabel(80-119)
src/components/Modals/Send/views/Address.tsx (5)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectIsAddressInAddressBook(52-82)selectInternalAccountIdByAddress(89-122)packages/caip/src/constants.ts (1)
ethChainId(60-60)src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
AddressBook(34-157)
src/state/slices/common-selectors.ts (1)
src/state/selector-utils.ts (1)
createDeepEqualOutputSelector(5-7)
src/state/slices/addressBookSlice/utils.ts (2)
src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)packages/caip/src/constants.ts (1)
CHAIN_NAMESPACE(77-82)
src/context/ModalProvider/types.ts (1)
src/components/Modals/Send/AddressBookSaveModal.tsx (1)
AddressBookSaveModalProps(29-33)
src/components/Modals/Send/views/SendAmountDetails.tsx (2)
src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
AddressInput(59-367)
src/components/Modals/Send/AddressBook/AddressBook.tsx (6)
src/state/store.ts (2)
useAppDispatch(151-151)useAppSelector(143-143)src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)src/state/slices/addressBookSlice/selectors.ts (2)
selectAddressBookEntriesByChainId(20-28)selectAddressBookEntriesBySearchQuery(30-46)src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(28-97)src/components/Text/Text.tsx (1)
Text(19-83)src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
ConfirmDelete(19-51)
src/components/Modals/Send/views/Confirm.tsx (6)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectExternalAddressBookEntryByAddress(128-143)selectInternalAccountIdByAddress(89-122)src/state/slices/portfolioSlice/selectors.ts (1)
selectPortfolioAccountMetadata(109-117)src/lib/utils/utxo.ts (1)
isUtxoAccountId(24-25)src/state/slices/portfolioSlice/utils/index.ts (1)
accountIdToLabel(80-119)src/components/InlineCopyButton.tsx (1)
InlineCopyButton(19-64)
🔇 Additional comments (11)
src/state/slices/assetsSlice/types.ts (1)
1-1: LGTM! Import path improvement.Updating the import from the internal
dist/cjspath to the package root entry is a good refactoring that decouples the code from the package's internal build structure and follows best practices.src/index.tsx (1)
26-26: Verify if this change is intentional.Enabling
react-scanappears unrelated to the address book feature described in the PR objectives. This looks like a development/debugging change that may have been committed accidentally.src/state/slices/addressBookSlice/types.ts (1)
3-17: Well-structured discriminated union.The type definition correctly uses discriminated unions to distinguish between internal and external address book entries, ensuring type safety when accessing the
accountIdproperty.src/state/selectors.ts (1)
85-85: LGTM!The new selector follows the established pattern and is properly typed via the
ParamFiltertype definition.src/components/Modals/Send/SendCommon.tsx (1)
8-8: Verify if the new route should be added to initialEntries.The new
Amountroute is added to theSendRoutesenum but is not included in theinitialEntriesarray (lines 11-18). Confirm whether this is intentional or if the route should be added to the array.src/state/slices/common-selectors.ts (1)
75-78: LGTM!The selector correctly filters non-EVM account IDs and follows the same pattern as
selectEvmAccountIdsfor consistency.src/context/ModalProvider/types.ts (1)
16-16: LGTM!The modal registration follows the established pattern for modal integration.
Also applies to: 47-47
src/state/slices/addressBookSlice/utils.ts (1)
22-31: EVM address deduplication logic is correct.The cross-chain duplicate detection for EVM addresses correctly uses case-insensitive comparison, which is appropriate since EVM addresses are case-insensitive.
src/state/slices/addressBookSlice/addressBookSlice.ts (1)
32-62: LGTM: EVM fan‑out + duplicate protection are solid.
- addAddress: dedup + EVM-wide insert behavior aligns with intended UX.
- deleteAddress: cascades across EVM chains by normalized address.
No blockers from me.
Also applies to: 63-91
src/components/Modals/Send/views/SendAmountDetails.tsx (1)
205-229: The review comment's concern about error handling is based on a misunderstanding of the actual function behavior.parseAddressInputWithChainIddoes not throw errors—it handles all error cases internally and returns safe values, including{ address: '', vanityAddress: '', chainId }when address validation fails. This is explicitly documented in the reference implementation (Address.tsx, line 160): "this does not throw, everything inside is handled."However, the suggestion to add try-finally remains architecturally valid:
Coding guidelines alignment: The project guidelines state "ALWAYS wrap async operations in try-catch blocks," which suggests defensive programming as a general pattern.
Consistency concern: There's inconsistency in the codebase—SharedRecipientAddress.tsx includes try-catch around the same function call, while Address.tsx (the reference implementation) does not.
Risk mitigation: While the primary function is safe, the helper functions it awaits internally could theoretically throw uncaught errors, leaving
setIsValidatingin a stuck state.Before applying this refactor, verify with the team whether:
- The architecture intentionally trusts
parseAddressInputWithChainIdto never throw (as Address.tsx suggests), or- Defensive try-catch should be enforced consistently across all call sites.
src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
145-149: Verify focus management doesn't conflict with other interactions.The effect on lines 145-149 programmatically focuses the input whenever
isFocusedis true and the input isn't already focused. This could interfere with user interactions (e.g., clicking other elements, using keyboard navigation) if another component or user action changes focus afterisFocusedbecomes true.Ensure this doesn't cause focus stealing during:
- Modal transitions or overlays (e.g.,
AddressBookSaveModal)- Keyboard navigation between form fields
- QR code scanning flow
If issues arise, consider guarding the focus call with additional conditions (e.g., checking if the modal is the active element).
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
79-79: Strengthen type safety for form values.The type assertion
as Partial<SendInput>bypasses TypeScript's type checking. Provide explicit field names touseWatchfor type safety.Apply this diff:
- const { vanityAddress, input: value } = useWatch() as Partial<SendInput> + const vanityAddress = useWatch<SendInput, SendFormFields.VanityAddress>({ + name: SendFormFields.VanityAddress + }) + const value = useWatch<SendInput, SendFormFields.Input>({ + name: SendFormFields.Input + })This ensures type safety and aligns with the pattern used elsewhere. As per coding guidelines.
🧹 Nitpick comments (3)
src/components/Modals/Send/AddressBook/AddressBook.tsx (2)
50-53: Strengthen type safety foruseWatchcall.The type assertion
as stringbypasses TypeScript checking. Provide an explicit generic touseWatchfor better type safety.Apply this diff:
- const input = useWatch({ - control, - name: SendFormFields.Input, - }) as string + const input = useWatch<SendInput, SendFormFields.Input>({ + control, + name: SendFormFields.Input, + })
89-93: Consider adding a clarifying comment for the filtering logic.The conditional logic determining when to show full entries vs. search results is non-obvious. A brief comment explaining the intent would help future maintainers.
+ // Show full entries when: address selected, no input yet, or valid address entered + // Show filtered search results when: input exists but has validation errors const entries = useMemo(() => { if (selectedEntry || !input || (input && !addressError)) return addressBookEntries return addressBookSearchEntries }, [selectedEntry, addressBookEntries, addressBookSearchEntries, input, addressError])src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
212-326: Consider extracting render modes into separate components.The
renderControllercallback contains three distinct rendering modes (~115 lines). Extracting each mode into a dedicated sub-component would improve readability and testability.For example:
EditableAddressInputfor lines 218-248AddressBookEntryDisplayfor lines 250-284CompactAddressDisplayfor lines 286-325Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
src/components/Modals/Send/AddressBook/AddressBook.tsx(1 hunks)src/components/Modals/Send/AddressInput/AddressInput.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsx
🧠 Learnings (1)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
🧬 Code graph analysis (2)
src/components/Modals/Send/AddressInput/AddressInput.tsx (6)
src/components/Modals/Send/Form.tsx (1)
SendInput(46-64)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectExternalAddressBookEntryByAddress(128-143)selectInternalAccountIdByAddress(89-122)src/state/slices/portfolioSlice/selectors.ts (1)
selectPortfolioAccountMetadata(109-117)src/lib/utils/utxo.ts (1)
isUtxoAccountId(24-25)src/state/slices/portfolioSlice/utils/index.ts (1)
accountIdToLabel(80-119)
src/components/Modals/Send/AddressBook/AddressBook.tsx (6)
src/state/store.ts (2)
useAppDispatch(151-151)useAppSelector(143-143)src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)src/state/slices/addressBookSlice/selectors.ts (2)
selectAddressBookEntriesByChainId(20-28)selectAddressBookEntriesBySearchQuery(30-46)src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(28-97)src/components/Text/Text.tsx (1)
Text(19-83)src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
ConfirmDelete(19-51)
🔇 Additional comments (4)
src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
95-155: LGTM!The avatar generation, button rendering, and JSX structure are well-implemented with proper memoization and clean patterns.
src/components/Modals/Send/AddressInput/AddressInput.tsx (3)
82-126: LGTM!The address book integration is well-structured with proper memoization, correct selector usage, and thoughtful fallback handling for internal account labels.
139-184: LGTM!The focus management logic and event handlers are well-implemented with proper memoization and clear intent. The
setTimeouttrick inhandlePasteis appropriately documented.
186-210: LGTM!The ENS/vanity address rendering logic is clean and properly memoized with correct dependencies.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/Modals/Send/views/Address.tsx (1)
108-117: Align save-button predicates with AddressInput.This includes internalAccountId in the condition, while AddressInput suppresses Save for internal accounts. Drop internalAccountId here for consistency.
- (!isInAddressBook || internalAccountId) && + (!isInAddressBook) &&Please confirm if internal accounts should be savable and update both places accordingly.
🧹 Nitpick comments (9)
src/components/CryptoFiatInput/CryptoFiatInput.tsx (2)
137-137: Consider making padding explicit for clarity.The combination of
p={1}andpx={2}works (resulting inpy={1}andpx={2}), but usingpy={1} px={2}would make the intent more explicit and improve maintainability.Apply this diff to clarify the padding intent:
- <Button variant='ghost' size='sm' p={1} minW='auto' h='auto' px={2}> + <Button variant='ghost' size='sm' py={1} px={2} minW='auto' h='auto'>
137-146: LGTM! Consider adding an accessibility label.The refactor successfully consolidates the toggle UI into a single Button component, improving the visual cohesion and click target area. The implementation is clean and aligns with the PR's UI consolidation objectives.
Optional: For improved accessibility, consider adding an
aria-labelto help screen reader users understand the button's purpose:- <Button variant='ghost' size='sm' p={1} minW='auto' h='auto' px={2}> + <Button + variant='ghost' + size='sm' + p={1} + minW='auto' + h='auto' + px={2} + aria-label={isFiat ? 'Switch to crypto amount' : 'Switch to fiat amount'} + >src/assets/translations/en/main.json (1)
1449-1460: Minor copy: consider plural “addresses”.Change “No address found.” to “No addresses found.” for empty states. Non-blocking.
Based on learnings
src/components/Modals/Send/views/SendAmountDetails.tsx (1)
140-171: DRY up duplicated address validation.The validateAddress logic duplicates Address.tsx. Extract to a shared hook/helper (e.g., useValidateSendAddress) to keep behavior in one place.
src/components/Modals/Send/AddressBook/AddressBook.tsx (2)
60-63: Normalize EVM address comparison.Use case-insensitive comparison for EVM addresses to avoid false negatives when matching the input against entries.
- return addressBookEntries?.find(entry => entry.isExternal and entry.address === input) + return addressBookEntries?.find(entry => + entry.isExternal && entry.address.toLowerCase() === input.toLowerCase() + )
146-153: Clear delete selection on modal close.Avoid retaining stale selectedDeleteEntry after closing ConfirmDelete.
- {selectedDeleteEntry && ( + {selectedDeleteEntry && ( <ConfirmDelete isOpen={isOpen} - onDelete={handleDelete(selectedDeleteEntry)} - onClose={onClose} + onDelete={handleDelete(selectedDeleteEntry)} + onClose={() => { setSelectedDeleteEntry(null); onClose() }} entryName={selectedDeleteEntry.label} /> )}src/components/Modals/Send/views/Address.tsx (1)
140-176: Deduplicate address validation logic.Same validateAddress implementation exists in SendAmountDetails.tsx. Extract to a shared utility/hook to avoid divergence.
src/components/Modals/Send/AddressInput/AddressInput.tsx (2)
83-88: Replace unsafe cast with typed useWatch.Avoid
as Partial<SendInput>. Watch specific fields with generics for type safety.- const { vanityAddress, input: value, to, assetId } = useWatch() as Partial<SendInput> + const vanityAddress = useWatch<SendInput, SendFormFields.VanityAddress>({ name: SendFormFields.VanityAddress }) + const value = useWatch<SendInput, SendFormFields.Input>({ name: SendFormFields.Input }) + const to = useWatch<SendInput, SendFormFields.To>({ name: SendFormFields.To }) + const assetId = useWatch<SendInput, SendFormFields.AssetId>({ name: SendFormFields.AssetId })As per coding guidelines.
46-51: Remove or use the unused ‘pe’ prop.
peis declared but not used. Either forward it to a container (e.g., InputGroup) or drop it.- pe?: SpaceProps['pe']
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (15)
.env.development(1 hunks)src/assets/translations/en/main.json(3 hunks)src/components/CryptoFiatInput/CryptoFiatInput.tsx(1 hunks)src/components/Modals/QrCode/QrCode.tsx(1 hunks)src/components/Modals/Send/AddressBook/AddressBook.tsx(1 hunks)src/components/Modals/Send/AddressBookSaveModal.tsx(1 hunks)src/components/Modals/Send/AddressInput/AddressInput.tsx(3 hunks)src/components/Modals/Send/Form.tsx(2 hunks)src/components/Modals/Send/views/Address.tsx(9 hunks)src/components/Modals/Send/views/SendAmountDetails.tsx(4 hunks)src/config.ts(1 hunks)src/state/migrations/index.ts(2 hunks)src/state/slices/common-selectors.ts(1 hunks)src/state/slices/preferencesSlice/preferencesSlice.ts(2 hunks)src/test/mocks/store.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/Modals/Send/AddressBookSaveModal.tsx
- src/state/slices/common-selectors.ts
- src/test/mocks/store.ts
- src/state/migrations/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/Modals/Send/Form.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/config.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/CryptoFiatInput/CryptoFiatInput.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Modals/Send/Form.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/CryptoFiatInput/CryptoFiatInput.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Modals/Send/Form.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/assets/translations/en/main.jsonsrc/config.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/CryptoFiatInput/CryptoFiatInput.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Modals/Send/Form.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/CryptoFiatInput/CryptoFiatInput.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/Modals/Send/Form.tsxsrc/components/Modals/QrCode/QrCode.tsxsrc/state/slices/preferencesSlice/preferencesSlice.tssrc/components/Modals/Send/views/Address.tsxsrc/components/Modals/Send/AddressBook/AddressBook.tsxsrc/config.tssrc/components/Modals/Send/AddressInput/AddressInput.tsxsrc/components/CryptoFiatInput/CryptoFiatInput.tsxsrc/components/Modals/Send/views/SendAmountDetails.tsx
🧠 Learnings (12)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
PR: shapeshift/web#10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
PR: shapeshift/web#10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/Modals/Send/views/SendAmountDetails.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
.env.development
📚 Learning: 2025-08-27T09:47:06.275Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Applied to files:
.env.development
📚 Learning: 2025-08-22T13:00:44.879Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Applied to files:
.env.development
📚 Learning: 2025-08-08T11:41:22.794Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Applied to files:
.env.development
📚 Learning: 2025-08-25T12:59:43.842Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/Fox/components/RFOXSection.tsx:234-248
Timestamp: 2025-08-25T12:59:43.842Z
Learning: NeOMakinG considers suggestions to refactor working modal state management from useState to useDisclosure as too nitpicky, preferring to keep such cosmetic code improvements out of feature PRs unless they address actual issues.
Applied to files:
.env.development
📚 Learning: 2025-08-08T15:00:22.321Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
Applied to files:
.env.development
📚 Learning: 2025-08-27T09:51:36.901Z
Learnt from: NeOMakinG
PR: shapeshift/web#10376
File: packages/contracts/src/viemClient.ts:39-40
Timestamp: 2025-08-27T09:51:36.901Z
Learning: NeOMakinG confirmed that issues flagged by CodeRabbit that predate the PR changes and are unrelated to the PR's core objective should be addressed separately, not within the current PR scope.
Applied to files:
.env.development
📚 Learning: 2025-09-02T12:38:46.940Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
Applied to files:
.env.development
📚 Learning: 2025-09-17T22:40:30.149Z
Learnt from: gomesalexandre
PR: shapeshift/web#10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Applied to files:
.env.development
🧬 Code graph analysis (6)
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
src/config.ts (1)
getConfig(210-212)
src/components/Modals/Send/views/Address.tsx (4)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectIsAddressInAddressBook(52-82)selectInternalAccountIdByAddress(89-122)packages/caip/src/constants.ts (1)
ethChainId(60-60)src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
AddressBook(34-156)
src/components/Modals/Send/AddressBook/AddressBook.tsx (7)
src/state/store.ts (2)
useAppDispatch(151-151)useAppSelector(143-143)src/state/slices/addressBookSlice/types.ts (1)
AddressBookEntry(3-17)src/state/slices/addressBookSlice/selectors.ts (2)
selectAddressBookEntriesByChainId(20-28)selectAddressBookEntriesBySearchQuery(30-46)src/state/slices/addressBookSlice/addressBookSlice.ts (1)
addressBookSlice(28-97)src/components/Text/Text.tsx (1)
Text(19-83)src/components/Modals/Send/AddressBook/AddressBookEntryButton.tsx (1)
AddressBookEntryButton(85-85)src/components/Modals/Send/AddressBook/ConfirmDelete.tsx (1)
ConfirmDelete(19-51)
src/components/Modals/Send/AddressInput/AddressInput.tsx (8)
src/components/Modals/Send/Form.tsx (1)
SendInput(50-68)src/hooks/useModal/useModal.tsx (1)
useModal(6-24)src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/assetsSlice/selectors.ts (1)
selectAssetById(16-20)src/state/slices/addressBookSlice/selectors.ts (2)
selectExternalAddressBookEntryByAddress(128-143)selectInternalAccountIdByAddress(89-122)src/state/slices/portfolioSlice/selectors.ts (1)
selectPortfolioAccountMetadata(109-117)src/lib/utils/utxo.ts (1)
isUtxoAccountId(24-25)src/state/slices/portfolioSlice/utils/index.ts (1)
accountIdToLabel(80-119)
src/components/CryptoFiatInput/CryptoFiatInput.tsx (2)
src/components/Text/Text.tsx (1)
Text(19-83)src/components/Amount/Amount.tsx (1)
Amount(21-42)
src/components/Modals/Send/views/SendAmountDetails.tsx (1)
src/components/Modals/Send/AddressInput/AddressInput.tsx (1)
AddressInput(61-390)
🪛 dotenv-linter (4.0.0)
.env.development
[warning] 9-9: [UnorderedKey] The VITE_FEATURE_ADDRESS_BOOK key should go before the VITE_FEATURE_THORCHAIN_TCY_ACTIVITY key
(UnorderedKey)
⏰ 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: Call / Static
🔇 Additional comments (7)
src/components/Modals/Send/Form.tsx (2)
215-236: LGTM! Navigation flow simplified correctly.The removal of viewport-based conditional navigation in favor of always routing through
SendRoutes.Addressaligns with the PR objectives to unify desktop and mobile send flows. The dependency array has been correctly updated to removeisSmallerThanMd, and the use ofrequestAnimationFrameensures navigation occurs after state updates complete.
248-298: LGTM! QR success handler navigation simplified correctly.The navigation after successful QR code scanning now consistently routes to
SendRoutes.Addressregardless of viewport size, which aligns with the unified navigation flow. The dependency array has been correctly updated to removeisSmallerThanMd.src/components/Modals/QrCode/QrCode.tsx (2)
4-4: LGTM! Routing simplification is clean.The import of
initialEntriesfromSendCommonsuccessfully removes the responsive routing complexity while maintaining functionality. This aligns with the PR's goal of unifying the routing approach.
21-22: No defensive refactor needed—SendRoutes.Scan is guaranteed to exist in initialEntries.SendRoutes.Scan is statically defined at index 2 in the initialEntries array, so
indexOf()will always return a valid index. The code is correct as written.src/assets/translations/en/main.json (1)
29-31: LGTM — common actions added.The new common.save/common.delete keys align with new UI.
src/state/slices/preferencesSlice/preferencesSlice.ts (1)
95-96: Feature flag wiring looks good.FeatureFlags.AddressBook and initial state sourcing from config are correct.
Also applies to: 221-222
src/config.ts (1)
197-198: Config validator added correctly.VITE_FEATURE_ADDRESS_BOOK boolean with default false is properly validated.
gomesalexandre
left a comment
gomesalexandre
left a comment
There was a problem hiding this comment.
Tested locally, see below - generally super happy with this however, just a bunch of smol things that make the experience meh (some of them to be fixed here, some of them TBD as follow-ups, and many which are really product oversights tbh, not really a you thing ser!)
https://jam.dev/c/b23464db-c9a0-4893-b01f-38f699570a2b
Going to stamp for the sake of progression since this is a large boi already.
Feel free to tackle whatever from the reviewsies at your convenience, then she's mergy /gnomes
- First time pasting an "internal" addy, it's displayed as "..." for a few secs which seems borked
https://jam.dev/c/201ddea7-a8e0-4859-8b3f-1ff62abe1fa7
- As previously mentioned, address book should merely be a mapping of CAIP-10 to label - as such, feels a bit weird I can see this guy labeled as "Account #0", but not rename it - feature would feel more complete if we already allowed renaming portfolio accounts, including in wallet page but happy to do this in a follow-up obviously!
i.e here, should really be quite an easy one to follow-up with making this guy a rename-able text field
- Feels weird having to click out of input for ENS, feels like it should just automagically work without me having to click out?
https://jam.dev/c/3aafb9a6-843a-461a-84be-36b75a631e89
- Feels like ENS should be pre-populated when saving?
https://jam.dev/c/68f3632e-69be-4929-bb03-215dc251632e
Also, feels weird to see ENS on initial new but not after clicking Save
- Can't see my portfolio addresses suggested anywhere here for cross-addies sends, assuming this is for a follow-up?
- Obviously not for this PR and not a you issue - from a product perspective just feels super hacked, like the data is here but isn't presented in the right/correct way. Not really sure I have enough brain cells to propose a better solution, but failed the jetlag litmus test fosho, doesn't feel nice or intuitive at all
- This guy would probably benefit from relevant chain icon or at the very least somin like "Account #0 - THORChain"
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/components/Modals/Send/views/Address.tsx (1)
88-102: Optional: Combine duplicate filter objects.Both
isInAddressBookFilterandinternalAccountIdFiltercompute the same object. You can extract it to a single memoized value to reduce duplication:+ const addressLookupFilter = useMemo( + () => ({ accountAddress: address, chainId: asset?.chainId }), + [address, asset?.chainId], + ) - const isInAddressBookFilter = useMemo( - () => ({ accountAddress: address, chainId: asset?.chainId }), - [address, asset?.chainId], - ) const isInAddressBook = useAppSelector(state => - selectIsAddressInAddressBook(state, isInAddressBookFilter), + selectIsAddressInAddressBook(state, addressLookupFilter), ) - const internalAccountIdFilter = useMemo( - () => ({ accountAddress: address, chainId: asset?.chainId }), - [address, asset?.chainId], - ) const internalAccountId = useAppSelector(state => - selectInternalAccountIdByAddress(state, internalAccountIdFilter), + selectInternalAccountIdByAddress(state, addressLookupFilter), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/components/Modals/Send/views/Address.tsx(9 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.{ts,tsx}: ALWAYS use Result<T, E> pattern for error handling in swappers and APIs
ALWAYS use Ok() and Err() from @sniptt/monads for monadic error handling
ALWAYS use custom error classes from @shapeshiftoss/errors
ALWAYS provide meaningful error codes for internationalization
ALWAYS include relevant details in error objects
ALWAYS wrap async operations in try-catch blocks
ALWAYS use AsyncResultOf utility for converting promises to Results
ALWAYS provide fallback error handling
ALWAYS use timeoutMonadic for API calls
ALWAYS provide appropriate timeout values for API calls
ALWAYS handle timeout errors gracefully
ALWAYS validate inputs before processing
ALWAYS provide clear validation error messages
ALWAYS use early returns for validation failures
ALWAYS log errors for debugging
ALWAYS use structured logging for errors
ALWAYS include relevant context in error logs
Throwing errors instead of using monadic patterns is an anti-pattern
Missing try-catch blocks for async operations is an anti-pattern
Generic error messages without context are an anti-pattern
Not handling specific error types is an anti-pattern
Missing timeout handling is an anti-pattern
No input validation is an anti-pattern
Poor error logging is an anti-pattern
Using any for error types is an anti-pattern
Missing error codes for internationalization is an anti-pattern
No fallback error handling is an anti-pattern
Console.error without structured logging is an anti-pattern
**/*.{ts,tsx}: ALWAYS use camelCase for variables, functions, and methods
ALWAYS use descriptive names that explain the purpose for variables and functions
ALWAYS use verb prefixes for functions that perform actions
ALWAYS use PascalCase for types, interfaces, and enums
ALWAYS use descriptive names that indicate the structure for types, interfaces, and enums
ALWAYS use suffixes like Props, State, Config, Type when appropriate for types and interfaces
ALWAYS use UPPER_SNAKE_CASE for constants and configuration values
ALWAYS use d...
Files:
src/components/Modals/Send/views/Address.tsx
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/error-handling.mdc)
**/*.tsx: ALWAYS wrap components in error boundaries
ALWAYS provide user-friendly fallback components in error boundaries
ALWAYS log errors for debugging in error boundaries
ALWAYS use useErrorToast hook for displaying errors
ALWAYS provide translated error messages in error toasts
ALWAYS handle different error types appropriately in error toasts
Missing error boundaries in React components is an anti-pattern
**/*.tsx: ALWAYS use PascalCase for React component names
ALWAYS use descriptive names that indicate the component's purpose
ALWAYS match the component name to the file name
Flag components without PascalCase
Flag default exports for components
Files:
src/components/Modals/Send/views/Address.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Modals/Send/views/Address.tsx
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
**/*.{tsx,jsx}: ALWAYS useuseMemofor expensive computations, object/array creations, and filtered data
ALWAYS useuseMemofor derived values and computed properties
ALWAYS useuseMemofor conditional values and simple transformations
ALWAYS useuseCallbackfor event handlers and functions passed as props
ALWAYS useuseCallbackfor any function that could be passed as a prop or dependency
ALWAYS include all dependencies inuseEffect,useMemo,useCallbackdependency arrays
NEVER use// eslint-disable-next-line react-hooks/exhaustive-depsunless absolutely necessary
ALWAYS explain why dependencies are excluded if using eslint disable
ALWAYS use named exports for components
NEVER use default exports for components
KEEP component files under 200 lines when possible
BREAK DOWN large components into smaller, reusable pieces
EXTRACT complex logic into custom hooks
USE local state for component-level state
LIFT state up when needed across multiple components
USE Context for avoiding prop drilling
ALWAYS wrap components in error boundaries for production
ALWAYS handle async errors properly
ALWAYS provide user-friendly error messages
ALWAYS use virtualization for lists with 100+ items
ALWAYS implement proper key props for list items
ALWAYS lazy load heavy components
ALWAYS use React.lazy for code splitting
Components receiving props are wrapped withmemo
Files:
src/components/Modals/Send/views/Address.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/react-best-practices.mdc)
USE Redux only for global state shared across multiple places
Files:
src/components/Modals/Send/views/Address.tsx
🧠 Learnings (38)
📓 Common learnings
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Learnt from: premiumjibles
PR: shapeshift/web#10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Learnt from: NeOMakinG
PR: shapeshift/web#10128
File: .cursor/rules/error-handling.mdc:266-274
Timestamp: 2025-07-29T10:35:22.059Z
Learning: NeOMakinG prefers less nitpicky suggestions on documentation and best practices files, finding overly detailed suggestions on minor implementation details (like console.error vs logger.error) too granular for cursor rules documentation.
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:42-86
Timestamp: 2025-08-08T11:41:22.794Z
Learning: NeOMakinG prefers not to include refactors in move-only PRs; such suggestions should be deferred to follow-up issues instead of being applied within the same PR.
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
📚 Learning: 2025-10-21T17:11:18.087Z
Learnt from: gomesalexandre
PR: shapeshift/web#10871
File: src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx:426-428
Timestamp: 2025-10-21T17:11:18.087Z
Learning: In src/components/Modals/Send/hooks/useSendDetails/useSendDetails.tsx, within the handleInputChange function, use .toFixed() without arguments (not .toString()) when converting BigNumber amounts for input field synchronization. This avoids exponential notation in the input while preserving precision for presentational components like <Amount.Crypto /> and <Amount.Fiat /> to format appropriately.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-29T07:07:49.332Z
Learnt from: premiumjibles
PR: shapeshift/web#10386
File: src/components/MultiHopTrade/components/VerifyAddresses/VerifyAddresses.tsx:272-294
Timestamp: 2025-08-29T07:07:49.332Z
Learning: In UTXO sell address verification flow in VerifyAddresses.tsx, the user wants address verification to be marked as "verified/complete" before starting the change address fetch, not after. The verification step and change address fetch should be treated as separate sequential operations in the UI flow.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-08T15:00:49.887Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/AssetSearch/components/AssetList.tsx:2-2
Timestamp: 2025-08-08T15:00:49.887Z
Learning: Project shapeshift/web: NeOMakinG prefers avoiding minor a11y/UI nitpicks (e.g., adding aria-hidden to decorative icons in empty states like src/components/AssetSearch/components/AssetList.tsx) within feature PRs; defer such suggestions to a follow-up instead of blocking the PR.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : ALWAYS validate inputs before processing
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-12T10:44:46.723Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/content/SendTransactionContent.tsx:0-0
Timestamp: 2025-09-12T10:44:46.723Z
Learning: gomesalexandre dismissed a clipboard error handling suggestion in PR #10461 for SendTransactionContent.tsx, demonstrating that the current navigator.clipboard.writeText implementation works as expected and preferring to keep it simple without additional try/catch error handling.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-03T22:09:37.542Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/error-handling.mdc:0-0
Timestamp: 2025-08-03T22:09:37.542Z
Learning: Applies to **/*.{ts,tsx} : No input validation is an anti-pattern
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-08T11:41:36.971Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/hooks/useGetTradeQuotes/hooks/useTrackTradeQuotes.ts:88-109
Timestamp: 2025-08-08T11:41:36.971Z
Learning: In MultiHopTrade Confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx), the Confirm route does not remount; navigating away goes to the swapper input page. Therefore, persistent deduplication across remounts for quote tracking is unnecessary; a ref-based single-mount dedupe is sufficient.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-08T15:53:09.362Z
Learnt from: gomesalexandre
PR: shapeshift/web#10442
File: src/components/TradeAssetSearch/components/GroupedAssetList/GroupedAssetList.tsx:34-35
Timestamp: 2025-09-08T15:53:09.362Z
Learning: In DefaultAssetList.tsx, the GroupedAssetList component already receives the activeChainId prop correctly on line ~58, contrary to automated analysis that may flag it as missing.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts expects a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter should have an assetId property. This pattern is consistent across portfolio selectors that use selectAssetIdParamFromFilter. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-07-24T09:43:11.699Z
Learnt from: CR
PR: shapeshift/web#0
File: .cursor/rules/swapper.mdc:0-0
Timestamp: 2025-07-24T09:43:11.699Z
Learning: Applies to packages/swapper/src/swappers/*/{*.ts,endpoints.ts} : Verify chain ID filtering in filterAssetIdsBySellable and filterBuyAssetsBySellAssetId methods.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-10T21:09:25.643Z
Learnt from: premiumjibles
PR: shapeshift/web#10215
File: src/components/MultiHopTrade/hooks/useGetTradeRateInput.ts:65-67
Timestamp: 2025-08-10T21:09:25.643Z
Learning: In the MultiHopTrade components, `selectInputBuyAsset` and `selectInputSellAsset` selectors from `tradeInputSlice` always return defined values because they have default values in the initial state (BTC for buyAsset, ETH for sellAsset, with fallback to defaultAsset). Null checks for these assets are unnecessary when using these selectors.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property, not a raw AssetId string. The selector signature is (state: ReduxState, filter) where filter is expected to have an assetId property. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-15T07:51:16.374Z
Learnt from: gomesalexandre
PR: shapeshift/web#10278
File: src/components/AssetHeader/hooks/useQuickBuy.ts:97-99
Timestamp: 2025-08-15T07:51:16.374Z
Learning: The selectPortfolioUserCurrencyBalanceByAssetId selector in src/state/slices/portfolioSlice/selectors.ts accepts a filter object with an assetId property (signature: (state, { assetId })), not a raw AssetId string. Passing a filter object like { assetId: someAssetId } is the correct usage pattern.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-22T12:58:36.070Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx:33-55
Timestamp: 2025-08-22T12:58:36.070Z
Learning: In RewardDistributionNotification component (src/components/Layout/Header/ActionCenter/components/Notifications/RewardDistributionNotification.tsx), NeOMakinG confirmed that runeAsset is expected to always be defined when the component renders, so defensive guards against undefined runeAsset are not needed.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-26T19:04:38.672Z
Learnt from: kaladinlight
PR: shapeshift/web#10369
File: packages/chain-adapters/src/cosmossdk/CosmosSdkBaseAdapter.ts:167-176
Timestamp: 2025-08-26T19:04:38.672Z
Learning: In packages/chain-adapters/src/cosmossdk/CosmosSdkBaseAdapter.ts, when processing assets from data.assets.reduce(), the team prefers using empty catch blocks to gracefully skip any assets that fail processing, rather than specific error type handling, to avoid useless noise and ensure robust asset filtering.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-04T17:29:59.479Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx:28-33
Timestamp: 2025-09-04T17:29:59.479Z
Learning: In shapeshift/web, the useGetPopularAssetsQuery function in src/components/TradeAssetSearch/hooks/useGetPopularAssetsQuery.tsx intentionally uses primaryAssets[assetId] instead of falling back to assets[assetId]. The design distributes primary assets across chains by iterating through their related assets and adding the primary asset to each related asset's chain. This ensures primary assets appear in all chains where they have related assets, supporting the grouped asset system.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-10-23T14:27:19.073Z
Learnt from: gomesalexandre
PR: shapeshift/web#10857
File: src/plugins/walletConnectToDapps/eventsManager/useWalletConnectEventsHandler.ts:101-104
Timestamp: 2025-10-23T14:27:19.073Z
Learning: In WalletConnect wallet_switchEthereumChain and wallet_addEthereumChain requests, the chainId parameter is always present as per the protocol spec. Type guards checking for missing chainId in these handlers (like `if (!evmNetworkIdHex) return`) are solely for TypeScript compiler satisfaction, not real runtime edge cases.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-27T09:51:36.901Z
Learnt from: NeOMakinG
PR: shapeshift/web#10376
File: packages/contracts/src/viemClient.ts:39-40
Timestamp: 2025-08-27T09:51:36.901Z
Learning: NeOMakinG confirmed that issues flagged by CodeRabbit that predate the PR changes and are unrelated to the PR's core objective should be addressed separately, not within the current PR scope.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-27T09:47:06.275Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/ButtonWalletPredicate/ButtonWalletPredicate.tsx:7-7
Timestamp: 2025-08-27T09:47:06.275Z
Learning: In shapeshift/web project, NeOMakinG consistently prefers to defer UI/UX improvements and refactoring work (like the Drawer.Close hack fix in ButtonWalletPredicate.tsx) to follow-up PRs rather than expanding the scope of feature PRs, even when the improvements would enhance robustness.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-17T22:40:30.149Z
Learnt from: gomesalexandre
PR: shapeshift/web#10569
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/WalletConnectModalSigningFooter.tsx:121-129
Timestamp: 2025-09-17T22:40:30.149Z
Learning: gomesalexandre maintains strict scope discipline even for style/UI PRs in shapeshift/web, declining functionally correct UX improvements (like keeping Cancel button enabled during gas simulation loading) when they fall outside the PR's stated styling objectives, demonstrating his consistent pattern of deferring valid but tangential improvements to separate efforts.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-25T12:59:43.842Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/Fox/components/RFOXSection.tsx:234-248
Timestamp: 2025-08-25T12:59:43.842Z
Learning: NeOMakinG considers suggestions to refactor working modal state management from useState to useDisclosure as too nitpicky, preferring to keep such cosmetic code improvements out of feature PRs unless they address actual issues.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-22T13:00:44.879Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/pages/RFOX/components/Stake/components/StakeSummary.tsx:112-114
Timestamp: 2025-08-22T13:00:44.879Z
Learning: NeOMakinG prefers to keep PR changes minimal and focused on the core objectives, avoiding cosmetic or defensive code improvements that aren't directly related to the PR scope, even when they would improve robustness.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-02T12:38:46.940Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/components/Table/Table.theme.ts:177-180
Timestamp: 2025-09-02T12:38:46.940Z
Learning: NeOMakinG prefers to defer technical debt and CSS correctness issues (like improper hover selectors) to follow-up PRs when the current PR is already large and focused on major feature implementation, even when the issues are valid from a usability/technical perspective.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-08T15:00:22.321Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/components/MultiHopTrade/components/TradeInput/components/HighlightedTokens.tsx:14-14
Timestamp: 2025-08-08T15:00:22.321Z
Learning: In shapeshift/web reviews for NeOMakinG, avoid nitpicks to change deep-relative imports to '@/…' alias paths within feature/non-refactor PRs; defer such style-only changes to a dedicated follow-up refactor unless they fix an issue.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-02T08:34:08.157Z
Learnt from: NeOMakinG
PR: shapeshift/web#10380
File: src/pages/Dashboard/components/AccountList/AccountTable.tsx:60-0
Timestamp: 2025-09-02T08:34:08.157Z
Learning: NeOMakinG prefers code review comments to focus only on actual PR changes, not pre-existing code issues, unless there are critical security or correctness concerns directly related to the new functionality.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-22T12:58:26.590Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx:108-111
Timestamp: 2025-08-22T12:58:26.590Z
Learning: In the RFOX GenericTransactionDisplayType flow in src/components/Layout/Header/ActionCenter/components/GenericTransactionActionCard.tsx, the txHash is always guaranteed to be present according to NeOMakinG, so defensive null checks for txLink are not needed in this context.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-13T16:45:17.166Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/components/WalletConnectSigningModal/StructuredMessage/StructuredMessage.tsx:0-0
Timestamp: 2025-09-13T16:45:17.166Z
Learning: gomesalexandre appreciates safety-focused suggestions for UI rendering in WalletConnect components, specifically defensive programming approaches that prevent null/undefined values from displaying as literal "null"/"undefined" strings in the user interface.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-25T23:32:13.876Z
Learnt from: premiumjibles
PR: shapeshift/web#10361
File: src/pages/Markets/components/CardWithSparkline.tsx:83-92
Timestamp: 2025-08-25T23:32:13.876Z
Learning: In shapeshift/web PR #10361, premiumjibles considered the nested button accessibility issue (ChartErrorFallback retry Button inside Card rendered as Button in CardWithSparkline.tsx) out of scope for the error boundaries feature PR, consistent with deferring minor a11y improvements to follow-up PRs.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-12T11:56:19.437Z
Learnt from: gomesalexandre
PR: shapeshift/web#10461
File: src/plugins/walletConnectToDapps/utils/tenderly/index.ts:0-0
Timestamp: 2025-09-12T11:56:19.437Z
Learning: gomesalexandre rejected verbose try/catch error handling for address validation in Tenderly integration (PR #10461), calling the approach "ugly" but still implemented safety measures in commit ad7e424b89, preferring cleaner safety implementations over defensive programming patterns.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-10T15:35:46.223Z
Learnt from: gomesalexandre
PR: shapeshift/web#10458
File: src/plugins/walletConnectToDapps/components/modals/EIP155SignTypedDataConfirmation.tsx:55-55
Timestamp: 2025-09-10T15:35:46.223Z
Learning: gomesalexandre prefers fail-fast early returns over graceful degradation when critical data is missing in WalletConnect flows (like peer metadata in EIP155SignTypedDataConfirmation.tsx). He favors "safety first, always double-wrap" approach and believes missing peer metadata indicates bigger problems that should be surfaced explicitly rather than masked with partial UI rendering.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-02T00:02:44.889Z
Learnt from: premiumjibles
PR: shapeshift/web#10380
File: src/components/AssetSearch/components/GroupedAssetRow.tsx:116-0
Timestamp: 2025-09-02T00:02:44.889Z
Learning: premiumjibles prefers avoiding store.getState() calls in favor of useAppSelector where reactivity is needed, to prevent accidentally breaking reactive updates in components.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-10-17T07:51:58.374Z
Learnt from: NeOMakinG
PR: shapeshift/web#10783
File: src/context/WalletProvider/NewWalletViews/NewWalletViewsSwitch.tsx:344-349
Timestamp: 2025-10-17T07:51:58.374Z
Learning: In the shapeshift/web codebase, Chakra UI's ModalContent component supports the containerProps prop. When using the useModalRegistration hook from ModalStackProvider, spreading {...modalContentProps} directly onto ModalContent is correct and properly applies z-index and pointer-events through containerProps.sx. Do not suggest extracting sx from modalContentProps.containerProps.sx.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-10-07T03:44:27.350Z
Learnt from: 0xApotheosis
PR: shapeshift/web#10760
File: src/components/ManageHiddenAssets/ManageHiddenAssetsList.tsx:78-84
Timestamp: 2025-10-07T03:44:27.350Z
Learning: In the ShapeShift web codebase, the following are stable references and do not need to be included in useCallback/useMemo dependency arrays:
- `navigate` from `useBrowserRouter()` hook
- Modal control objects (like `walletDrawer`) from `useModal()` hook (including their `isOpen`, `close`, and `open` methods)
- These are backed by stable context providers
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-09-04T10:18:34.140Z
Learnt from: gomesalexandre
PR: shapeshift/web#10427
File: src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx:40-40
Timestamp: 2025-09-04T10:18:34.140Z
Learning: In the shapeshift/web codebase, src/state/slices/selectors.ts uses wildcard exports (`export * from './[sliceName]/selectors'`) to re-export all selectors from individual slice selector files, making them available through the barrel import. This means selectors like selectTxByFilter from txHistorySlice/selectors are properly accessible via '@/state/slices/selectors' even though they don't appear in explicit named exports.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-05T22:41:35.473Z
Learnt from: premiumjibles
PR: shapeshift/web#10187
File: src/pages/Assets/Asset.tsx:1-1
Timestamp: 2025-08-05T22:41:35.473Z
Learning: In the shapeshift/web codebase, component imports use direct file paths like '@/components/ComponentName/ComponentName' rather than barrel exports. The AssetAccountDetails component should be imported as '@/components/AssetAccountDetails/AssetAccountDetails', not from a directory index.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-14T17:54:32.563Z
Learnt from: gomesalexandre
PR: shapeshift/web#10276
File: src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx:97-108
Timestamp: 2025-08-14T17:54:32.563Z
Learning: In ReusableLpStatus component (src/pages/ThorChainLP/components/ReusableLpStatus/ReusableLpStatus.tsx), the txAssets dependency is stable from first render because poolAsset, baseAsset, actionSide, and action are all defined first render, making the current txAssetsStatuses initialization pattern safe without needing useEffect synchronization.
Applied to files:
src/components/Modals/Send/views/Address.tsx
📚 Learning: 2025-08-27T09:47:59.394Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Modal/components/DialogCloseButton.tsx:29-37
Timestamp: 2025-08-27T09:47:59.394Z
Learning: In shapeshift/web, for DialogCloseButton.tsx, the onClick handler must remain on Drawer.Close (not moved to IconButton with asChild) because stopPropagation() needs to be called at the Drawer.Close level to properly handle drawer closing behavior; moving event handling to the child IconButton would break the event flow that Drawer.Close expects.
Applied to files:
src/components/Modals/Send/views/Address.tsx
🧬 Code graph analysis (1)
src/components/Modals/Send/views/Address.tsx (4)
src/state/store.ts (1)
useAppSelector(143-143)src/state/slices/addressBookSlice/selectors.ts (2)
selectIsAddressInAddressBook(64-97)selectInternalAccountIdByAddress(104-131)packages/caip/src/constants.ts (1)
ethChainId(60-60)src/components/Modals/Send/AddressBook/AddressBook.tsx (1)
AddressBook(34-154)
⏰ 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: Install and Cache
🔇 Additional comments (4)
src/components/Modals/Send/views/Address.tsx (4)
107-111: LGTM: Save button logic correctly prevents saving internal accounts.The boolean conditions properly gate the save button to only show for:
- Addresses not already in the address book
- External addresses (not internal accounts)
- Valid addresses without errors
- When the feature flag is enabled
190-194: LGTM: Properly clears all address-related fields.The callback correctly resets
Input,To, andVanityAddresswhen an address book entry is cleared, ensuring form state remains consistent.
214-217: LGTM: AddressInput integration props are well-structured.The new props correctly wire up the address book functionality:
resolvedAddressprovides the validated address for displaychainIdscopes the address book to the asset's chainonEmptiedhandles clearing when the user deselects an entryshouldShowSaveButtoncontrols the save CTA visibility
242-244: LGTM: AddressBook feature properly gated and wired.The AddressBook component is correctly:
- Feature-flagged to enable gradual rollout
- Given the asset's chainId for namespace-scoped filtering
- Wired to populate the input and navigate on selection
2 participants
Description
This PR adds the address book into the send flow:
Issue (if applicable)
closes #10694
Risk
Low, new feature behind a flag
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
New Features
UI
Chores