Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThis pull request introduces several changes across various components and files. Notably, it adds a new environment variable Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🧹 Outside diff range and nitpick comments (33)
app/positions/components/onboarding/SmartOnboarding.tsx (2)
6-6: Add type safety for step valuesDefine an enum or union type for possible step values to prevent typos and improve maintainability.
+type OnboardingStep = 'asset-selection' | 'risk-selection'; + function OnboardingContent() { - const { step } = useOnboarding(); + const { step } = useOnboarding<OnboardingStep>();Also applies to: 10-11
8-13: Add error state handlingConsider adding a fallback UI for unknown step values.
return ( <div className="w-full"> {step === 'asset-selection' && <AssetSelection />} {step === 'risk-selection' && <RiskSelection />} + {!['asset-selection', 'risk-selection'].includes(step) && ( + <div>Invalid step</div> + )} </div> );app/positions/components/onboarding/types.ts (1)
3-14: Consider using more specific types for numeric fieldsThe
decimalsandnetworkfields could benefit from more precise types:
decimalsis typically 0-18networkshould match specific chain IDs- decimals: number; - network: number; + decimals: 0 | 6 | 8 | 18; + network: 1 | 137 | 42161; // mainnet | polygon | arbitrumapp/positions/onboarding/page.tsx (2)
11-14: Add type safety for step valuesDefine an enum or const object for steps to avoid typos and improve maintainability.
+const ONBOARDING_STEPS = { + ASSET_SELECTION: 'asset-selection', + RISK_SELECTION: 'risk-selection', + SETUP: 'setup', + SUCCESS: 'success', +} as const; + +type OnboardingStep = typeof ONBOARDING_STEPS[keyof typeof ONBOARDING_STEPS]; + export default function OnboardingPage() { const searchParams = useSearchParams(); - const step = searchParams.get('step') ?? 'asset-selection'; + const step = (searchParams.get('step') ?? ONBOARDING_STEPS.ASSET_SELECTION) as OnboardingStep;
30-39: Add semantic HTML and ARIA labelsImprove accessibility by adding proper landmarks and labels.
return ( <div className="flex min-h-screen flex-col font-zen"> <Header /> - <div className="mx-auto flex w-full max-w-7xl flex-1 flex-col px-4 py-8"> + <main className="mx-auto flex w-full max-w-7xl flex-1 flex-col px-4 py-8" aria-label="Onboarding Process"> <OnboardingProvider> <div className="flex min-h-0 flex-1 flex-col">{renderStep()}</div> </OnboardingProvider> - </div> + </main> </div> );app/positions/components/onboarding/OnboardingContext.tsx (3)
6-15: Add step transition validationConsider defining valid step transitions to prevent jumping to arbitrary steps.
type OnboardingStep = 'asset-selection' | 'risk-selection' | 'setup'; + +const VALID_TRANSITIONS: Record<OnboardingStep, OnboardingStep[]> = { + 'asset-selection': ['risk-selection'], + 'risk-selection': ['asset-selection', 'setup'], + 'setup': ['risk-selection'] +};
27-31: Extract URL handlingMove URL manipulation to a utility function to handle future path changes.
+const ONBOARDING_BASE_PATH = '/positions/onboarding'; + +const updateStepInUrl = (params: URLSearchParams, step: OnboardingStep) => { + params.set('step', step); + return `${ONBOARDING_BASE_PATH}?${params.toString()}`; +}; + const setStep = (newStep: OnboardingStep) => { const params = new URLSearchParams(searchParams.toString()); - params.set('step', newStep); - router.push(`/positions/onboarding?${params.toString()}`); + router.push(updateStepInUrl(params, newStep)); };
49-55: Enhance error messageThe error message could guide developers better.
- throw new Error('useOnboarding must be used within an OnboardingProvider'); + throw new Error('useOnboarding must be used within an OnboardingProvider. Wrap your component tree with <OnboardingProvider> at the entry point of the onboarding flow.');app/positions/components/onboarding/SuccessPage.tsx (2)
29-31: Extract animation delays to constantsMove delays to constants for easier maintenance.
+const ANIMATION_DELAYS = { + ICON: 0.2, + HEADING: 0.4, + TEXT: 0.6, + BUTTON: 0.8 +};Also applies to: 37-39, 46-48, 55-57
60-60: Use NextUI's built-in radius propReplace className='rounded' with radius prop for consistency.
-<Button className='rounded' color="primary" +<Button radius="md" color="primary"app/settings/page.tsx (2)
33-40: Add link to Permit2 docsHelp users learn more about Permit2 by adding a link to its documentation.
<p className="text-sm text-secondary"> Enable signature-based token approvals using Permit2. This bundles approvals and - actions into a single transaction, saving gas. + actions into a single transaction, saving gas.{' '} + <a href="https://docs.uniswap.org/contracts/permit2/overview" className="text-primary hover:underline" target="_blank" rel="noopener noreferrer">Learn more</a> </p>
Line range hint
8-8: Add error handling for local storageLocal storage operations can fail. Add try-catch and fallback value.
-const [usePermit2, setUsePermit2] = useLocalStorage('usePermit2', true); +const [usePermit2, setUsePermit2] = useLocalStorage('usePermit2', true, { + onError: () => console.warn('Failed to access local storage, using default settings'), +}); const handlePermit2Toggle = useCallback( (checked: boolean) => { + try { setUsePermit2(checked); + } catch (error) { + console.warn('Failed to save settings:', error); + } }, [setUsePermit2], );Also applies to: 11-17
tailwind.config.ts (3)
5-5: Fix import orderMove type imports before regular imports.
-import type { Config } from 'tailwindcss'; -import plugin from 'tailwindcss/plugin'; +import plugin from 'tailwindcss/plugin'; +import type { Config } from 'tailwindcss';🧰 Tools
🪛 eslint
[error] 5-5:
tailwindcss/pluginimport should occur before type import oftailwindcss(import/order)
57-78: Simplify theme configurationBoth themes use identical radius values. Consider extracting to a shared object.
+const radiusConfig = { + small: '0.375rem', + medium: '0.375rem', + large: '0.375rem', +}; nextui({ themes: { light: { layout: { - radius: { - small: '0.375rem', - medium: '0.375rem', - large: '0.375rem', - }, + radius: radiusConfig, }, }, dark: { layout: { - radius: { - small: '0.375rem', - medium: '0.375rem', - large: '0.375rem', - }, + radius: radiusConfig, }, }, }, }),
79-85: Use arrow functionConvert to arrow function syntax for consistency.
-plugin(function({ addBase }) { +plugin(({ addBase }) => {🧰 Tools
🪛 eslint
[error] 79-85: Unexpected function expression.
(prefer-arrow-callback)
app/api/balances/route.ts (1)
30-42: Consider adding rate limitingAdd rate limiting to prevent API abuse.
Consider using a rate limiting middleware or implementing a simple in-memory rate limiter.
src/hooks/useUserBalances.ts (2)
5-19: Add JSDoc comments to typesDocument the types to help other developers understand the data structure.
+/** + * Represents a token balance with metadata + */ type TokenBalance = { address: string; balance: string; chainId: number; decimals: number; logoURI?: string; symbol: string; } +/** + * API response structure for token balances + */ type TokenResponse = { tokens: { address: string; balance: string; }[]; }
27-42: Add retry mechanism for failed requestsNetwork requests can fail temporarily. Consider adding a retry mechanism.
const fetchBalances = useCallback( async (chainId: number): Promise<TokenResponse['tokens']> => { + const MAX_RETRIES = 3; + let attempts = 0; + + while (attempts < MAX_RETRIES) { try { const response = await fetch(`/api/balances?address=${address}&chainId=${chainId}`); if (!response.ok) { throw new Error('Failed to fetch balances'); } const data = (await response.json()) as TokenResponse; return data.tokens; } catch (err) { console.error('Error fetching balances:', err); + attempts++; + if (attempts === MAX_RETRIES) { throw err instanceof Error ? err : new Error('Unknown error occurred'); + } + await new Promise(resolve => setTimeout(resolve, 1000 * attempts)); } + } + throw new Error('Max retries exceeded'); }, [address], );src/components/layout/header/Navbar.tsx (1)
121-121: Consider using dynamic positioningThe fixed
top-[72px]value might break if header height changes. Consider using dynamic positioning or CSS variables.-<div className="bg-surface absolute left-0 right-0 top-[72px] z-50 p-4 shadow-lg md:hidden"> +<div className="bg-surface absolute left-0 right-0 top-full z-50 p-4 shadow-lg md:hidden">app/markets/components/OracleFilter.tsx (2)
43-44: Add focus styles for keyboard navigationThe hover and transition styles look good, but consider adding explicit focus styles.
-className={`bg-surface min-w-48 cursor-pointer rounded p-2 shadow-sm transition-all duration-200 hover:bg-gray-200 dark:hover:bg-gray-700 ${ +className={`bg-surface min-w-48 cursor-pointer rounded p-2 shadow-sm transition-all duration-200 hover:bg-gray-200 focus:ring-2 focus:ring-primary dark:hover:bg-gray-700 ${
80-117: Consider adding loading state for imagesThe dropdown implementation is solid. Consider adding blur placeholder for oracle icons.
<Image src={OracleVendorIcons[oracle]} alt={oracle} width={16} height={16} + placeholder="blur" + blurDataURL="data:image/jpeg;base64,/9j/4AAQSkZJRg==" className="rounded-full" />app/positions/components/onboarding/AssetSelection.tsx (2)
38-71: Optimize market filtering and APY calculationsTwo potential improvements:
- Move toLowerCase() out of the filter loop
- Use reduce() instead of spread for min/max APY
balances.forEach((balance) => { + const balanceAddress = balance.address.toLowerCase(); const relevantMarkets = markets.filter( (market) => market.morphoBlue.chain.id === balance.chainId && - market.loanAsset.address.toLowerCase() === balance.address.toLowerCase(), + market.loanAsset.address.toLowerCase() === balanceAddress, ); if (relevantMarkets.length === 0) return; - const apys = relevantMarkets.map((market) => market.state.supplyApy); - const minApy = Math.min(...apys); - const maxApy = Math.max(...apys); + const { min: minApy, max: maxApy } = relevantMarkets.reduce( + (acc, market) => ({ + min: Math.min(acc.min, market.state.supplyApy), + max: Math.max(acc.max, market.state.supplyApy) + }), + { min: Infinity, max: -Infinity } + );
79-89: Add loading skeletonReplace generic loading text with a skeleton UI for better UX.
- <div className="mt-6">Loading...</div> + <div className="mt-6 grid grid-cols-1 gap-4 sm:grid-cols-2 lg:grid-cols-3"> + {[...Array(6)].map((_, i) => ( + <div key={i} className="animate-pulse rounded-lg border p-4"> + <div className="h-12 w-12 rounded-full bg-gray-200 dark:bg-gray-700" /> + <div className="mt-4 h-4 w-24 rounded bg-gray-200 dark:bg-gray-700" /> + </div> + ))} + </div>src/components/supplyModal.tsx (2)
79-79: Remove debug console.logThis logging statement should be removed before production deployment.
- console.log('isApproved', isApproved);
218-221: Consider using a constant for delay durationsThe hardcoded delays (500ms, 1000ms) should be defined as constants at the top of the file for better maintainability.
+ const UI_TRANSITION_DELAY = 500; + const APPROVAL_TRANSITION_DELAY = 1000; - await new Promise((resolve) => setTimeout(resolve, 500)); + await new Promise((resolve) => setTimeout(resolve, UI_TRANSITION_DELAY)); - await new Promise((resolve) => setTimeout(resolve, 1000)); + await new Promise((resolve) => setTimeout(resolve, APPROVAL_TRANSITION_DELAY));Also applies to: 244-246
src/hooks/useMultiMarketSupply.ts (3)
79-79: Remove console.log to avoid exposing sensitive dataThe
console.logstatement may leak sensitive information. Remove it before deploying.- console.log('Signed for bundlers:', { sigs, permitSingle });
135-135: Avoid hard-coded timeouts for reliabilityUsing a fixed delay may not reliably prevent 'rabby' reverting. Consider a more robust solution.
186-189: Notify user upon errors in 'approveAndSupply'If an error occurs, the user isn't informed. Add a toast error message.
} catch (error) { setShowProcessModal(false); console.error('Error in approveAndSupply:', error); + toast.error('An error occurred. Please try again.'); }src/components/SupplyProcessModal.tsx (2)
49-49: Clarify Approval Detail MessageConsider rephrasing the detail for clarity.
Apply this change:
- detail: `This one-time approval makes sure you don't need to send approval tx again in the future.`, + detail: `This one-time approval ensures you won't need to approve again.`
117-120: Enhance Modal Title and DescriptionConsider specifying the market name when supplying to a single market.
Apply this change:
- <p className="mt-1 text-sm text-gray-500"> - {isMultiMarket ? `Supplying to ${supplies.length} markets` : 'Supplying to market'} - </p> + <p className="mt-1 text-sm text-gray-500"> + {isMultiMarket + ? `Supplying to ${supplies.length} markets` + : `Supplying to ${supplies[0].market.name}`} + </p>app/positions/components/onboarding/RiskSelection.tsx (1)
201-201: Ensuremarket.uniqueKeyis long enough when slicingWhen slicing
market.uniqueKeyat line 201, make sure it has at least 8 characters to prevent errors.app/positions/components/onboarding/SetupPositions.tsx (2)
87-94: Clear error state when input is validThe error state is set on invalid input but not cleared when corrected. Reset the error when input becomes valid.
try { // Validate the new amount can be converted to BigInt parseUnits(cleanValue || '0', tokenDecimals); + setError(null); setTotalAmount(cleanValue); } catch (e) { setError('Invalid amount'); }
230-234: Provide specific error messages inhandleNextCatching all exceptions and setting a generic error message may hide issues. Log the actual error or display a more informative message.
} catch (e) { - setError('Invalid amount format'); + setError(e.message || 'An error occurred during supply'); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (42)
.env.test(1 hunks)app/api/balances/route.ts(1 hunks)app/history/components/HistoryTable.tsx(1 hunks)app/home/_components/HomeHeader.tsx(1 hunks)app/info/components/info.tsx(3 hunks)app/info/components/sectionData.tsx(1 hunks)app/markets/components/OracleFilter.tsx(3 hunks)app/markets/components/markets.tsx(2 hunks)app/positions/components/FromAndToMarkets.tsx(2 hunks)app/positions/components/PositionsContent.tsx(4 hunks)app/positions/components/RebalanceActionInput.tsx(1 hunks)app/positions/components/RebalanceModal.tsx(3 hunks)app/positions/components/SmartOnboarding.tsx(1 hunks)app/positions/components/onboarding/AssetSelection.tsx(1 hunks)app/positions/components/onboarding/OnboardingContext.tsx(1 hunks)app/positions/components/onboarding/RiskSelection.tsx(1 hunks)app/positions/components/onboarding/SetupPositions.tsx(1 hunks)app/positions/components/onboarding/SmartOnboarding.tsx(1 hunks)app/positions/components/onboarding/SuccessPage.tsx(1 hunks)app/positions/components/onboarding/types.ts(1 hunks)app/positions/onboarding/page.tsx(1 hunks)app/rewards/components/RewardContent.tsx(1 hunks)app/settings/page.tsx(2 hunks)src/components/ButtonGroup.tsx(2 hunks)src/components/RiskNotificationModal.tsx(1 hunks)src/components/Status/EmptyScreen.tsx(1 hunks)src/components/Status/LoadingScreen.tsx(1 hunks)src/components/SupplyProcessModal.tsx(4 hunks)src/components/layout/banner/banner.tsx(1 hunks)src/components/layout/header/AccountConnect.tsx(1 hunks)src/components/layout/header/AccountDropdown.tsx(1 hunks)src/components/layout/header/AccountInfoPanel.tsx(4 hunks)src/components/layout/header/Navbar.tsx(4 hunks)src/components/providers/ClientProviders.tsx(1 hunks)src/components/supplyModal.tsx(5 hunks)src/contexts/MarketsContext.tsx(3 hunks)src/hooks/useLocalStorage.ts(1 hunks)src/hooks/useMultiMarketSupply.ts(1 hunks)src/hooks/useUserBalances.ts(1 hunks)src/store/createWagmiConfig.ts(1 hunks)src/utils/tokens.ts(1 hunks)tailwind.config.ts(2 hunks)
✅ Files skipped from review due to trivial changes (18)
- .env.test
- app/history/components/HistoryTable.tsx
- app/home/_components/HomeHeader.tsx
- app/info/components/info.tsx
- app/info/components/sectionData.tsx
- app/markets/components/markets.tsx
- app/positions/components/FromAndToMarkets.tsx
- app/positions/components/RebalanceActionInput.tsx
- app/positions/components/RebalanceModal.tsx
- app/rewards/components/RewardContent.tsx
- src/components/Status/LoadingScreen.tsx
- src/components/layout/header/AccountConnect.tsx
- src/components/layout/header/AccountDropdown.tsx
- src/components/layout/header/AccountInfoPanel.tsx
- src/components/providers/ClientProviders.tsx
- src/contexts/MarketsContext.tsx
- src/hooks/useLocalStorage.ts
- src/store/createWagmiConfig.ts
🧰 Additional context used
🪛 Biome
app/positions/components/onboarding/SetupPositions.tsx
[error] 108-108: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
🪛 eslint
tailwind.config.ts
[error] 5-5: tailwindcss/plugin import should occur before type import of tailwindcss
(import/order)
[error] 79-85: Unexpected function expression.
(prefer-arrow-callback)
🔇 Additional comments (33)
app/positions/components/SmartOnboarding.tsx (1)
7-7: Verify max-width constraint
Check if max-w-7xl works well with the content width requirements.
✅ Verification successful
max-w-7xl is consistently used for onboarding layouts
The same max-width constraint (max-w-7xl) is used in the parent onboarding page layout, confirming this is the intended width for this section. Other sections like info pages use smaller widths (max-w-3xl) for content-focused layouts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find similar max-width usage
rg "max-w-\d?xl" -A 2 -B 2
Length of output: 1363
app/positions/components/onboarding/SmartOnboarding.tsx (1)
1-22: Clean implementation of the onboarding flow
Good use of context and component composition. The code is well-structured and maintainable.
src/components/Status/EmptyScreen.tsx (1)
11-11: Clean styling update
Styling changes match the app's design system and improve the empty state's visual presentation.
app/positions/components/onboarding/types.ts (2)
16-16: LGTM!
Clear and descriptive step definitions.
18-25: LGTM!
Well-structured context type with proper optionals and setters.
app/positions/components/onboarding/OnboardingContext.tsx (1)
17-17: LGTM!
Clean context creation with proper typing.
app/settings/page.tsx (1)
29-29: LGTM: Style change aligns with app-wide updates
src/components/layout/banner/banner.tsx (1)
16-16: Border radius change looks good.
Part of the design system update to use rounded instead of rounded-lg.
app/api/balances/route.ts (1)
9-12: LGTM!
Clean and precise type definition.
src/components/ButtonGroup.tsx (2)
60-60: LGTM: Consistent border radius
Matches the standardized border radius used across other components.
77-78: LGTM: Cleaner corner rounding logic
Good simplification of the button edge styling.
src/hooks/useUserBalances.ts (2)
21-26: LGTM!
Clean initialization with proper typing and state management.
88-98: LGTM!
Clean effect implementation with proper dependency array and return values.
src/components/RiskNotificationModal.tsx (1)
73-73: LGTM!
The border styling creates clear visual separation for the checkbox area.
src/components/layout/header/Navbar.tsx (2)
99-99: LGTM: Mobile menu button styling
Clean styling change that properly aligns the menu icon.
110-110: LGTM: Theme toggle styling
Good addition of hover feedback and proper centering.
app/markets/components/OracleFilter.tsx (1)
19-19: LGTM: Clean click outside handler
The simplified logic is correct and cleanup is properly handled.
app/positions/components/PositionsContent.tsx (3)
6-6: LGTM!
Icon imports align with the UI updates.
25-25: LGTM!
Safer null check implementation.
88-98: LGTM!
Clear call-to-action for new users.
app/positions/components/onboarding/AssetSelection.tsx (2)
27-31: LGTM!
Clean hook usage and proper loading state handling.
73-77: LGTM!
Clean state management and navigation.
src/utils/tokens.ts (2)
Line range hint 324-397: Helper functions look good
The utility functions only use address and chainId for lookups. No direct impact from the decimal change.
150-150: Verify EURC decimals change
The change from 18 to 6 decimals for EURC needs verification against the token contract.
✅ Verification successful
EURC decimals value is correct
The EURC token configuration shows 6 decimals, which matches the Circle EURC token standard. No other files in the codebase depend on this value.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Get EURC token decimals from contract
MAINNET_ADDRESS="0x1aBaEA1f7C830bD89Acc67eC4af516284b1bC33c"
BASE_ADDRESS="0x60a3E35Cc302bFA44Cb288Bc5a4F316Fdb1adb42"
# Search for decimal-related function calls or variables
ast-grep --pattern 'decimals()'
rg -A 2 'EURC.*decimals|decimals.*EURC'
Length of output: 194
Script:
#!/bin/bash
# Search for EURC token configuration
rg -A 10 -B 5 'EURC.*=' src/utils/tokens.ts
# Search for any decimal calculations or references
rg -l 'decimals.*EURC|EURC.*decimals'
rg -l 'formatUnits.*EURC|parseUnits.*EURC'
Length of output: 130
Script:
#!/bin/bash
# Look for token configurations in the file
cat src/utils/tokens.ts
# Search for any files using EURC token
rg -l "EURC"
Length of output: 12814
src/components/supplyModal.tsx (2)
304-320: Verify SupplyProcessModal props usage
The component now passes supplies as an array, which is a breaking change. Ensure all consumers of SupplyProcessModal are updated.
187-195:
Add missing dependency to useCallback
The dependency array is missing useEth which is used in the callback.
}, [
account,
market,
supplyAmount,
sendTransactionAsync,
useEth,
signForBundlers,
usePermit2Setting,
+ useEth,
]);Likely invalid or redundant comment.
src/components/SupplyProcessModal.tsx (5)
11-14: Type Definition and Props Updated Correctly
Defining MarketSupply and updating the supplies prop makes sense for handling multiple supplies.
Also applies to: 17-17
26-29: Destructuring Props
Destructuring the updated props is appropriate and keeps the component clean.
92-92: Check Multi-Market Condition
Setting isMultiMarket to supplies.length > 1 correctly determines if multiple markets are involved.
156-157: Confirm Token Decimals
Verify that supply.market.loanAsset.decimals matches the token's decimals for accurate balance formatting.
Run the following script to confirm token decimals:
✅ Verification successful
Token decimals are correctly used from the market data
The verification shows that supply.market.loanAsset.decimals is consistently used across the codebase for formatting token balances. The decimals value comes from the market data structure defined in src/utils/types.ts and is populated from on-chain data via GraphQL queries (seen in src/graphql/queries.ts).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check that loanAsset decimals match token decimals.
# Expect: Display of loanAsset decimals for each market.
rg 'loanAsset' -A 5
Length of output: 46649
152-152: Verify LTV Formatting
Ensure that supply.market.lltv uses the correct decimals when formatting.
Run the following script to check the decimals:
✅ Verification successful
LTV Formatting Verified
The code correctly uses 16 decimals when formatting the LLTV value with formatUnits(BigInt(supply.market.lltv), 16). This matches the decimal format used consistently across the codebase for LLTV values, as evidenced by:
- Same 16 decimal formatting in supply modal:
formatUnits(BigInt(market.lltv), 16) - LLTV values defined as uint256 in contract ABIs
- Consistent BigInt conversion and 16 decimal formatting in all UI components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the decimals used for formatting lltv values.
# Expect: Occurrences of 'lltv' and associated decimals.
rg 'lltv' -A 5
Length of output: 145823
app/positions/components/onboarding/RiskSelection.tsx (2)
227-227: Verify correct decimals in formatUnits for market.lltv
Confirm that using 16 decimals in formatUnits for market.lltv accurately represents the intended percentage.
1-281: Code is well-organized and functions correctly
The component logic is clear, and the implementation follows best practices.
| const url = getNetworkImg(networkId); | ||
| return ( | ||
| <Image | ||
| src={url as string} |
There was a problem hiding this comment.
Add type check before string assertion
The forced type assertion could fail if getNetworkImg returns undefined.
- src={url as string}
+ src={url ?? '/fallback-network-icon.png'}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Outside diff range and nitpick comments (20)
src/types/token.ts (1)
7-16: Consider adding address validation.Add address format validation using a string template type:
+type EthereumAddress = `0x${string}` export type NetworkToken = { symbol: string; decimals: number; network: SupportedNetworks; - address: string; + address: EthereumAddress; };app/positions/components/onboarding/SuccessPage.tsx (2)
1-4: Fix import orderMove
@nextui-org/reactimport beforenext/link-import Link from 'next/link'; -import { Button } from '@nextui-org/react'; +import { Button } from '@nextui-org/react'; +import Link from 'next/link'; import { FaCheckCircle } from 'react-icons/fa'; import { useOnboarding } from './OnboardingContext';🧰 Tools
🪛 eslint (1.23.1)
[error] 2-2:
@nextui-org/reactimport should occur before import ofnext/link(import/order)
27-31: Improve Link styling consistencyMove
bg-surfaceandborder-1to Button component for better style organization-<Link href="/positions/onboarding" className="no-underline border-1 bg-surface"> - <Button variant="light" className="min-w-[120px] rounded"> +<Link href="/positions/onboarding" className="no-underline"> + <Button variant="light" className="min-w-[120px] rounded border-1 bg-surface">tailwind.config.ts (2)
5-5: Fix import orderMove type import after regular imports.
-import type { Config } from 'tailwindcss'; import plugin from 'tailwindcss/plugin'; +import type { Config } from 'tailwindcss';🧰 Tools
🪛 eslint (1.23.1)
[error] 5-5:
tailwindcss/pluginimport should occur before type import oftailwindcss(import/order)
79-85: Use arrow functionPlugin works fine but could use modern syntax.
- plugin(function ({ addBase }) { + plugin(({ addBase }) => {🧰 Tools
🪛 eslint (1.23.1)
[error] 79-85: Unexpected function expression.
(prefer-arrow-callback)
src/hooks/useAllowance.ts (2)
77-77: Remove console.log statementDebug logs shouldn't be in production code.
- console.log('data', data);
79-81: Consider enhancing loading state handlingisLoadingAllowance could also account for chain switching states to improve the onboarding UX.
- const isLoadingAllowance = data === undefined; + const isLoadingAllowance = data === undefined || chain?.id !== chainIdFromArgumentOrConnectedWallet;src/hooks/useUserBalances.ts (2)
1-4: Fix import orderReorder imports to follow the style guide.
import { useCallback, useEffect, useState } from 'react'; import { useAccount } from 'wagmi'; -import { findToken } from '@/utils/tokens'; import { SupportedNetworks } from '@/utils/networks'; +import { findToken } from '@/utils/tokens';🧰 Tools
🪛 eslint (1.23.1)
[error] 4-4:
@/utils/networksimport should occur before import of@/utils/tokens(import/order)
28-43: Add input validation and specific error typesThe fetch function could be more robust:
- Validate address format before API call
- Add specific error types for different failure cases
const fetchBalances = useCallback( async (chainId: number): Promise<TokenResponse['tokens']> => { try { + if (!address || !/^0x[a-fA-F0-9]{40}$/.test(address)) { + throw new Error('Invalid address format'); + } const response = await fetch(`/api/balances?address=${address}&chainId=${chainId}`); if (!response.ok) { - throw new Error('Failed to fetch balances'); + throw new Error(`Failed to fetch balances: ${response.statusText}`); } const data = (await response.json()) as TokenResponse; return data.tokens; } catch (err) { console.error('Error fetching balances:', err); throw err instanceof Error ? err : new Error('Unknown error occurred'); } }, [address], );app/positions/components/PositionsContent.tsx (1)
91-101: Add analytics trackingConsider tracking clicks on "Start Lending" to measure conversion.
<button type="button" + onClick={() => { + analytics.track('empty_state_cta_click'); + }} className="bg-monarch-orange mx-auto rounded px-8 py-3 font-zen text-white opacity-90 transition-all duration-200 ease-in-out hover:opacity-100" >src/components/SupplyProcessModal.tsx (1)
122-170: Market details section needs error boundaryAdd error handling for failed token lookups or formatting errors.
+ import { ErrorBoundary } from '@/components/ErrorBoundary'; {/* Market details */} + <ErrorBoundary fallback={<div>Failed to load market details</div>}> <div className="mt-4 space-y-3"> {supplies.map((supply) => { // ... existing code })} </div> + </ErrorBoundary>src/hooks/useMultiMarketSupply.ts (2)
147-147: Extract timeout values to constantsMove magic number 800 to a named constant for better maintainability.
+ const RABBY_TIMEOUT_MS = 800; - await new Promise((resolve) => setTimeout(resolve, 800)); + await new Promise((resolve) => setTimeout(resolve, RABBY_TIMEOUT_MS));
222-230: Enhance error message specificityCurrent error messages could be more specific about what failed during approval.
- if (error.message.includes('User rejected')) { - toast.error('Approval rejected by user'); - } else { - toast.error('Failed to approve token'); - } + if (error.message.includes('User rejected')) { + toast.error('Approval rejected by user'); + } else if (error.message.includes('insufficient funds')) { + toast.error('Insufficient funds for approval'); + } else if (error.message.includes('nonce')) { + toast.error('Transaction nonce error. Please try again'); + } else { + toast.error(`Failed to approve token: ${error.message}`); + }app/positions/components/onboarding/SetupPositions.tsx (5)
81-99: Consider using a numeric input typeThe current implementation manually handles numeric input validation. Consider using a numeric input type with built-in validation.
- const cleanValue = value.replace(/[^0-9.]/g, ''); - - // Ensure only one decimal point - const parts = cleanValue.split('.'); - if (parts.length > 2) return; - - // Limit decimal places to token decimals - if (parts[1] && parts[1].length > tokenDecimals) return; + const numValue = Number(value); + if (isNaN(numValue)) return; + + const formatted = numValue.toFixed(tokenDecimals);
234-235: Enhance error loggingThe error message lacks context. Consider adding more details about the operation that failed.
- console.error('Supply failed:', err); + console.error(`Supply failed for token ${selectedToken?.symbol}:`, err);
298-298: Avoid hardcoded viewport heightsThe
calc(100vh-500px)height calculation might break on different screen sizes.Consider using CSS Grid or Flexbox with dynamic sizing:
- <div className="h-[calc(100vh-500px)] overflow-auto rounded border border-gray-200 dark:border-gray-700"> + <div className="min-h-0 max-h-[80vh] overflow-auto rounded border border-gray-200 dark:border-gray-700">
459-459: Add tooltip for disabled stateThe Execute button can be disabled but doesn't explain why.
<Button color="primary" isDisabled={error !== null || !totalAmount || supplies.length === 0} + title={error ? error : !totalAmount ? "Enter amount" : supplies.length === 0 ? "Select markets" : ""} isLoading={supplyPending || isLoadingPermit2} onPress={handleSupply} className="min-w-[120px] rounded" >
325-328: Add aria-label for better accessibilityThe market ID link lacks an accessible label.
<a href={`/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`} target="_blank" rel="noopener noreferrer" + aria-label={`View market ${market.uniqueKey}`} className="text-primary hover:text-primary-400" >app/positions/components/onboarding/AssetSelection.tsx (2)
28-29: Add Error Handling for Data FetchingConsider handling errors from
useUserBalancesanduseMarketsto inform the user if data fails to load.
158-158: Simplify APY Display When Rates Are EqualIf
minApyandmaxApyare the same, displaying them as a single value might improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
app/api/balances/route.ts(1 hunks)app/markets/components/OracleFilter.tsx(3 hunks)app/positions/components/FromAndToMarkets.tsx(2 hunks)app/positions/components/PositionsContent.tsx(4 hunks)app/positions/components/onboarding/AssetSelection.tsx(1 hunks)app/positions/components/onboarding/OnboardingContext.tsx(1 hunks)app/positions/components/onboarding/RiskSelection.tsx(1 hunks)app/positions/components/onboarding/SetupPositions.tsx(1 hunks)app/positions/components/onboarding/SuccessPage.tsx(1 hunks)app/positions/components/onboarding/types.ts(1 hunks)src/components/Input/Input.tsx(1 hunks)src/components/Status/EmptyScreen.tsx(1 hunks)src/components/SupplyProcessModal.tsx(4 hunks)src/hooks/useAllowance.ts(2 hunks)src/hooks/useMultiMarketSupply.ts(1 hunks)src/hooks/useUserBalances.ts(1 hunks)src/types/token.ts(1 hunks)tailwind.config.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/components/Input/Input.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- app/api/balances/route.ts
- app/markets/components/OracleFilter.tsx
- app/positions/components/FromAndToMarkets.tsx
- app/positions/components/onboarding/RiskSelection.tsx
- app/positions/components/onboarding/types.ts
- src/components/Status/EmptyScreen.tsx
🧰 Additional context used
🪛 eslint (1.23.1)
app/positions/components/onboarding/SetupPositions.tsx
[error] 10-10: 'MarketSupply' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 233-233: 'error' is already declared in the upper scope on line 27 column 10.
(@typescript-eslint/no-shadow)
app/positions/components/onboarding/SuccessPage.tsx
[error] 2-2: @nextui-org/react import should occur before import of next/link
(import/order)
src/hooks/useUserBalances.ts
[error] 4-4: @/utils/networks import should occur before import of @/utils/tokens
(import/order)
tailwind.config.ts
[error] 5-5: tailwindcss/plugin import should occur before type import of tailwindcss
(import/order)
[error] 79-85: Unexpected function expression.
(prefer-arrow-callback)
🔇 Additional comments (15)
src/types/token.ts (1)
1-16: Clean type definition with good documentation.
The type structure is clear and well-documented.
tailwind.config.ts (1)
57-78: Looks good
Consistent radius settings for both themes.
app/positions/components/onboarding/OnboardingContext.tsx (3)
1-15: LGTM! Well-structured types.
The type definitions are clear and type-safe.
17-17: LGTM! Proper context initialization.
66-72: LGTM! Clean hook implementation.
src/hooks/useUserBalances.ts (3)
6-20: LGTM!
Types are well-defined with all necessary fields.
53-56: LGTM!
Good use of Promise.all for concurrent chain requests.
89-98: LGTM!
Clean effect implementation and complete return interface.
app/positions/components/PositionsContent.tsx (3)
6-6: LGTM: Clean imports and robust checks
The new icon imports and market position check look good.
Also applies to: 25-25
34-62: LGTM: Clean button implementation
Good job on the consistent styling and accessibility.
54-54: Verify onboarding route exists
Multiple links point to "/positions/onboarding". Let's verify the route.
Also applies to: 93-93
✅ Verification successful
Route exists and is properly integrated
The /positions/onboarding route exists at app/positions/onboarding/page.tsx and is used consistently across multiple onboarding components with proper step navigation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if onboarding route exists
rg -g "app/**/*" -A 5 "positions/onboarding"
Length of output: 5775
Script:
#!/bin/bash
# Check if onboarding route/page exists
fd "onboarding" app/positions/page.tsx app/positions/onboarding/page.tsx
Length of output: 262
Script:
#!/bin/bash
# Check for Next.js page files in positions directory
fd "page.tsx" app/positions/
Length of output: 170
src/components/SupplyProcessModal.tsx (4)
11-14: Clean type definition
The MarketSupply type is well-structured and properly typed.
17-17: Props structure looks good
Props interface cleanly handles both single and multi-market cases.
Also applies to: 26-31
95-105: Animation implementation looks good
Clean framer-motion setup with proper opacity and scale transitions.
189-189: Fix color usage
The text-primary class is used with monarch-orange which was previously flagged as an issue.
| const { address: account } = useAccount(); | ||
| const chainId = loanAsset?.network; | ||
| const tokenSymbol = loanAsset?.symbol; | ||
| const totalAmount = supplies.reduce((sum, supply) => sum + supply.amount, 0n); |
There was a problem hiding this comment.
Add validation for empty supplies array
Check if supplies array is empty before reduce operation to prevent unexpected behavior.
- const totalAmount = supplies.reduce((sum, supply) => sum + supply.amount, 0n);
+ const totalAmount = supplies.length > 0
+ ? supplies.reduce((sum, supply) => sum + supply.amount, 0n)
+ : 0n;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const totalAmount = supplies.reduce((sum, supply) => sum + supply.amount, 0n); | |
| const totalAmount = supplies.length > 0 | |
| ? supplies.reduce((sum, supply) => sum + supply.amount, 0n) | |
| : 0n; |
1 participant
Much better onboarding flow!
Summary by CodeRabbit
New Features
Bug Fixes
Style
Documentation