fix: category selection #532
Conversation
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughRemoves footer-outlet contexts/components; adds PageCta type and PageCtaButton; hooks/providers now return CTAs; pages render PageCtaButton; earn/dashboard selection gains category-awareness; price-per-share/min-stake formatters and reward UI consolidated; layout/animation and tests updated. ChangesFooter Outlet Removal and PageCta Integration
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
packages/widget/src/pages/review/hooks/use-pending-review.hook.ts (1)
151-158: ⚡ Quick winDrop the CTA
useMemounless a consumer needs identity stability.This object is derived from local state and translations only. Under the React Compiler guideline, prefer a plain
PageCtahere and keep memoization for boundaries that truly need stable identity.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: "React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/review/hooks/use-pending-review.hook.ts` around lines 151 - 158, The CTA object is unnecessarily wrapped in useMemo; remove the useMemo wrapper and return a plain PageCta object named cta constructed from t("shared.confirm"), onClickRef.current, disabled: false, and isLoading: actionPendingMutation.isPending so its identity is not artificially memoized; update references to the existing cta variable (created where useMemo was) and remove useMemo import if now unused.Source: Coding guidelines
packages/widget/src/pages/complete/hooks/use-complete.hook.ts (1)
58-68: ⚡ Quick winDrop the CTA
useMemounless a consumer needs identity stability.This is just local render data in the supplied context. With React Compiler enabled, prefer a plain
PageCtaobject here and only memoize at the boundary that actually depends on referential stability.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: "React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/complete/hooks/use-complete.hook.ts` around lines 58 - 68, The CTA is unnecessarily wrapped in useMemo (const cta = useMemo<PageCta>(...)) despite being local render state; remove the useMemo and return a plain PageCta object instead (preserve properties: disabled, isLoading, label: t("complete.continue", { context: isLedgerLive ? "ledger" : undefined }), onClick: () => onClickRef.current(), hide: !!activityReviewMatch) so consumers that do need referential stability can be memoized at their boundary; update any references to the cta variable to use the plain object and remove useMemo import if now unused.Source: Coding guidelines
packages/widget/src/pages-dashboard/position-details/components/position-details-actions.tsx (1)
85-114: ⚡ Quick winDrop the CTA
useMemounless a consumer needs identity stability.Here the value is immediately consumed as a render prop for
PageCtaButton, so a plainPageCtaobject is the better fit under the React Compiler rule.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: "React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages-dashboard/position-details/components/position-details-actions.tsx` around lines 85 - 114, Remove the unnecessary useMemo and replace unstakeCta with a plain PageCta value computed inline (still returning null when isLoading is true) since identity stability isn't required by consumers; keep the same fields (disabled: unstakeDisabled, isLoading: false, label: t(...getExtendedYieldType(integrationData)), onClick: onUnstakeClick) and the Maybe.fromRecord mapping logic or its equivalent so PageCtaButton receives identical props but without wrapping in useMemo.Source: Coding guidelines
packages/widget/src/pages/review/hooks/use-action-review.hook.ts (1)
146-154: ⚡ Quick winDrop the CTA
useMemounless a consumer needs identity stability.Nothing in the supplied context shows a boundary that requires this object to stay referentially stable. Prefer a plain
PageCtavalue and memoize only where identity is actually observed.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: "React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/review/hooks/use-action-review.hook.ts` around lines 146 - 154, The CTA object currently created with useMemo (const cta = useMemo<PageCta>(...)) should be converted to a plain PageCta value since no consumer requires referential identity; remove the useMemo call and return a simple object for cta with the same properties (label: t(`activity.review.${labelKey}`), onClick: () => navigate(`/activity/${path}/steps`), disabled: false, isLoading: false, hide: actionOlderThan7Days), keeping the same referenced symbols (cta, PageCta, labelKey, navigate, path, actionOlderThan7Days, t) so behavior is unchanged while complying with the React Compiler guideline against unnecessary useMemo.Source: Coding guidelines
packages/widget/src/pages/steps/hooks/use-steps.hook.ts (1)
175-186: ⚡ Quick winDrop the CTA
useMemounless a consumer needs identity stability.The new CTA is plain derived render state in the supplied code. Prefer returning a normal
PageCtavalue here and reserve manual memoization for an actual identity-sensitive boundary.As per coding guidelines,
packages/widget/src/**/*.{ts,tsx}: "React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/steps/hooks/use-steps.hook.ts` around lines 175 - 186, The CTA currently wrapped in useMemo (const cta = useMemo<PageCta>(...)) should be converted to a plain derived value: remove the useMemo import/usage and return a direct PageCta (or null) based on txStates.length, preserving the same shape (disabled, isLoading, label: t("shared.cancel"), onClick calling onClickRef.current(), variant: "secondary") and keeping the PageCta type annotation; this drops unnecessary identity memoization while retaining onClickRef, txStates and t usage.Source: Coding guidelines
packages/widget/src/pages/review/hooks/use-stake-review.hook.ts (1)
198-212: ⚡ Quick winPrefer a plain CTA object here.
The hook already returns a fresh object on Line 231, so memoizing only
ctadoes not buy semantic stability and just adds dependency bookkeeping. As per coding guidelines,packages/widget/src/**/*.{ts,tsx}: React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/review/hooks/use-stake-review.hook.ts` around lines 198 - 212, The cta is unnecessarily wrapped in useMemo; remove the useMemo call and return a plain PageCta object instead — construct cta as { disabled: kycGateIsBlocking, isLoading: enterMutation.isPending || yieldKycGate.isLoading, label: t("shared.confirm"), onClick: () => onClickRef.current() } so you still reference onClickRef, enterMutation, yieldKycGate, kycGateIsBlocking and t but avoid memo/dependency bookkeeping; ensure the hook's returned value remains the fresh object used later.Source: Coding guidelines
packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx (1)
751-789: ⚡ Quick winDrop the
useMemoaround this CTA.
EarnPageContext.Providerstill receives a freshvalueobject below, so memoizing onlyctadoes not provide the semantic stability the compiler guideline allows it for. A plainconst cta = ...is simpler here. As per coding guidelines,packages/widget/src/**/*.{ts,tsx}: React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx` around lines 751 - 789, Remove the useMemo wrapper around the cta variable and replace it with a plain const cta = ... expression so the CTA is computed directly (keep the same conditional structure and all referenced symbols: buttonCTAText, buttonDisabled, isConnected, isLedgerLiveAccountPlaceholder, externalProviders, appLoading, isFetching, yieldKycGate.isLoading, onClickRef, connectClickRef, registerFooterButton, hasNotYieldsForToken, and t); this aligns with the React Compiler guideline because EarnPageContext.Provider still creates a fresh value object and there is no semantic identity requirement that needs memoization.Source: Coding guidelines
packages/widget/src/pages/review/hooks/use-unstake-review.hook.ts (1)
159-167: ⚡ Quick winPrefer a plain CTA object here.
This hook also returns a new object below, so memoizing just
ctadoes not create any semantic-stability guarantee for consumers. As per coding guidelines,packages/widget/src/**/*.{ts,tsx}: React Compiler is enabled. Do not adduseMemo,useCallback, orReact.memoonly for render-performance optimization; prefer plain values/functions. Use manual memoization only when required for semantic stability, such as an external API dependency or context value identity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/review/hooks/use-unstake-review.hook.ts` around lines 159 - 167, The cta currently uses useMemo (const cta = useMemo<PageCta>(...)) which is unnecessary and prevents semantic-stability guarantees because the hook already returns a new object later; replace the useMemo with a plain PageCta object (const cta: PageCta = { label: t("shared.confirm"), onClick: () => onClickRef.current(), disabled: kycGateIsBlocking, isLoading: unstakeIsLoading || yieldKycGate.isLoading }) so you return a regular value and preserve the same referenced symbols (cta, PageCta, onClickRef, kycGateIsBlocking, unstakeIsLoading, yieldKycGate.isLoading) without using useMemo/useCallback.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`:
- Around line 476-480: onTokenBalanceSelect currently always dispatches { type:
"token/select" } which ignores dashboard context and can change
selectedDashboardYieldCategory; instead, read selectedDashboardYieldCategory
from state (or props) and when a dashboard category is active dispatch the
category-aware action (e.g. { type: "dashboard/token/select", data: { token:
tokenBalance.token, category: selectedDashboardYieldCategory } } or the existing
category-aware action your reducer expects) otherwise fall back to dispatching {
type: "token/select", data: tokenBalance.token }; update the
onTokenBalanceSelect useCallback to branch on selectedDashboardYieldCategory and
call the appropriate dispatch so token picks on dashboard flows preserve the
active category.
---
Nitpick comments:
In
`@packages/widget/src/pages-dashboard/position-details/components/position-details-actions.tsx`:
- Around line 85-114: Remove the unnecessary useMemo and replace unstakeCta with
a plain PageCta value computed inline (still returning null when isLoading is
true) since identity stability isn't required by consumers; keep the same fields
(disabled: unstakeDisabled, isLoading: false, label:
t(...getExtendedYieldType(integrationData)), onClick: onUnstakeClick) and the
Maybe.fromRecord mapping logic or its equivalent so PageCtaButton receives
identical props but without wrapping in useMemo.
In `@packages/widget/src/pages/complete/hooks/use-complete.hook.ts`:
- Around line 58-68: The CTA is unnecessarily wrapped in useMemo (const cta =
useMemo<PageCta>(...)) despite being local render state; remove the useMemo and
return a plain PageCta object instead (preserve properties: disabled, isLoading,
label: t("complete.continue", { context: isLedgerLive ? "ledger" : undefined }),
onClick: () => onClickRef.current(), hide: !!activityReviewMatch) so consumers
that do need referential stability can be memoized at their boundary; update any
references to the cta variable to use the plain object and remove useMemo import
if now unused.
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`:
- Around line 751-789: Remove the useMemo wrapper around the cta variable and
replace it with a plain const cta = ... expression so the CTA is computed
directly (keep the same conditional structure and all referenced symbols:
buttonCTAText, buttonDisabled, isConnected, isLedgerLiveAccountPlaceholder,
externalProviders, appLoading, isFetching, yieldKycGate.isLoading, onClickRef,
connectClickRef, registerFooterButton, hasNotYieldsForToken, and t); this aligns
with the React Compiler guideline because EarnPageContext.Provider still creates
a fresh value object and there is no semantic identity requirement that needs
memoization.
In `@packages/widget/src/pages/review/hooks/use-action-review.hook.ts`:
- Around line 146-154: The CTA object currently created with useMemo (const cta
= useMemo<PageCta>(...)) should be converted to a plain PageCta value since no
consumer requires referential identity; remove the useMemo call and return a
simple object for cta with the same properties (label:
t(`activity.review.${labelKey}`), onClick: () =>
navigate(`/activity/${path}/steps`), disabled: false, isLoading: false, hide:
actionOlderThan7Days), keeping the same referenced symbols (cta, PageCta,
labelKey, navigate, path, actionOlderThan7Days, t) so behavior is unchanged
while complying with the React Compiler guideline against unnecessary useMemo.
In `@packages/widget/src/pages/review/hooks/use-pending-review.hook.ts`:
- Around line 151-158: The CTA object is unnecessarily wrapped in useMemo;
remove the useMemo wrapper and return a plain PageCta object named cta
constructed from t("shared.confirm"), onClickRef.current, disabled: false, and
isLoading: actionPendingMutation.isPending so its identity is not artificially
memoized; update references to the existing cta variable (created where useMemo
was) and remove useMemo import if now unused.
In `@packages/widget/src/pages/review/hooks/use-stake-review.hook.ts`:
- Around line 198-212: The cta is unnecessarily wrapped in useMemo; remove the
useMemo call and return a plain PageCta object instead — construct cta as {
disabled: kycGateIsBlocking, isLoading: enterMutation.isPending ||
yieldKycGate.isLoading, label: t("shared.confirm"), onClick: () =>
onClickRef.current() } so you still reference onClickRef, enterMutation,
yieldKycGate, kycGateIsBlocking and t but avoid memo/dependency bookkeeping;
ensure the hook's returned value remains the fresh object used later.
In `@packages/widget/src/pages/review/hooks/use-unstake-review.hook.ts`:
- Around line 159-167: The cta currently uses useMemo (const cta =
useMemo<PageCta>(...)) which is unnecessary and prevents semantic-stability
guarantees because the hook already returns a new object later; replace the
useMemo with a plain PageCta object (const cta: PageCta = { label:
t("shared.confirm"), onClick: () => onClickRef.current(), disabled:
kycGateIsBlocking, isLoading: unstakeIsLoading || yieldKycGate.isLoading }) so
you return a regular value and preserve the same referenced symbols (cta,
PageCta, onClickRef, kycGateIsBlocking, unstakeIsLoading,
yieldKycGate.isLoading) without using useMemo/useCallback.
In `@packages/widget/src/pages/steps/hooks/use-steps.hook.ts`:
- Around line 175-186: The CTA currently wrapped in useMemo (const cta =
useMemo<PageCta>(...)) should be converted to a plain derived value: remove the
useMemo import/usage and return a direct PageCta (or null) based on
txStates.length, preserving the same shape (disabled, isLoading, label:
t("shared.cancel"), onClick calling onClickRef.current(), variant: "secondary")
and keeping the PageCta type annotation; this drops unnecessary identity
memoization while retaining onClickRef, txStates and t usage.
🪄 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: 6c685530-9601-45ef-aa14-02bc6b706101
📒 Files selected for processing (37)
packages/widget/src/Widget.tsxpackages/widget/src/domain/types/yields.tspackages/widget/src/hooks/api/use-dashboard-yield-catalog.tspackages/widget/src/navigation/containers/animation-layout.tsxpackages/widget/src/navigation/containers/animation-page.tsxpackages/widget/src/pages-dashboard/activity/activity-details.page.tsxpackages/widget/src/pages-dashboard/common/components/footer-outlet/index.tsxpackages/widget/src/pages-dashboard/common/components/tabs/index.tsxpackages/widget/src/pages-dashboard/overview/earn-page/index.tsxpackages/widget/src/pages-dashboard/overview/index.tsxpackages/widget/src/pages-dashboard/position-details/components/position-details-actions.tsxpackages/widget/src/pages-dashboard/position-details/components/position-details-stake-actions.tsxpackages/widget/src/pages-dashboard/position-details/index.tsxpackages/widget/src/pages/complete/hooks/use-complete.hook.tspackages/widget/src/pages/complete/pages/common.page.tsxpackages/widget/src/pages/complete/state/index.tsxpackages/widget/src/pages/components/footer-outlet/context.tspackages/widget/src/pages/components/footer-outlet/index.tsxpackages/widget/src/pages/components/footer-outlet/styles.css.tspackages/widget/src/pages/components/page-cta.tsxpackages/widget/src/pages/details/earn-page/earn.page.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsxpackages/widget/src/pages/details/earn-page/state/types.tspackages/widget/src/pages/review/hooks/use-action-review.hook.tspackages/widget/src/pages/review/hooks/use-pending-review.hook.tspackages/widget/src/pages/review/hooks/use-stake-review.hook.tspackages/widget/src/pages/review/hooks/use-unstake-review.hook.tspackages/widget/src/pages/review/pages/action-review.page.tsxpackages/widget/src/pages/review/pages/common-page/common.page.tsxpackages/widget/src/pages/review/pages/pending-review.page.tsxpackages/widget/src/pages/review/pages/stake-review.page.tsxpackages/widget/src/pages/review/pages/unstake-review.page.tsxpackages/widget/src/pages/steps/hooks/use-steps.hook.tspackages/widget/src/pages/steps/pages/common.page.tsxpackages/widget/src/providers/index.tsxpackages/widget/tests/pages-dashboard/position-details-action-tabs.test.tsx
💤 Files with no reviewable changes (8)
- packages/widget/src/pages/components/footer-outlet/index.tsx
- packages/widget/src/Widget.tsx
- packages/widget/src/pages/components/footer-outlet/context.ts
- packages/widget/src/pages-dashboard/overview/index.tsx
- packages/widget/src/pages/components/footer-outlet/styles.css.ts
- packages/widget/src/pages-dashboard/position-details/index.tsx
- packages/widget/src/pages-dashboard/common/components/footer-outlet/index.tsx
- packages/widget/tests/pages-dashboard/position-details-action-tabs.test.tsx
| const onTokenBalanceSelect = useCallback( | ||
| async (tokenBalance: TokenBalanceScanResponseDto) => { | ||
| const category = dashboardVariant ? selectedDashboardYieldCategory : null; | ||
|
|
||
| if (!category) { | ||
| dispatch({ type: "token/select", data: tokenBalance.token }); | ||
| return; | ||
| } | ||
|
|
||
| const selectionRequestId = dashboardTokenSelectionRequestRef.current + 1; | ||
| dashboardTokenSelectionRequestRef.current = selectionRequestId; | ||
| dispatch({ type: "token-only/select", data: tokenBalance.token }); | ||
|
|
||
| const tokenYield = await getDashboardTokenYield({ | ||
| category, | ||
| tokenBalance, | ||
| }).catch((error) => { | ||
| console.log(error); | ||
| return null; | ||
| }); | ||
|
|
||
| if ( | ||
| selectionRequestId !== dashboardTokenSelectionRequestRef.current || | ||
| !tokenYield | ||
| ) { | ||
| return; | ||
| } | ||
|
|
||
| selectDashboardTokenYield({ | ||
| token: tokenBalance.token, | ||
| yieldDto: tokenYield.yieldDto, | ||
| yieldId: tokenYield.yieldId, | ||
| }); | ||
| }, | ||
| [ | ||
| dashboardVariant, | ||
| dispatch, | ||
| getDashboardTokenYield, | ||
| selectedDashboardYieldCategory, | ||
| selectDashboardTokenYield, | ||
| ] | ||
| (tokenBalance: TokenBalanceScanResponseDto) => | ||
| dispatch({ type: "token/select", data: tokenBalance.token }), | ||
| [dispatch] | ||
| ); |
There was a problem hiding this comment.
Keep token picks category-aware on dashboard flows.
This now routes every token click through "token/select". That reducer path recomputes the initial yield from the token alone, so picking a token while the user is on rwa or defi can auto-select a yield from another category and immediately change selectedDashboardYieldCategory to that new yield's category. Route dashboard token picks through the category-aware selection path when a dashboard category is active.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/widget/src/pages/details/earn-page/state/earn-page-context.tsx`
around lines 476 - 480, onTokenBalanceSelect currently always dispatches { type:
"token/select" } which ignores dashboard context and can change
selectedDashboardYieldCategory; instead, read selectedDashboardYieldCategory
from state (or props) and when a dashboard category is active dispatch the
category-aware action (e.g. { type: "dashboard/token/select", data: { token:
tokenBalance.token, category: selectedDashboardYieldCategory } } or the existing
category-aware action your reducer expects) otherwise fall back to dispatching {
type: "token/select", data: tokenBalance.token }; update the
onTokenBalanceSelect useCallback to branch on selectedDashboardYieldCategory and
call the appropriate dispatch so token picks on dashboard flows preserve the
active category.
Move stake initialization into the earn page reducer. Skip catalog probes while wallet connection is pending.
petar-omni
force-pushed
the
refactor-page-cta-flow
branch
from
3be9f08 to
5f2d2d1
Compare
| selectedDashboardYieldCategory, | ||
| selectDashboardTokenYield, | ||
| ] | ||
| (tokenBalance: TokenBalanceScanResponseDto) => |
There was a problem hiding this comment.
possible dashboard regression? token selection no longer preserves the active category and can fall back to a yield from a different tab
| @@ -684,12 +491,14 @@ export const EarnPageContextProvider = ({ | |||
| setSelectedDashboardYieldCategoryFallback(category); | |||
|
|
|||
| const target = | |||
There was a problem hiding this comment.
same here: switching back to a category now restores its initial seed instead of the last token/yield previously selected in that tab
Move dashboard yield category into earn page reducer state and filter first-eligible-yield by category when initializing stakes. Remove unused token-list-yields helpers and fetchAllYieldSummaries pagination.
ProviderIcon used providerIcon for the main image and mainUrl for the network badge. Use mainUrl for the token logo and providerIcon for the badge when present. Add min-width on earn-page via provider images to prevent layout overflow.
Show pricePerShareState.price in earn and position detail rows when the yield provides it. Rename cooldown copy to redemption time in EN and FR.
Some yields expose reward metadata through outputToken rather than rewardRate components. Include it so reward badges and meta info render correctly for those yields.
Use "Minimum subscription" instead of "Min stake" for RWA yields in earn and position details. Stretch the earn details panel to fill available height so long detail lists scroll within the panel.
The stake/enter confirmation screen showed an "Estimated rewards" heading with no yearly or monthly values. Reuse the shared earn-page reward amounts on review so users see the same projections before confirming.
Use absolute positioning for the earn details scroll container so content scrolls independently of the page footer CTA. Remove redundant bottom margins from PageContainer and PageCtaButton.
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 (1)
packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx (1)
120-154:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffDashboard category selection always resets to the catalog's initial seed.
When the user switches dashboard categories (line 121-123), the reducer uses
dashboardYieldCatalog.initialSelectionByCategory.get(action.data)to pick the token and yield. This means switching back to a category always restores the catalog's initial seed for that category, not the user's last selection within that category.For example:
- User is on "rwa" tab, selects tokenA → yieldX
- User switches to "defi" tab
- User switches back to "rwa" tab → resets to the catalog seed, losing tokenA/yieldX selection
If you want to preserve per-category selections across tab switches, consider tracking
lastSelectionByCategoryin state and using that instead of the catalog seed.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx` around lines 120 - 154, The reducer currently always uses dashboardYieldCatalog.initialSelectionByCategory.get(action.data) which forces the catalog seed on category switches; modify the logic in the "dashboard/yield-category/select" case to first look up a per-category last selection stored in state (e.g. state.lastSelectionByCategory.get(action.data)) and fall back to dashboardYieldCatalog.initialSelectionByCategory only if no last selection exists, and ensure you add/initialize a lastSelectionByCategory Map in the state shape and update it whenever onYieldSelectState (or wherever selections change) is called so selections are persisted per category instead of resetting to the catalog seed; keep the existing equality checks (selectedToken, selectedStakeId) and use Maybe semantic as before.
🧹 Nitpick comments (1)
packages/widget/src/pages/review/pages/common-page/components/review-top-section.tsx (1)
82-89: 💤 Low valueConsider using
.alt()instead of??for Maybe fallback.The prop
estimatedRewardAmountsis typed asMaybe<{...}>, so when it'sMaybe.empty()the?? Maybe.empty()fallback will never trigger (sinceMaybe.empty()is truthy). For idiomatic purify-ts usage, use.alt(Maybe.empty())or rely on the optional prop:- {(estimatedRewardAmounts ?? Maybe.empty()) + {estimatedRewardAmounts .map((amounts) => (🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/widget/src/pages/review/pages/common-page/components/review-top-section.tsx` around lines 82 - 89, The code uses a truthy JavaScript nullish fallback (estimatedRewardAmounts ?? Maybe.empty()) which never runs because Maybe.empty() is truthy; replace that expression with the purify-ts alternative operator so the Maybe fallback works — e.g. use estimatedRewardAmounts.alt(Maybe.empty()). Leave the subsequent .map(...) and .extractNullable() calls unchanged so the mapping and extraction on the Maybe returned by .alt() behave correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx`:
- Around line 87-116: The token/select reducer currently recomputes
selectedDashboardYieldCategory from the returned yieldDto (via
getDashboardYieldCategory) which allows token selection to change the active
dashboard tab; to fix, stop overwriting the current category or ensure yields
are pre-filtered by it: either (A) keep selectedDashboardYieldCategory:
state.selectedDashboardYieldCategory in the .map that builds State so the
existing dashboard tab is preserved, or (B) modify the getInitYield flow to
filter eligible yields by state.selectedDashboardYieldCategory before
auto-selecting a yield (so getInitYield/onYieldSelectState return a yieldDto
that already respects the dashboard category). Reference symbols: token/select
reducer, getInitYield, getDashboardYieldCategory, onYieldSelectState,
state.selectedDashboardYieldCategory.
---
Outside diff comments:
In
`@packages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsx`:
- Around line 120-154: The reducer currently always uses
dashboardYieldCatalog.initialSelectionByCategory.get(action.data) which forces
the catalog seed on category switches; modify the logic in the
"dashboard/yield-category/select" case to first look up a per-category last
selection stored in state (e.g. state.lastSelectionByCategory.get(action.data))
and fall back to dashboardYieldCatalog.initialSelectionByCategory only if no
last selection exists, and ensure you add/initialize a lastSelectionByCategory
Map in the state shape and update it whenever onYieldSelectState (or wherever
selections change) is called so selections are persisted per category instead of
resetting to the catalog seed; keep the existing equality checks (selectedToken,
selectedStakeId) and use Maybe semantic as before.
---
Nitpick comments:
In
`@packages/widget/src/pages/review/pages/common-page/components/review-top-section.tsx`:
- Around line 82-89: The code uses a truthy JavaScript nullish fallback
(estimatedRewardAmounts ?? Maybe.empty()) which never runs because Maybe.empty()
is truthy; replace that expression with the purify-ts alternative operator so
the Maybe fallback works — e.g. use estimatedRewardAmounts.alt(Maybe.empty()).
Leave the subsequent .map(...) and .extractNullable() calls unchanged so the
mapping and extraction on the Maybe returned by .alt() behave correctly.
🪄 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: 168f6923-a830-4b23-a737-e7396a0eaea1
📒 Files selected for processing (31)
packages/widget/src/components/atoms/token-icon/provider-icon/index.tsxpackages/widget/src/components/molecules/estimated-reward-amounts/index.tsxpackages/widget/src/domain/types/yields.tspackages/widget/src/hooks/api/use-multi-yields.tspackages/widget/src/hooks/api/use-token-list-yields.tspackages/widget/src/hooks/api/use-yield-summaries.tspackages/widget/src/pages-dashboard/overview/earn-details/earn-details-formatters.tspackages/widget/src/pages-dashboard/overview/earn-details/earn-details-model.tsxpackages/widget/src/pages-dashboard/overview/earn-details/styles.css.tspackages/widget/src/pages-dashboard/position-details/position-details-model.tsxpackages/widget/src/pages/components/page-container.tsxpackages/widget/src/pages/components/page-cta.tsxpackages/widget/src/pages/details/activity-page/hooks/use-activity-filters.tspackages/widget/src/pages/details/earn-page/components/select-yield-section/select-yield-reward-details.tsxpackages/widget/src/pages/details/earn-page/components/select-yield-section/styles.css.tspackages/widget/src/pages/details/earn-page/state/earn-page-context.tsxpackages/widget/src/pages/details/earn-page/state/earn-page-state-context.tsxpackages/widget/src/pages/details/earn-page/state/types.tspackages/widget/src/pages/details/earn-page/state/use-get-init-yield.tspackages/widget/src/pages/details/earn-page/state/use-init-yield.tspackages/widget/src/pages/review/hooks/use-stake-review.hook.tspackages/widget/src/pages/review/pages/common-page/common.page.tsxpackages/widget/src/pages/review/pages/common-page/components/review-top-section.tsxpackages/widget/src/pages/review/pages/stake-review.page.tsxpackages/widget/src/translation/English/translations.jsonpackages/widget/src/translation/French/translations.jsonpackages/widget/tests/domain/token-list-yields.test.tspackages/widget/tests/domain/yield-summaries.test.tspackages/widget/tests/pages-dashboard/earn-details-model.test.tsxpackages/widget/tests/pages-dashboard/position-details-model.test.tsxpackages/widget/tests/use-cases/staking-flow/staking-flow.test.tsx
💤 Files with no reviewable changes (5)
- packages/widget/src/pages/components/page-container.tsx
- packages/widget/src/hooks/api/use-token-list-yields.ts
- packages/widget/tests/domain/token-list-yields.test.ts
- packages/widget/tests/domain/yield-summaries.test.ts
- packages/widget/src/hooks/api/use-yield-summaries.ts
✅ Files skipped from review due to trivial changes (3)
- packages/widget/tests/use-cases/staking-flow/staking-flow.test.tsx
- packages/widget/src/translation/French/translations.json
- packages/widget/src/translation/English/translations.json
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/widget/src/pages/review/pages/stake-review.page.tsx
- packages/widget/src/pages/components/page-cta.tsx
- packages/widget/src/pages/review/hooks/use-stake-review.hook.ts
- packages/widget/src/pages/review/pages/common-page/common.page.tsx
- packages/widget/src/pages/details/earn-page/state/types.ts
petar-omni
force-pushed
the
refactor-page-cta-flow
branch
from
6be5b4b to
1f77fe2
Compare
Activity filter pills were hidden unless loaded actions spanned two or more dashboard categories. Show All plus the matching category filters whenever at least one categorized activity is loaded.
petar-omni
force-pushed
the
refactor-page-cta-flow
branch
from
1f77fe2 to
ef989a3
Compare
5 participants
Summary by CodeRabbit
Refactor
New Features
Other