-
Notifications
You must be signed in to change notification settings - Fork 122
fix(trade): fixed yield widget navigation issue #5790
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe updates adjust the logic for activating the Yield menu link and for resetting trade state in the frontend. The active state logic for the Yield route now accounts for chain ID, and the trade state reset is prevented when navigating to a Yield route with empty tokens, ensuring the Yield widget can open as intended. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TradeWidgetLinks
participant useSetupTradeState
User->>TradeWidgetLinks: Navigate to Yield widget
TradeWidgetLinks->>TradeWidgetLinks: Check if Yield route is active (with chain ID)
Note right of TradeWidgetLinks: Active state set using stricter path check
User->>useSetupTradeState: Open Yield widget with empty tokens
useSetupTradeState->>useSetupTradeState: Check if tokens are empty & if route is Yield
alt Route is Yield & tokens are empty
useSetupTradeState-->>User: Do not reset trade state
else
useSetupTradeState-->>User: Reset trade state
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes found. Suggested reviewers
Poem
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Ready to review, but putting in DRAFT to not have 3 PRs open at a time. |
const INITIAL_CHAIN_ID_FROM_URL = getRawCurrentChainIdFromUrl() | ||
const EMPTY_TOKEN_ID = '_' | ||
|
||
// TODO: Break down this large function into smaller functions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also addressed this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @fairlighteth , great job!
There is only one issue:
-
open yield tab
-
connect to a wallet
-
using network selector, switch to another network --> tokens will be prepopulated into the form, however, should not
-
go to the swap/limit/twap --> tokens will not be picked, tokens from the previous network will be displayed in the URL
I'm approving this PR with this issue as it can be addressed when we start working on the Yield again in the future.
Summary
Fixes #5424
Fixed Yield widget navigation issue where clicking the Yield menu item would redirect users back to Swap page instead of
opening the Yield widget.
To Test
Navigate to Yield widget
/yield
route without redirecting back to SwapUnlock screen functionality
Other trade types still work
Background
The issue was in
useSetupTradeState.ts
where the trade state logic was resetting routes with empty tokens (no input/output currency) back to the default Swap page. This reset logic was intended for Swap/Limit/Advanced routes that require tokens, but was incorrectly being applied to the Yield route which is designed to work without pre-selected tokens and has its own token selection interface.The fix excludes Yield routes from the "tokens are empty" reset logic while preserving the existing behavior for other trade types.
Summary by CodeRabbit