fix: yield details style polish #11734
Conversation
Remove titleOverride props so single yields show their metadata.name (e.g., "Aave v3 USDC Lending") instead of just the asset symbol. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Display warning badges on yield cards and alerts on detail pages when yields are marked as underMaintenance or deprecated in the API. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…badges Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Create reusable YieldExplainers component that shows reward schedule, unbonding periods, and other yield-specific info based on mechanics type. Add it to YieldEnterModal and EarnConfirm for consistent user education across all enter flows. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show a "Learn more" link with external icon when a yield has metadata.documentation available. Appears below the description text on the yield detail page. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add YieldExplainers component to YieldForm.tsx (yield page enter modal) - Update ProviderDto to include website and references fields - Use provider.references[0] for documentation link (protocol website) - Fix docs link styling: inline icon next to description - Prefer provider documentation over yield metadata documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add 3-tab structure: All | Available to Earn | My Positions - Auto-navigate to "Available to Earn" tab when wallet connects - Each tab has explicit URL param for proper navigation - Keep recommended strip visible in all tabs - Add YieldTab enum for type safety - Rename Earn button action to navigate to Available tab Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix filters not working on My Positions tab (use unfiltered data) - Fix filters affecting Recommended For You (should be independent) - Fix "Show all" button navigating to All tab when on Available tab - Show full metadata.name on yield detail page below provider pill - Hide Withdraw button when user has no balance (instead of disabled) - Add YieldProviderInfo component with provider descriptions - Add YieldRelatedMarkets component showing other yields for same token - Make Yields first tab in Earn menu when feature flag enabled - Code simplification: remove unnecessary useMemo wrappers Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show fiat value as primary (large text) and crypto as secondary. This matches user expectations where fiat is the more relevant metric. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Add a professional two-column layout for desktop screens with info on the left and actions on the right. Create YieldInfoCard and YieldAvailableToDeposit components. Mobile layout remains unchanged. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix validator mismatch on yield detail page by passing selectedValidatorAddress from YieldDetail to YieldPositionCard (ensures Cosmos uses ShapeShift DAO) - Add "New" badge support to NavigationDropdown component - Mark Yields menu item with New badge - Apply react-best-practices cleanup to yield components Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create getYieldDisplayName that returns curator name for Morpho/Yearn vaults (e.g., "Steakhouse High Yield") and symbol for simple yields - Integrate into YieldItem, YieldDetail, YieldInfoCard, YieldHero - Remove useless secondary row on mobile showing ugly metadata name - Fix Visit Website alignment in YieldProviderInfo - Fix Lending badge from UPPERCASE to Capitalize - Simplify components by removing unnecessary useMemo calls Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replace cramped pill with clean icon + label pairs - Show Asset (icon + symbol), Chain (icon + name), Protocol (icon + name) - Apply same pattern to both desktop (YieldInfoCard) and mobile (YieldHero) - Remove redundant subtitle section - Clean up unused code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… skill - Prefer arrow functions over function keyword in memo components - Prefer useMemo over IIFE in TSX - Prefer implicit returns in useMemo/useCallback - Document avoidance of nested ternaries Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix dangerous fallback to 18 decimals in YieldAvailableToDeposit - Remove unnecessary Box wrapper in YieldEnterModal - Simplify nested ternary in YieldDetail using useMemo - Refactor YieldExplainers with memoized translations - Convert function keywords to arrow functions in memo components - Use implicit returns in useMemo/useCallback - Simplify enum mappings in YieldsList using array pattern - Remove NEAR_IMPROVEMENTS.md Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Fixes potential runtime error when allBalances[id] is undefined, which would cause .minus() to throw or produce NaN. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Check aggregatedAmount > 0 for hasClaimable, consistent with hasWithdrawable check, to avoid showing empty claimable UI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds translation entries, optional display props to AccountSelector, exports avatar size constants/types, tweaks button/card themes, and implements a broad refactor of the yields feature: new components (YieldAddMore, YieldCompareItem), updated page layouts (Main/PageHeader), enriched data flows (market data and balances), and various UI updates across yield components. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/components/LazyLoadAvatar.tsx (1)
9-28: Constrainsizeprop to non-responsiveAvatarSize
AvatarProps.sizesupports responsive values (e.g.,{ base: 'sm', md: 'lg' }), but the dictionary lookup ignores them and falls back tomdsilently. Type-assert without validation, violating the guideline. Constrain the prop toAvatarSize(string literal union) to define clear boundaries for this wrapper component and eliminate the unvalidated assertion.♻️ Suggested refactor
-export type LazyLoadAvatarProps = SkeletonProps & - Pick<AvatarProps, 'src' | 'boxSize' | 'name' | 'icon' | 'bg' | 'size'> +export type LazyLoadAvatarProps = SkeletonProps & + Pick<AvatarProps, 'src' | 'boxSize' | 'name' | 'icon' | 'bg'> & { + size?: AvatarSize + }- const skeletonSize = useMemo(() => { - return AVATAR_SIZES[size as AvatarSize] ?? AVATAR_SIZES.md - }, [size]) + const skeletonSize = useMemo(() => AVATAR_SIZES[size] ?? AVATAR_SIZES.md, [size])src/pages/Yields/components/YieldAvailableToDeposit.tsx (1)
47-80: Handle zero‑decimal tokens correctly.
Line 47–51 and Line 74–80 treat precision0as falsy, which hides the card for 0‑decimal assets.🔧 Proposed fix
- const availableBalance = useMemo( - () => - inputTokenPrecision - ? bnOrZero(availableBalanceBaseUnit).shiftedBy(-inputTokenPrecision) - : bnOrZero(0), - [availableBalanceBaseUnit, inputTokenPrecision], - ) + const availableBalance = useMemo(() => { + if (inputTokenPrecision === undefined) return bnOrZero(0) + return bnOrZero(availableBalanceBaseUnit).shiftedBy(-inputTokenPrecision) + }, [availableBalanceBaseUnit, inputTokenPrecision]) @@ - if (!inputTokenPrecision) return null + if (inputTokenPrecision === undefined) return null @@ - if (!hasAvailableBalance || hasPosition) return null + if (!hasAvailableBalance || hasPosition) return nullsrc/pages/Yields/components/YieldPositionCard.tsx (1)
132-314: Don’t hide the card when rewards are only claimable.
Line 313 returnsnullwhenhasAnyPositionis false, buthasAnyPositionexcludes claimable rewards. If a user has only claimable rewards (no active/entering/exiting/withdrawable), the claim UI vanishes.🔧 Proposed fix
- const hasAnyPosition = totalAmount.gt(0) + const hasAnyPosition = totalAmount.gt(0) || hasClaimable
🤖 Fix all issues with AI agents
In `@src/lib/yieldxyz/providerDescriptions.ts`:
- Around line 1-47: The PROVIDER_DESCRIPTIONS object currently embeds hardcoded
English strings which breaks i18n rules; change its shape to export mapping
entries that reference translation keys (e.g., replace description fields with
translationKey fields such as 'providers.morpho.description') so the UI reads
text via the translation system; update any consumers that access
PROVIDER_DESCRIPTIONS to call the translator with the provided translationKey
(or resolve keys to strings at render time) and add the corresponding entries
(providers.*.description) to the locale files so all provider copy is kept in
translations.
In `@src/pages/Yields/components/YieldAddMore.tsx`:
- Around line 100-108: The UI currently hardcodes the "/yr" suffix in
YieldAddMore.tsx inside the Amount.Fiat component; replace that fixed string
with a localized translation by calling translate('yieldXYZ.perYear') and
passing the result as the suffix prop to Amount.Fiat (use the existing translate
function in this file), and add/update the corresponding translation key
(yieldXYZ.perYear) in the i18n resource so the suffix is localized and available
for all locales; ensure potentialYearlyEarningsFiat.toFixed() remains the value
prop and keep the component props (as, color, fontWeight) unchanged.
- Around line 37-65: The component treats inputTokenPrecision as a falsy flag
which breaks 0‑decimal tokens; update the checks to explicitly test for
undefined/null instead of truthiness: in the availableBalance useMemo (the
computation that depends on inputTokenPrecision), the availableBalanceFiat and
potentialYearlyEarningsFiat calculations, and the final render guard (the if
that returns null), replace the truthy checks with an explicit check such as
"inputTokenPrecision !== undefined && inputTokenPrecision !== null" or "typeof
inputTokenPrecision === 'number'" so a precision value of 0 is handled correctly
while still guarding against missing precision.
- Around line 57-62: handleEnter currently replaces the entire query string
losing existing params; change it to parse and merge the existing query (using
location.search) with { action: 'enter', modal: 'yield' } before calling
navigate (keep using qs.stringify), and add location.search to the useCallback
dependency array so navigation updates when existing params change; reference
the handleEnter function, navigate call, location.search, and qs.stringify when
making the change.
In `@src/pages/Yields/components/YieldAvailableToDeposit.tsx`:
- Around line 121-133: The Amount.Fiat component currently hardcodes the "/yr"
suffix; replace that with a localized string by calling
translate('yieldXYZ.perYear') and passing its result into the Amount.Fiat suffix
prop (in YieldAvailableToDeposit.tsx where Amount.Fiat is rendered and
potentialYearlyEarningsFiat is used). Ensure you add the new translation key
yieldXYZ.perYear to the i18n resource files so the UI displays the localized
per-year suffix.
In `@src/pages/Yields/components/YieldCompareItem.tsx`:
- Around line 52-66: The Card component is rendered as a button (Card
as='button') but lacks an explicit type, which can cause unintended form
submissions; update the Card JSX in YieldCompareItem.tsx to include
type='button' on the Card element (where as='button' is set) so clicks trigger
handleClick without submitting enclosing forms, keeping all other props
(onClick={handleClick}, cursor='pointer', etc.) unchanged.
In `@src/pages/Yields/components/YieldHero.tsx`:
- Around line 87-103: The code treats inputTokenPrecision as falsy so 0‑decimal
tokens get zeroed and multiplying by a possibly undefined reward rate yields
NaN; update the availableBalance computation to check for null/undefined (e.g.,
inputTokenPrecision != null) so precision of 0 is honored, ensure
availableBalanceFiat uses bnOrZero(inputTokenMarketData?.price) as it already
does, and compute potentialYearlyEarningsFiat using
bnOrZero(yieldItem?.rewardRate?.total) (or default 0) before multiplying (e.g.,
availableBalanceFiat.times(bnOrZero(yieldItem?.rewardRate?.total))) so missing
rates produce 0 not NaN; adjust the useMemo dependency arrays to reference the
exact values used (inputTokenPrecision and yieldItem.rewardRate.total)
accordingly.
🧹 Nitpick comments (3)
src/pages/Yields/components/YieldItem.tsx (1)
81-113: Reintroduce memoization for derived UI valuesThese IIFE-derived objects/JSX are recreated on every render and drop the memoization guarantees expected in this codebase. Consider restoring
useMemofor these derived values (stats, iconElement, cardStatElement, mobileBalanceKey/Element, rowBalanceElement) to keep stable references and avoid unnecessary re-renders.♻️ Suggested refactor (example for stats)
- const stats = (() => { + const stats = useMemo(() => { if (isSingle) { const y = data.yieldItem return { apy: y.rewardRate.total, apyLabel: y.rewardRate.rateType, tvlUsd: y.statistics?.tvlUsd ?? '0', providers: [{ id: y.providerId, logo: data.providerIcon }], chainIds: y.chainId ? [y.chainId] : [], count: 1, name: y.metadata.name, canEnter: y.status.enter, } } const yields = data.yields const maxApy = Math.max(0, ...yields.map(y => y.rewardRate.total)) const totalTvlUsd = yields .reduce((acc, y) => acc.plus(bnOrZero(y.statistics?.tvlUsd)), bnOrZero(0)) .toFixed() const providerIds = [...new Set(yields.map(y => y.providerId))] const chainIds = [...new Set(yields.map(y => y.chainId).filter(Boolean))] as string[] return { apy: maxApy, apyLabel: 'APY', tvlUsd: totalTvlUsd, providers: providerIds.map(id => ({ id, logo: yieldProviders?.[id]?.logoURI })), chainIds, count: yields.length, name: data.assetName, canEnter: true, } - })() + }, [isSingle, data, yieldProviders])As per coding guidelines, consider
useMemofor derived values.Also applies to: 137-151, 190-225, 229-233, 235-255, 257-304
src/pages/Yields/YieldAssetDetails.tsx (1)
534-545: Extract the back-navigation handler into useCallbackThe inline
onClickrecreates a new handler whenheaderComponentrememoizes. Hoist it to auseCallbackto keep a stable reference.♻️ Suggested refactor
+ const handleBackClick = useCallback(() => navigate('/yields'), [navigate]) + const headerComponent = useMemo( () => ( <> <Button leftIcon={<ArrowBackIcon />} variant='ghost' - onClick={() => navigate('/yields')} + onClick={handleBackClick} mb={6} > {translate('common.back')} </Button>As per coding guidelines, event handlers passed as props should be memoized.
src/pages/Yields/components/YieldStats.tsx (1)
83-99: Consider adding a comment or type documentation clarifying whetherValidatorDto.tvlandYieldStatistics.tvlare in token units or another denomination, since the code assumes token units but this is not explicitly documented.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Yields/components/YieldAvailableToDeposit.tsx (2)
47-53: Handle 0-decimal tokens in available balance calculation.The truthy check on
inputTokenPrecision(line 49) fails for 0-decimal tokens wheredecimals === 0is falsy. This was fixed inYieldHero.tsx(line 89) but not applied here, causing inconsistent behavior.🩹 Proposed fix
const availableBalance = useMemo( () => - inputTokenPrecision + inputTokenPrecision != null ? bnOrZero(availableBalanceBaseUnit).shiftedBy(-inputTokenPrecision) : bnOrZero(0), [availableBalanceBaseUnit, inputTokenPrecision], )
60-63: Wrap reward rate with bnOrZero to prevent NaN.Multiplying by a potentially undefined
rewardRate.totalcan yieldNaN. This was fixed inYieldHero.tsx(line 101) but not applied here.🩹 Proposed fix
const potentialYearlyEarningsFiat = useMemo( - () => availableBalanceFiat.times(yieldItem.rewardRate.total), + () => availableBalanceFiat.times(bnOrZero(yieldItem.rewardRate.total)), [availableBalanceFiat, yieldItem.rewardRate.total], )
🤖 Fix all issues with AI agents
In `@src/pages/Yields/components/YieldHero.tsx`:
- Around line 327-334: The hardcoded "/yr" suffix on Amount.Fiat should be
replaced with the i18n translation key used elsewhere; update the Amount.Fiat
prop suffix to use translate('yieldXYZ.perYear') (or the same translate import
used in YieldAvailableToDeposit.tsx), ensure the translate function is imported
at top of YieldHero.tsx, and remove the hardcoded string so the suffix is fully
localized.
🧹 Nitpick comments (2)
src/pages/Yields/components/YieldCompareItem.tsx (1)
69-82: Consider usingLazyLoadAvatarfor the provider icon for consistency.The chain icon overlay (lines 72-80) uses
LazyLoadAvatarwhich provides loading skeleton and lazy loading benefits, but the main provider avatar on line 70 uses the standardAvatar. For consistency and to benefit from lazy loading for external provider icons, consider usingLazyLoadAvatarhere as well.♻️ Suggested change
<Box position='relative'> - <Avatar size='sm' src={providerIcon} name={displayProviderName} /> + <LazyLoadAvatar size='sm' src={providerIcon} name={displayProviderName} /> {chainIcon && (src/pages/Yields/components/YieldHero.tsx (1)
291-291: Redundant condition.This
!hasPositioncheck is inside theelsebranch (line 285) wherehasPositionis alreadyfalse, making the condition always true.♻️ Suggested simplification
- {!hasPosition && descriptionSection} + {descriptionSection}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/pages/Yields/components/YieldHero.tsx`:
- Around line 67-107: YieldHero is using the portfolio-wide balance because
selectPortfolioCryptoBalanceBaseUnitByFilter is called with only { assetId:
inputTokenAssetId }; remove the inline comment near the top, add an accountId
prop to the YieldHero component (e.g. accept accountId: string | undefined) and
pass it into the selector filter like
selectPortfolioCryptoBalanceBaseUnitByFilter(state, { accountId, assetId:
inputTokenAssetId }); ensure the prop name matches uses in parent components
(YieldDetail via useYieldAccountSync) so the selectedAccountId is forwarded into
YieldHero.
🧹 Nitpick comments (1)
src/pages/Yields/components/YieldHero.tsx (1)
286-349: Preserve fiat precision before formatting (optional).
toFixed()defaults to 0 decimals, which may truncate cents. Consider passing string values via.toString()(or specify decimals) soAmount.Fiatcan format with locale rules.♻️ Optional refinement
- <Amount.Fiat value={availableBalanceFiat.toFixed()} /> + <Amount.Fiat value={availableBalanceFiat.toString()} /> @@ - value={potentialYearlyEarningsFiat.toFixed()} + value={potentialYearlyEarningsFiat.toString()}Based on learnings, prefer
.toString()for BigNumber-derived values before passing toAmount.*components.
| const iconSource = resolveYieldInputAssetIcon(yieldItem) | ||
| const apy = bnOrZero(yieldItem.rewardRate.total).times(100).toFixed(2) | ||
| const hasExitBalance = bnOrZero(userBalanceCrypto).gt(0) | ||
| const hasPosition = bnOrZero(userBalanceCrypto).gt(0) | ||
|
|
||
| // Available to deposit logic | ||
| const inputToken = yieldItem.inputTokens[0] | ||
| const inputTokenAssetId = inputToken?.assetId ?? '' | ||
| const inputTokenPrecision = inputToken?.decimals | ||
|
|
||
| const assetIcon = useMemo(() => { | ||
| if (iconSource.assetId) { | ||
| return <AssetIcon assetId={iconSource.assetId} size='2xl' /> | ||
| } | ||
| return <AssetIcon src={iconSource.src} size='2xl' /> | ||
| }, [iconSource]) | ||
|
|
||
| const availableBalanceBaseUnit = useAppSelector(state => | ||
| selectPortfolioCryptoBalanceBaseUnitByFilter(state, { assetId: inputTokenAssetId }), | ||
| ) | ||
|
|
||
| const availableBalance = useMemo( | ||
| () => | ||
| inputTokenPrecision != null | ||
| ? bnOrZero(availableBalanceBaseUnit).shiftedBy(-inputTokenPrecision) | ||
| : bnOrZero(0), | ||
| [availableBalanceBaseUnit, inputTokenPrecision], | ||
| ) | ||
|
|
||
| const availableBalanceFiat = useMemo( | ||
| () => availableBalance.times(bnOrZero(inputTokenMarketData?.price)), | ||
| [availableBalance, inputTokenMarketData?.price], | ||
| ) | ||
|
|
||
| const potentialYearlyEarningsFiat = useMemo( | ||
| () => availableBalanceFiat.times(bnOrZero(yieldItem?.rewardRate?.total)), | ||
| [availableBalanceFiat, yieldItem?.rewardRate?.total], | ||
| ) | ||
|
|
||
| const hasAvailableBalance = availableBalance.gt(0) | ||
| const showAvailableToDeposit = !hasPosition && hasAvailableBalance | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if YieldDetail passes accountId to YieldHero
rg -n "YieldHero" src/pages/Yields/YieldDetail.tsx -A 5 -B 5
# Also check for account-related variables in YieldDetail
rg -n "accountId|selectedAccountId|accountIdParam|AccountSelector" src/pages/Yields/YieldDetail.tsx -A 2 -B 2Repository: shapeshift/web
Length of output: 3386
🏁 Script executed:
# Inspect the selector signature to understand its parameters
rg -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/portfolioSlice/selectors.ts -A 10 -B 2Repository: shapeshift/web
Length of output: 40
🏁 Script executed:
# Check the full YieldHero component file to understand its props
head -150 src/pages/Yields/components/YieldHero.tsxRepository: shapeshift/web
Length of output: 5014
🏁 Script executed:
# Search for the selector definition in selectors files
find src/state -name "*selector*" -type f | head -20Repository: shapeshift/web
Length of output: 898
🏁 Script executed:
# Search for selectPortfolioCryptoBalanceBaseUnitByFilter across the codebase
rg "selectPortfolioCryptoBalanceBaseUnitByFilter" --type ts --type tsx -A 5 -B 2Repository: shapeshift/web
Length of output: 85
🏁 Script executed:
# Check if there's an account-scoped variant of the balance selector
rg "selectPortfolioBalance.*Filter\|selectCryptoBalance.*accountId" src/state/slices/portfolioSlice/selectors.ts -A 3Repository: shapeshift/web
Length of output: 40
🏁 Script executed:
# Read the portfolio selectors file to find the function definition
rg -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/portfolioSlice/selectors.ts -A 10Repository: shapeshift/web
Length of output: 40
🏁 Script executed:
# Also check the main selectors export file
grep -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/selectors.tsRepository: shapeshift/web
Length of output: 40
🏁 Script executed:
# List contents of portfolioSlice directory
ls -la src/state/slices/portfolioSlice/Repository: shapeshift/web
Length of output: 577
🏁 Script executed:
# Read the selectors.ts file to find the function
grep -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/portfolioSlice/selectors.tsRepository: shapeshift/web
Length of output: 40
🏁 Script executed:
# If it's not found there, check if it's exported from src/state/slices/selectors.ts
grep -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/selectors.tsRepository: shapeshift/web
Length of output: 40
🏁 Script executed:
# Search the entire codebase for where this selector is defined
rg "selectPortfolioCryptoBalanceBaseUnitByFilter\s*=" --type tsRepository: shapeshift/web
Length of output: 176
🏁 Script executed:
# Read the common-selectors.ts file to understand the selector definition
grep -n "selectPortfolioCryptoBalanceBaseUnitByFilter" src/state/slices/common-selectors.ts -A 20 -B 2Repository: shapeshift/web
Length of output: 1189
🏁 Script executed:
# Also check if there's an account-scoped version available
grep -n "selectPortfolioBalance\|selectCryptoBalance" src/state/slices/common-selectors.ts | head -30Repository: shapeshift/web
Length of output: 40
Account-scope the available balance and remove the inline comment.
The selector selectPortfolioCryptoBalanceBaseUnitByFilter supports account scoping via an accountId parameter in its filter object. When accountId is provided, it returns the account-specific balance; without it, it falls back to the portfolio-wide balance. Currently, YieldHero passes only the assetId, which causes line 81 to display the portfolio-wide balance instead of the selected account's balance. YieldDetail already provides selectedAccountId (via useYieldAccountSync), but it's not passed to YieldHero—this needs to be fixed.
Also, remove the inline comment on line 71 per coding guidelines.
Proposed cleanup
- // Available to deposit logicTo fix the account scoping, YieldHero needs to receive accountId as a prop and pass it to the selector's filter object:
selectPortfolioCryptoBalanceBaseUnitByFilter(state, { accountId, assetId: inputTokenAssetId })🤖 Prompt for AI Agents
In `@src/pages/Yields/components/YieldHero.tsx` around lines 67 - 107, YieldHero
is using the portfolio-wide balance because
selectPortfolioCryptoBalanceBaseUnitByFilter is called with only { assetId:
inputTokenAssetId }; remove the inline comment near the top, add an accountId
prop to the YieldHero component (e.g. accept accountId: string | undefined) and
pass it into the selector filter like
selectPortfolioCryptoBalanceBaseUnitByFilter(state, { accountId, assetId:
inputTokenAssetId }); ensure the prop name matches uses in parent components
(YieldDetail via useYieldAccountSync) so the selectedAccountId is forwarded into
YieldHero.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/pages/Yields/components/YieldProviderInfo.tsx (1)
19-28: UnusedproviderIdprop in type definition.The
providerIdproperty is defined inYieldProviderInfoProps(line 20) but is not destructured or used anywhere in the component (line 28). This creates dead code since callers pass this prop but it has no effect.🧹 Proposed fix to remove unused prop
type YieldProviderInfoProps = { - providerId: string providerName: string providerLogoURI?: string providerWebsite?: string description?: string } export const YieldProviderInfo = memo( - ({ providerName, providerLogoURI, providerWebsite, description }: YieldProviderInfoProps) => { + ({ providerName, providerLogoURI, providerWebsite, description }: YieldProviderInfoProps) => {Then update callers in
YieldDetail.tsxto remove theproviderIdprop:<YieldProviderInfo - providerId={yieldItem.providerId} providerName={validatorOrProvider.name}src/pages/Yields/YieldDetail.tsx (1)
291-298: Missingdescriptionprop in mobile layout.The mobile
YieldProviderInfodoes not receive thedescriptionprop, while the desktop version (line 323) does passdescription={validatorOrProvider.description}. This creates an inconsistent experience where mobile users won't see the provider description.🐛 Proposed fix to add missing prop
<YieldProviderInfo providerId={yieldItem.providerId} providerName={validatorOrProvider.name} providerLogoURI={validatorOrProvider.logoURI} providerWebsite={validatorOrProvider.documentation} + description={validatorOrProvider.description} />
🧹 Nitpick comments (1)
src/pages/Yields/YieldDetail.tsx (1)
262-262: Consider extracting static responsive value outside the component.The
containerPaddingXis a static object with no dependencies. Per coding guidelines, static values can be defined as constants outside the component to avoid unnecessary memoization overhead.♻️ Optional refactor
+const containerPaddingX = { base: 4, xl: 0 } + export const YieldDetail = memo(() => { // ... - const containerPaddingX = useMemo(() => ({ base: 4, xl: 0 }), [])
| const addressBadge = useMemo(() => { | ||
| if (!address) return null | ||
| return ( | ||
| <Badge variant='subtle' colorScheme='blue' borderRadius='full' px={2} py={0.5}> | ||
| {addressBadgeText} | ||
| </Badge> | ||
| ) | ||
| }, [address, addressBadgeText]) | ||
|
|
There was a problem hiding this comment.
Is this an intended change? Personally, that badge helps me to identify the address and I find it useful as a user
| ]) | ||
|
|
||
| if (!accountId) return null | ||
| if (!isBalancesLoading && !hasAnyPosition) return null |
There was a problem hiding this comment.
q: is this an intended change? This may produce flashes, also, it removes the changes introduced in #11703 re: showing out position as zero'd out
This will now produce a layout shift from no/wallet state vs. zero'd out state before.
Also, this produces an inconsistent layout for positions without balance.
There was a problem hiding this comment.
I was thinking we would show either of them, but not at the same time. So I can adjust this logic if we want to show them buttons they can't interact with.
There was a problem hiding this comment.
While yield parts has some sexy improvements, it also has regressions (missing position data e.g unstakings, claims etc not available on mobile now), regressed bits like zero'd out position without a wallet, inconsistent layout for balance/apy depending on no/position.
Most importantly, the app is in a fully broken state now. If going with a fully vibe-coded PR, at the very least sanity check the changeset and make Claude itself do some review pass and test the changes according to the surface area diffed.
Tbh just rage quitted commenting the code and went for functional testing - pls salvage actual yield visual changes from this, be mindful about global theme changes and yield logic/visual regressions introduced and test them well ty
Spotted issues:
- All app buttons are now broken in light mode
https://jam.dev/c/e9f2f354-eec9-4e24-9d11-f7a3c72eecef
- We broke cards in the app across the board
- We broke avatars in the app across the board
- We lost section with unstaking, rewards etc on mobile, see develop vs this diff - how to see my unstaking and claims my rewards on mobile now?
-
Also lost description of yield on mobile (see screenshot above, the yield-specific description)
-
Asset icons looking pixelated on mobile w/ this new bigger icons
- While it looks a lot better for some assets with more info, looks a bit empty for others now i.e seemed to fill a teeny tiny bit more space before
- Not all yields looking the same re: APY/balance depending on active, is this visual discrepancy expected?
- Inconsistent use of APY green badge vs. gradient apy (directly related to the discrepancy above)
- Available to deposit doesn't display a fiat value anymore
- Double provider reference, once in subheading, then in tags, is this expected?
- Yield symbol list is now visually broken on desktop and mobile alike
- Personal taste but I find things way less readable and when stretched more to take full screen width with 3 col layout, see this diff vs. develop
- The new related yields list view doesn't show TVL for comparison
- Not sure if intentional, but the new related yields list does not display the parsed name with Curator
-
On a tangent, personally not fan of a list view for related at the very least on desktop as related cards of sorts is a nice familiar UX, a list doesn't feel very "related" kind of UX - for mobile, it is currently list-y already and saves a bit more space with the new way so why not
-
We're displaying way too much precision in the new available to earn in yield details page
| size={size} | ||
| icon={icon} | ||
| boxSize={boxSize} | ||
| boxSize='100%' |
| boxShadow: | ||
| '0 1px 0 var(--chakra-colors-border-base) inset, 0 0 0 1px var(--chakra-colors-border-base) inset', |
There was a problem hiding this comment.
Yeah also not sure about this one
| typeof inputTokenPrecision === 'number' | ||
| ? bnOrZero(availableBalanceBaseUnit).shiftedBy(-inputTokenPrecision) | ||
| : bnOrZero(0), |
There was a problem hiding this comment.
This doesn't display on mobile, is this expected?
| [translate], | ||
| ) | ||
|
|
||
| const containerPaddingX = useMemo(() => ({ base: 4, xl: 0 }), []) |
There was a problem hiding this comment.
nitpick but this doesn't need to be a useMemo'd constant and also conventional px ?
There was a problem hiding this comment.
Why was this file diffed so much?
| [isStaking, selectedValidator], | ||
| ) | ||
|
|
||
| if (variant === 'list') { |
There was a problem hiding this comment.
This is used only in one place, not sure we need the variant complexity for only one usage.
| .sort((a, b) => b.potentialEarnings.minus(a.potentialEarnings).toNumber()) | ||
| .slice(0, 3) | ||
| }, [unfilteredAvailableYields, userCurrencyBalances]) | ||
| }, [isConnected, yields?.unfiltered, userCurrencyBalances, assetBalancesBaseUnit]) |
There was a problem hiding this comment.
q: why the exhaustive assetBalancesBaseUnit dep if unused?
| } | ||
| return validatorParam || defaultValidator | ||
| }, [yieldItem.id, validatorParam, defaultValidator]) | ||
| }, [yieldItem.id, defaultValidator, validatorParam]) |
There was a problem hiding this comment.
q: why did the order changes if those are the same as before?
|
@reallybeard closing to not make you spend more time on this one, I don't think this PR is in a salvage-able state so no point to tackle. If you're confident in reopening a PR with the few yield style chunks only (with all the comments above taken into consideration), will take a look, else I'll salvage said style bits tomo. |
Description
Visual polish everywhere:
New "Add More" feature:
Related Markets redesign:
Better page layouts:
Provider info cards:
Account selector flexibility:
Misc cleanup:
Issue (if applicable)
closes #
Risk
low risk mostly UI updates
Testing
Engineering
Operations
Screenshots (if applicable)
Summary by CodeRabbit
New Features
Improvements
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.