PRO-3800 - Added Warning Message on Sell#429
Conversation
WalkthroughAdds a $1 minimum-value check to the Sell flow by introducing minAmount state, updating input/change validation, extending notEnoughLiquidity gating, and adjusting error and button-disable logic accordingly in Sell.tsx. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Sell as Sell.tsx
participant Token as Token Data
participant Button as SellButton
User->>Sell: Enter token amount
Sell->>Token: Read token.usdValue
alt usdValue available
Sell->>Sell: Compute USD = amount * usdValue
Sell->>Sell: Set minAmount = (USD < $1)
else usdValue unavailable
Sell->>Sell: Set minAmount = false
end
Sell->>Sell: Validate balance/liquidity + minAmount
Sell->>Button: notEnoughLiquidity = liquidityFail || minAmount
Note over Sell,Button: Render errors:<br/>- Not enough balance (liquidity fail)<br/>- Min amount is $1 worth of tokens (minAmount)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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: 2
🧹 Nitpick comments (1)
src/apps/pulse/components/Sell/Sell.tsx (1)
67-67: Consider checkingminAmountin the offer fetch logic.The
fetchSellOfferfunction checks!notEnoughLiquiditybut doesn't check!minAmount. This means the component will fetch sell offers even when the amount is below the $1 minimum, which is wasteful since the user cannot proceed with the transaction anyway.Apply this diff to add the
minAmountcheck:if ( debouncedTokenAmount && token && isInitialized && parseFloat(debouncedTokenAmount) > 0 && - !notEnoughLiquidity + !notEnoughLiquidity && + !minAmount ) { setIsLoadingOffer(true); try {And update the dependency array on line 92:
- }, [debouncedTokenAmount, token, isInitialized, notEnoughLiquidity]); + }, [debouncedTokenAmount, token, isInitialized, notEnoughLiquidity, minAmount]);Also update the else-if condition:
- } else if (notEnoughLiquidity) { + } else if (notEnoughLiquidity || minAmount) { setLocalSellOffer(null); setIsLoadingOffer(false); }Also applies to: 87-89
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apps/pulse/components/Sell/Sell.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
📚 Learning: 2025-09-09T12:40:15.629Z
Learnt from: RanaBug
PR: pillarwallet/x#391
File: src/apps/pulse/components/Sell/Sell.tsx:113-130
Timestamp: 2025-09-09T12:40:15.629Z
Learning: In the Pulse app Sell component, when a user changes/switches tokens, the input amount automatically resets to 0, which means liquidity validation state doesn't become stale when tokens change.
Applied to files:
src/apps/pulse/components/Sell/Sell.tsx
🔇 Additional comments (4)
src/apps/pulse/components/Sell/Sell.tsx (4)
54-54: LGTM!The new
minAmountstate variable is properly declared to track the $1 minimum requirement.
137-137: LGTM!Correctly resets
minAmountwhen the input is empty.
315-315: Consider the error message precedence.The warning condition correctly checks for all three error states. However, the error message rendering uses OR chaining, which means if both
notEnoughLiquidityandminAmountare true, only "Not enough balance" will be displayed. While this prioritization may be intentional, consider whether users might benefit from seeing both messages or if a combined message would be clearer.The change from "Not enough liquidity" to "Not enough balance" aligns with the PR objectives.
Verify this behavior is acceptable: when a user enters an amount that both exceeds their balance AND is below $1 USD, only "Not enough balance" is shown.
Also applies to: 329-330
427-427: LGTM!Correctly combines both
notEnoughLiquidityandminAmountconditions to disable the sell button when either validation fails.
| if (!Number.isNaN(token?.usdValue)) { | ||
| setMinAmount(inputAmount * Number(token?.usdValue) < 1); | ||
| } else { | ||
| setMinAmount(false); | ||
| } |
There was a problem hiding this comment.
Handle the edge case when usdValue is zero.
If token.usdValue is 0 (price data unavailable or zero-value token), the calculation inputAmount * 0 < 1 will always be true for any positive input, incorrectly blocking valid sell operations.
Apply this diff to properly handle the edge case:
- if (!Number.isNaN(token?.usdValue)) {
+ if (!Number.isNaN(token?.usdValue) && Number(token?.usdValue) > 0) {
setMinAmount(inputAmount * Number(token?.usdValue) < 1);
} else {
setMinAmount(false);
}📝 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.
| if (!Number.isNaN(token?.usdValue)) { | |
| setMinAmount(inputAmount * Number(token?.usdValue) < 1); | |
| } else { | |
| setMinAmount(false); | |
| } | |
| if (!Number.isNaN(token?.usdValue) && Number(token?.usdValue) > 0) { | |
| setMinAmount(inputAmount * Number(token?.usdValue) < 1); | |
| } else { | |
| setMinAmount(false); | |
| } |
🤖 Prompt for AI Agents
In src/apps/pulse/components/Sell/Sell.tsx around lines 130 to 134, the current
logic treats a usdValue of 0 as valid and therefore marks minAmount true for any
positive input; change the condition to first ensure usdValue is a finite number
greater than 0 before computing inputAmount * usdValue < 1, otherwise set
minAmount to false; ensure you coerce token?.usdValue to a Number safely (handle
token undefined), and use Number.isFinite or explicit checks (usdValue !==
undefined && usdValue > 0) to avoid treating zero as causing a minimum amount
restriction.
| if (!Number.isNaN(token?.usdValue)) { | ||
| setMinAmount(inputAmount * Number(token?.usdValue) < 1); | ||
| } else { | ||
| setMinAmount(false); | ||
| } | ||
| setNotEnoughLiquidity(inputAmount > tokenBalance); | ||
| } else { | ||
| setNotEnoughLiquidity(false); | ||
| setMinAmount(false); |
There was a problem hiding this comment.
Handle the edge case when usdValue is zero and eliminate code duplication.
This validation logic has the same critical issue as lines 130-134: if token.usdValue is 0, it will incorrectly block valid operations. Additionally, this logic is duplicated from the input change handler.
Apply this diff to fix the edge case and consider extracting the validation logic into a helper function to eliminate duplication:
- if (!Number.isNaN(token?.usdValue)) {
+ if (!Number.isNaN(token?.usdValue) && Number(token?.usdValue) > 0) {
setMinAmount(inputAmount * Number(token?.usdValue) < 1);
} else {
setMinAmount(false);
}
setNotEnoughLiquidity(inputAmount > tokenBalance);
} else {
setNotEnoughLiquidity(false);
setMinAmount(false);
}Consider extracting the validation logic into a helper function:
const calculateMinAmount = (amount: number, usdValue: string | number | undefined): boolean => {
const numericUsdValue = Number(usdValue);
return !Number.isNaN(numericUsdValue) && numericUsdValue > 0 && amount * numericUsdValue < 1;
};
2 participants
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit