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

Add basic demos of launching TWAs. #19

Merged
merged 3 commits into from
Sep 24, 2019
Merged

Add basic demos of launching TWAs. #19

merged 3 commits into from
Sep 24, 2019

Conversation

PEConn
Copy link
Collaborator

@PEConn PEConn commented Sep 5, 2019

Launching a TWA without using LauncherActivity is really ergonomic, except for the case of using a custom referrer.

I wanted to get these demos up so we had a vague use case for before dealing with #13 .

@PEConn
Copy link
Collaborator Author

PEConn commented Sep 5, 2019

Andre, Pavel - looking at the code it looks like the appropriate place for this to go is TrustedWebActivityIntentBuilder alongside setAdditionalTrustedOrigins, would you agree?

We could add it in TwaLauncher instead as a stop gap for a quicker turn around.

Or Andre suggested we could have some sort of "customize Intent callback" that TwaLauncher takes and then executes just before launching the Intent. That would be really powerful (in both the good and bad way).

@pshmakov
Copy link
Collaborator

pshmakov commented Sep 5, 2019

"customize Intent callback" sounds good. Alternatively, a less powerful option may work - receive a Bundle of additional extras to put into Intent.

demo/build.gradle Outdated Show resolved Hide resolved
Copy link
Member

@andreban andreban left a comment

Choose a reason for hiding this comment

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

It seems most of the new files ended without the license header. Can we add them before merging?

@andreban
Copy link
Member

andreban commented Sep 6, 2019

Or Andre suggested we could have some sort of "customize Intent callback" that TwaLauncher takes and then executes just before launching the Intent. That would be really powerful (in both the good and bad way).

The already has this power (eg: writing their own TwaLauncher) or just launching their own Intent. It's a matter of how easy we are making it for them to change the Intent.

@PEConn
Copy link
Collaborator Author

PEConn commented Sep 6, 2019

The already has this power (eg: writing their own TwaLauncher) or just launching their own Intent. It's a matter of how easy we are making it for them to change the Intent.

It's also a matter of the public API. What they can do today is messy, but that's fine because it uses the building blocks we provide. If we add a "customize intent callback" that then becomes part of TwaLauncher's API and people will start relying on it as well - especially if we move the Referrer stuff into AndroidX, then we'll have this method people will have been using be redundant.

The other aspect is letting developers shoot themselves in the foot - the EXTRA_REFERRER has a fairly specific format. If we give them raw access to it, they may get it wrong. I think we should provide an interface like Bundle and then construct it ourselves (as Pavel suggested).

However these do seem like orthogonal points - maybe we should create a createReferrerFromKeyValuePairs method and let them call it in the customizeIntentCallback method.

@andreban
Copy link
Member

andreban commented Sep 6, 2019

I agree that the higher level API for setting the referral, etc is the best approach. It's more obvious to developers and less likely for them to shoot themselves on the foot.

Regarding the intent callback, it provides a good path for developers who have use-cases we don't know about yet and would need access to the Intent.

new TwaLauncher(this).launch(builder, null, null);
}

public void launcherWithMultipleOrigins(View view) {

Choose a reason for hiding this comment

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

this is great!

@andreban andreban added the enhancement New feature or request label Sep 18, 2019
@PEConn
Copy link
Collaborator Author

PEConn commented Sep 23, 2019

Hey Andre, added the licenses.

Copy link
Member

@andreban andreban left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM, Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants