Skip to content

Conversation

@crutch12
Copy link
Contributor

@crutch12 crutch12 commented Dec 3, 2025

Summary

Fixes #6603

To Test

  1. Switch to mobile mode (screen width <960px)
  2. Click Network Selector
  3. Click Network Selector again

Network Selector should be closed on the second click

Background

I'm not sure why useOnClickOutside doesn't accept node ref in mobile mode. So I've added Control button ref in useOnClickOutside arg array on mobile. Now second click doesn't trigger useOnClickOutside

Summary by CodeRabbit

  • Bug Fixes
    • Improved network selector's click-outside detection to ensure it reliably closes when clicking outside the component, enhancing the click detection area accuracy.

✏️ Tip: You can customize this high-level summary in your review settings.

@vercel
Copy link

vercel bot commented Dec 3, 2025

@crutch12 is attempting to deploy a commit to the cow Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Walkthrough

A new DOM reference (nodeSelector) is introduced to the NetworkSelector component and integrated into the click-outside detection logic. On medium-sized viewports, both the mobile node and selector control node are now monitored, fixing the double toggle behavior that occurred when clicking the network selector on mobile devices.

Changes

Cohort / File(s) Summary
Network Selector Click-Outside Detection Fix
apps/cowswap-frontend/src/legacy/components/Header/NetworkSelector/index.tsx
Added nodeSelector ref to SelectorControls element and updated the useOnClickOutside hook to monitor both nodeMobile and nodeSelector on medium viewports, preventing outside-click detection from triggering on the selector control itself

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Single file modification with a focused bug fix
  • Straightforward addition of a ref and array update
  • No complex logic or new functionality introduced

Suggested reviewers

  • elena-zh
  • limitofzero

Poem

🐰 A click twice told, now works as planned,
The selector stays where it should stand,
Mobile no longer toggles in jest,
Outside detection knows best! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the fix for the mobile-specific bug where the Network Selector closes and immediately reopens on the second click.
Linked Issues check ✅ Passed The PR successfully addresses the core objective of issue #6603 by including the Control button in useOnClickOutside for mobile, preventing the second click from being treated as an outside click.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the mobile Network Selector outside-click behavior described in issue #6603; no extraneous modifications are present.
Description check ✅ Passed The PR description includes the required Summary, To Test, and Background sections with clear, actionable information.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

@vercel
Copy link

vercel bot commented Dec 3, 2025

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

Project Deployment Preview Updated (UTC)
swap-dev Ready Ready Preview Dec 3, 2025 10:40am

Copy link
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 for fixing this

@shoom3301 shoom3301 merged commit 443a61e into cowprotocol:develop Dec 4, 2025
9 of 15 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Dec 4, 2025
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.

Mobile: Network selector doesn't close on the second click

3 participants