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 example app to use two activities #1497

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

stianjensen
Copy link

@stianjensen stianjensen commented Sep 12, 2023

Using singleTask in an Android application for the activity that has an intent-filter on the LAUNCHER, means that any activities running on top of this activity will disappear when the app's icon is pressed again from the launcher.

This is a problem, because when confirming a SetupIntent or PaymentIntent, Stripe may need to present a web browser (its own activity) for verification with a customer's bank or similar, and the user may even need to leave this web browser activity temporarily to navigate to their bank's own application. If the user is not careful when navigating back to the original app, and taps the app's icon from the launcher, the app will open to its original activity, and the web browser is gone. Instead, the user will need to navigate back only using Android's app switcher, which will correctly bring you back into the open web browser activity.

The change in this commit side-steps this issue by splitting the example application into two acitivites. One that is only responsible for launching the application, and a second activity that actually represents the react native application, and will be started by the first activity, ensuring to only start it once.

This fixes #355.

Testing

  • I tested this manually
  • I added automated tests

Documentation

Select one:

  • I have added relevant documentation for my changes.
  • This PR does not result in any developer-facing changes.

Using singleTask in an Android application for the activity that has an
intent-filter on the LAUNCHER, means that any activities running on top
of this activity will disappear when the app's icon is pressed again
from the launcher.

This is a problem, because when confirming a SetupIntent or
PaymentIntent, Stripe may need to present a web browser (its own
activity) for verification with a customer's bank or similar, and the
user may even need to leave this web browser activity temporarily to
navigate to their bank's own application. If the user is not careful
when navigating back to the original app, and taps the app's icon from
the launcher, the app will open to its original activity, and the web
browser is gone. Instead, the user will need to navigate back only using
Android's app switcher, which will correctly bring you back into the
open web browser activity.

The change in this commit side-steps this issue by splitting the example
application into two acitivites. One that is only responsible for
launching the application, and a second activity that actually
represents the react native application, and will be started by the
first activity, ensuring to only start it once.

This fixes stripe#355.
@stianjensen
Copy link
Author

I based this on the fix originally described here:
proyecto26/react-native-inappbrowser#213 (comment)

It may be that the code can be further simplified, by not even maintaining a list, since in my experiments, I've never seen that it's possible for the activity to be started more than once (which makes sense, since it has launchMode 'singleTask). But for now I kept it like this, since it's equivalent to the code we ended up deploying to production.

@stianjensen stianjensen marked this pull request as ready for review September 12, 2023 20:52
@zdnk
Copy link

zdnk commented Sep 19, 2023

what about expo apps using managed workflow?

<category android:name="android.intent.category.LAUNCHER" />
</intent-filter>
</activity>

<activity
android:name=".MainActivity"
android:label="@string/app_name"
android:configChanges="keyboard|keyboardHidden|orientation|screenSize|uiMode"
android:launchMode="singleTask"
Copy link
Contributor

Choose a reason for hiding this comment

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

Replacing singleTask by singleTop may improve the app compatibility with deep linking.

Opening a deep link with that launchMode will cause the app to be restarted and may cause the lost of the current state.

Copy link
Author

Choose a reason for hiding this comment

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

singleTask is what is generally recommended (and the only thing that seems to work well in practice) by Branch.io for instance. As long as the activity that handles deeplinks is singleTask, but the one that handles the launcher intent is not, both the Stripe use case, and the Branch use case seem to work well in the same app at the same time.

@stianjensen
Copy link
Author

what about expo apps using managed workflow?

I'm not sure, and not really that familiar with android activites except what I've been forced to try to understand while debugging and fixing the issue in our react native app.

I would assume that the issue is something expo would need to support in their native code – but unsure if they could do it without risking breaking some other use case.

I still think there's not much Stripe could do about it without abandoning the use of chrome custom tabs altogether though (and that's not really an option for 3d secure flows either).

@@ -15,18 +15,20 @@
<meta-data
android:name="com.google.android.gms.wallet.api.enabled"
android:value="true" />
<activity android:name=".LaunchActivity" android:exported="true">
Copy link
Contributor

Choose a reason for hiding this comment

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

To handle deeplink .LaunchActivity should be singleTop otherwise it may cause two instances of the app

Suggested change
<activity android:name=".LaunchActivity" android:exported="true">
<activity android:launchMode="singleTop" android:name=".LaunchActivity" android:exported="true">

Copy link
Author

Choose a reason for hiding this comment

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

The trick with this split into two activities is that deeplinks should be handled by MainActivity directly, not LaunchActivity. MainActivity can then be singleTask or singleTop depending on what you prefer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, and kept deeplinks handled by MainActivity with singleTask launchMode (required to prevent issue with 3DS) will cause multiple instances. Using android:launchMode="singleTop" for LaunchActivity will prevent the multiple instances, as the deeplink will route the intent to the existing instance instead of creating a new one.

But I agree that it may depend on the use case you have to handle deeplink. This change is not required, but may be necessary depending on the project configuration.

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] 3DSecure confirmation screen closed if app is put in background
4 participants