Skip to content
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

Promise based API #3

Merged
merged 3 commits into from
Oct 25, 2016
Merged

Promise based API #3

merged 3 commits into from
Oct 25, 2016

Conversation

akshaybhalotia
Copy link
Contributor

Changes as per discussion on #2 .

Supports promise as well as callbacks (for example, as in razorpay/razorpay-cordova )

```
4. Call RazorpayCheckout's `open` method with `options`, preferably on a user action:
2. Call RazorpayCheckout's `open` method with `options` (preferably on a user
action) as a **JS Promise**. The `then` part corresponds to a successful payment,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

open method with options as a JS Promise

sounds a bit weird. How about the below ?

Call RazorpayCheckout.open method with the options. The method returns a promise where then part corresponds to a successful payment and the catch part corresponds to payment failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, sounds great. Wasn't sure on terminology around Promises.


static open(options, successCallback, errorCallback) {
return new Promise(function(resolve, reject) {
let removeSubscriptions = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move the function outside open method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

static open(options, successCallback, errorCallback) {
return new Promise(function(resolve, reject) {
let removeSubscriptions = () => {
razorpayEvents.removeAllListeners('Razorpay::onPaymentSuccess');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onPaymentSuccess, onPaymentError looks like a handler name.

How about Razorpay::PAYMENT_SUCCESS, Razorpay::PAYMENT_ERROR event names ?


}

export {RazorpayCheckout};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make it as default export like

export default RazorpayCheckout

const { RazorpayCheckout, RazorpayEventEmitter } = Razorpay;

const razorpayEvents = new NativeEventEmitter(RazorpayEventEmitter);
import { RazorpayCheckout } from 'react-native-razorpay';
Copy link
Contributor

@selvagsz selvagsz Oct 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with default export, this will be

import RazorpayCheckout from 'react-native-razorpay'

{ alert("Success: " + data.payment_id) }
).catch(data =>
{ alert("Error: " + data.code + " | " + data.description) }
);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change it to below style

RazorpayCheckout.open(options).then((data) => {
    // handle success
    alert(`Success: ${data.payment_id}`)
}).catch((error) => {
    // handle error
    alert(`Error: ${error.code} | ${error.description}`)    
})

@selvagsz
Copy link
Contributor

lgtm 👍

@selvagsz selvagsz merged commit 1a5805d into f/release_config Oct 25, 2016
@selvagsz selvagsz deleted the f/promise_based_api branch October 25, 2016 13:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants