This repository was archived by the owner on Jul 23, 2026. It is now read-only.
chore: webapp and mobile improve performance for earn - #2790
Closed
mmioana wants to merge 21 commits into
Closed
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughSplits the Earn filtering context into separate data and filter providers, introduces a cached server helper for initial opportunities, shifts several UI pieces to client-only dynamic imports, adds toggle filter categories and toggle view, memoizes card components, and updates image remote patterns and lodash imports. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Browser as Browser (Client)
participant Page as /[lng]/earn Page (Server)
participant Cache as unstable_cache
participant API as getOpportunitiesFiltered (API)
participant Provider as EarnFilteringProvider (Client)
participant DynamicUI as EarnOpportunitiesAllClient (Client)
Browser->>Page: GET /[lng]/earn
Page->>Cache: call cached helper (key: earn-initial-all-opportunities)
alt cache miss
Cache->>API: getOpportunitiesFiltered({})
API-->>Cache: response.data
end
Cache-->>Page: return cached result (data or fallback)
Page-->>Browser: render page with initialAllOpportunities prop
Browser->>DynamicUI: mount EarnOpportunitiesAllClient(initialAllOpportunities)
DynamicUI->>Provider: initialize EarnData & EarnFilter providers with initial data
Browser->>DynamicUI: user opens Filters
DynamicUI->>DynamicUI: load dynamic Filter modal/drawer (ssr:false)
DynamicUI->>Provider: apply/clear filters (useEarnFilter)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
Contributor
|
✅ All snapshot tests passed |
Contributor
Playwright test results
Details
Failed testschromium › earnPage.spec.ts › Should be able to navigate to the "Your Positions" tab › Should be able to navigate to the "Your Positions" tab (Qase ID: 56) Flaky testschromium › mainMenu.spec.ts › Main Menu flows › Should be able to navigate to the Jumper Learn (Qase ID: 22) Skipped testschromium › themeManipulation.spec.ts › Switch between dark and light theme and check the background color › Partner theme should appear in theme menu and apply background color (Qase ID: 49) |
mmioana
changed the title
mmioana
force-pushed
the
feature/jum-609-webapp-and-mobile-improve-performance-for-earn
branch
from
April 28, 2026 15:17
304ca17 to
225d2cb
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
src/components/Cards/HeroEarnCard/HeroEarnCard.tsx (1)
170-178:⚠️ Potential issue | 🟠 MajorMemo comparator misses rendered props.
The comparator only checks
data?.slug, but the component rendersdata.asset,data.protocol,data.forYou,data.tags,data.latest.apy,data.name,data.lpToken, andprimaryAction. If any of these change whileslugremains stable, the memo blocks necessary re-renders, leaving stale content.Suggested fix: use default shallow comparison
-export const HeroEarnCard = memo(HeroEarnCardBase, (prev, next) => { - return ( - prev.data?.slug === next.data?.slug && - prev.isLoading === next.isLoading && - prev.copy === next.copy && - prev.isMain === next.isMain && - prev.href === next.href - ); -}); +export const HeroEarnCard = memo(HeroEarnCardBase);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/Cards/HeroEarnCard/HeroEarnCard.tsx` around lines 170 - 178, The custom memo comparator for HeroEarnCard (wrapping HeroEarnCardBase) only compares data?.slug and a few props, causing stale renders when other rendered fields (e.g., data.asset, data.protocol, data.forYou, data.tags, data.latest.apy, data.name, data.lpToken, primaryAction) change; fix by removing the custom comparator so React.memo uses the default shallow comparison (i.e., export const HeroEarnCard = memo(HeroEarnCardBase)) or, alternatively, expand the comparator to explicitly compare all rendered props (data.asset, data.protocol, data.forYou, data.tags, data.latest?.apy, data.name, data.lpToken, primaryAction and the existing fields) to ensure re-renders occur when any rendered value changes.src/components/EarnFilterBar/hooks.tsx (2)
338-339:⚠️ Potential issue | 🟠 Major
Clear allmay not reset pending UI state.
onClearis wired directly tohandleClearAllFilters, which clears the applied filters but may not reset thependingValuestracked byusePendingFilters. If the applied state is already empty, pressing Clear all after making pending selections in the modal could leave the pending UI unchanged.Verify that
usePendingFilters.clearAllinternally resets pending state, or explicitly reset it alongside the applied clear.#!/bin/bash # Check how usePendingFilters handles clearAll and whether it resets pending state ast-grep --pattern 'export function usePendingFilters($$$) { $$$ }' rg -n -A 20 'clearAll' src/components/composite/MultiLayer/hooks.ts🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EarnFilterBar/hooks.tsx` around lines 338 - 339, The Clear all button currently calls handleClearAllFilters but may leave pending UI selections intact; update the onClear flow so it also resets the pending state from usePendingFilters (call the clearAll/reset method on the pending filters instance) — either modify handleClearAllFilters to invoke pendingFilters.clearAll (or pendingFilters.resetPending) or wrap handleClearAllFilters so onClear calls both handleClearAllFilters() and pendingFilters.clearAll(); ensure you reference the pending filter hook instance used in this file (usePendingFilters / pendingFilters) and the onClear handler so both applied and pending states are cleared.
323-336:⚠️ Potential issue | 🟠 MajorNormalize filter values to
nullwhen empty/default.The
onApplyhandler passes empty arrays and full-range values directly instead of usingnullto indicate "unset". This inconsistency with the established convention (wherenullmeans "no filter") can leave stale filter state in context and query params.Suggested fix
onApply: (values) => { - const minRewardsAPY = values.rewardsAPY ? 0.0 : undefined; + const hasApyFilter = + values.apy[0] !== apyMin || values.apy[1] !== apyMax; + const hasTvlFilter = + values.tvl[0] !== tvlMin || values.tvl[1] !== tvlMax; handleApplyAllFilters({ - chains: values.chains.map(Number) ?? [], - protocols: values.protocols ?? [], - tags: values?.tags ?? [], - assets: values.assets ?? [], - minAPY: values.apy[0] / 100, - maxAPY: values.apy[1] / 100, - minTVL: values.tvl[0], - maxTVL: values.tvl[1], - minRewardsAPY, + chains: values.chains.length ? values.chains.map(Number) : null, + protocols: values.protocols.length ? values.protocols : null, + tags: values.tags.length ? values.tags : null, + assets: values.assets.length ? values.assets : null, + minAPY: hasApyFilter ? values.apy[0] / 100 : null, + maxAPY: hasApyFilter ? values.apy[1] / 100 : null, + minTVL: hasTvlFilter ? values.tvl[0] : null, + maxTVL: hasTvlFilter ? values.tvl[1] : null, + minRewardsAPY: values.rewardsAPY ? 0.0 : null, }); handleSortBy(values.sortBy ?? ''); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EarnFilterBar/hooks.tsx` around lines 323 - 336, The onApply handler should map empty/default inputs to null before calling handleApplyAllFilters to avoid leaving stale filters: update the onApply block in hooks.tsx so that chains, protocols, tags, and assets become null when their arrays are empty (instead of []), convert minAPY/maxAPY to null when values.apy represents the full slider range (so you only pass a number when the range is actually narrowed), convert minTVL/maxTVL to null when values.tvl represents the full TVL range, and set minRewardsAPY to null (not undefined) when rewardsAPY is not set; keep calling handleSortBy(values.sortBy ?? '') but optionally normalize empty string to null if your app expects that. Ensure these checks reference the existing symbols values.apy, values.tvl, values.rewardsAPY, and the call handleApplyAllFilters to implement the normalization.
🧹 Nitpick comments (2)
src/components/EarnFilterBar/EarnFilterBarSkeleton.tsx (1)
9-13: Makespace-betweendeterministic with explicit width.At Line 11,
justifyContent: 'space-between'depends on available row width; addingwidth: '100%'makes spacing behavior consistent.Suggested tweak
sx={{ display: 'flex', justifyContent: 'space-between', alignItems: 'center', + width: '100%', }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EarnFilterBar/EarnFilterBarSkeleton.tsx` around lines 9 - 13, The layout uses justifyContent: 'space-between' inside the sx prop of EarnFilterBarSkeleton which yields variable spacing; update the sx object for the container (the object that currently contains display: 'flex', justifyContent: 'space-between', alignItems: 'center') to include width: '100%' so the space-between calculation is deterministic and consistent across layouts.src/components/EarnFilterBar/EarnFilterBar.tsx (1)
57-67: Minor: Redundant nested Stack wrapper.The inner
Stack(lines 57-67) has identical styling to its parentStack(lines 48-55) and only wrapsEarnListMode. This nesting appears unnecessary.Suggested simplification
<Stack direction="row" sx={{ gap: 1, display: 'flex', alignItems: 'center', flexShrink: 0, }} > - <Stack - direction="row" - sx={{ - gap: 1, - display: 'flex', - alignItems: 'center', - flexShrink: 0, - }} - > - <EarnListMode variant={variant} setVariant={setVariant} /> - </Stack> + <EarnListMode variant={variant} setVariant={setVariant} /> {!isForYouTab && <EarnFilterBarContentAll />} </Stack>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/EarnFilterBar/EarnFilterBar.tsx` around lines 57 - 67, The inner Stack wrapper around EarnListMode in EarnFilterBar is redundant because it duplicates the parent's sx props; remove the nested Stack and render <EarnListMode variant={variant} setVariant={setVariant} /> directly inside the parent Stack (keep the parent Stack's sx and children order) to simplify the component and avoid unnecessary DOM nodes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/components/Cards/EarnCard/EarnCard.tsx`:
- Around line 17-25: The memo comparator on EarnCard (memo(EarnCardBase, ...))
currently only compares variant, data.slug, isLoading, isMissingPosition and
href, omitting primaryAction, headerBadge, and fullWidth which can change
rendering; fix by either (A) adding comparisons for prev.primaryAction ===
next.primaryAction, prev.headerBadge === next.headerBadge, and prev.fullWidth
=== next.fullWidth inside the custom comparator for EarnCard, or (B) remove the
custom comparator entirely so React.memo uses the default shallow comparison for
EarnCardBase to catch changes to those props.
In `@src/components/EarnFilterBar/EarnFilterBarSkeleton.tsx`:
- Around line 16-22: The skeleton currently renders two BaseSkeleton pills
(Array.from({ length: 2 })) at height 32 which doesn't match the desktop
filter's three MD tabs and causes CLS; update EarnFilterBarSkeleton (where
BaseSkeleton is mapped) to render three placeholders (use length: 3) and adjust
the BaseSkeleton props (variant="rounded", width, height) to match the desktop
MD tab dimensions used by the real tabs so the skeleton layout matches the final
UI.
---
Duplicate comments:
In `@src/components/Cards/HeroEarnCard/HeroEarnCard.tsx`:
- Around line 170-178: The custom memo comparator for HeroEarnCard (wrapping
HeroEarnCardBase) only compares data?.slug and a few props, causing stale
renders when other rendered fields (e.g., data.asset, data.protocol,
data.forYou, data.tags, data.latest.apy, data.name, data.lpToken, primaryAction)
change; fix by removing the custom comparator so React.memo uses the default
shallow comparison (i.e., export const HeroEarnCard = memo(HeroEarnCardBase))
or, alternatively, expand the comparator to explicitly compare all rendered
props (data.asset, data.protocol, data.forYou, data.tags, data.latest?.apy,
data.name, data.lpToken, primaryAction and the existing fields) to ensure
re-renders occur when any rendered value changes.
In `@src/components/EarnFilterBar/hooks.tsx`:
- Around line 338-339: The Clear all button currently calls
handleClearAllFilters but may leave pending UI selections intact; update the
onClear flow so it also resets the pending state from usePendingFilters (call
the clearAll/reset method on the pending filters instance) — either modify
handleClearAllFilters to invoke pendingFilters.clearAll (or
pendingFilters.resetPending) or wrap handleClearAllFilters so onClear calls both
handleClearAllFilters() and pendingFilters.clearAll(); ensure you reference the
pending filter hook instance used in this file (usePendingFilters /
pendingFilters) and the onClear handler so both applied and pending states are
cleared.
- Around line 323-336: The onApply handler should map empty/default inputs to
null before calling handleApplyAllFilters to avoid leaving stale filters: update
the onApply block in hooks.tsx so that chains, protocols, tags, and assets
become null when their arrays are empty (instead of []), convert minAPY/maxAPY
to null when values.apy represents the full slider range (so you only pass a
number when the range is actually narrowed), convert minTVL/maxTVL to null when
values.tvl represents the full TVL range, and set minRewardsAPY to null (not
undefined) when rewardsAPY is not set; keep calling handleSortBy(values.sortBy
?? '') but optionally normalize empty string to null if your app expects that.
Ensure these checks reference the existing symbols values.apy, values.tvl,
values.rewardsAPY, and the call handleApplyAllFilters to implement the
normalization.
---
Nitpick comments:
In `@src/components/EarnFilterBar/EarnFilterBar.tsx`:
- Around line 57-67: The inner Stack wrapper around EarnListMode in
EarnFilterBar is redundant because it duplicates the parent's sx props; remove
the nested Stack and render <EarnListMode variant={variant}
setVariant={setVariant} /> directly inside the parent Stack (keep the parent
Stack's sx and children order) to simplify the component and avoid unnecessary
DOM nodes.
In `@src/components/EarnFilterBar/EarnFilterBarSkeleton.tsx`:
- Around line 9-13: The layout uses justifyContent: 'space-between' inside the
sx prop of EarnFilterBarSkeleton which yields variable spacing; update the sx
object for the container (the object that currently contains display: 'flex',
justifyContent: 'space-between', alignItems: 'center') to include width: '100%'
so the space-between calculation is deterministic and consistent across layouts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c936d523-1b2b-4174-8d20-72848eb46173
⛔ Files ignored due to path filters (6)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yamlsrc/components/Cards/EarnCard/__snapshots__/EarnCard.snapshot.spec.tsx.snapis excluded by!**/*.snapsrc/components/Cards/HeroEarnCard/__snapshots__/HeroEarnCard.snapshot.spec.tsx.snapis excluded by!**/*.snapsrc/components/composite/WalletBalanceCard/__snapshots__/WalletBalanceCard.snapshot.spec.tsx.snapis excluded by!**/*.snapsrc/components/composite/cards/ProcessingTransactionCard/__snapshots__/ProcessingTransactionCard.snapshot.spec.tsx.snapis excluded by!**/*.snapsrc/components/core/AvatarStack/__snapshots__/AvatarStack.snapshot.spec.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (48)
next.config.mjspackage.jsonsrc/app/[lng]/earn/[slug]/page.tsxsrc/app/[lng]/earn/layout.tsxsrc/app/[lng]/earn/page.tsxsrc/app/lib/getEarnInitialAllOpportunitiesCached.tssrc/app/ui/earn/EarnEmptyList/EarnEmptyList.tsxsrc/app/ui/earn/EarnEmptyList/EarnEmptyListAllMarkets.tsxsrc/app/ui/earn/EarnEmptyList/EarnEmptyListForYou.tsxsrc/app/ui/earn/EarnEmptyList/EarnEmptyListYourPositions.tsxsrc/app/ui/earn/EarnFilteringContext.tsxsrc/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAll.tsxsrc/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAllClient.tsxsrc/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAllSkeleton.tsxsrc/app/ui/earn/EarnOpportunitiesCards.tsxsrc/app/ui/earn/EarnsPage.tsxsrc/app/ui/earn/EarnsPageSkeleton.tsxsrc/app/ui/earn/filterOpportunities.tssrc/app/ui/earn/index.tssrc/components/Cards/EarnCard/EarnCard.tsxsrc/components/Cards/HeroEarnCard/HeroEarnCard.tsxsrc/components/Cards/ProtocolCard/ProtocolCard.tsxsrc/components/EarnFilterBar/EarnFilterBar.stories.tsxsrc/components/EarnFilterBar/EarnFilterBar.tsxsrc/components/EarnFilterBar/EarnFilterBarSkeleton.tsxsrc/components/EarnFilterBar/components/EarnFilterBarContentForYou.tsxsrc/components/EarnFilterBar/components/EarnFilterSort.tsxsrc/components/EarnFilterBar/components/EarnListMode.tsxsrc/components/EarnFilterBar/hooks.tsxsrc/components/EarnFilterBar/layouts/EarnFilterBarContentAll.tsxsrc/components/EarnFilterBar/layouts/EarnFilterBarContentAllDesktop.tsxsrc/components/EarnFilterBar/layouts/EarnFilterBarContentAllTablet.tsxsrc/components/EarnFilterBar/layouts/EarnFilterViewDesktop.tsxsrc/components/EarnFilterBar/layouts/EarnFilterViewTablet.tsxsrc/components/EarnFilterBar/utils.tssrc/components/composite/DepositFlow/DepositFlow.tsxsrc/components/composite/MultiLayer/MultiLayer.types.tssrc/components/composite/MultiLayer/components/LeafCategoryRenderer.tsxsrc/components/composite/MultiLayer/utils.tssrc/components/composite/MultiLayer/views/ToggleView.tsxsrc/components/composite/RequestRedeemFlow/RequestRedeemFlow.tsxsrc/components/composite/WithdrawFlow/WithdrawFlow.tsxsrc/components/core/AvatarStack/AvatarItem.tsxsrc/components/core/form/Select/Select.styles.tssrc/hooks/earn/useEarnFilterOpportunities.tssrc/hooks/earn/useEarnTopOpportunities.tssrc/i18n/resources.d.tssrc/i18n/translations/en/translation.json
💤 Files with no reviewable changes (6)
- src/app/[lng]/earn/layout.tsx
- src/app/ui/earn/index.ts
- src/app/ui/earn/EarnsPageSkeleton.tsx
- src/components/EarnFilterBar/components/EarnFilterSort.tsx
- src/components/EarnFilterBar/layouts/EarnFilterBarContentAllDesktop.tsx
- src/components/EarnFilterBar/layouts/EarnFilterBarContentAllTablet.tsx
✅ Files skipped from review due to trivial changes (13)
- src/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAllSkeleton.tsx
- src/i18n/translations/en/translation.json
- src/app/ui/earn/filterOpportunities.ts
- src/components/composite/MultiLayer/utils.ts
- src/components/EarnFilterBar/components/EarnListMode.tsx
- src/app/[lng]/earn/[slug]/page.tsx
- src/components/EarnFilterBar/layouts/EarnFilterViewDesktop.tsx
- package.json
- src/app/ui/earn/EarnOpportunitiesCards.tsx
- src/components/EarnFilterBar/layouts/EarnFilterViewTablet.tsx
- src/components/EarnFilterBar/components/EarnFilterBarContentForYou.tsx
- next.config.mjs
- src/hooks/earn/useEarnFilterOpportunities.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- src/components/Cards/ProtocolCard/ProtocolCard.tsx
- src/hooks/earn/useEarnTopOpportunities.ts
- src/app/ui/earn/EarnEmptyList/EarnEmptyListForYou.tsx
- src/app/ui/earn/EarnEmptyList/EarnEmptyList.tsx
- src/app/lib/getEarnInitialAllOpportunitiesCached.ts
- src/components/composite/WithdrawFlow/WithdrawFlow.tsx
- src/i18n/resources.d.ts
- src/components/composite/MultiLayer/MultiLayer.types.ts
- src/components/EarnFilterBar/utils.ts
- src/components/core/form/Select/Select.styles.ts
- src/components/composite/DepositFlow/DepositFlow.tsx
- src/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAllClient.tsx
- src/components/composite/MultiLayer/views/ToggleView.tsx
- src/components/EarnFilterBar/layouts/EarnFilterBarContentAll.tsx
- src/app/ui/earn/EarnEmptyList/EarnEmptyListYourPositions.tsx
- src/app/ui/earn/EarnOpportunitiesAll/EarnOpportunitiesAll.tsx
- src/app/ui/earn/EarnEmptyList/EarnEmptyListAllMarkets.tsx
Comment on lines
+17
to
+25
| export const EarnCard = memo(EarnCardBase, (prev, next) => { | ||
| return ( | ||
| prev.variant === next.variant && | ||
| prev.data?.slug === next.data?.slug && | ||
| prev.isLoading === next.isLoading && | ||
| prev.isMissingPosition === next.isMissingPosition && | ||
| prev.href === next.href | ||
| ); | ||
| }); |
Contributor
There was a problem hiding this comment.
Memo comparator misses rendered props primaryAction, headerBadge, and fullWidth.
The custom comparator doesn't check primaryAction, headerBadge, or fullWidth, all of which are passed to the variant components and affect rendering. Changes to these props while slug remains stable will be ignored, causing stale UI.
Option 1: Include missing props in comparator
export const EarnCard = memo(EarnCardBase, (prev, next) => {
return (
prev.variant === next.variant &&
prev.data?.slug === next.data?.slug &&
prev.isLoading === next.isLoading &&
prev.isMissingPosition === next.isMissingPosition &&
- prev.href === next.href
+ prev.href === next.href &&
+ prev.fullWidth === next.fullWidth &&
+ prev.primaryAction === next.primaryAction &&
+ prev.headerBadge === next.headerBadge
);
});Option 2: Use default shallow comparison
-export const EarnCard = memo(EarnCardBase, (prev, next) => {
- return (
- prev.variant === next.variant &&
- prev.data?.slug === next.data?.slug &&
- prev.isLoading === next.isLoading &&
- prev.isMissingPosition === next.isMissingPosition &&
- prev.href === next.href
- );
-});
+export const EarnCard = memo(EarnCardBase);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export const EarnCard = memo(EarnCardBase, (prev, next) => { | |
| return ( | |
| prev.variant === next.variant && | |
| prev.data?.slug === next.data?.slug && | |
| prev.isLoading === next.isLoading && | |
| prev.isMissingPosition === next.isMissingPosition && | |
| prev.href === next.href | |
| ); | |
| }); | |
| export const EarnCard = memo(EarnCardBase, (prev, next) => { | |
| return ( | |
| prev.variant === next.variant && | |
| prev.data?.slug === next.data?.slug && | |
| prev.isLoading === next.isLoading && | |
| prev.isMissingPosition === next.isMissingPosition && | |
| prev.href === next.href && | |
| prev.fullWidth === next.fullWidth && | |
| prev.primaryAction === next.primaryAction && | |
| prev.headerBadge === next.headerBadge | |
| ); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/Cards/EarnCard/EarnCard.tsx` around lines 17 - 25, The memo
comparator on EarnCard (memo(EarnCardBase, ...)) currently only compares
variant, data.slug, isLoading, isMissingPosition and href, omitting
primaryAction, headerBadge, and fullWidth which can change rendering; fix by
either (A) adding comparisons for prev.primaryAction === next.primaryAction,
prev.headerBadge === next.headerBadge, and prev.fullWidth === next.fullWidth
inside the custom comparator for EarnCard, or (B) remove the custom comparator
entirely so React.memo uses the default shallow comparison for EarnCardBase to
catch changes to those props.
Comment on lines
+16
to
+22
| {Array.from({ length: 2 }).map((_, index) => ( | ||
| <BaseSkeleton | ||
| key={index} | ||
| variant="rounded" | ||
| width={104} | ||
| height={32} | ||
| /> |
Contributor
There was a problem hiding this comment.
Skeleton tab placeholders don’t match the desktop tabs and can introduce avoidable CLS.
Line 16-22 renders only 2 pills at 32px height, while the desktop filter view renders 3 MD tabs. This mismatch can cause visible layout shift when the real tabs hydrate/load, which is counterproductive for this performance-focused PR.
Proposed adjustment
- {Array.from({ length: 2 }).map((_, index) => (
+ {Array.from({ length: 3 }).map((_, index) => (
<BaseSkeleton
key={index}
variant="rounded"
width={104}
- height={32}
+ height={40}
/>
))}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {Array.from({ length: 2 }).map((_, index) => ( | |
| <BaseSkeleton | |
| key={index} | |
| variant="rounded" | |
| width={104} | |
| height={32} | |
| /> | |
| {Array.from({ length: 3 }).map((_, index) => ( | |
| <BaseSkeleton | |
| key={index} | |
| variant="rounded" | |
| width={104} | |
| height={40} | |
| /> | |
| ))} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/EarnFilterBar/EarnFilterBarSkeleton.tsx` around lines 16 - 22,
The skeleton currently renders two BaseSkeleton pills (Array.from({ length: 2
})) at height 32 which doesn't match the desktop filter's three MD tabs and
causes CLS; update EarnFilterBarSkeleton (where BaseSkeleton is mapped) to
render three placeholders (use length: 3) and adjust the BaseSkeleton props
(variant="rounded", width, height) to match the desktop MD tab dimensions used
by the real tabs so the skeleton layout matches the final UI.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/testData/earnPageFunctions.ts (1)
62-65: Prefer condition-based waiting over fixed 1s sleep after apply.
waitForTimeout(1000)can still be flaky in slower CI runs. Wait for a deterministic UI condition instead.Suggested fix
// Apply the filter and close the modal await page.getByTestId(`${DESKTOP_FILTER_MODAL}-apply-button`).click(); - - await page.waitForTimeout(1000); + await expect( + page.getByTestId(`${DESKTOP_FILTER_MODAL}-apply-button`), + ).toBeHidden(); + await expect(page.getByTestId('earn-opportunities-cards-grid')).toBeVisible();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/testData/earnPageFunctions.ts` around lines 62 - 65, Replace the fixed 1s sleep after clicking the apply button with a condition-based wait: after clicking the locator `${DESKTOP_FILTER_MODAL}-apply-button`, wait for the filter modal to be removed/hidden or for a deterministic UI change that indicates filters were applied (for example wait for the `${DESKTOP_FILTER_MODAL}` test-id to be hidden or for the results list/test-id that reflects applied filters to be visible/updated). Update the test that contains the apply-click (the code using `${DESKTOP_FILTER_MODAL}-apply-button`) to use Playwright's locator.waitFor or page.waitForSelector with an appropriate state ('hidden' or 'visible') instead of waitForTimeout.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/testData/earnPageFunctions.ts`:
- Around line 13-15: The closeFilterModal function currently only presses Escape
and can race with later actions; update closeFilterModal to press Escape and
then wait for the filter modal's apply button to become hidden/removed (e.g.,
waitForSelector('selector-for-apply-button', { state: 'hidden' }) or equivalent)
so callers can assume the modal is fully closed before proceeding; reference the
function name closeFilterModal and the apply-button element when applying the
wait.
---
Nitpick comments:
In `@tests/testData/earnPageFunctions.ts`:
- Around line 62-65: Replace the fixed 1s sleep after clicking the apply button
with a condition-based wait: after clicking the locator
`${DESKTOP_FILTER_MODAL}-apply-button`, wait for the filter modal to be
removed/hidden or for a deterministic UI change that indicates filters were
applied (for example wait for the `${DESKTOP_FILTER_MODAL}` test-id to be hidden
or for the results list/test-id that reflects applied filters to be
visible/updated). Update the test that contains the apply-click (the code using
`${DESKTOP_FILTER_MODAL}-apply-button`) to use Playwright's locator.waitFor or
page.waitForSelector with an appropriate state ('hidden' or 'visible') instead
of waitForTimeout.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b1c425f1-f380-45e5-a1a3-5c06636c9df3
📒 Files selected for processing (1)
tests/testData/earnPageFunctions.ts
Comment on lines
+13
to
+15
| async function closeFilterModal(page: Page) { | ||
| await page.keyboard.press('Escape'); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n tests/testData/earnPageFunctions.ts | head -30Repository: jumperexchange/jumper-exchange
Length of output: 1212
🏁 Script executed:
rg "DESKTOP_FILTER_MODAL" -A 2 -B 2Repository: jumperexchange/jumper-exchange
Length of output: 1032
🏁 Script executed:
rg "closeFilterModal" -A 5 -B 2Repository: jumperexchange/jumper-exchange
Length of output: 3135
🏁 Script executed:
rg "apply-button" -B 5 -A 5 tests/Repository: jumperexchange/jumper-exchange
Length of output: 1829
🏁 Script executed:
rg "close.*modal\|modal.*close" -i tests/testData/earnPageFunctions.ts -B 3 -A 3Repository: jumperexchange/jumper-exchange
Length of output: 56
🏁 Script executed:
rg "modal.*hidden\|hidden.*modal" -i tests/ -B 2 -A 2Repository: jumperexchange/jumper-exchange
Length of output: 56
🏁 Script executed:
rg "Escape.*modal\|modal.*Escape" -i tests/ -B 3 -A 3Repository: jumperexchange/jumper-exchange
Length of output: 56
🏁 Script executed:
rg "earn-filters-desktop-modal" -B 2 -A 5Repository: jumperexchange/jumper-exchange
Length of output: 1421
🏁 Script executed:
grep -rn "toBeHidden\|waitFor.*hidden" tests/ | head -20Repository: jumperexchange/jumper-exchange
Length of output: 186
🏁 Script executed:
cat -n tests/testData/earnPageFunctions.ts | sed -n '35,65p'Repository: jumperexchange/jumper-exchange
Length of output: 1330
🏁 Script executed:
rg "getByTestId.*apply-button" tests/testData/earnPageFunctions.ts -B 3 -A 3Repository: jumperexchange/jumper-exchange
Length of output: 308
🏁 Script executed:
rg "Modal" src/components/EarnFilterBar -A 10 | head -40Repository: jumperexchange/jumper-exchange
Length of output: 2484
🏁 Script executed:
rg "onClose\|Escape" src/components/EarnFilterBar -B 2 -A 2Repository: jumperexchange/jumper-exchange
Length of output: 56
🏁 Script executed:
fd -name "FilterSortModal*" src/Repository: jumperexchange/jumper-exchange
Length of output: 305
🏁 Script executed:
rg "keyboard.press.*Escape\|onClose" src/components/composite/FilterSortModal -B 2 -A 2Repository: jumperexchange/jumper-exchange
Length of output: 56
🏁 Script executed:
fd "FilterSortModal" src/Repository: jumperexchange/jumper-exchange
Length of output: 246
🏁 Script executed:
rg "onClose" src/components/composite/FilterSortModal -B 3 -A 3Repository: jumperexchange/jumper-exchange
Length of output: 1802
🏁 Script executed:
cat -n src/components/composite/FilterSortModal/FilterSortModal.tsx | head -80Repository: jumperexchange/jumper-exchange
Length of output: 2971
🏁 Script executed:
rg "ModalContainer" src/components/composite -B 2 -A 10 | grep -A 10 "FilterSortModal"Repository: jumperexchange/jumper-exchange
Length of output: 4478
🏁 Script executed:
cat -n src/components/core/modals/ModalContainer/ModalContainer.tsx | head -60Repository: jumperexchange/jumper-exchange
Length of output: 1818
🏁 Script executed:
rg "useFullScreenDrawer" src/components/core/FullScreenDrawer -B 2 -A 15 | head -50Repository: jumperexchange/jumper-exchange
Length of output: 1288
🏁 Script executed:
cat -n src/components/core/FullScreenDrawer/hooks.tsRepository: jumperexchange/jumper-exchange
Length of output: 765
🏁 Script executed:
rg "toBeVisible\|toBeHidden" tests/testData/earnPageFunctions.tsRepository: jumperexchange/jumper-exchange
Length of output: 56
Add postcondition assertion to closeFilterModal to prevent race conditions.
closeFilterModal sends Escape key but doesn't verify the modal is closed, which can cause race conditions in subsequent code. The apply-button should become hidden when the modal closes.
Suggested fix
async function closeFilterModal(page: Page) {
await page.keyboard.press('Escape');
+ await expect(
+ page.getByTestId(`${DESKTOP_FILTER_MODAL}-apply-button`),
+ ).toBeHidden();
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function closeFilterModal(page: Page) { | |
| await page.keyboard.press('Escape'); | |
| } | |
| async function closeFilterModal(page: Page) { | |
| await page.keyboard.press('Escape'); | |
| await expect( | |
| page.getByTestId(`${DESKTOP_FILTER_MODAL}-apply-button`), | |
| ).toBeHidden(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tests/testData/earnPageFunctions.ts` around lines 13 - 15, The
closeFilterModal function currently only presses Escape and can race with later
actions; update closeFilterModal to press Escape and then wait for the filter
modal's apply button to become hidden/removed (e.g.,
waitForSelector('selector-for-apply-button', { state: 'hidden' }) or equivalent)
so callers can assume the modal is fully closed before proceeding; reference the
function name closeFilterModal and the apply-button element when applying the
wait.
Kayanski
reviewed
May 4, 2026
| case CategoryContentType.SingleSelect: | ||
| return <SingleSelectView category={category} slotProps={slotProps} />; | ||
|
|
||
| case CategoryContentType.Toggle: |
Contributor
There was a problem hiding this comment.
Toggle view button doesn't show as an active filter in the layout.
Other filters show 1 or N hen filtering for N assets or n chains.
The only rewards button does not which makes the UI less clear
Kayanski
reviewed
May 4, 2026
| ? createToggleCategory({ | ||
| id: 'rewardsAPY', | ||
| label: t('earn.filter.rewards.label'), | ||
| value: pendingValues.rewardsAPY, |
Contributor
There was a problem hiding this comment.
adding a badgeLabel here makes sense I think, UX wise
Kayanski
reviewed
May 4, 2026
| const initialAllOpportunities = await getEarnInitialAllOpportunitiesCached(); | ||
|
|
||
| return ( | ||
| <Suspense fallback={<EarnsPageSkeleton />}> |
Contributor
There was a problem hiding this comment.
Not using EarnPageSkeleton anymore ?
Kayanski
approved these changes
May 4, 2026
Kayanski
left a comment
Kayanski
left a comment
Contributor
There was a problem hiding this comment.
Ok, Very nice ! Apart the small changes prompted :)
mmioana
marked this pull request as draft
June 2, 2026 15:37
Contributor
Author
|
Closing this as there has been a different performance improvement done for earn. Will treat the filters refactoring in a different PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which Jira task belongs to this PR?
Closes https://linear.app/lifi-linear/issue/JUM-609/webapp-and-mobile-improve-performance-for-earn
Reports from 03.04.26
Note
On preview branch SEO results will be low as this page is not allowed to be indexed
Testing steps
/earnand/earn?tab=allWhy did I implement it this way?
Checklist before requesting a review
Summary by CodeRabbit
New Features
Refactor
Chores