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

Bug Fix: Android app not focused when using urlHandler or customActionHandler #79

Merged
merged 1 commit into from
Oct 5, 2020

Conversation

jehartzog
Copy link
Contributor

Fixes #76.

I went tracing through this and iterable/iterable-android-sdk to figure out why we had some many issues with our Android app not bring brought into focus. I'll lay out what I found here, starting at the top level.

  1. The actual config.customActionHandler you assign is fired here, via RNEventEmitter. Note that nothing is actually done with the return value from your JS handler.

  2. The event which executes the above is triggered in native Android here. The key issue here is this function always returns true, which tells the native code that any custom urls/actions are always handled. This also happens for urls.

  3. The above function is called here, where if we have a customActionHandler defined, then we return the result of the above (which as we saw, is always true). Otherwise we return false.

  4. The problem comes here, where we execute the action, and if handled === true than we don't bring the app into focus. I believe this may have been doing sensibly for native Android apps, where they want to intercept a URL/action and can easily bring the app into foreground. However this is not the same for RN, as this code need to be run on native side.

I initially created a fork of iterable/iterable-android-sdk which fixed the issue, but I later realize it would likely to be easier to fix for the RN use case, without breaking changes to the existing iterable-android-sdk, if we made the changes in this PR.

Copy link
Contributor

@vbabenkoru vbabenkoru left a comment

Choose a reason for hiding this comment

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

Thank you for the PR! Top notch investigative work @jehartzog! You're absolutely right: with RN, we can't assume the app calls startActivity, because RN apps are generally single-activity apps, and it's ok to send a launcher activity intent.
Thank you for the contribution. Very helpful.

@roninopf roninopf merged commit 840cf35 into Iterable:master Oct 5, 2020
@roninopf
Copy link
Contributor

roninopf commented Oct 5, 2020

Thank you @jehartzog! I've gone ahead and merged this, and it will be released soon.

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.

Android: App not focused on push notification press if backgrounded and custom url is handled
3 participants