fix: claim cta changes on navigation b/w intial and confirm view#696
fix: claim cta changes on navigation b/w intial and confirm view#696jjramirezn merged 1 commit intodevelopfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
WalkthroughThis pull request introduces updates to two claim link components. In the Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant ICV as InitialClaimLinkView
participant Router as Route Manager
User->>ICV: Select chain & token
ICV->>ICV: Evaluate isCrossChainTransaction
alt Cross-chain transaction
ICV->>Router: Reset selected route & update state
else Same-chain transaction
ICV->>Router: Maintain current route
end
User->>ICV: Click claim button
sequenceDiagram
participant User as User
participant CCV as ConfirmClaimLinkView
participant State as App State
User->>CCV: Click "Go Back" button
CCV->>CCV: Execute handleGoBack (reset selectedRoute, setHasFetchedRoute false)
CCV->>State: Invoke onPrev callback
Possibly related PRs
Suggested reviewers
Poem
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: 0
🧹 Nitpick comments (3)
src/components/Claim/Link/Initial.view.tsx (3)
398-403: Consider reusing the existing chain and token comparison.The
isCrossChainTransactionlogic duplicates the inverse ofisSameChainAndToken. Consider refactoring to reuse the existing comparison:-const isCrossChainTransaction = useMemo(() => { - return ( - selectedChainID !== claimLinkData.chainId || - !areTokenAddressesEqual(selectedTokenAddress, claimLinkData.tokenAddress) - ) -}, [selectedChainID, selectedTokenAddress, claimLinkData.chainId, claimLinkData.tokenAddress]) +const isCrossChainTransaction = useMemo(() => !isSameChainAndToken, [isSameChainAndToken])
406-418: Add error handling for route fetching.While the effect correctly manages states, it should handle potential errors from
fetchRoute:useEffect(() => { if (isCrossChainTransaction && !selectedRoute && !isXchainLoading) { setIsXchainLoading(true) setLoadingState('Fetching route') setHasFetchedRoute(true) setErrorState({ showError: false, errorMessage: '', }) - fetchRoute() + fetchRoute().catch((error) => { + console.error('Error fetching route:', error) + setErrorState({ + showError: true, + errorMessage: 'Failed to fetch route. Please try again.', + }) + }) } }, [isCrossChainTransaction])
495-529: Consider extracting route display into a separate component.The route display logic is complex and could benefit from being extracted into a reusable component:
// RouteDisplay.tsx interface RouteDisplayProps { selectedRoute: any isXchainLoading: boolean claimLinkData: any supportedSquidChainsAndTokens: any supportedPeanutChains: any } const RouteDisplay: React.FC<RouteDisplayProps> = ({ selectedRoute, isXchainLoading, claimLinkData, supportedSquidChainsAndTokens, supportedPeanutChains, }) => { if (isXchainLoading || !selectedRoute) { return <div className="h-2 w-12 animate-colorPulse rounded bg-slate-700"></div> } const fromChain = supportedPeanutChains.find( (chain) => chain.chainId === selectedRoute.route.params.fromChain )?.name const toChain = supportedSquidChainsAndTokens[selectedRoute.route.params.toChain]?.axelarChainName return ( <> {fromChain} <Icon name={'arrow-next'} className="h-4 fill-gray-1" /> {toChain} <MoreInfo text={`You are bridging ${claimLinkData.tokenSymbol.toLowerCase()} on ${fromChain} to ${selectedRoute.route.estimate.toToken.symbol.toLowerCase()} on ${toChain}.`} /> </> ) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/components/Claim/Link/Initial.view.tsx(6 hunks)src/components/Claim/Link/Onchain/Confirm.view.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: prettier
🔇 Additional comments (5)
src/components/Claim/Link/Onchain/Confirm.view.tsx (3)
28-29: LGTM! Props addition for route state management.The new props
setSelectedRouteandsetHasFetchedRouteare correctly added to manage route state during back navigation.
102-108: LGTM! Clean implementation of back navigation logic.The
handleGoBackfunction follows the single responsibility principle by encapsulating the route state reset logic. It correctly checks forselectedRouteexistence before resetting the state, preventing potential issues with stale route data.
261-264: LGTM! Button implementation with proper state handling.The back button correctly uses the new
handleGoBackfunction while maintaining loading state handling and dark mode styling.src/components/Claim/Link/Initial.view.tsx (2)
295-308: LGTM! Clear and maintainable chain and token comparison.The refactored code improves readability by using a descriptive variable name
isSameChainAndToken. The reset logic is correctly applied with helpful comments.
572-572: LGTM! Button logic correctly handles cross-chain scenarios.The button logic correctly uses
isCrossChainTransactionfor navigation and properly disables the button when a route is required but not available.Also applies to: 586-586
2 participants
fixes TASK-8847 where claim cta would change when navigating b/w views in claim flow, now fixed it to be consitent
Summary by CodeRabbit