cashout-fix: disable cashout inputs when not available + minor FAQ issue #411
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request introduce enhancements to the cashout components in the application. A new hyperlink for KYC information is added to the FAQ section, improving user navigation. Additionally, the logic for cashout eligibility based on the selected blockchain is refined in the Changes
Possibly related PRs
Suggested reviewers
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: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Cashout/Components/Faq.comp.tsx (1)
26-30: LGTM: KYC link added to FAQThe addition of the Link component for the KYC text improves user experience by providing easy access to KYC information, as outlined in the PR objectives.
Consider adding an
aria-labelto the Link component for improved accessibility. For example:- <Link href="/kyc" className="text-blue-600 underline"> + <Link href="/kyc" className="text-blue-600 underline" aria-label="Learn more about KYC (Know Your Customer)">src/components/Cashout/Components/Initial.view.tsx (2)
38-40: LGTM:cannotCashoutOnSelectedChainconstant addedThe addition of this constant improves code readability and centralizes the logic for determining cashout availability. This aligns well with the PR objective of disabling cashout inputs when not available for a specific blockchain chain.
However, there's a minor optimization that can be made:
Consider removing the redundant
Booleancall:const cannotCashoutOnSelectedChain = - !Boolean(crossChainDetails.find((chain: any) => chain.chainId.toString() === selectedChainID.toString())) || + !crossChainDetails.find((chain: any) => chain.chainId.toString() === selectedChainID.toString()) || selectedChainID === '1'This change doesn't affect the logic but makes the code slightly cleaner.
🧰 Tools
🪛 Biome
[error] 39-39: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
388-389: LGTM: "Proceed" button and warning messages updatedThe changes to the "Proceed" button's disabled condition and the addition of a new warning message effectively implement the PR objective of preventing interactions with unavailable cashout options. This improves the user experience by providing clear feedback.
Consider enhancing the warning message to provide more specific information:
{cannotCashoutOnSelectedChain && ( <span className=" text-h8 font-normal "> - <ChakraIcon name="warning" className="-mt-0.5" /> You cannot cashout on this chain. + <ChakraIcon name="warning" className="-mt-0.5" /> Cashout is not available on the selected chain. Please choose a supported chain to proceed. </span> )}This change provides more context to the user and guides them towards a solution.
Also applies to: 418-422
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/components/Cashout/Components/Faq.comp.tsx (2 hunks)
- src/components/Cashout/Components/Initial.view.tsx (8 hunks)
🧰 Additional context used
🪛 Biome
src/components/Cashout/Components/Initial.view.tsx
[error] 39-39: Avoid redundant
BooleancallIt is not necessary to use
Booleancall when a value will already be coerced to a boolean.
Unsafe fix: Remove redundantBooleancall(lint/complexity/noExtraBooleanCast)
🔇 Additional comments (8)
src/components/Cashout/Components/Faq.comp.tsx (3)
4-4: LGTM: New import for Link componentThe addition of the Link import from 'next/link' is appropriate for implementing the KYC link in the FAQ section, as mentioned in the PR objectives.
36-36: LGTM: Capitalization fix in fee descriptionThe capitalization of "We" in the fee description has been corrected, addressing the minor typographical error mentioned in the PR objectives. This change enhances the overall quality of the FAQ text.
Line range hint
1-48: Verify implementation of disabling cashout inputsThe changes in this file address the addition of the KYC link to the FAQ and fix the minor typographical error, as mentioned in the PR objectives. However, the implementation for disabling cashout inputs when they are not available is not present in this file.
To ensure all PR objectives are met, please run the following script to check for the implementation of disabling cashout inputs:
src/components/Cashout/Components/Initial.view.tsx (5)
21-21: LGTM: Import oftwMergeaddedThe addition of the
twMergeimport from 'tailwind-merge' is appropriate for the conditional styling changes made in the component.
268-276: LGTM: Bank account selection UI updatedThe changes to the bank account selection UI effectively implement the PR objective of disabling cashout inputs when not available. The use of
twMergefor conditional styling and the added check before allowing selection are well-implemented.
320-324: LGTM: New bank account input UI updatedThe changes to the new bank account input UI effectively implement the PR objective of disabling cashout inputs when not available. The use of
twMergefor conditional styling and the addition of thedisabledattribute ensure that the input is both visually and functionally disabled when cashout is not available on the selected chain.Also applies to: 329-332, 339-339
349-354: LGTM: Bank account input UI for non-authenticated users updatedThe changes to the bank account input UI for non-authenticated users mirror those made for authenticated users, ensuring consistent behavior across the application. The use of
twMergefor conditional styling and the addition of thedisabledattribute effectively implement the PR objective of disabling cashout inputs when not available.Also applies to: 358-361, 368-368
Line range hint
1-424: Overall assessment: Well-implemented changesThe modifications in this file effectively achieve the PR objective of disabling cashout inputs when they are not available for a specific blockchain chain. The implementation is consistent across different parts of the component, including the bank account selection, new bank account input, and the "Proceed" button.
Key points:
- The introduction of
cannotCashoutOnSelectedChaincentralizes the logic for determining cashout availability.- The use of
twMergefor conditional styling provides clear visual cues to the user.- Proper disabling of inputs and the "Proceed" button prevents unintended interactions.
- The addition of a warning message improves user feedback.
The suggested minor improvements, if implemented, will further enhance the code quality and user experience. Great job on this implementation!
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
src/components/Global/TokenAmountInput/index.tsx (1)
51-54: Approve form element changes with a minor suggestion.The modifications to the form element enhance user interaction and align with the PR objectives:
- The
ref={formRef}addition is consistent with the earlierformRefdeclaration.- Adding
cursor-textto the className improves visual feedback.- The
onClick={handleContainerClick}implementation adds the desired focus functionality.These changes effectively improve the user experience as intended.
Consider using the
onMouseDownevent instead ofonClickfor a more responsive user experience:- onClick={handleContainerClick} + onMouseDown={handleContainerClick}This change will trigger the focus immediately on mouse press, rather than waiting for the click to complete.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/components/Global/TokenAmountInput/index.tsx (1 hunks)
🔇 Additional comments (3)
src/components/Global/TokenAmountInput/index.tsx (3)
43-47: LGTM: handleContainerClick improves user experience.The
handleContainerClickfunction is a good addition that enhances user interaction by focusing the input when the container is clicked. It correctly uses theinputRefand follows React best practices.
Line range hint
1-124: Summary: PR objectives achieved with minor suggestions for improvement.This PR successfully implements the changes to improve user experience in the TokenAmountInput component. The modifications align with the stated objectives by enhancing input interaction and visual feedback.
Key improvements:
- Added click-to-focus functionality for better user interaction.
- Updated cursor style to indicate interactivity.
While the changes effectively meet the PR goals, consider the following suggestions:
- Verify the usage of
formRefand remove if unused.- Consider using
onMouseDowninstead ofonClickfor more responsive focus behavior.Overall, the implementation is sound and follows React best practices. Great job on improving the component's functionality!
41-42: Approve formRef addition, but verify its usage.The addition of
formRefis a good practice for accessing the form element directly. However, it doesn't appear to be used elsewhere in the component.Let's verify if
formRefis used elsewhere in the codebase:If
formRefis indeed unused, consider removing it to keep the code clean.✅ Verification successful
Confirm
formRefusage within the component.The
formRefis correctly used as a reference to the form element in theTokenAmountInputcomponent. However, there was an issue checking its usage in other files due to an unrecognized file type. Please run the following corrected script to ensureformRefisn't used elsewhere:
Confirm
formRefusage is confined to the component.The
formRefis correctly used within theTokenAmountInputcomponent. To ensure it's not utilized elsewhere, please run the following corrected script:
Confirm
formRefusage is confined to the component.The
formRefis correctly used within theTokenAmountInputcomponent. To ensure it's not utilized elsewhere, please run the following corrected script:
formRefis correctly used within the component and not referenced elsewhere.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if formRef is used elsewhere in the component or other files # Test 1: Check usage within this file echo "Checking usage within TokenAmountInput component:" rg "formRef" src/components/Global/TokenAmountInput/index.tsx # Test 2: Check usage in other files echo "Checking usage in other files:" rg "formRef" --type typescript --type tsxLength of output: 394
Script:
#!/bin/bash # Description: Check if formRef is used elsewhere in the component or other files # Test 1: Check usage within this file echo "Checking usage within TokenAmountInput component:" rg "formRef" src/components/Global/TokenAmountInput/index.tsx # Test 2: Check usage in other files echo "Checking usage in other files:" rg "formRef" --type ts --type tsxLength of output: 379
Script:
#!/bin/bash # Description: Check if formRef is used elsewhere in the component or other files # Test 1: Check usage within this file echo "Checking usage within TokenAmountInput component:" rg "formRef" src/components/Global/TokenAmountInput/index.tsx # Test 2: Check usage in other files echo "Checking usage in other files:" rg "formRef" --type typescript --type typescriptreactLength of output: 406
Script:
#!/bin/bash # Description: Check if formRef is used elsewhere in the component or other files # Test 1: Check usage within this file echo "Checking usage within TokenAmountInput component:" rg "formRef" src/components/Global/TokenAmountInput/index.tsx # Test 2: Check usage in other files echo "Checking usage in other files:" rg "formRef" -g "*.ts" -g "*.tsx"Length of output: 525
| Cashing out requires KYC. <br></br>Min cashout: $10, max $100k. | ||
| Cashing out requires{' '} | ||
| <Link href="/kyc" className="text-blue-600 underline"> | ||
| KYC |
There was a problem hiding this comment.
praise: awesome to see a QA item in code so fast!
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
src/components/Cashout/Components/Initial.view.tsx (1)
41-43: Improve Type Safety by TypingcrossChainDetailsCurrently,
chainis typed asanyin thefindmethod. To enhance type safety and leverage TypeScript's benefits, define a proper type forchain.Apply this diff:
const xchainAllowed = useMemo( - () => Boolean(crossChainDetails.find((chain: any) => chain.chainId.toString() === selectedChainID.toString())), + () => Boolean(crossChainDetails.find((chain: { chainId: number }) => chain.chainId.toString() === selectedChainID.toString())), [crossChainDetails, selectedChainID] )Alternatively, consider defining an interface or type for
crossChainDetailsitems.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/components/Cashout/Components/Initial.view.tsx (6 hunks)
🔇 Additional comments (1)
src/components/Cashout/Components/Initial.view.tsx (1)
409-412: Clear User Feedback When Cashout Is Not AllowedDisplaying a warning message when cashout is not allowed effectively informs the user.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
src/components/Cashout/Components/Initial.view.tsx (3)
260-269: Enhanced UI feedback for cashout availabilityThe changes to the bank account selection UI effectively communicate cashout availability to the user:
- Conditional styling based on
xchainAllowedimproves visual feedback.- Disabling the click functionality when cashout is not allowed prevents user confusion.
These updates enhance the user experience by providing clear visual and interactive cues.
Consider adding an
aria-disabledattribute whenxchainAllowedis false to improve accessibility:<div key={index} className={twMerge( 'flex w-full items-center justify-between border border-black p-2', selectedBankAccount === account.account_identifier ? 'bg-purple-1' : 'hover:bg-gray-100', xchainAllowed && 'cursor-pointer', !xchainAllowed && 'opacity-60' )} + aria-disabled={!xchainAllowed} onClick={() => { if (!xchainAllowed) return // ... rest of the onClick handler }} >
313-318: Consistent UI treatment for new bank account inputThe changes to the new bank account input UI align well with the bank account selection UI:
- Conditional styling based on
xchainAllowedprovides consistent visual feedback.- Disabling the input when cashout is not allowed prevents user interaction.
These updates maintain a cohesive user experience across different input methods.
For consistency with the bank account selection UI, consider adding
cursor-not-allowedwhenxchainAllowedis false:<div className={twMerge( 'flex w-full border border-black p-2', xchainAllowed && 'cursor-pointer', - !xchainAllowed && 'opacity-60' + !xchainAllowed && 'opacity-60 cursor-not-allowed' )} >Also applies to: 323-326, 333-333
Line range hint
412-416: Clear warning message for unavailable cashoutThe addition of a warning message when cashout is not available is a good improvement:
- It provides explicit feedback to the user about why they can't proceed with the cashout.
- The message complements the visual cues provided by the disabled UI elements.
This change enhances the user experience by clearly communicating the reason for the unavailable cashout option.
Consider providing more specific information about why cashout is not available on the current chain:
{!xchainAllowed && ( <span className=" text-h8 font-normal "> <ChakraIcon name="warning" className="-mt-0.5" /> - You cannot cashout on this chain. + Cashout is not available on the current chain. Please switch to a supported chain to proceed. </span> )}This more detailed message could help users understand what action they need to take to enable cashout.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/components/Cashout/Components/Initial.view.tsx (6 hunks)
🔇 Additional comments (3)
src/components/Cashout/Components/Initial.view.tsx (3)
21-21: Improved styling management and chain compatibility checkThe addition of
twMergeand thexchainAllowedconstant are good improvements:
twMergewill help manage Tailwind CSS classes more efficiently, reducing conflicts and improving maintainability.xchainAllowedcentralizes the logic for determining cashout availability on the selected chain, which can be reused throughout the component.These changes enhance code readability and maintainability.
Also applies to: 40-43
Line range hint
380-380: Consistent disabling of "Proceed" buttonThe addition of
!xchainAllowedto the button's disabled condition is a good change:
- It prevents users from attempting to proceed when cashout is not available on the selected chain.
- This change maintains consistency with the other UI elements that are disabled when
xchainAllowedis false.This update completes the user experience by ensuring all relevant UI elements respect the cashout availability.
Line range hint
1-418: Summary: Comprehensive implementation of cashout availability checksThe changes in this file effectively implement a consistent check for cashout availability across the UI:
- The
xchainAllowedconstant centralizes the logic for determining cashout availability.- UI elements (bank account selection, new bank account input, and "Proceed" button) are consistently disabled when cashout is not available.
- Visual feedback is provided through conditional styling and opacity changes.
- An explicit warning message informs users when cashout is not available on the current chain.
- The use of
twMergeimproves the management of conditional Tailwind CSS classes.These changes significantly enhance the user experience by providing clear feedback and preventing invalid actions when cashout is not available on the selected chain.
nezz0746
force-pushed
the
fix/cashout-minor-issues
branch
from
5a6c6a5
to
0f780c4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/components/Cashout/Components/Initial.view.tsx (2)
269-276: LGTM: Updated UI elements withtwMergeThe use of
twMergefor conditional styling based onxchainAllowedis well-implemented:
- Dynamic class application enhances the UI responsiveness to the cashout availability.
- Cursor style, opacity, and input state are correctly managed based on
xchainAllowed.For consistency, consider applying the same
hover:bg-gray-100class to the new bank account input div whenxchainAllowedis true, similar to the bank account selection div.Also applies to: 322-327, 332-335
Line range hint
421-425: LGTM: New warning message for disallowed cashoutThe addition of a warning message when cashout is not allowed on the selected chain is well-implemented:
- The message is clear and consistent with other warnings in the component.
- The condition for displaying the message is correct.
Consider providing more specific information in the warning message, such as "Cashout is not available on [chain name]" or "Please switch to a supported chain for cashout". This would give users more context and guidance on how to proceed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- src/components/Cashout/Components/Faq.comp.tsx (2 hunks)
- src/components/Cashout/Components/Initial.view.tsx (7 hunks)
- src/components/Global/TokenAmountInput/index.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/Cashout/Components/Faq.comp.tsx
- src/components/Global/TokenAmountInput/index.tsx
🔇 Additional comments (3)
src/components/Cashout/Components/Initial.view.tsx (3)
21-21: LGTM: New import andxchainAllowedconstantThe addition of the
twMergeimport and thexchainAllowedconstant are well-implemented:
- The
twMergeimport will be useful for conditional class merging.- The
xchainAllowedconstant effectively encapsulates the logic for determining cashout availability.- Using
useMemoforxchainAllowedis a good optimization.Also applies to: 40-52
Line range hint
386-391: LGTM: Updated cashout button disabled stateThe addition of
!xchainAllowedto the button's disabled conditions is correct and consistent with the overall changes. It ensures that the button is disabled when cashout is not allowed on the selected chain, while preserving other necessary conditions.
Line range hint
1-426: Summary: Well-implemented cashout availability checkThe changes in this file effectively implement a new cashout availability check based on the selected chain:
- The new
xchainAllowedconstant encapsulates the logic for determining cashout availability.- UI elements are consistently updated to reflect the cashout availability status, including:
- Conditional styling for bank account selection and input
- Disabling the cashout button when not allowed
- Displaying a warning message for unsupported chains
- The implementation uses
twMergefor efficient conditional styling anduseMemofor performance optimization.These changes improve the user experience by providing clear visual feedback and preventing interactions when cashout is not available on the selected chain.
Great job on implementing this feature! The code is clean, consistent, and improves the overall functionality of the cashout process.
| Cashing out requires KYC. <br></br>Min cashout: $10, max $100k. | ||
| Cashing out requires{' '} | ||
| <Link href="/kyc" className="text-blue-600 underline"> | ||
| KYC |
| </p> | ||
| <br></br> | ||
| <p>Fees:</p> | ||
| <ul className="list-disc pl-5"> | ||
| <li>Peanut sponsors gas</li> | ||
| <li>we have to charge a $1 fee for EU cashouts, and $0.5 for US transfers</li> | ||
| <li>We have to charge a $1 fee for EU cashouts, and $0.5 for US transfers</li> |
| setRecipient, | ||
| recipient, |
There was a problem hiding this comment.
thought: both setRecipient and recipient are now not-used in InitialCashoutView now. We should prob clean it up
| @@ -49,10 +46,10 @@ export const InitialCashoutView = ({ | |||
| * There may be chains that are not supported to conduct that cross-chain operation (e.g., due to gas costs, | |||
| * business strategy, etc.), so we'd like to block user action in that case. | |||
| @@ -269,12 +266,16 @@ export const InitialCashoutView = ({ | |||
| ?.map((account, index) => ( | |||
| <div | |||
| key={index} | |||
| className={`flex w-full cursor-pointer items-center justify-between border border-black p-2 ${ | |||
| className={twMerge( | |||
| : 'hover:bg-gray-100', | ||
| xchainAllowed && 'cursor-pointer', | ||
| !xchainAllowed && 'opacity-60' | ||
| )} | ||
| onClick={() => { |
There was a problem hiding this comment.
thought: this logic doesn't seem super legible
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Style