-
Notifications
You must be signed in to change notification settings - Fork 15
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
iOS #1
base: master
Are you sure you want to change the base?
iOS #1
Conversation
… information, and logging.
Thanks for the PR but there are some issues with it:
|
I'm aware that there's no JavaScript compatibility right now, I'm working on that. Like I said, it's a work in progress, and I don't expect you to merge before it's finished. As for the licensing issues, let's just hope @MattFoley will add one. Or are you guys already working on an iOS implementation yourselves? |
We'll need one by the next month for a project, so if we do not find a suitable implementation till there we'll build one.
|
I'd be happy to add a license. Sorry about that, usually my code is MIT licensed. |
I'd be very happy if you guys went ahead and used this. I won't be able to implement Android support and I'm very much a javascript novice. Let me know if you need anything else. |
@MattFoley Thanks for the license, if I ever come to Boston I'll buy you that beer ;) I moved the license to @cartolari I've got it working. I'll do some more (manual) testing tomorrow, but it seems like single payments work just like in Android, with the same API for the user. Internally, the API is different though. I'm not an iOS developer and I'm not familiar with the PayPal SDK and the differences between the iOS and the Android SDK. You can either merge this PR and then refactor, or at least take a look at my implementation and then do it from scratch. I struggled with integrating the PayPal SDK into the library and linking it all together, so if you do it from scratch, you might want to have a look at the updated README about the installation instructions for the end user and at my commits dbcab7c and e73c131. Basically I followed the manual instructions from PayPal, doing steps 1-4 for the library project, and steps 4-5 for the app (note the duplicate step 4). I also played around with CocoaPods but failed. In the end, I think the way I did it is quite nice for the users of the library. But like I said, I'm not an iOS developer, so maybe there are easier / better ways. For development, the PayPal SDK is in the repository as a submodule. |
Thanks @amiuhle, |
If you guys are still working on this, you may want to eyeball some updates I made to my repository recently. A few issues were found when used with the latest version of React Native and they're all fixed and in the master branch over there now. |
Looks like iOS support was never merged here? Should I go ahead and bring in this repository into mine, so that we have one unified iOS and Android paypal wrapper? |
I'm also interested in working on a unified wrapper. My intention was to start with this pull request and unify the two APIs. Is there a better starting point? |
Hey @MattFoley and @miracle2k, Sorry for the delay... I am without time to spend in the iOS support by now (since the product that we were using this package is no longer using PayPal). I am adding you both as collaborators here, but feel free to make the universal wrapper on your own repositories. Best regards, |
If that's the case, I'd love to move an Android wrapper into my repository. We use the iOS version at Skillz, and will need an Android version in the coming months. I can't imagine we would stop using it, so it will have a stable maintainer for a long time to come.
|
@MattFoley I think It'd be easier to just merge my PR into this repository, but either way, if you need any help, let me know. |
Sorry about the long wait on getting this merged together. I've opened a copy of your PR against our repository, which we'll be using in production, so will be stable and maintained. MattFoley/react-native-paypal#11 I did make a few minor adjustments: Will this suit your needs going forward @amiuhle? I'm going to work with another engineer on our team to review the code, but it should be merged in tomorrow. |
@MattFoley I'm guessing so, but unfortunately I don't have time to check right now. Just go ahead and merge. If there are any necessary adjustments, I'll create a PR when I'm working on the project the next time. Thanks for the work! |
I started working on an iOS implementation. So far I've only merged in MattFoley/react-native-paypal, so it's still a work in progress.
I just wanted to let people know to avoid duplicate work on this.