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

Update Checkout Android SDK to v1.2.0, add compatibilty changes for RN 0.39 #9

Merged
merged 5 commits into from
Dec 12, 2016

Conversation

mb-14
Copy link
Contributor

@mb-14 mb-14 commented Dec 12, 2016

This should also fix #8

@mb-14 mb-14 requested a review from akshaybhalotia December 12, 2016 08:22
"homepage": "https://github.com/razorpay/react-native-razorpay#readme",
"dependencies": {
"react-native": "^0.39.1"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this as optional? My guess is that since this a breaking change (kind of), not many people will upgrade to this version of react-native. We should not be forcing this.

}
});

AppRegistry.registerComponent('example', () => example);
Copy link
Contributor

Choose a reason for hiding this comment

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

How and why is this different than the standard index.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Each platform requires its own entry file. We can point both entrypoint files (index.android.js and index.ios.js) to the common code in index.js to avoid code duplication

Copy link
Contributor

Choose a reason for hiding this comment

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

We should have been doing this already. Please make the necessary changes.

@@ -16,6 +16,9 @@
import com.facebook.react.bridge.Arguments;
import com.facebook.react.modules.core.DeviceEventManagerModule;
import com.razorpay.CheckoutActivity;
import com.razorpay.PaymentData;
Copy link
Contributor

Choose a reason for hiding this comment

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

Have we decided explicitly that we are exposing this? Because then, it needs to be consistent across all public repos (cordova, sample app, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We've already exposed this for the auto-capture flow, no harm in exposing it here

@@ -66,4 +66,4 @@ const styles = StyleSheet.create({
}
});

AppRegistry.registerComponent('example', () => example);
AppRegistry.registerComponent('example', () => example);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you confirm that this does not break on iOS?

@akshaybhalotia akshaybhalotia merged commit b70089b into master Dec 12, 2016
@mb-14 mb-14 deleted the f/android_update branch December 13, 2016 04:25
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.

Not working with latest react-native 0.39
2 participants