-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat: MUSD-188 refactor musd conversion cta visibility #24335
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: feat/musd-187-musd-conversion-design-adjustments-ahead-of-uat
Are you sure you want to change the base?
feat: MUSD-188 refactor musd conversion cta visibility #24335
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
6aa40ad to
a2df8f0
Compare
…of-uat' into feat/musd-188-refactor-musd-conversion-cta-visibility
🔍 Smart E2E Test Selection⏭️ Smart E2E selection skipped - base branch is not main (base: feat/musd-187-musd-conversion-design-adjustments-ahead-of-uat) All E2E tests pre-selected. |
| }, [balancesPerChainId]); | ||
|
|
||
| const hasMusdBalanceOnChain = (chainId: Hex) => | ||
| Boolean(balancesByChain[chainId]); |
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.
Unmemoized function causes unnecessary callback recreations
Medium Severity
The hasMusdBalanceOnChain function is created inline without useCallback, producing a new function reference on every render. Since this function is included in the dependency arrays of shouldShowBuyGetMusdCta and shouldShowTokenListItemCta callbacks in useMusdCtaVisibility, those callbacks will be recreated on every render, defeating memoization. This can cause unnecessary re-renders in consuming components, particularly impactful in the token list which may display many items.
Additional Locations (1)
| describe('hook structure', () => { | ||
| it('returns object with hasMusdBalance and balancesByChain properties', () => { | ||
| const { result } = renderHook(() => useHasMusdBalance()); | ||
| it('returns object with hasMusdBalanceOnAnyChainOnAnyChain and balancesByChain properties', () => { |
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.
Test names contain repeated word typo (Bugbot Rules)
Low Severity
Test names contain hasMusdBalanceOnAnyChainOnAnyChain with duplicated text instead of hasMusdBalanceOnAnyChain. This violates the unit testing guidelines rule that test names must be "specific about the behavior being tested" - the typo makes the test descriptions confusing and inaccurate as documentation.
Additional Locations (1)
|



Description
Updated rendering conditions of mUSD conversion CTAs and added feature flags for each CTA.
Primary and Seconday CTA Rendering Conditions
Feature flags added
selectIsMusdCtaEnabledFlagtoselectIsMusdGetBuyCtaEnabledFlagselectIsMusdConversionTokenListItemCtaEnabledFlagselectIsMusdConversionAssetOverviewEnabledFlagselectIsMusdConversionRewardsUiEnabledFlagChangelog
CHANGELOG entry: refactored mUSD conversion CTA rendering conditions
Related issues
Fixes:
Manual testing steps
See rendering conditions table above.
Screenshots/Recordings
Before
Primary and Secondary CTAs could be rendered at the same time.
After
Primary or Secondary CTA rendered but never both at the same time.
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Consolidates and refines mUSD conversion CTA rendering across the app with explicit feature flags and shared visibility logic.
useMusdCtaVisibility(decidesBuy/Get mUSD, token list item, and asset overview CTAs) anduseMusdBalance(detects mUSD balances per chain)selectIsMusdGetBuyCtaEnabledFlag,selectIsMusdConversionTokenListItemCtaEnabledFlag,selectIsMusdConversionAssetOverviewEnabledFlag,selectIsMusdConversionRewardsUiEnabledFlag, andselectMusdConversionCTATokensuseMusdConversionTokens(removes CTA-specific checks; keeps filtering/support and output chain resolution)MusdConversionAssetListCta→shouldShowBuyGetMusdCta,TokenListItem→shouldShowTokenListItemCta,EarnLendingBalance→shouldShowAssetOverviewCta.js.env.examplewithMM_MUSD_CTA_ENABLED,MM_MUSD_CONVERSION_ASSET_OVERVIEW_CTA,MM_MUSD_CONVERSION_TOKEN_LIST_ITEM_CTA,MM_MUSD_CONVERSION_REWARDS_UI_ENABLEDact, testIDs)Written by Cursor Bugbot for commit 1a41c05. This will update automatically on new commits. Configure here.