Skip to content

fix: prevent surplus modal from re-queuing auto-shown orders#7213

Merged
kernelwhisperer merged 5 commits intocowprotocol:developfrom
minato32:fix/surplus-modal-auto-requeue-prevention
Apr 7, 2026
Merged

fix: prevent surplus modal from re-queuing auto-shown orders#7213
kernelwhisperer merged 5 commits intocowprotocol:developfrom
minato32:fix/surplus-modal-auto-requeue-prevention

Conversation

@minato32
Copy link
Copy Markdown
Contributor

@minato32 minato32 commented Mar 23, 2026

Summary

Fixes #7189

Fix progress modal not opening when clicking "Show progress" from the account modal after navigating away from the swap page.

When a swap is submitted, tradeConfirmStateAtom is set to { isOpen: true, transactionHash: orderId }. If the user navigates away from the swap page (e.g., to /account/tokens), TradeWidgetModals unmounts but the atom state persists. This causes SurplusModalSetup to think the confirmation modal is already displaying the order, so it refuses to open the progress modal and immediately removes the order from the surplus queue.

The fix resets the trade confirm state when TradeWidgetModals unmounts, so SurplusModalSetup correctly sees isConfirmationModalOpen: false after navigation.

To Test

  1. Connect wallet and initiate a swap on the swap page
  • Verify the confirmation/progress modal appears on the swap page
  1. While the order is still pending, navigate to the account tokens page (/account/tokens)
  • Verify the confirmation modal closes on navigation
  1. Open the account modal (click wallet address in the header) and find the pending order
  • Verify the "Show progress" link is visible next to the pending order
  • Click "Show progress" — verify the progress modal opens successfully
  1. Dismiss the progress modal
  • Verify the modal stays dismissed and does not reappear
  1. Navigate back to the swap page and submit another swap
  • Verify the confirmation modal still works normally on the swap page
  • Verify dismissing the confirmation modal works as expected

Background

SurplusModalSetup (rendered in TopLevelModals, always mounted) checks tradeConfirmStateAtom.isOpen to avoid showing a duplicate modal when the trade confirmation is already visible. However, tradeConfirmStateAtom is global Jotai state that was never cleaned up on navigation — only on account/chain changes. This meant the stale isOpen: true blocked the surplus modal from ever opening after the user left the swap page.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where trade confirmation modal state could persist with stale data when navigating away from the trade widget, preventing incorrect modal visibility on subsequent interactions.

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 23, 2026

@minato32 is attempting to deploy a commit to the cow-dev Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 23, 2026

Walkthrough

A cleanup useEffect was added to the TradeWidgetModals component to invoke closeTradeConfirm() when unmounting, preventing stale modal state visibility. The file-level ESLint complexity directive was removed.

Changes

Cohort / File(s) Summary
Modal State Cleanup
apps/cowswap-frontend/src/modules/trade/containers/TradeWidget/TradeWidgetModals.tsx
Added cleanup useEffect hook that triggers closeTradeConfirm() on component unmount to clear trade confirm modal state and prevent stale UI state in downstream SurplusModalSetup behavior. Removed file-level ESLint complexity disable directive.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 A modal state that lingered long,
Now springs to life, then clears ere long.
With cleanup hooks, the tale is told—
Fresh starts await, no stale to hold!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main fix: preventing the surplus modal from being blocked by stale trade confirm state when re-queuing orders after navigation.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description check ✅ Passed The pull request description is comprehensive and well-structured, following the template with all major sections completed including Summary with issue reference, detailed testing steps with checkboxes, and background context explaining the root cause.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@minato32 minato32 force-pushed the fix/surplus-modal-auto-requeue-prevention branch from e1469d6 to d67da26 Compare March 24, 2026 08:06
@minato32 minato32 marked this pull request as ready for review March 24, 2026 08:09
@vercel
Copy link
Copy Markdown

vercel bot commented Mar 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
swap-dev Ready Ready Preview Mar 24, 2026 11:09am

Request Review

@minato32
Copy link
Copy Markdown
Contributor Author

fixes #7189

@elena-zh elena-zh requested a review from a team March 24, 2026 14:36
Copy link
Copy Markdown
Contributor

@elena-zh elena-zh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you, LGTM!

@kernelwhisperer kernelwhisperer enabled auto-merge (squash) April 7, 2026 12:50
@kernelwhisperer kernelwhisperer disabled auto-merge April 7, 2026 12:50
@kernelwhisperer kernelwhisperer enabled auto-merge (squash) April 7, 2026 13:05
@kernelwhisperer kernelwhisperer merged commit b8d38ed into cowprotocol:develop Apr 7, 2026
9 of 16 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 7, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Impossible to open Progress bar modal from Account pages

5 participants