Skip to content
This repository has been archived by the owner on May 15, 2024. It is now read-only.

GH-1180 ; Don't launch new task on custom tabs. #1183

Merged
merged 8 commits into from
Mar 26, 2020
Merged

Conversation

jamesmontemagno
Copy link
Collaborator

Description of Change

Based off feedback of launching new task for system. Can't find google docs showing to do it how we have it and my old plugin didn't have it.

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

Behavioral Changes

Will now launch in same activity.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

@jamesmontemagno jamesmontemagno added this to the 1.5.2 milestone Mar 19, 2020
@jamesmontemagno jamesmontemagno added the awaiting-review This PR needs to have a set of eyes on it label Mar 19, 2020
@Redth
Copy link
Member

Redth commented Mar 25, 2020

The original issue #225 was basically:

Calling StartActivity() from outside of an Activity context requires the FLAG_ACTIVITY_NEW_TASK flag.

Which makes me think we should maybe check if we have the Activity's context first, and if not, still add the NewTask flag because it's all we can do at that point.

@jamesmontemagno
Copy link
Collaborator Author

I think we want to add back in:

#if ANDROID_24
if (Platform.HasApiLevelN)
flags |= ActivityFlags.LaunchAdjacent;
#endif

@rickclephas
Copy link

I think we want to add back in:

#if ANDROID_24
if (Platform.HasApiLevelN)
flags |= ActivityFlags.LaunchAdjacent;
#endif

Good point but the LaunchAdjacent flag requires the NewTask flag. See FLAG_ACTIVITY_LAUNCH_ADJACENT.

IMHO multi-window support should be an option in BrowserLaunchOptions.

This can be a place to expose some platform specific isms.

For now on Android we have a `PrefereAdjacent`  and on iOS a `PreferModal`.
@Redth
Copy link
Member

Redth commented Mar 26, 2020

So, I've added to this PR a new BrowserLaunchFlags and property on BrowserLaunchOptions. My thinking here is we can use this to set some platform specific things like Launching adjacent on android or presenting as Modal on iOS. Things that are useful to expose but don't really have a cross platform meaning.

This takes care of adding optionally adding LAUNCH_ADJACENT flag for Android where we otherwise wouldn't want NewTask. Thoughts?

@rickclephas
Copy link

So, I've added to this PR a new BrowserLaunchFlags and property on BrowserLaunchOptions. My thinking here is we can use this to set some platform specific things like Launching adjacent on android or presenting as Modal on iOS. Things that are useful to expose but don't really have a cross platform meaning.

This takes care of adding optionally adding LAUNCH_ADJACENT flag for Android where we otherwise wouldn't want NewTask. Thoughts?

Sounds good to me! 🙂

@jamesmontemagno
Copy link
Collaborator Author

Just running final test and should be good

@jamesmontemagno jamesmontemagno merged commit 2bcb47d into develop Mar 26, 2020
@Redth Redth deleted the bug/gh-1180 branch March 27, 2020 01:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
awaiting-review This PR needs to have a set of eyes on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants