Skip to content

Fix Search to clear the token address from token-atlas#410

Merged
RanaBug merged 1 commit intostagingfrom
fix/PRO-3690/change-token-atlas-buy-to-pulse
Sep 25, 2025
Merged

Fix Search to clear the token address from token-atlas#410
RanaBug merged 1 commit intostagingfrom
fix/PRO-3690/change-token-atlas-buy-to-pulse

Conversation

@RanaBug
Copy link
Collaborator

@RanaBug RanaBug commented Sep 24, 2025

Description

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Summary by CodeRabbit

  • New Features

    • After selecting a token, the app now cleans up the URL by removing temporary query parameters, providing cleaner, shareable links and smoother navigation.
  • Bug Fixes

    • Sell mode searches no longer auto-populate from URL token parameters; token prefill from URL applies only in Buy mode, preventing incorrect asset selection and reducing confusion.

@RanaBug RanaBug requested a review from IAmKio September 24, 2025 15:58
@RanaBug RanaBug self-assigned this Sep 24, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 24, 2025

Walkthrough

Updates Buy and Search components to adjust token selection flow: adds navigation-based query cleanup after selecting a token and restricts reading the asset URL parameter to buy mode. Minor eslint dependency comment added.

Changes

Cohort / File(s) Summary
Buy flow URL cleanup
src/apps/pulse/components/Buy/Buy.tsx
Imports useNavigate, adds removeQueryParams helper to clear query params via navigation, invokes it after successful token selection from token-atlas and search results, and adds an eslint-disable for effect deps related to the cleanup call.
Search asset param gating
src/apps/pulse/components/Search/Search.tsx
Reads asset from URL only when isBuy is true; guards isAddress(tokenAddress) check to prevent sell searches from using asset query; adds explanatory comment.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant Search as Search (isBuy)
  participant Atlas as Token Atlas
  participant Buy as Buy
  participant Nav as Router

  rect rgba(220,245,255,0.5)
    Note over Search,Atlas: Token selection via URL or search (Buy mode)
    U->>Search: Open Buy with ?asset=...
    Search-->>Atlas: Resolve token (if isBuy)
    Atlas-->>Buy: Token selected
  end

  rect rgba(230,255,230,0.5)
    Note over Buy,Nav: Post-selection URL cleanup (new)
    Buy->>Nav: navigate(currentPath, { replace, search: "" })
    Nav-->>U: URL without query params
  end

  rect rgba(255,240,220,0.5)
    Note over Search: Sell mode behavior (unchanged + guard)
    U->>Search: Open Sell with ?asset=...
    Search-->>Search: Ignore asset when !isBuy
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • IAmKio

Poem

A hop and a pop through URLs I go,
Tidying trails where query seeds grow.
Buy-bunny selects, then sweeps the lane,
While Sell stays neat, untouched by the chain.
With a twirl of the tail, I navigate clean—
Tokens in basket, paths pristine. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR uses the repository template but the required sections are essentially empty: the Description and How Has This Been Tested fields contain only placeholders and lack a concise summary, test steps, environments, or expected results; there are no screenshots or implementation details documenting the changes shown in the raw_summary (modifications to Buy.tsx and Search.tsx to clear query params and guard asset reads). Because these required details are missing the description is incomplete and does not meet the repository's template requirements. Please update the PR description to include a concise Description that lists the files changed and the intent (what was fixed and why), a How Has This Been Tested section with step-by-step test instructions and environment details, any relevant screenshots or repro steps, and ensure the Types of changes checkboxes accurately reflect the change.
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 (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title "Fix Search to clear the token address from token-atlas" concisely and accurately reflects the primary change in the diffs: updates to Search/Buy flows to clear token-address query parameters originating from token-atlas. It is short, specific, and directly tied to the file-level summaries provided.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/PRO-3690/change-token-atlas-buy-to-pulse

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.

@cloudflare-workers-and-pages
Copy link

Deploying x with  Cloudflare Pages  Cloudflare Pages

Latest commit: 9bccc4e
Status: ✅  Deploy successful!
Preview URL: https://193111e3.x-e62.pages.dev
Branch Preview URL: https://fix-pro-3690-change-token-at.x-e62.pages.dev

View logs

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/apps/pulse/components/Search/Search.tsx (1)

145-145: Consider using the shared removeQueryParams helper.

The removeQueryParams function defined in the Buy component (lines 84-86) could be extracted to a shared utility to avoid duplication, since both components perform identical URL cleanup operations.

Apply this refactor to share the utility:

+// Create a shared utility file: src/apps/pulse/utils/navigation.ts
+import { useLocation, useNavigate } from 'react-router-dom';
+
+export const useRemoveQueryParams = () => {
+  const navigate = useNavigate();
+  const location = useLocation();
+  
+  return () => {
+    navigate(location.pathname, { replace: true });
+  };
+};

Then update both components to use the shared utility:

-  const removeQueryParams = () => {
-    navigate(location.pathname, { replace: true });
-  };
+  const removeQueryParams = useRemoveQueryParams();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a430b17 and 9bccc4e.

📒 Files selected for processing (2)
  • src/apps/pulse/components/Buy/Buy.tsx (3 hunks)
  • src/apps/pulse/components/Search/Search.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.
📚 Learning: 2025-03-28T09:22:22.712Z
Learnt from: RanaBug
PR: pillarwallet/x#275
File: src/apps/the-exchange/components/DropdownTokensList/DropdownTokenList.tsx:180-195
Timestamp: 2025-03-28T09:22:22.712Z
Learning: In the Exchange app, `swapTokenList` and `receiveTokenList` are derived from `searchTokenResult` when search is active, so including `searchToken` in the useEffect dependency array that uses these lists would be redundant as the lists will update when search results change.

Applied to files:

  • src/apps/pulse/components/Buy/Buy.tsx
  • src/apps/pulse/components/Search/Search.tsx
⏰ 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 (5)
src/apps/pulse/components/Search/Search.tsx (1)

124-128: LGTM! Clean guard implementation for buy mode token address reading.

The replace: true option prevents the URL from being added to the browser history, which is appropriate for cleaning up URL parameters. The guard ensures the asset parameter is only read when in buy mode, aligning with the PR objective to prevent token addresses from appearing in sell searches.

src/apps/pulse/components/Buy/Buy.tsx (4)

14-14: LGTM! Added useNavigate for URL cleanup functionality.

The import aligns with the PR objective to clear query parameters after token selection.


77-77: LGTM! Clean URL parameter cleanup implementation.

The navigate hook and removeQueryParams helper provide a clean way to remove URL parameters by navigating to the current path without search parameters. Using replace: true prevents adding a new history entry, which is appropriate for cleanup operations.

Also applies to: 84-86


394-396: LGTM! Proper cleanup after token-atlas auto-selection.

Calling removeQueryParams() after successfully selecting a token from the token-atlas flow ensures the URL is cleaned up, preventing the asset parameter from interfering with subsequent searches.


399-399: ESLint disable is appropriate for the cleanup dependency.

The eslint-disable comment is justified because the removeQueryParams function call in the useEffect is intentionally excluded from dependencies to avoid unnecessary re-renders when the cleanup function changes.

Copy link
Collaborator

@IAmKio IAmKio left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants