-
Notifications
You must be signed in to change notification settings - Fork 6
Update for newer versions of React Native (and some other goodies) #2
base: master
Are you sure you want to change the base?
Conversation
This works around a race condition in RN < 0.38.0, wherein hasActiveCatalystInstance() might return true on an instance that is not in fact fully set up and ready to receive messages.
Side effect of these improvements is that |
@artdent Thank you so, so much for this pull request! I apologize on the silence; I wasn't aware that GitHub doesn't automatically notify you of activity on your own repos. I'm going to take a look at this PR this evening, but from what I see I don't have any major objections. Does this have any breaking API changes? I'm okay putting it out as a major semver update, but only if need be. Again, thank you so much! |
It has been superceded by .android.js and .ios.js.
Yay, thanks for taking a look! Good call about backward compatibility. I just pushed a change to make sure the old constructor for ChromeCustomTabsPackage still works. With that, I think this PR is now backward-compatible. (The JS changes didn't affect the exposed API.) I also just pushed a fix to 4753b75, which should have deleted the platform-independent JS entry point. The point of cfd6404 is to expose the ability to customize the look and feel of the Chrome custom tab. Here's what its usage looks like in my project, for example:
It would be friendlier to expose these knobs to JavaScript, but some of them require referencing native code anyway, so I went with the simpler solution. |
Hello everyone, thank you all for making this (and updating it), I'm on RN 0.43, how long till we can get this on npm? :) |
@artdent Okay, finally had the chance to sit down and give this a proper review! I remembered playing around with customizing the custom tab, so I wanted to see how I did that. It looks like I was passing a config object into the Other than that, I think this is good to merge! Since I never released a 1.0.0, I'd like to tag this as such :) Thanks again for submitting this! I really, really appreciate it! |
🎉 |
Cool. That all sounds good. It looks like specifying animations and bitmaps from JavaScript should be possible with some difficulty. It can be done in a manner similar to this code from the react Image implementation: int id = context.getResources().getIdentifier(name, "drawable", context.getPackageName()); That said, since you need to be using Android Studio to manage those resources anyway, I personally would find it more convenient to customize the intent on the native side. I would rather make available both JavaScript props and a native hook that can reference CustomTabsIntent.Builder directly. But if you think that's overcomplicated then I can drop cfd6404 from this PR. |
If we can support a limited set of customizations from JavaScript, and then the full power of the SDK from Java, that'd be 🔥! Forgive me as I'm not a Java/Android developer, but would customizing the IntentEditor happen in For customizing from JS, could we precreate the client.launchCustomTab('http://i.imgur.com/xjdem.gif' {
tabEntranceAnimation: 'slide_up',
activityExitAnimation: 'fade_out',
tabExitAnimation: 'slide_down',
activityEntranceAnimation: 'fade_id'
}); and then this on the Java side (please forgive me if this doesn't compile, I don't have an IDE handy): Map<String, Integer> mResourceDrawableIdMap = new HashMap<String, Integer>();
mResourceDrawableIdMap.put("slide_up", R.anim.slide_up);
mResourceDrawableIdMap.put("slide_down", R.anim.slide_down);
mResourceDrawableIdMap.put("fade_in", R.anim.fade_in);
mResourceDrawableIdMap.put("fade_out", R.anim.fade_out);
if (config.hasKey("tabEntranceAnimation") && config.hasKey("activityExitAnimation")) {
String tabEntranceAnimationKey = config.getString("tabEntranceAnimation");
String activityExitAnimationKey = config.getString("activityExitAnimation");
if (mResourceDrawableIdMap.hasKey(tabEntranceAnimationKey)
&& mResourceDrawableIdMap.hasKey(activityExitAnimationKey)) {
builder.setStartAnimations(
mContext.getApplicationContext(),
mResourceDrawableIdMap.get(tabEntranceAnimationKey),
mResourceDrawableIdMap.get(activityExitAnimationKey)
);
}
} The downside to this approach is that it would grow unwieldy as the number of animations grow, but I think that we can provide just a small set (maybe 8?), and then allow anything outside of those to be done in Java natively. |
So is this available in npm now? |
@vivnau123 No, not yet. If you need this functionality in the meantime, use npm to install the package from the pull request. |
I was using the package with the pull request in rn0.35 and it was working fine. But I upgraded to 0.43.4 and it gives module not found error with the pull request even though the module is there in my node modules |
Will this PR also work on RN > 0.40? |
Hi! I'm using this module in my project, and I wanted to share some improvements. I could make separate PRs for each one if you want, or you can just take a look at all of them here.
The two most useful are:
If you happen to not be interested anymore, I'm happy to maintain my own fork, but it'd be nice to get these changes into a released version on NPM.