Sell quote and gas estimation pause fix#425
Conversation
WalkthroughIntroduces a pause mechanism for the sell flow. HomeScreen manages isSellFlowPaused and gates refresh/auto-refresh. PreviewSell accepts setSellFlowPaused, updates it based on transaction phases, and propagates pause to gas estimation. useGasEstimation adds an isPaused flag to suppress estimations during paused states. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant HS as HomeScreen
participant PS as PreviewSell
participant GE as useGasEstimation
U->>HS: Open Sell flow
HS->>PS: Render with setSellFlowPaused(...)
note over HS: Maintains isSellFlowPaused<br/>Gates refresh/auto-refresh
PS->>PS: Detect isWaitingForSignature / isExecuting
PS->>HS: setSellFlowPaused(true)
HS->>HS: Suppress refresh while paused
PS->>GE: useGasEstimation({ ..., isPaused: true })
GE-->>PS: Skip estimation (paused)
U->>PS: Confirm tx / execution completes
PS->>HS: setSellFlowPaused(false)
PS->>GE: useGasEstimation({ ..., isPaused: false })
GE-->>PS: Estimate gas (active)
HS->>HS: Auto-refresh resumes (not paused)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Deploying x with
|
| Latest commit: |
bc1f544
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://81e136ee.x-e62.pages.dev |
| Branch Preview URL: | https://feat-pro-3681-transaction-st.x-e62.pages.dev |
![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 (1)
src/apps/pulse/components/Sell/PreviewSell.tsx (1)
350-425: Consider removing redundant pause state updates.The manual calls to
setSellFlowPausedat lines 361, 383, and 423 are redundant since the effect at lines 118-123 already synchronizes the pause state wheneverisWaitingForSignatureorisExecutingchanges. The effect will automatically callsetSellFlowPaused(true)whenisExecutingbecomestrue(line 360) andsetSellFlowPaused(false)when it becomesfalse(lines 382, 422).Remove the manual calls for cleaner code:
setIsWaitingForSignature(true); setIsExecuting(true); - if (setSellFlowPaused) setSellFlowPaused(true); try {setIsTransactionSuccess(true); setIsWaitingForSignature(false); setIsExecuting(false); - if (setSellFlowPaused) setSellFlowPaused(false); // Clean up the batch from kit after successful executionsetIsExecuting(false); - if (setSellFlowPaused) setSellFlowPaused(false); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/apps/pulse/components/App/HomeScreen.tsx(6 hunks)src/apps/pulse/components/Sell/PreviewSell.tsx(9 hunks)src/apps/pulse/hooks/useGasEstimation.ts(4 hunks)
⏰ 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). (3)
- GitHub Check: unit-tests
- GitHub Check: lint
- GitHub Check: build
🔇 Additional comments (10)
src/apps/pulse/hooks/useGasEstimation.ts (3)
18-26: LGTM! Well-designed pause mechanism.The
isPausedflag is properly integrated with clear defaults and correct parameter passing. This design allows callers to pause gas estimation during critical transaction phases without additional complexity.
46-48: Correct guard logic for paused state.The short-circuit condition properly prevents gas estimation when paused, avoiding unnecessary network calls and potential state conflicts during transaction execution.
141-141: Dependencies correctly include isPaused.Both the
useCallbackanduseEffectdependency arrays properly includeisPaused, ensuring the hook responds to pause state changes and maintains correct closure semantics.Also applies to: 153-158
src/apps/pulse/components/App/HomeScreen.tsx (4)
121-121: LGTM! Clear pause state management.The
isSellFlowPausedstate provides centralized control for pausing sell flow operations in HomeScreen.
184-186: Correct guard conditions prevent refresh during pause.The guards properly prevent manual and sell offer refreshes when the sell flow is paused, ensuring data consistency during transaction execution.
Also applies to: 195-201
738-759: Auto-refresh correctly respects pause state.The auto-refresh logic is properly gated by
isSellFlowPaused, preventing background updates during critical transaction phases. The dependency array is complete.
801-801: Pause control correctly delegated to PreviewSell.Passing
setSellFlowPausedenables PreviewSell to control the pause state based on transaction phases, establishing proper parent-child state coordination.src/apps/pulse/components/Sell/PreviewSell.tsx (3)
40-52: LGTM! Clean prop integration and computed pause state.The
setSellFlowPausedprop is properly typed as optional and the computedisPausedstate correctly combines transaction phases for gas estimation.Also applies to: 81-82
118-130: LGTM! Declarative pause state synchronization.The effect at lines 118-123 provides clean, declarative synchronization of the pause state to the parent component based on
isWaitingForSignatureandisExecuting. The cleanup effect ensures proper reset on unmount, preventing stale pause states.
226-229: Correct guard prevents refresh during transaction phases.Blocking refresh while waiting for signature or executing prevents quote staleness and potential user confusion during active transactions.
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Bug Fixes