[TASK-6611] feat: add max button to token amount input #495
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. 📝 Walkthrough📝 WalkthroughWalkthroughThe pull request introduces significant enhancements across several components related to cashout and link creation functionalities. Key updates include the addition of a Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (8)
src/components/Global/TokenAmountInput/index.tsx (1)
96-103: Enhance max button accessibility
While the max button implementation is good, consider these accessibility improvements:
- Add aria-label to better describe the action
- Add tooltip to explain what max value represents
{maxValue && maxValue !== tokenValue && (
<button
onClick={handleMaxClick}
className="absolute right-1 ml-1 border border-n-1 px-3 py-2 text-h7 uppercase hover:bg-n-3/10"
+ aria-label="Set maximum token amount"
+ title={`Set maximum amount: ${maxValue}`}
>
Max
</button>
)}src/hooks/useBalance.tsx (2)
167-175: Add TypeScript types and documentation.
The new balanceByToken function is well-implemented with proper memoization and null checks. Consider these improvements:
+/**
+ * Finds a balance entry for a specific token on a specific chain.
+ * @param chainId - The chain ID to look for
+ * @param tokenAddress - The token address to look for
+ * @returns The balance entry if found, undefined otherwise
+ */
const balanceByToken = useCallback(
- (chainId: string, tokenAddress: string) => {
+ (chainId: interfaces.ChainId, tokenAddress: interfaces.TokenAddress) => {177-177: Consider adding return type annotation.
The return value correctly includes the new balanceByToken function. Consider adding an explicit return type for better type safety:
-return { balances, fetchBalances, valuePerChain, refetchBalances, hasFetchedBalances, balanceByToken }
+return {
+ balances,
+ fetchBalances,
+ valuePerChain,
+ refetchBalances,
+ hasFetchedBalances,
+ balanceByToken
+} as const satisfies interfaces.UseBalanceReturnsrc/components/Cashout/Components/Initial.view.tsx (2)
194-198: Consider token-specific decimal precision
The maxValue calculation is implemented correctly, but consider the following improvements:
- The fixed precision of 6 decimal places might not be suitable for all tokens, as some tokens have different decimal places (e.g., USDC has 6, while most ERC20 tokens have 18).
- Consider adding validation for negative balances, although this should be rare.
Consider updating the implementation to handle token-specific decimals:
const maxValue = useMemo(() => {
const balance = balanceByToken(selectedChainID, selectedTokenAddress)
if (!balance) return ''
- return floorFixed(balance.amount, 6)
+ const tokenDecimals = balance.decimals ?? 6 // Fallback to 6 if decimals not provided
+ return balance.amount > 0 ? floorFixed(balance.amount, tokenDecimals) : '0'
}, [selectedChainID, selectedTokenAddress, balanceByToken])Line range hint 102-193: Consider improving error handling and component organization
The handleOnNext function has grown quite complex and handles multiple responsibilities:
- Bank account validation
- User fetching and type determination
- Link preparation
- KYC status management
Consider breaking this down into smaller, more focused functions or custom hooks for better maintainability and testing.
Consider:
- Extracting user management logic into a custom hook (e.g.,
useUserManagement) - Creating a separate service for bank account operations
- Implementing proper error boundaries for better error handling
Example structure:
// hooks/useUserManagement.ts
const useUserManagement = () => {
// Extract user fetching and type determination logic here
}
// services/bankAccountService.ts
const validateAndPrepareBankAccount = async (bankAccountNumber: string) => {
// Extract bank account validation logic here
}
// hooks/useLinkPreparation.ts
const useLinkPreparation = () => {
// Extract link preparation logic here
}src/utils/general.utils.ts (1)
238-240: Add input validation and JSDoc documentation.
The function implementation is correct for handling token amounts. However, consider these improvements:
- Add input validation:
export function floorFixed(value: number, decimals: number) {
+ if (isNaN(value) || !isFinite(value)) return '0'
+ if (decimals < 0) decimals = 0
return (Math.floor(value * 10 ** decimals) / 10 ** decimals).toFixed(decimals)
}- Add JSDoc documentation:
/**
* Formats a number to a fixed number of decimal places using floor rounding.
* @param value - The number to format
* @param decimals - Number of decimal places (non-negative)
* @returns The formatted number as a string with exact decimal places
* @example
* floorFixed(1.23456, 2) // returns "1.23"
* floorFixed(1.23456, 0) // returns "1"
*/src/components/Create/Link/Input.view.tsx (1)
171-173: Improve the clarity of the error message
The error message could be rephrased for better readability.
Apply this diff to update the error message:
setErrorState({
showError: true,
errorMessage:
- 'You do not have enough balance to cover the transaction fees, try again with suggested amount',
+ 'You do not have enough balance to cover the transaction fees. Please try again with the suggested amount.',
})src/components/Create/useCreateLink.tsx (1)
Line range hint 193-237: Improve error handling in the estimateGasFee function.
The function currently throws an error if it fails to fetch the token price. However, it would be better to handle this error gracefully and provide a fallback value or a default gas fee estimate.
Here's a suggested fix:
-if (!nativeTokenPrice || typeof nativeTokenPrice.price !== 'number' || isNaN(nativeTokenPrice.price)) {
- throw new Error('Failed to fetch token price')
-}
-const transactionCostUSD = Number(transactionCostNative) * nativeTokenPrice.price
+let transactionCostUSD = 0
+if (nativeTokenPrice && typeof nativeTokenPrice.price === 'number' && !isNaN(nativeTokenPrice.price)) {
+ transactionCostUSD = Number(transactionCostNative) * nativeTokenPrice.price
+} else {
+ console.warn('Failed to fetch token price. Using default transaction cost.')
+ transactionCostUSD = Number(transactionCostNative) * 0.0001 // Default to a small value
+}🧰 Tools
🪛 Biome
[error] 203-203: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/components/Cashout/Components/Initial.view.tsx (4 hunks)
- src/components/Create/Link/Input.view.tsx (8 hunks)
- src/components/Create/useCreateLink.tsx (13 hunks)
- src/components/Global/TokenAmountInput/index.tsx (4 hunks)
- src/hooks/useBalance.tsx (3 hunks)
- src/utils/general.utils.ts (1 hunks)
🧰 Additional context used
🪛 Biome
src/components/Create/useCreateLink.tsx
[error] 653-653: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🔇 Additional comments (13)
src/components/Global/TokenAmountInput/index.tsx (3)
11-11: LGTM: Appropriate prop type for maxValue
The optional string type for maxValue is appropriate for handling token amounts without floating-point precision issues.
14-14: LGTM: Clean prop integration
The maxValue prop is correctly integrated into the component signature.
14-14: Verify gas fee handling for native tokens
Per PR objectives, the system should check combined value of transaction and gas fees for native tokens. Ensure that the maxValue being passed to this component already accounts for gas fees where appropriate.
✅ Verification successful
Let me search for more context about how maxValue is calculated and used.
Based on the search results, I can now generate a final response as I have found evidence of gas fee handling in the codebase.
Gas fees are properly handled for native tokens
The codebase already accounts for gas fees when using native tokens. Specifically in src/components/Create/Link/Input.view.tsx, before setting the maxValue:
- Gas fees are estimated using
estimateGasFee - For native tokens, the code checks if users have enough balance to cover both the transaction amount and gas fees
- The check is done by adding the gas amount to the token value:
tokenValue: String(Number(tokenValue) + maxGasAmount)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for maxValue calculations in parent components
rg -A 5 "maxValue.*TokenAmountInput"
Length of output: 751
Script:
#!/bin/bash
# Search for maxValue calculations and gas fee handling
rg -A 10 "maxValue.*=|const.*maxValue"
# Search for gas fee related calculations
rg -A 5 "gasFee|gasLimit|estimateGas"
# Search for TokenAmountInput usage to see where maxValue is passed
ast-grep --pattern 'TokenAmountInput maxValue={$_}'
Length of output: 13728
src/hooks/useBalance.tsx (3)
3-4: LGTM! Clean import additions.
The new imports support the added functionality while promoting code reuse through utility functions.
126-127: LGTM! Improved code clarity.
Good refactor to use the dedicated utility function while maintaining the same sorting behavior.
167-177: Verify integration with consuming components.
Since this hook provides critical functionality for the max button feature, let's verify its integration:
✅ Verification successful
No issues found with balanceByToken integration
The verification shows that balanceByToken is properly integrated in both components that use it:
- In
Create/Link/Input.view.tsx, it's used within auseMemohook to calculate the max value for the input, with proper null checks and formatting:
const maxValue = useMemo(() => {
const balance = balanceByToken(selectedChainID, selectedTokenAddress)
if (!balance) return ''
return floorFixed(balance.amount, 6)
}, [selectedChainID, selectedTokenAddress, balanceByToken])- Similarly in
Cashout/Components/Initial.view.tsx, it follows the same pattern with proper null handling:
const maxValue = useMemo(() => {
const balance = balanceByToken(selectedChainID, selectedTokenAddress)
if (!balance) return ''
return floorFixed(balance.amount, 6)
}, [selectedChainID, selectedTokenAddress, balanceByToken])Both implementations properly handle undefined returns and have appropriate dependency arrays in their hooks. The integration is consistent with the hook's design and includes proper error handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the usage of balanceByToken in components and error handling
# Test 1: Find components using balanceByToken
echo "Components using balanceByToken:"
rg "balanceByToken\s*=" src/
# Test 2: Check error handling in those components
echo "\nError handling in components:"
rg "balanceByToken.*\s*(try|catch|if.*undefined|if.*null)" src/
Length of output: 309
Script:
#!/bin/bash
# Let's try a broader search to find usages of the hook and its returned values
# Find files importing or using useBalance
echo "Files importing useBalance:"
rg "useBalance" src/
# Find direct usage of the hook
echo -e "\nDirect hook usage:"
rg "const.*=.*useBalance\(" src/
# Find any usage of balanceByToken method
echo -e "\nUsage of balanceByToken method:"
rg "balanceByToken\(" src/
# Look for max button related components
echo -e "\nMax button related components:"
rg -i "max.*button|maxbutton" src/
Length of output: 4895
Script:
#!/bin/bash
# Let's examine the specific components that use balanceByToken to verify error handling
echo "Content of Create/Link/Input.view.tsx:"
cat src/components/Create/Link/Input.view.tsx
echo -e "\nContent of Cashout/Components/Initial.view.tsx:"
cat src/components/Cashout/Components/Initial.view.tsx
Length of output: 33484
src/components/Cashout/Components/Initial.view.tsx (3)
35-37: LGTM: Context extraction is properly updated
The addition of selectedTokenAddress to the context extraction is well-integrated and necessary for the max button functionality.
39-39: LGTM: Balance hook usage is properly updated
The addition of balanceByToken is consistent with the requirements and is properly utilized for max value calculations.
230-230: LGTM: TokenAmountInput properly configured with maxValue
The maxValue prop is correctly passed to TokenAmountInput, enabling the max button functionality as intended.
src/utils/general.utils.ts (1)
238-240: LGTM! The implementation is correct for token amount formatting.
The function correctly handles the formatting of token amounts by:
- Using
Math.floorto round down, which is appropriate for financial calculations - Avoiding floating-point precision issues by using integer math
- Ensuring exact decimal places in the output string
src/components/Create/Link/Input.view.tsx (1)
293-293: Verify TokenAmountInput handles maxValue correctly
By adding maxValue to TokenAmountInput, ensure that the component properly handles cases where maxValue might be '0' or another edge case.
Consider adding validations within TokenAmountInput to handle these scenarios and prevent potential errors.
src/components/Create/useCreateLink.tsx (2)
71-111: LGTM!
The generateLinkDetails function looks good. It correctly retrieves the token details, generates the base URL, and returns the link details object.
653-653:
Remove the unnecessary catch clause.
The catch clause that only rethrows the original error is useless and can be confusing.
-try {
- await checkUserHasEnoughBalance({ tokenValue })
- const linkDetails = generateLinkDetails({ tokenValue, walletType, envInfo: environmentInfo })
- const password = await generatePassword()
- await switchNetwork(selectedChainID)
-
- const _isGaslessDepositPossible = isGaslessDepositPossible({
- chainId: selectedChainID,
- tokenAddress: selectedTokenAddress,
- })
-
- if (_isGaslessDepositPossible) {
- return prepareGaslessDeposit(linkDetails, password)
- } else {
- return prepareStandardDeposit(linkDetails, password, tokenValue)
- }
-} catch (error) {
- throw error
-}
+await checkUserHasEnoughBalance({ tokenValue })
+const linkDetails = generateLinkDetails({ tokenValue, walletType, envInfo: environmentInfo })
+const password = await generatePassword()
+await switchNetwork(selectedChainID)
+
+const _isGaslessDepositPossible = isGaslessDepositPossible({
+ chainId: selectedChainID,
+ tokenAddress: selectedTokenAddress,
+})
+
+if (_isGaslessDepositPossible) {
+ return prepareGaslessDeposit(linkDetails, password)
+} else {
+ return prepareStandardDeposit(linkDetails, password, tokenValue)
+}🧰 Tools
🪛 Biome
[error] 653-653: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
jjramirezn
force-pushed
the
fix/limit-with-gas-native-token
branch
from
23dc13b to
c62d6d1
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (5)
src/hooks/useBalance.tsx (1)
126-127: Consider adding a comment to explain the sorting priority.
The sorting logic prioritizes native tokens (zero address) over other tokens when values are equal. While the implementation is correct, a brief comment would help explain this business logic.
if (valueA === valueB) {
+ // Prioritize native tokens (zero address) when values are equal
if (isAddressZero(a.address)) return -1
if (isAddressZero(b.address)) return 1src/components/Create/Link/Input.view.tsx (1)
159-176: Improve gas fee handling readability and user experience.
While the number conversions are fine as per previous feedback, consider these improvements:
- Document the 1.3 multiplier (30% buffer) used in the calculation
- Make the error message more user-friendly
Apply this diff:
if (undefined !== _feeOptions && isNativeCurrency(selectedTokenAddress)) {
+ // Calculate maximum gas cost with current gas prices
const maxGasAmount = Number(
formatEther(_feeOptions.gasLimit.mul(_feeOptions.maxFeePerGas || _feeOptions.gasPrice))
)
try {
await checkUserHasEnoughBalance({
tokenValue: String(Number(tokenValue) + maxGasAmount),
})
} catch (error) {
+ // Reduce token value by gas amount plus 30% buffer for price fluctuations
_setTokenValue((Number(tokenValue) - maxGasAmount * 1.3).toFixed(6))
setErrorState({
showError: true,
errorMessage:
- 'You do not have enough balance to cover the transaction fees, try again with suggested amount',
+ 'Insufficient balance for transaction fees. A new amount has been suggested that includes gas fees.',
})
return
}
}src/components/Create/useCreateLink.tsx (2)
Line range hint 188-232: Reduce code duplication in gas fee estimation.
The error handling block duplicates most of the code from the try block. Consider extracting the common logic into a separate function.
+const calculateGasFee = async (
+ chainId: string,
+ preparedTx: any,
+ gasLimit?: BigNumber
+) => {
+ const feeOptions = await peanut.setFeeOptions({
+ chainId: chainId,
+ unsignedTx: preparedTx,
+ gasLimit,
+ })
+
+ let transactionCostWei = feeOptions.gasLimit.mul(
+ feeOptions.maxFeePerGas || feeOptions.gasPrice
+ )
+ let transactionCostNative = formatEther(transactionCostWei)
+ const nativeTokenPrice = await fetchTokenPrice(
+ '0x0000000000000000000000000000000000000000',
+ chainId
+ )
+ if (!nativeTokenPrice || typeof nativeTokenPrice.price !== 'number' || isNaN(nativeTokenPrice.price)) {
+ throw new Error('Failed to fetch token price')
+ }
+ const transactionCostUSD = Number(transactionCostNative) * nativeTokenPrice.price
+
+ return {
+ feeOptions,
+ transactionCostUSD,
+ }
+}
+
const estimateGasFee = useCallback(
async ({ chainId, preparedTx }: { chainId: string; preparedTx: any }) => {
try {
- const feeOptions = await peanut.setFeeOptions({
- chainId: chainId,
- unsignedTx: preparedTx,
- })
-
- let transactionCostWei = feeOptions.gasLimit.mul(feeOptions.maxFeePerGas || feeOptions.gasPrice)
- let transactionCostNative = formatEther(transactionCostWei)
- const nativeTokenPrice = await fetchTokenPrice('0x0000000000000000000000000000000000000000', chainId)
- if (!nativeTokenPrice || typeof nativeTokenPrice.price !== 'number' || isNaN(nativeTokenPrice.price)) {
- throw new Error('Failed to fetch token price')
- }
- const transactionCostUSD = Number(transactionCostNative) * nativeTokenPrice.price
-
- return {
- feeOptions,
- transactionCostUSD,
- }
+ return await calculateGasFee(chainId, preparedTx)
} catch (error) {
try {
- const feeOptions = await peanut.setFeeOptions({
- chainId: chainId,
- unsignedTx: preparedTx,
- gasLimit: BigNumber.from(100000),
- })
-
- let transactionCostWei = feeOptions.gasLimit.mul(feeOptions.maxFeePerGas || feeOptions.gasPrice)
- let transactionCostNative = formatEther(transactionCostWei)
- const nativeTokenPrice = await fetchTokenPrice('0x0000000000000000000000000000000000000000', chainId)
- if (!nativeTokenPrice) {
- throw new Error('Failed to fetch token price')
- }
- const transactionCostUSD = Number(transactionCostNative) * nativeTokenPrice.price
-
- return {
- feeOptions,
- transactionCostUSD,
- }
+ return await calculateGasFee(chainId, preparedTx, BigNumber.from(100000))
} catch (error) {
console.error('Failed to estimate gas fee:', error)
throw error
}
}
},
[]
)🧰 Tools
🪛 Biome
[error] 198-198: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
Line range hint 419-430: Review native token value precision handling.
For native token transfers, the value is being fixed to 7 decimal places which might cause precision loss. Consider using the full precision of the input value.
-tokenValue = Number(tokenValue).toFixed(7)
-const amount = parseEther(tokenValue)
+const amount = parseEther(tokenValue)src/utils/general.utils.ts (1)
238-240: The implementation looks correct but could be more robust.
The function correctly floors numbers to specified decimal places, which is crucial for token amount calculations. However, consider adding input validation and edge case handling.
Consider this enhanced implementation:
export function floorFixed(value: number, decimals: number) {
+ if (!Number.isFinite(value)) return '0';
+ if (!Number.isInteger(decimals) || decimals < 0) {
+ throw new Error('Decimals must be a non-negative integer');
+ }
return (Math.floor(value * 10 ** decimals) / 10 ** decimals).toFixed(decimals);
}This version:
- Handles NaN and Infinity
- Validates that decimals is a non-negative integer
- Maintains the same precision-handling logic
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- src/components/Cashout/Components/Initial.view.tsx (4 hunks)
- src/components/Create/Link/Input.view.tsx (8 hunks)
- src/components/Create/useCreateLink.tsx (12 hunks)
- src/components/Global/TokenAmountInput/index.tsx (4 hunks)
- src/hooks/useBalance.tsx (3 hunks)
- src/utils/general.utils.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Global/TokenAmountInput/index.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Create/Link/Input.view.tsx (2)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#495
File: src/components/Create/Link/Input.view.tsx:244-248
Timestamp: 2024-10-29T12:20:47.207Z
Learning: In the `TokenAmountInput` component within `src/components/Global/TokenAmountInput/index.tsx`, when `balance` is undefined, the `maxValue` prop should be set to an empty string `''`.
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#495
File: src/components/Create/Link/Input.view.tsx:159-176
Timestamp: 2024-10-29T12:23:32.778Z
Learning: Number conversions are acceptable as they are in the code and do not require changes.
🪛 Biome
src/components/Create/useCreateLink.tsx
[error] 648-648: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
🔇 Additional comments (10)
src/hooks/useBalance.tsx (3)
3-4: LGTM! Import statements are properly organized.
The new imports for useCallback and utility functions are correctly added and align with their usage in the code.
177-177: LGTM! Return value properly includes the new balanceByToken method.
The addition of balanceByToken to the returned object aligns with the PR objectives and maintains good code organization.
167-177: Verify the usage patterns of balanceByToken in consuming components.
Let's ensure the new balanceByToken method is properly integrated with the max button functionality mentioned in the PR objectives.
✅ Verification successful
balanceByToken is correctly integrated with max button functionality
The verification shows proper integration:
- Used in both Create/Link and Cashout components to calculate maxValue
- Correctly used with TokenAmountInput component's maxValue prop
- Implementation properly considers chain ID and token address
- Used in conjunction with useCallback and useMemo for performance
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using balanceByToken
echo "Searching for balanceByToken usage patterns..."
rg "balanceByToken" --type tsx -B 2 -A 2
# Search for max button implementations that should use this function
echo "Searching for max button implementations..."
rg "max.*button|maxValue|max.*amount" --type tsx -B 2 -A 2
Length of output: 357
Script:
#!/bin/bash
# Search for balanceByToken usage without file type restriction
echo "Searching for balanceByToken usage patterns..."
rg "balanceByToken" -B 2 -A 2
# Search for max button implementations
echo "Searching for max button implementations..."
rg "max.*button|maxValue|max.*amount" -B 2 -A 2
Length of output: 8421
src/components/Create/Link/Input.view.tsx (2)
244-248: LGTM! Well-implemented balance handling.
The maxValue calculation is correctly memoized and follows the established pattern of returning an empty string for undefined balances.
275-277: LGTM! Clean UI implementation.
The recipient display logic properly handles both ENS names and addresses, and the maxValue integration with TokenAmountInput is clean.
Also applies to: 293-293
src/components/Cashout/Components/Initial.view.tsx (3)
35-39: LGTM: Context and hook changes are well-integrated
The addition of selectedTokenAddress and balanceByToken properly supports the new max button functionality while maintaining clean integration with existing code.
Line range hint 230-236: LGTM: Clean integration of maxValue with TokenAmountInput
The maxValue prop is properly integrated with the TokenAmountInput component while maintaining existing wallet connection and submission functionality.
230-230: Verify TokenAmountInput implementation
Let's verify that the TokenAmountInput component properly implements the max button functionality.
✅ Verification successful
Let me check the actual implementation of the max button functionality in the TokenAmountInput component.
TokenAmountInput implementation is correctly handling max value functionality
Based on the code inspection and the learning from jjramirezn, the TokenAmountInput component correctly implements the max button functionality:
- It properly accepts the
maxValueprop - The
handleMaxClickfunction sets the input denomination to 'TOKEN' (which is intentional as per the learning) - The max button is only shown when
maxValueexists and differs from the currenttokenValue - When clicked, it correctly sets the token value to the maximum amount
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check TokenAmountInput implementation for max button functionality
# Search for TokenAmountInput component definition
echo "Searching for TokenAmountInput component implementation..."
rg -l "TokenAmountInput" src/components/Global
# Check maxValue prop handling
echo "Checking maxValue prop handling..."
ast-grep --pattern 'interface $_ {
$$$
maxValue?: $_
$$$
}'
Length of output: 931
Script:
#!/bin/bash
# Check the implementation of TokenAmountInput component
echo "Checking TokenAmountInput implementation..."
rg -A 20 "const TokenAmountInput.*=.*{" src/components/Global/TokenAmountInput/index.tsx
# Look for max button handling
echo "Checking max button handling..."
rg -A 10 "maxValue" src/components/Global/TokenAmountInput/index.tsx
Length of output: 1969
src/components/Create/useCreateLink.tsx (2)
705-705: LGTM!
The token price handling aligns with the PR objectives and is implemented correctly.
89-97: Review token amount precision handling.
The token amount is being fixed to 6 decimal places which might not be appropriate for all tokens. Consider using the token's decimal places from tokenDetails.tokenDecimals to determine the appropriate precision.
| const maxValue = useMemo(() => { | ||
| const balance = balanceByToken(selectedChainID, selectedTokenAddress) | ||
| if (!balance) return '' | ||
| return floorFixed(balance.amount, 6) |
There was a problem hiding this comment.
Agree with coderabbit here, why 6 ?
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
src/hooks/useBalance.tsx (1)
126-127: Consider documenting the sorting priority.
The sorting logic correctly prioritizes native tokens, which aligns with the PR's objective of improving token handling.
Add a comment explaining the sorting priority:
if (valueA === valueB) {
+ // Prioritize native tokens (zero address) in the list
if (isAddressZero(a.address)) return -1
if (isAddressZero(b.address)) return 1src/components/Create/Link/Input.view.tsx (1)
277-277: Consider improving readability of recipient display logic
While the logic is correct, the nested ternary could be made more readable.
Consider this more readable alternative:
-? `Send to ${recipient.name?.endsWith('.eth') ? recipient.name : shortenAddressLong(recipient.address ?? '')}`
+? `Send to ${getDisplayName(recipient)}`
// Add this helper function:
const getDisplayName = (recipient: { name?: string, address?: string }) => {
if (recipient.name?.endsWith('.eth')) return recipient.name;
return shortenAddressLong(recipient.address ?? '');
};src/components/Create/useCreateLink.tsx (1)
Line range hint 419-430: Consider precision handling for native token amounts.
While the ERC20 token parsing is correct, the native token amount is fixed to 7 decimal places which might lead to precision loss for some values.
Consider removing the fixed decimal places or increasing the precision:
-tokenValue = Number(tokenValue).toFixed(7)
+tokenValue = Number(tokenValue).toString()📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- src/components/Cashout/Components/Initial.view.tsx (4 hunks)
- src/components/Create/Link/Input.view.tsx (8 hunks)
- src/components/Create/useCreateLink.tsx (13 hunks)
- src/components/Global/TokenAmountInput/index.tsx (4 hunks)
- src/hooks/useBalance.tsx (6 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/components/Global/TokenAmountInput/index.tsx
🧰 Additional context used
📓 Learnings (1)
src/components/Create/Link/Input.view.tsx (1)
Learnt from: jjramirezn
PR: peanutprotocol/peanut-ui#495
File: src/components/Create/Link/Input.view.tsx:244-248
Timestamp: 2024-10-29T12:20:47.207Z
Learning: In the `TokenAmountInput` component within `src/components/Global/TokenAmountInput/index.tsx`, when `balance` is undefined, the `maxValue` prop should be set to an empty string `''`.
🔇 Additional comments (15)
src/hooks/useBalance.tsx (3)
2-4: LGTM! Clean import organization.
The imports are well-organized, using direct imports instead of namespace imports, which is a TypeScript best practice.
11-13: LGTM! Consistent type usage.
The state declarations correctly use the directly imported types, maintaining consistency with the import changes.
177-177: Verify the impact of the new return value.
The addition of balanceByToken to the return object is correct, but we should verify its usage in consuming components.
Let's verify the usage of this hook:
✅ Verification successful
Let me search for the actual usage of balanceByToken to verify its impact.
The addition of balanceByToken is properly integrated and used
The new balanceByToken function is being correctly used in three components:
src/components/Create/useCreateLink.tsx: For checking token balance during link creationsrc/components/Create/Link/Input.view.tsx: For displaying token balance in the input viewsrc/components/Cashout/Components/Initial.view.tsx: For showing balance in the cashout flow
All usages follow a consistent pattern of retrieving and displaying token balances for specific chain and token combinations, replacing what would otherwise require manual filtering of the balances array.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for useBalance hook usage
rg "useBalance\(\)" -A 5
Length of output: 6410
Script:
#!/bin/bash
# Search for balanceByToken usage
rg "balanceByToken" -A 3
Length of output: 3728
src/components/Create/Link/Input.view.tsx (6)
7-7: LGTM: Import optimizations
The imports have been refined to include only the necessary functions, improving code maintainability.
Also applies to: 11-11, 13-13, 21-21
60-60: LGTM: Balance management enhancement
The addition of balanceByToken enables precise token balance retrieval, supporting the max value feature.
107-111: LGTM: Improved gasless deposit check
The gasless deposit check has been refactored for better readability and reusability.
184-187: LGTM: Points estimation logic
The points estimation logic correctly handles both gasless and regular transactions.
🧰 Tools
🪛 Biome
[error] 186-187: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
245-250: LGTM: Efficient max value calculation
The max value calculation is properly memoized and follows the established pattern of returning an empty string for undefined balances, as confirmed by previous learnings.
295-295: LGTM: TokenAmountInput enhancement
The maxValue prop has been correctly added to the TokenAmountInput component.
src/components/Cashout/Components/Initial.view.tsx (4)
35-37: LGTM: Context extraction is properly updated
The addition of selectedTokenAddress to the context extraction is consistent with the requirements for implementing the max button functionality.
39-39: LGTM: Balance hook usage is properly updated
The addition of balanceByToken from the useBalance hook provides the necessary functionality to fetch token-specific balances.
194-199: LGTM: maxValue calculation is properly implemented
The implementation correctly calculates the maximum value based on the selected token's balance and is properly memoized for performance optimization.
231-231: LGTM: maxValue prop is properly passed to TokenAmountInput
The maxValue prop is correctly passed to the TokenAmountInput component, enabling the max button functionality as required.
src/components/Create/useCreateLink.tsx (2)
33-64: LGTM! Balance checking implementation is robust.
The balance checking logic has been well-implemented with:
- Clear validation of chain and token address
- Proper balance retrieval using
balanceByToken - Appropriate minimum amount validation
628-642: LGTM! Comprehensive native token balance validation.
The implementation correctly:
- Checks if the token is native currency
- Calculates the maximum gas amount
- Validates the total amount (token value + gas fees)
3 participants
This PR does the following