Migrate FragmentActivity Parameters to ComponentActivity #76
Conversation
| @@ -107,7 +107,7 @@ private boolean isValidRequestCode(int requestCode) { | |||
| * | |||
| * @param activity the activity that received the deep link back into the app | |||
| */ | |||
| public BrowserSwitchResult deliverResult(@NonNull FragmentActivity activity) { | |||
| public BrowserSwitchResult deliverResult(@NonNull ComponentActivity activity) { | |||
There was a problem hiding this comment.
Hey for BrowserSwitch v3 we may may be able to get rid of this method in favor of parseBrowserSwitchResult(). The hope is to make the manual browser switching flow mentioned in the migration guide the primary method of browser switching. It only needs a Context and an Intent instead of a particular Activity class.
There was a problem hiding this comment.
Ah ok yeah before v3 I'll do some more cleanup to see if we even want to support this pattern (and we are already using parseResult in core now). But it's the start method that is still requiring activity. The only place the method actually needs an activity currently is to check if the activity is finishing and throw an error. This check may not be as necessary now that we are giving more control to merchants over invoking the method - do you think we can remove it and change the start parameter to context as well?
There was a problem hiding this comment.
Oh yeah for start() I would definitely keep ComponentActivity. That's a really good point to make it Compose friendly.
It also makes sense to investigate turning Browser Switch into a launcher. That may help here, and apparently Compose has a launcher API that should simplify onCreate() instantiation.
There was a problem hiding this comment.
Only downside is we don't get a result back from Chrome Custom Tabs, but it does allow us to move away from Activity.startActivity(). It may be worth taking a look to see if it somehow makes sense for browser switching.
There was a problem hiding this comment.
^ there's a lot to discuss here, we have a ton of options. For the most part if we go with parseResult() and start() for now we should be good.
|
Replaced by #81 |
Summary of changes
FragmentActivityparameters toComponentActivity. This is more compatible with Jetpack Compose integrations, and we don't use any specificFragmentActivityfeatures within the SDK, so we don't need to require itChecklist
Authors