-
-
Notifications
You must be signed in to change notification settings - Fork 120
Fix(rnpsw) remove default currency #214
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix(rnpsw) remove default currency #214
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR updates the Paystack integration by removing the default currency from the PaystackProvider and adding XOF as a supported currency.
- Updated generatePaystackParams to conditionally include the currency parameter
- Expanded the Currency type to include XOF in development/types.ts
- Removed the default 'NGN' currency in PaystackProvider and now conditionally includes the currency prop
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| development/utils.ts | Conditionally includes currency in the generated parameters |
| development/types.ts | Adds XOF as a supported currency |
| development/PaystackProvider.tsx | Removes default currency and conditionally passes currency to the Paystack params |
Comments suppressed due to low confidence (1)
development/PaystackProvider.tsx:20
- Removing the default currency value changes the component's API; please ensure that integrations explicitly handle a fallback or default currency as intended.
currency,
|
Hey @tolu-paystack 👋 Thanks for this thoughtful contribution! This change addresses an important usability concern—defaulting to NGN in the provider can unintentionally prioritize Nigerian merchants, requiring others to pass in an extra prop unnecessarily. Your decision to remove the hardcoded currency aligns well with Paystack’s own default behavior (using the merchant’s integration currency), and improves the developer experience by ensuring consistency and reducing surprises across different regions. I reviewed the code, and it follows the style and simplicity we aim for in this package—clean, focused, and purposeful. Great job keeping the changes minimal and scoped only to what was necessary. A couple of suggestions before we merge:
Other than that, this is solid work! Thanks again for helping improve the project for the wider community 🌍🚀 |
just1and0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @tolu-paystack, again, great job on the PR, just one last thing before we merge it in.
|
Hey @just1and0 just seen your comment. Makes sense, I'll update the README now |
|
@just1and0 updated |
just1and0
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
@all-contributors add @tolu-paystack for code |
|
I've put up a pull request to add @tolu-paystack! 🎉 |
|
🎉 This PR is included in version 5.0.2 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Description
Addresses #212. Makes it such that if you don't pass a currency to PaystackProvider, your integration's default currency is used.
Also adds XOF as one of the supported currencies (Paystack is in beta in CIV)
Issue URL
#212