feat: enable custom fee address and fee amount #140
Conversation
WalkthroughThe changes introduce new properties to the payment widget components to support transaction fees, allowing the platform to take fees during payment processing. New properties Changes
Assessment against linked issues
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: 13
🧹 Outside diff range comments (1)
packages/payment-widget/src/lib/payment-widget.svelte (1)
Line range hint
4-4: Remove unused Button import and LGTM on button implementationThe standard HTML button implementation looks good and provides more direct control over styling. However, the Button component import on line 4 is no longer used and should be removed.
Please remove the following unused import:
- import { Button } from "@requestnetwork/shared-components/button";The button implementation and its click handler logic are correct, properly handling both connected and disconnected states.
Also applies to: 199-212
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (4)
- packages/payment-widget/src/lib/components/payment-confirmation.svelte (7 hunks)
- packages/payment-widget/src/lib/payment-widget.svelte (6 hunks)
- packages/payment-widget/src/lib/react/PaymentWidget.d.ts (2 hunks)
- packages/payment-widget/src/lib/utils/request.ts (6 hunks)
🔇 Additional comments (3)
packages/payment-widget/src/lib/payment-widget.svelte (3)
25-25: LGTM: New fee-related properties addedThe addition of
feeAddressandfeeAmountInUSDproperties aligns with the PR objectives. The default values are appropriately set, withethers.constants.AddressZeroused forfeeAddress.Also applies to: 41-42
236-237: LGTM: Fee properties passed to PaymentConfirmationThe
feeAddressandfeeAmountInUSDprops are correctly passed to the PaymentConfirmation component, ensuring that the fee information is available for processing and display during payment confirmation.
Line range hint
1-486: Summary: Implementation of custom fee functionality looks goodThe changes in this file successfully implement the custom fee functionality as per the PR objectives. The new properties
feeAddressandfeeAmountInUSDare properly integrated into the component and passed to the relevant child components.Key points:
- New fee-related properties are added and properly used.
- The button implementation has been updated for better control and styling.
- CSS styles are well-implemented with a minor suggestion for improvement.
Next steps:
- Address the minor suggestions in the review comments.
- Ensure that the
PaymentConfirmationcomponent (not shown in this file) properly handles the new fee properties.- Update tests to cover the new fee functionality.
- Consider adding documentation for the new fee-related properties in the component's JSDoc or README.
Overall, the changes look good and align well with the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
- packages/payment-widget/src/lib/payment-widget.svelte (6 hunks)
🔇 Additional comments (3)
packages/payment-widget/src/lib/payment-widget.svelte (3)
8-8: LGTM: Import of ethers libraryThe addition of the
ethersimport is appropriate for usingethers.constants.AddressZeroin the newfeeAddressproperty.
235-236: LGTM: Fee properties passed to PaymentConfirmationThe
feeAddressandfeeAmountInUSDproperties are correctly passed to the PaymentConfirmation component. This change is consistent with the new properties added to the parent component and allows for proper handling of fee information in the confirmation step.
374-407: 🧹 Nitpick (assertive)LGTM on button styles with a minor suggestion
The new button styles are well-implemented, providing a consistent look and feel with proper hover and disabled states. The use of SCSS nesting and CSS variables is good for maintainability and theming.
For consistency with other color definitions in the file, consider using an SCSS variable for the button's background color:
+ $button-bg-color: #0bb489; - background-color: #0bb489; + background-color: $button-bg-color; - background-color: rgba($color: #0bb489, $alpha: 0.8); + background-color: rgba($color: $button-bg-color, $alpha: 0.8);This change would make it easier to update the color scheme in the future if needed.
aimensahnoun
deleted the
127-payment-widget-expose-feeaddress-and-feeamount-so-platform-can-take-a-fee
branch
| <section class="rn-payment-widget-body"> | ||
| <h2>Pay with crypto</h2> | ||
| <Button | ||
| <button |
There was a problem hiding this comment.
@aimensahnoun for future reference, odd changes like this warrant a comment in the code to explain why we're breaking from convention.
resolves #127
feeAddressandfeeAmountInUSD.feeAmountInUSDto selected payment currency.feeAddressandfeeAmountInUSDare valid, we show them to the buyer in the confirmation step and calculate the total.feeAddressandfeeAmountInUSDare valid.Payment without fee
Payment with fee
Summary by CodeRabbit
New Features
Documentation
Bug Fixes