feat: new swap action card flow #10353
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds AwaitingApproval/AwaitingSwap statuses across types, icons, tags, translations, and SwapActionCard titles. Refactors useSwapActionSubscriber to sync swap actions for all swaps from confirmedTradeExecution, upserting/updating status and approval metadata. Simplifies useAllowanceApproval by removing Action Center/toast side effects, recording txHash and completion only. Broadens SwapActionCard collapsibility. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant TC as TradeConfirm (useAllowanceApproval)
participant BC as Blockchain
participant ST as Store (Redux)
participant SUB as useSwapActionSubscriber
participant UI as Action Center UI
U->>TC: Initiate swap
alt Approval required
TC->>BC: Submit allowance tx
TC->>ST: setAllowanceApprovalTxPending
BC-->>TC: txHash
TC->>ST: setAllowanceApprovalTxHash
TC->>BC: wait for receipt
BC-->>TC: receipt
TC->>ST: setAllowanceApprovalTxComplete
end
loop Poll/sync
SUB->>ST: read swaps + confirmedTradeExecution
SUB->>SUB: Map to ActionStatus (AwaitingApproval / AwaitingSwap / Pending / ...)
SUB->>ST: Upsert/update SwapAction with status + approval metadata
end
ST-->>UI: Action updates
UI-->>U: Show status tag/icon and title
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit can use your project's `biome` configuration to improve the quality of JS/TS/CSS/JSON code reviews.Add a configuration file to your project to customize how CodeRabbit runs ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
premiumjibles
force-pushed
the
10209_swap_action_approvals
branch
3 times, most recently
from
123c678 to
a78597d
Compare
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
Show resolved
Hide resolved
premiumjibles
force-pushed
the
10209_swap_action_approvals
branch
3 times, most recently
from
fd5f66f to
993cddd
Compare
premiumjibles
force-pushed
the
10209_swap_action_approvals
branch
from
993cddd to
b36ec7a
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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsx (1)
45-45: Fix: Possible crash when swap is undefined
swap.swapperNameis dereferenced without a null check, whileswapcan be undefined (computed from store and used with optional chaining elsewhere). This will throw on initial renders or transient states.Apply one of these diffs (keeping useMemo is preferred per our hooks guidelines):
- const isArbitrumBridge = useMemo(() => swap.swapperName === SwapperName.ArbitrumBridge, [swap]) + const isArbitrumBridge = useMemo( + () => swap?.swapperName === SwapperName.ArbitrumBridge, + [swap?.swapperName], + )Alternatively (if you relax the memo requirement):
- const isArbitrumBridge = useMemo(() => swap.swapperName === SwapperName.ArbitrumBridge, [swap]) + const isArbitrumBridge = swap?.swapperName === SwapperName.ArbitrumBridge
🧹 Nitpick comments (4)
src/assets/translations/en/main.json (1)
3067-3069: Add a “swap.initiated” message to match Initiated status (used for Arbitrum bridge special-case).SwapActionCard may need a description string for Initiated. Add a key for consistency with other statuses.
Apply this diff near the existing swap messages:
"swap": { "processing": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} is being processed.", "complete": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} is complete.", "failed": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} has failed.", + "initiated": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} has been initiated.", "streaming": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} is streaming.", "awaitingApproval": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} is awaiting approval.", "awaitingSwap": "Your swap of %{sellAmountAndSymbol} to %{buyAmountAndSymbol} is awaiting execution." },src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx (1)
97-117: Guard viem receipt wait with try/catch and mark failure on errors/timeouts.If waitForTransactionReceipt throws, we’ll have set the txHash but never mark the step failed, leaving UI stuck. Wrap in try/catch and dispatch failure + toast on error; optionally add a timeout to avoid indefinite waits.
Apply this diff:
- const publicClient = assertGetViemClient(tradeQuoteStep.sellAsset.chainId) - await publicClient.waitForTransactionReceipt({ hash: txHash as Hash }) - - dispatch( - tradeQuoteSlice.actions.setAllowanceApprovalTxComplete({ - hopIndex, - id: confirmedTradeId, - }), - ) + try { + const publicClient = assertGetViemClient(tradeQuoteStep.sellAsset.chainId) + await publicClient.waitForTransactionReceipt({ + hash: txHash as Hash, + // consider: confirmations: 1, timeout: 300000 + }) + dispatch( + tradeQuoteSlice.actions.setAllowanceApprovalTxComplete({ + hopIndex, + id: confirmedTradeId, + }), + ) + } catch (e) { + dispatch( + tradeQuoteSlice.actions.setAllowanceApprovalTxFailed({ + hopIndex, + id: confirmedTradeId, + }), + ) + showErrorToast(e) + }src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx (1)
121-182: Sync effect is correct; consider a light optimization to avoid unnecessary upserts.Current logic only upserts when status changes — good. If allowanceApproval metadata changes without a status change, we won’t update the action. If you want the txHash to surface immediately in the card (e.g., collapsible state), also upsert when allowanceApproval.txHash transitions from undefined → defined.
Example tweak inside the else-if branch:
- } else if (isSwapAction(existingAction) && existingAction.status !== targetStatus) { + } else if ( + isSwapAction(existingAction) && + (existingAction.status !== targetStatus || + existingAction.swapMetadata.allowanceApproval?.txHash !== approvalMetadata?.txHash) + ) {src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsx (1)
48-50: Consider auto-opening for actionable Awaiting states*Opening only on Pending might hide the actionable “Awaiting Approval/Swap” steps by default. Consider expanding for these statuses to reduce friction.
Example:
const { isOpen, onToggle } = useDisclosure({ - defaultIsOpen: - action.status === ActionStatus.Pending && Boolean(swap?.isStreaming || swap?.txLink), + defaultIsOpen: + [ActionStatus.Pending, ActionStatus.AwaitingApproval, ActionStatus.AwaitingSwap].includes( + action.status, + ) && + Boolean( + swap?.isStreaming || + swap?.txLink || + action.swapMetadata?.allowanceApproval?.txHash, + ), })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (8)
src/assets/translations/en/main.json(2 hunks)src/components/Layout/Header/ActionCenter/ActionCenter.tsx(1 hunks)src/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsx(1 hunks)src/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsx(1 hunks)src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsx(1 hunks)src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx(1 hunks)src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx(4 hunks)src/state/slices/actionSlice/types.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{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/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/state/slices/actionSlice/types.tssrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.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/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
**/*
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/*: ALWAYS use appropriate file extensions
Flag files without kebab-case
Files:
src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/state/slices/actionSlice/types.tssrc/assets/translations/en/main.jsonsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.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/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.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/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsxsrc/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/state/slices/actionSlice/types.tssrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
**/use*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/naming-conventions.mdc)
**/use*.{ts,tsx}: ALWAYS use use prefix for custom hooks
ALWAYS use descriptive names that indicate the hook's purpose
ALWAYS use camelCase after the use prefix for custom hooks
Files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
🧠 Learnings (15)
📚 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} : Avoid side effects in swap logic within swapper implementation files.
Applied to files:
src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsxsrc/components/Layout/Header/ActionCenter/ActionCenter.tsxsrc/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-08T14:59:40.422Z
Learnt from: NeOMakinG
PR: shapeshift/web#10231
File: src/pages/Explore/ExploreCategory.tsx:231-238
Timestamp: 2025-08-08T14:59:40.422Z
Learning: In src/pages/Explore/ExploreCategory.tsx, for the PageHeader filter trigger, NeOMakinG considers changing a clickable Chakra Icon to IconButton too nitpicky for this PR and prefers to keep the current Icon-based trigger; such minor a11y/UI nitpicks should be deferred to a follow-up if needed.
Applied to files:
src/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsx
📚 Learning: 2025-08-22T12:59:01.210Z
Learnt from: NeOMakinG
PR: shapeshift/web#10323
File: src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx:26-29
Timestamp: 2025-08-22T12:59:01.210Z
Learning: In src/components/Layout/Header/ActionCenter/components/RewardDistributionActionCard.tsx, NeOMakinG declined wrapping the RewardDistributionActionCard component with React.memo, saying it was "too much", suggesting that like other action center components, memoization is not beneficial for this specific use case.
Applied to files:
src/components/Layout/Header/ActionCenter/ActionCenter.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/Layout/Header/ActionCenter/ActionCenter.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} : Leverage shared utilities (e.g., executeEvmTransaction, checkEvmSwapStatus) in swapper implementations when applicable.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-22T15:07:18.021Z
Learnt from: kaladinlight
PR: shapeshift/web#10326
File: src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx:37-41
Timestamp: 2025-08-22T15:07:18.021Z
Learning: In src/hooks/useActionCenterSubscribers/useThorchainLpActionSubscriber.tsx, kaladinlight prefers not to await the upsertBasePortfolio call in the Base chain handling block, indicating intentional fire-and-forget behavior for Base portfolio upserts in the THORChain LP completion flow.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.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} : Include comments explaining swap logic in swapper implementation files.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.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 **/swapper/**/*.{ts,tsx} : ALWAYS use TradeQuoteError enum for error codes in swapper errors
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsxsrc/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.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/**/*.ts : Use TypeScript with explicit types (e.g., SupportedChainIds) in all swapper-related files.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-04T15:36:25.122Z
Learnt from: NeOMakinG
PR: shapeshift/web#10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T15:36:25.122Z
Learning: In swap transaction handling, buy transaction hashes should always use the swapper's explorer (stepSource) because they are known by the swapper immediately upon swap execution. The conditional logic for using default explorers applies primarily to sell transactions which need to be detected/indexed by external systems like Thorchain or ViewBlock.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-04T16:02:27.360Z
Learnt from: NeOMakinG
PR: shapeshift/web#10171
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandedStepperSteps.tsx:458-458
Timestamp: 2025-08-04T16:02:27.360Z
Learning: In multi-hop swap transactions, last hop sell transactions might not be detected by the swapper (unlike buy transactions which are always known immediately). The conditional stepSource logic for last hop buy transactions (`isLastHopSellTxSeen ? stepSource : undefined`) serves as defensive programming for future multi-hop support with intermediate chains, even though multi-hop functionality is not currently supported in production.
Applied to files:
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
📚 Learning: 2025-08-08T11:40:55.734Z
Learnt from: NeOMakinG
PR: shapeshift/web#10234
File: src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx:41-41
Timestamp: 2025-08-08T11:40:55.734Z
Learning: In MultiHopTrade confirm flow (src/components/MultiHopTrade/components/TradeConfirm/TradeConfirm.tsx and related hooks), there is only one active trade per flow. Because of this, persistent (module/Redux) dedupe for QuotesReceived in useTrackTradeQuotes is not necessary; the existing ref-based dedupe is acceptable.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-07-29T15:04:28.083Z
Learnt from: NeOMakinG
PR: shapeshift/web#10139
File: src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx:109-115
Timestamp: 2025-07-29T15:04:28.083Z
Learning: In src/components/MultiHopTrade/components/TradeConfirm/components/ExpandableStepperSteps.tsx, the component is used under an umbrella that 100% of the time contains the quote, making the type assertion `activeTradeQuote?.steps[currentHopIndex] as TradeQuoteStep` safe. Adding conditional returns before hooks would violate React's Rules of Hooks.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.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/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
📚 Learning: 2025-08-11T09:46:41.060Z
Learnt from: gomesalexandre
PR: shapeshift/web#10219
File: src/components/MultiHopTrade/components/TradeInput/TradeInput.tsx:167-172
Timestamp: 2025-08-11T09:46:41.060Z
Learning: In the shapeshift/web repository, the display cache logic for trade quotes (using `selectUserAvailableTradeQuotes` and `selectUserUnavailableTradeQuotes`) is intentionally kept the same between `TradeInput.tsx` and `TradeQuotes.tsx` components. The `hasQuotes` computation in `TradeInput.tsx` uses these display cache selectors by design, matching the pattern used in `TradeQuotes.tsx`.
Applied to files:
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
🧬 Code graph analysis (1)
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx (7)
packages/swapper/src/types.ts (1)
Swap(384-407)packages/caip/src/constants.ts (1)
ethChainId(60-60)src/state/store.ts (2)
useAppSelector(143-143)store(137-137)src/state/slices/tradeQuoteSlice/selectors.ts (1)
selectConfirmedTradeExecution(540-547)src/state/slices/actionSlice/selectors.ts (1)
selectSwapActionBySwapId(123-137)src/state/slices/actionSlice/actionSlice.ts (1)
actionSlice(11-39)src/state/slices/actionSlice/types.ts (1)
isSwapAction(168-172)
⏰ 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 (13)
src/state/slices/actionSlice/types.ts (1)
30-31: All clear: downstream selectors use strictstatus === ActionStatus.Pendingchecks, so the newAwaitingApprovalandAwaitingSwapvalues won’t be counted. No changes needed.src/assets/translations/en/main.json (1)
3052-3055: Status translations added — consistent with new enum values.Keys and casing match ActionStatus. No issues.
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx (1)
8-9: Type-only import split is fine.Import changes are correct and keep types tree-shakeable. No issues.
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx (4)
46-51: Direct mapping table is clear and aligns with enum semantics.The SwapStatus → ActionStatus mapping is straightforward and maintainable.
399-410: Polling queries limited to Pending swaps — matches desired flow.Query keys include swap id and sellTxHash, and enabled gating prevents unnecessary polling for Awaiting* states. Looks good.
53-88: Initiated state handling confirmed; no further UI updates requiredVerified explicit handling for
ActionStatus.Initiatedin:
SwapActionCard.tsx(returnsactionCenter.bridge.initiated)SwapNotification.tsx(returnsactionCenter.bridge.initiated)ActionStatusTag.tsx(renders a tag forActionStatus.Initiated)ActionStatusIcon.tsx(includes a case forActionStatus.Initiated)en/main.jsontranslations (maps"initiated": "Initiated")All concerns around UI coverage for the Initiated state are addressed.
184-185: No external swapStatusHandler callers found—safe to proceedI’ve searched the entire repo and confirmed that
swapStatusHandleris only declared and used withinuseSwapActionSubscriber.tsx(at lines 183, 408, and 414). There are no other invocations elsewhere, so no additional call sites need updating.src/components/Layout/Header/ActionCenter/ActionCenter.tsx (1)
79-81: Collapsibility now considers allowance approval txHash — good UX improvement.This makes the card collapsible as soon as the approval hash exists, even before the swap txLink is known.
src/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsx (1)
53-59: LGTM: Awaiting statuses render with an orange GenericTx icon*The visual treatment and grouping of both statuses are consistent with the rest of the switch and with ActionStatusTag.
src/components/Layout/Header/ActionCenter/components/SwapActionCard.tsx (3)
122-126: Guard against undefined components prop
swapNotificationTranslationComponentsreturnsundefinedwhenswapis falsy. Most translation components handle this, but confirm the implementation toleratescomponents={undefined}. If not, default to{}.You can make this change proactively:
- const description = useMemo(() => { - return ( - <Text fontSize='sm' translation={title} components={swapNotificationTranslationComponents} /> - ) - }, [title, swapNotificationTranslationComponents]) + const description = useMemo(() => { + return ( + <Text + fontSize='sm' + translation={title} + components={swapNotificationTranslationComponents ?? {}} + /> + ) + }, [title, swapNotificationTranslationComponents])
144-156: Confirm error boundary coverage at the Action Center levelPer our TSX guidelines, components should be wrapped by an error boundary in production. If ActionCard or a parent wrapper already provides this, we’re good; otherwise, consider adding one at the Action Center container level to avoid per-item boundaries.
104-105: Missing translation keys in non-English locales for AwaitingApproval/AwaitingSwapOur verification shows that only the English locale defines the four new keys:
actionCenter.swap.awaitingApprovalactionCenter.swap.awaitingSwapactionCenter.status.awaitingApprovalactionCenter.status.awaitingSwapAll other
src/assets/translations/<locale>/main.jsonfiles (de,es,fr,id,ja,ko,pt,ru,tr,uk,zh) are missing one or more of these entries. As a result, users in those locales will fall back (or see missing text) instead of the intended translations.Please add the missing keys (with appropriate translations) to each
main.json. For example:• In
src/assets/translations/de/main.json:+ "actionCenter": { + "swap": { + "awaitingApproval": "Deine Swap von %{sellAmountAndSymbol} zu %{buyAmountAndSymbol} wartet auf Genehmigung.", + "awaitingSwap": "Deine Swap von %{sellAmountAndSymbol} zu %{buyAmountAndSymbol} wartet auf Ausführung." + }, + "status": { + "awaitingApproval": "Wartet auf Genehmigung", + "awaitingSwap": "Wartet auf Swap" + } + }(Repeat for each locale.)
After updating, please re-run:
for file in src/assets/translations/*/main.json; do jq -e ' .actionCenter.swap.awaitingApproval and .actionCenter.swap.awaitingSwap and .actionCenter.status.awaitingApproval and .actionCenter.status.awaitingSwap ' "$file" && echo "$file: OK" || echo "$file: Missing keys" doneto confirm all translation files include the new entries.
⛔ Skipped due to learnings
Learnt from: 0xApotheosis PR: shapeshift/web#10073 File: src/components/Layout/Header/ActionCenter/components/Details/ClaimDetails.tsx:10-11 Timestamp: 2025-07-24T21:05:13.642Z Learning: In the ShapeShift web repository, translation workflow follows a two-step process: 1) First PR adds only English translations to src/assets/translations/en/main.json, 2) Globalization team handles follow-up PRs to add keys to remaining language files (de, es, fr, id, ja, ko, pt, ru, tr, uk, zh). Don't suggest verifying all locale files simultaneously during initial feature PRs.Learnt from: premiumjibles PR: shapeshift/web#10291 File: src/assets/translations/en/main.json:216-218 Timestamp: 2025-08-17T23:39:00.407Z Learning: In the shapeshift/web project, translations for non-English locales are handled as a separate follow-up process by language experts, not as part of the initial PR that adds English keys to src/assets/translations/en/main.json.src/components/Layout/Header/ActionCenter/components/ActionStatusTag.tsx (1)
78-89: LGTM: Orange tags for AwaitingApproval/AwaitingSwapConsistent color scheme and translation keys; matches the icon semantics and the new flow.
NeOMakinG
left a comment
NeOMakinG
left a comment
There was a problem hiding this comment.
https://jam.dev/c/22db1ded-813d-4311-8d8b-8d351458b890
Looks super sexy, only one thing looking weird to me:
If you start a swap, then leave to do something else and never do it back again, you will always have a "waiting for swap" action card until you do another swap, can be done as a follow up but at some point we might want to consider a swap abandoned, we would need to find the perfect heuristics/way to do that
src/components/Layout/Header/ActionCenter/components/ActionStatusIcon.tsx
Show resolved
Hide resolved
src/components/MultiHopTrade/components/TradeConfirm/hooks/useAllowanceApproval.tsx
Show resolved
Hide resolved
src/hooks/useActionCenterSubscribers/useSwapActionSubscriber.tsx
Outdated
Show resolved
Hide resolved
|
@NeOMakinG actioned swap status change and rename here d9543b2 |
2 participants
Description
Polish the action card flow for swaps where it moves from Awaiting Approval -> Awaiting Swap -> Pending -> Complete/Failed.
Removes the dedicated approval action card as that's incorporated in the one card.
Issue (if applicable)
closes #10351
Risk
Low-medium risk, only affects the action cards for swaps
Testing
Test swaps on both native and non-native wallets
Test swaps that require and don't require approvals
Test swaps that require permit
Test bailing out of a swap before executing/approving. Card should stay but starting a new swap should wipe it out.
All swaps should be successful but should show the additional statuses of "awaiting approval" (if required) and "awaiting swap". No dedicated approval card should show but you should be able to see approval tx by expanding card as soon as approval is done
Engineering
☝️
Operations
☝️
Screenshots (if applicable)
https://jam.dev/c/f3987d13-b268-449a-8592-46df1b6eb330
Summary by CodeRabbit
New Features
Bug Fixes