feat: Add 'Supply' Button to Position Page#77
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes updates to several components related to market and position management. Key changes involve modifying import paths, enhancing state management for modal visibility, and restructuring GraphQL queries. The 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 (
|
antoncoding
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (5)
src/utils/types.ts (1)
268-271: Add dailyApys documentationAdd JSDoc to explain APY calculation method and time window.
+ /** + * Daily APY rates calculated over 24h period + * @property netSupplyApy - Net supply APY including rewards + * @property netBorrowApy - Net borrow APY including fees + */ dailyApys: { netSupplyApy: number; netBorrowApy: number; };app/positions/components/SuppliedMarketsDetail.tsx (1)
198-218: Consider extracting button stylesBoth buttons share identical styles. Consider creating a shared style class or component.
+// Add to your CSS file or styled component +.action-button { + @apply bg-hovered rounded-sm p-1 text-xs duration-300 ease-in-out hover:bg-orange-500; +} -className="bg-hovered rounded-sm p-1 text-xs duration-300 ease-in-out hover:bg-orange-500" +className="action-button"app/markets/components/markets.tsx (2)
Line range hint
64-65: Fix potential type safety issue with selectedMarketThe code assumes selectedMarket is defined when showing the modal, but the type allows undefined:
-const [selectedMarket, setSelectedMarket] = useState<Market | undefined>(undefined); +const [selectedMarket, setSelectedMarket] = useState<Market | null>(null); -{showSupplyModal && ( +{showSupplyModal && selectedMarket && ( <SupplyModal - market={selectedMarket as Market} + market={selectedMarket} onClose={() => setShowSupplyModal(false)} /> )}Also applies to: 171-176
Line range hint
170-270: Add ARIA labels for better accessibilityInteractive elements need proper ARIA labels:
-<div className="container h-full gap-8" style={{ padding: '0 5%' }}> +<div className="container h-full gap-8" style={{ padding: '0 5%' }} role="main"> -<h1 className="py-8 font-zen"> Markets </h1> +<h1 className="py-8 font-zen" id="markets-heading"> Markets </h1>src/graphql/queries.ts (1)
83-83: Reconsider the inclusion of__typename.Remove
__typenameif it's not needed.Also applies to: 140-140
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- app/markets/components/markets.tsx (1 hunks)
- app/positions/components/PositionsContent.tsx (3 hunks)
- app/positions/components/PositionsSummaryTable.tsx (5 hunks)
- app/positions/components/SuppliedMarketsDetail.tsx (4 hunks)
- src/graphql/queries.ts (1 hunks)
- src/hooks/useMarkets.ts (2 hunks)
- src/hooks/useUserPositions.ts (4 hunks)
- src/utils/types.ts (2 hunks)
🧰 Additional context used
🔇 Additional comments (20)
src/hooks/useUserPositions.ts (3)
4-4: LGTM! Good separation of concerns.Moving the GraphQL query to a dedicated file improves maintainability.
80-85: Verify consumers of position data.Market warnings moved to
market.warningsWithDetail. Check if UI components handle this change.#!/bin/bash # Find components using position data rg "warningsWithDetail" --type tsx
34-34: Verify query consistency across networks.The same query is used for both networks. Let's verify the query definition.
Also applies to: 47-47
✅ Verification successful
The previous command had issues with file type recognition. Let's try a broader search to find both the query usage and its definition. The second command looks for GraphQL queries containing "position" in template literals.
Query is properly imported and used consistently
The query is imported from
@/graphql/queriesand used identically for both Mainnet and Base networks. The query definition inqueries.tsfetches user market positions with the correct parameters (address and chainId).🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find and display the query definition rg -l "userPositionsQuery" --type tsLength of output: 137
Script:
#!/bin/bash # Let's try a more comprehensive search for the query rg "userPositionsQuery" -A 5 # Also search for potential query definition rg "gql`.*position.*`" --multilineLength of output: 1723
app/positions/components/PositionsContent.tsx (3)
18-19: LGTM!Clean separation of modal states improves code clarity.
54-73: LGTM!Modal implementations are consistent and handle cleanup properly.
83-84: LGTM!Props naming clearly reflects their purpose.
src/hooks/useMarkets.ts (1)
5-5: LGTM!Moving the query to a separate file improves code organization.
src/utils/types.ts (2)
Line range hint
1-285: Types look goodClean structure with logical grouping and consistent naming.
8-8: Check all MarketPosition usagesType change from inline object to Market reference could break existing code.
✅ Verification successful
Based on the search results, I can see that the change from inline object to Market reference is being used consistently throughout the codebase. The
marketproperty is accessed in a type-safe manner across all files, with proper access to nested properties like:
market.loanAssetmarket.collateralAssetmarket.morphoBluemarket.uniqueKeymarket.statemarket.warningsWithDetail- etc.
No issues with Market type usage
The type change is properly integrated across all components and doesn't break existing code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find all files using MarketPosition type rg -l "MarketPosition" # Find potential usage of old market property structure rg "market\." -A 3 -B 3Length of output: 64137
app/positions/components/SuppliedMarketsDetail.tsx (2)
14-15: Props update looks goodClear prop names that match their purpose.
120-120: Warning handling update looks goodConsistent use of market-level warnings throughout the component.
Also applies to: 127-129
app/markets/components/markets.tsx (2)
Line range hint
149-164: LGTM! Clean filtering implementationGood separation of concerns with filtering logic in utils.
Line range hint
134-148: Verify URL update behaviorCheck if multiple rapid filter changes create separate history entries:
✅ Verification successful
Let me check for more context about the URL parameter handling.
Let me check one more thing about the router implementation.
URL updates look good, no history entry issues
The code uses a single
router.pushwithscroll: falseand maintains previous params state. No conflicting router calls or complex state updates found. The URL updates are straightforward and won't create unnecessary history entries.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Look for other router.push calls that might conflict rg "router.push.*scroll.*false" -A 2Length of output: 251
Script:
#!/bin/bash # Look for URL parameter handling patterns ast-grep --pattern 'const params = new URLSearchParams' # Check for any debouncing or batching of URL updates rg "useEffect.*params" -A 5Length of output: 182
Script:
#!/bin/bash # Check for router usage and any debouncing logic rg "router\s*=\s*useRouter" -A 10 rg "useCallback.*router" -A 10Length of output: 881
app/positions/components/PositionsSummaryTable.tsx (5)
19-24: LGTM: Clean type definitions and props structure.The new props structure cleanly separates supply and withdraw modal controls.
Also applies to: 30-35
72-73: LGTM: Safe warning handling.Good use of optional chaining to prevent undefined errors.
288-289: LGTM: Consistent modal props.Modal controls are properly passed down to child components.
269-280: Verify CSS class usage.Check if
bg-hoveredclass is defined in your CSS system.#!/bin/bash # Search for bg-hovered class definition rg 'bg-hovered'
80-80: Verify APY calculation change.Changed from
dailyApys.netSupplyApytostate.supplyApy. Please confirm this is intentional.✅ Verification successful
Let me analyze the APY calculation changes with one more verification to understand the data flow.
This will help us understand if these are two different APY calculations or if one is derived from the other.
Let me try one more search with a simpler pattern to understand the APY types.
APY calculation change is intentional and correct
The change from
dailyApys.netSupplyApytostate.supplyApyin weighted APY calculation is valid because:
state.supplyApyrepresents the base supply APYdailyApys.netSupplyApyincludes additional calculations for net returns- For weight calculations, using the base
supplyApyis more appropriate🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other APY calculations to ensure consistency rg -A 2 'supplyApy|netSupplyApy'Length of output: 4048
Script:
#!/bin/bash # Check the market state and daily APY types/interfaces ast-grep --pattern 'interface $_ { $$$ state: { $$$ supplyApy: $_ $$$ } $$$ dailyApys: { $$$ netSupplyApy: $_ $$$ } $$$ }'Length of output: 162
Script:
#!/bin/bash # Look for interface or type definitions containing APY fields ast-grep --pattern 'interface Market { $$$ }' ast-grep --pattern 'type Market = { $$$ }'Length of output: 3140
src/graphql/queries.ts (2)
150-150: Confirm if$chainIdshould be optional.Check if making
$chainIdoptional is intentional in the query.
159-159: Good use of theMarketFieldsfragment.Reusing the fragment promotes consistency.
1 participant
Improved Features, Enhanced and Simplified Components, Bug Fixes in State Management, URL Sync, Modal Logic, and Data Structures.