Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive refactoring of the onboarding process for creating new positions. The changes centralize the onboarding experience into a modal within the Positions component, removing separate onboarding pages and streamlining user interaction. The implementation includes new context management, step-based navigation, and enhanced state handling for selecting assets, managing risks, and setting up positions. Changes
Sequence DiagramsequenceDiagram
participant User
participant PositionsContent
participant OnboardingModal
participant AssetSelection
participant RiskSelection
participant SetupPositions
participant SuccessPage
User->>PositionsContent: Click "New Position"
PositionsContent->>OnboardingModal: Open Modal
OnboardingModal->>AssetSelection: Render First Step
User->>AssetSelection: Select Token
AssetSelection->>RiskSelection: Navigate Next
User->>RiskSelection: Configure Risk
RiskSelection->>SetupPositions: Navigate Next
User->>SetupPositions: Supply Assets
SetupPositions->>SuccessPage: Show Completion
SuccessPage->>PositionsContent: Close Modal
Possibly Related PRs
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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: 4
🔭 Outside diff range comments (1)
app/positions/components/onboarding/SetupPositions.tsx (1)
Line range hint
305-332: Fix hardcoded table heightThe table height uses a hardcoded calculation that might break on different screen sizes.
- className="h-[calc(100vh-500px)]" + className="max-h-[600px] h-full"
🧹 Nitpick comments (11)
app/positions/components/onboarding/SetupPositions.tsx (2)
21-21: Add error boundaryAdd error boundaries to handle hook failures gracefully.
+ import { ErrorBoundary } from '@/components/ErrorBoundary'; export function SetupPositions() { + return ( + <ErrorBoundary fallback={<div>Failed to load positions setup</div>}> {/* existing component content */} + </ErrorBoundary> + ); }Also applies to: 220-220
242-243: Add specific error messagesThe error handling relies on useMultiMarketSupply's generic error toast. Consider catching specific errors.
- await approveAndSupply(); + try { + await approveAndSupply(); + } catch (error) { + if (error.code === 4001) { + toast.error('Transaction rejected by user'); + } else if (error.code === -32603) { + toast.error('Internal JSON-RPC error'); + } else { + toast.error('Failed to supply'); + } + throw error; + }app/positions/components/onboarding/OnboardingContext.tsx (1)
73-73: Remove debugconsole.logThe
console.logseems left over from debugging.Apply this diff:
- console.log('can go next', currentStep, canGoNext);app/positions/components/onboarding/RiskSelection.tsx (1)
87-92: Simplify market selection logicYou can streamline
toggleMarketSelection.Apply this diff:
const toggleMarketSelection = (market: Market) => { - const ids = selectedMarkets.map((m) => m.uniqueKey); - if (ids.includes(market.uniqueKey)) { + if (selectedMarkets.some((m) => m.uniqueKey === market.uniqueKey)) { setSelectedMarkets(selectedMarkets.filter((m) => m.uniqueKey !== market.uniqueKey)); } else { setSelectedMarkets([...selectedMarkets, market]); } };docs/Styling.md (1)
14-15: Add examples for rounded corner usageShow when to use each variant with specific component examples.
app/positions/components/onboarding/SuccessPage.tsx (2)
15-16: Remove or track the TODO commentThe TODO comment about enabling agent features needs tracking.
Want me to create an issue to track this feature?
19-24: Consider extracting reset logichandleFinished combines multiple state resets. Consider moving this to the context.
- const handleFinished = () => { - onClose(); - setStep('asset-selection'); - setSelectedToken(null); - setSelectedMarkets([]); - }; + const { resetOnboarding } = useOnboarding(); + const handleFinished = () => { + onClose(); + resetOnboarding(); + };src/components/common/MarketInfoBlock.tsx (1)
Line range hint
26-32: Add meaningful alt text for token imageCurrent alt text only shows symbol. Add more context for screen readers.
- alt={market.collateralAsset.symbol} + alt={`${market.collateralAsset.symbol} token icon`}app/positions/components/PositionsContent.tsx (1)
125-130: Remove redundant modal visibility checkThe
showOnboardingModalcheck is redundant since it's already checked in the condition.- {showOnboardingModal && ( <OnboardingModal - isOpen={showOnboardingModal} + isOpen={true} onClose={() => setShowOnboardingModal(false)} /> - )}app/positions/components/onboarding/AssetSelection.tsx (1)
134-154: Remove commented codeRemove the commented tooltip code to keep the codebase clean.
- {/* if base network, show agent badge */} - {token.network === SupportedNetworks.Base && ( - // <Tooltip - // content={ - // <div className="flex flex-col gap-2 p-2 font-zen"> - // <div className="text-base">Monarch Autopilot 🎉</div> - // <div className="text-sm"> - // Monarch Autopilot is now in beta on Base! Setup the agent to start - // automating your reallocations. - // </div> - // </div> - // } - // > - // <div className="flex gap-2 rounded bg-primary bg-opacity-50 px-1.5 py-0.5 text-xs text-gray-100"> - // 🤖 - // <span className="opacity-100">beta</span> - // </div> - // </Tooltip> - <div /> - )}app/positions/report/components/ReportTable.tsx (1)
Line range hint
189-220: Consider extracting the filter and sort logic.The chain of filters and sorts could be more readable if extracted into separate functions.
+ const isMarketActive = (endBalance: bigint) => endBalance > 0n; + const sortByActiveStatusAndInterest = (a: MarketReport, b: MarketReport) => { + const aActive = isMarketActive(BigInt(a.endBalance)); + const bActive = isMarketActive(BigInt(b.endBalance)); + if (aActive !== bActive) return bActive ? 1 : -1; + return Number(b.interestEarned - a.interestEarned); + }; + const hasActivityInTimeframe = (report: MarketReport) => report.effectiveTime !== 0; + {report.marketReports - .filter((m) => m.market.collateralAsset !== null) - .slice() - .sort((a, b) => { - const aActive = BigInt(a.endBalance) > 0n; - const bActive = BigInt(b.endBalance) > 0n; - if (aActive !== bActive) return bActive ? 1 : -1; - return Number(b.interestEarned - a.interestEarned); - }) - .filter((marketReport) => marketReport.effectiveTime !== 0) + .filter(m => m.market.collateralAsset !== null) + .sort(sortByActiveStatusAndInterest) + .filter(hasActivityInTimeframe)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
src/imgs/tokens/usol.pngis excluded by!**/*.pngsrc/imgs/tokens/usui.pngis excluded by!**/*.png
📒 Files selected for processing (25)
app/positions/components/PositionsContent.tsx(3 hunks)app/positions/components/SmartOnboarding.tsx(0 hunks)app/positions/components/onboarding/AssetSelection.tsx(5 hunks)app/positions/components/onboarding/Modal.tsx(1 hunks)app/positions/components/onboarding/OnboardingContent.tsx(0 hunks)app/positions/components/onboarding/OnboardingContext.tsx(2 hunks)app/positions/components/onboarding/RiskSelection.tsx(4 hunks)app/positions/components/onboarding/SetupPositions.tsx(5 hunks)app/positions/components/onboarding/SmartOnboarding.tsx(0 hunks)app/positions/components/onboarding/SuccessPage.tsx(1 hunks)app/positions/components/onboarding/content.tsx(1 hunks)app/positions/onboarding/page.tsx(0 hunks)app/positions/report/components/ReportTable.tsx(3 hunks)docs/Styling.md(1 hunks)src/abis/monarch-agent-v1.ts(1 hunks)src/components/OracleVendorBadge.tsx(1 hunks)src/components/SupplyProcessModal.tsx(2 hunks)src/components/common/MarketInfoBlock.tsx(2 hunks)src/components/providers/ClientProviders.tsx(2 hunks)src/components/supplyModal.tsx(2 hunks)src/components/withdrawModal.tsx(2 hunks)src/hooks/useMultiMarketSupply.ts(2 hunks)src/hooks/useTransactionWithToast.tsx(1 hunks)src/utils/monarch-agent.ts(1 hunks)src/utils/tokens.ts(2 hunks)
💤 Files with no reviewable changes (4)
- app/positions/components/SmartOnboarding.tsx
- app/positions/onboarding/page.tsx
- app/positions/components/onboarding/OnboardingContent.tsx
- app/positions/components/onboarding/SmartOnboarding.tsx
✅ Files skipped from review due to trivial changes (2)
- src/hooks/useTransactionWithToast.tsx
- src/utils/monarch-agent.ts
🔇 Additional comments (12)
src/abis/monarch-agent-v1.ts (1)
1-210: LGTM
The ABI looks correctly defined.
src/components/providers/ClientProviders.tsx (1)
15-18: LGTM!
Clean provider implementation with proper component hierarchy.
src/components/SupplyProcessModal.tsx (1)
123-123: LGTM: Component rename looks good
app/positions/components/PositionsContent.tsx (1)
89-99: LGTM: Button implementation looks good
The button correctly handles the modal trigger and ownership check.
app/positions/components/onboarding/AssetSelection.tsx (2)
79-82: LGTM: Loading state improvement
Spinner provides better UX than static text.
103-103: Verify mobile layout
The grid layout changed from responsive to fixed columns. Verify this works well on mobile.
✅ Verification successful
Grid layout follows mobile-first pattern
The layout is properly responsive:
- Mobile (default): 1 column
- Tablet (sm): 1 column
- Desktop (lg): 2 columns
This matches other grid layouts in the codebase which use similar mobile-first patterns (e.g., 1 column on mobile, multiple columns on larger screens).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other grid layouts in the codebase for consistency
rg "grid-cols-\d+" --type tsx
Length of output: 59
Script:
#!/bin/bash
# Search for grid layouts in all files (not limiting to tsx)
rg "grid-cols-\d+"
# Also search for the specific file to see its history
rg "grid-cols" "app/positions/components/onboarding/AssetSelection.tsx" -A 2 -B 2
Length of output: 975
src/hooks/useMultiMarketSupply.ts (1)
25-25: LGTM!
Clean implementation of the success callback.
Also applies to: 70-70
src/components/withdrawModal.tsx (1)
10-10: LGTM!
Consistent update of the component name.
Also applies to: 131-131
app/positions/report/components/ReportTable.tsx (1)
59-59: LGTM!
Clean rename of the component.
src/utils/tokens.ts (2)
243-246: LGTM: Network expansion for lBTC
The addition of base chain support for lBTC follows the established pattern.
388-399: LGTM: New tokens added
The uSOL and uSui token entries follow the standard format and include all required fields.
src/components/supplyModal.tsx (1)
22-22: Verify MarketInfoBlock component compatibility
The component has been renamed from MarketAmountBlock to MarketInfoBlock.
Also applies to: 344-344
✅ Verification successful
Component rename looks safe
The MarketInfoBlock component is a new component with proper type definitions and is used across multiple files. No traces of MarketAmountBlock in the codebase, suggesting this is a clean rename with updated imports.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if MarketInfoBlock exports the same props as MarketAmountBlock
ast-grep --pattern 'export function MarketInfoBlock({ $_ }: { market: $_ })'
Length of output: 78
Script:
#!/bin/bash
# Search for MarketInfoBlock component definition
rg -A 5 "export (const|function) MarketInfoBlock"
# Search for MarketAmountBlock component definition
rg -A 5 "export (const|function) MarketAmountBlock"
# Find files containing MarketInfoBlock
fd -t f -e tsx -e ts -x rg -l "MarketInfoBlock"
# Find files containing MarketAmountBlock
fd -t f -e tsx -e ts -x rg -l "MarketAmountBlock"
Length of output: 1017
Script:
#!/bin/bash
# Check MarketInfoBlock props interface
rg -B 5 "MarketInfoBlockProps" src/components/common/MarketInfoBlock.tsx
# Look for previous MarketAmountBlock in git history
git log -p --all -S "MarketAmountBlock"
Length of output: 453
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
app/positions/components/onboarding/SetupPositions.tsx (1)
Line range hint
219-242: Improve error feedback in supply handlingThe error handling is too generic. Users would benefit from more specific error messages.
} catch (supplyError) { console.error('Supply failed:', supplyError); - // Error toast is already shown in useMultiMarketSupply + toast.error( + supplyError instanceof Error + ? supplyError.message + : 'Failed to process supply transaction' + ); } finally {
🧹 Nitpick comments (4)
app/positions/components/onboarding/SetupPositions.tsx (3)
20-21: Add error handling for navigation functionsThe navigation functions could fail if the context is not properly initialized.
- const { selectedToken, selectedMarkets, goToNextStep, goToPrevStep } = useOnboarding(); + const { selectedToken, selectedMarkets, goToNextStep, goToPrevStep } = useOnboarding() ?? {}; + if (!goToNextStep || !goToPrevStep) { + console.error('Navigation functions not available'); + return null; + }
321-331: Enhance link accessibilityThe market link needs proper aria labels for better accessibility.
<a href={`/market/${market.morphoBlue.chain.id}/${market.uniqueKey}`} target="_blank" rel="noopener noreferrer" className="no-underline" + aria-label={`View details for ${market.loanToken.symbol}/${market.collateralToken.symbol} market`} >
413-424: Add loading state messageThe Execute button should indicate what operation is in progress.
<Button variant="cta" isDisabled={error !== null || !totalAmount || supplies.length === 0} isLoading={supplyPending || isLoadingPermit2} + loadingText={isLoadingPermit2 ? "Approving..." : "Supplying..."} onPress={() => void handleSupply()} className="min-w-[120px]" >app/positions/components/onboarding/RiskSelection.tsx (1)
209-211: Use consistent event handler namesThe
Buttoncomponent uses bothonClickandonPressas event handlers. For consistency, consider standardizing on one. IfonPressis preferred, update this instance:-<Button - onClick={(e) => handleMarketDetails(market, e)} +<Button + onPress={(e) => handleMarketDetails(market, e)}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/positions/components/PositionsContent.tsx(3 hunks)app/positions/components/onboarding/Modal.tsx(1 hunks)app/positions/components/onboarding/OnboardingContext.tsx(2 hunks)app/positions/components/onboarding/RiskSelection.tsx(4 hunks)app/positions/components/onboarding/SetupPositions.tsx(6 hunks)app/positions/components/onboarding/SuccessPage.tsx(1 hunks)src/components/OracleVendorBadge.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/components/OracleVendorBadge.tsx
- app/positions/components/PositionsContent.tsx
- app/positions/components/onboarding/SuccessPage.tsx
- app/positions/components/onboarding/Modal.tsx
🧰 Additional context used
📓 Learnings (1)
app/positions/components/onboarding/OnboardingContext.tsx (1)
Learnt from: antoncoding
PR: antoncoding/monarch#97
File: app/positions/components/onboarding/OnboardingContext.tsx:36-43
Timestamp: 2024-12-16T02:01:51.219Z
Learning: In `app/positions/components/onboarding/OnboardingContext.tsx`, the `defaultStep` variable is no longer needed and can be removed.
🔇 Additional comments (2)
app/positions/components/onboarding/OnboardingContext.tsx (1)
Line range hint 1-90: Looks good to me
The onboarding context is well-structured, and the navigation logic is clear.
app/positions/components/onboarding/RiskSelection.tsx (1)
228-235: Duplicate: Use consistent event handler names
As above, ensure all Button components use the same event handler prop for consistency.
1 participant
Summary by CodeRabbit
Release Notes
New Features
uSOLanduSui, expanding network options.Improvements
Bug Fixes
Documentation