-
Notifications
You must be signed in to change notification settings - Fork 169
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
Firebase App Invite integration #668
Conversation
Had to remove home gdg on the server. It is not really used and causing problems now.
- Invite button is added to NavDrawer that just creates a dynamic url. - AppInvite related code from MainActivity is removed because it will be forwarded to AppInviteDeepLinkActivity by default using the deeplink.
The first page will be the welcome invite page. When we don't have invite, it will move on to the next page.
And windowBackground is set through themes
and remove old AppInviteDeepLinkActivity
Revert the changes in PlusUtils Upgrade play services
Move intent filters from MainActivity to StartActivity
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
@@ -136,10 +136,11 @@ dependencies { | |||
compile "com.android.support.test.espresso:espresso-idling-resource:$rootProject.espressoVersion" | |||
|
|||
//Play Services | |||
compile "com.google.android.gms:play-services-auth:$rootProject.playServicesVersion" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you need auth? It is not mentioned in the firebase integration documentation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The piece of code that we have in Settings it's not working anymore. The class was removed. I first deleted that and realized that I can just add this dependency. The reason was the upgrade of the gms version.
private void shareAppInviteLink() { | ||
AppInviteLinkGenerator linkGenerator = AppInviteLinkGenerator.create(); | ||
String gplusId = PlusUtils.getCurrentPersonId(getGoogleApiClient()); | ||
HttpUrl appInviteLink = gplusId != null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AboutActivity should not decide on how to handle not signed users. This should go into the linkGenerator.
@@ -303,6 +312,19 @@ private void navigateTo(Class<? extends GdgActivity> activityClass, Bundle addit | |||
mDrawerLayout.closeDrawers(); | |||
} | |||
|
|||
private void shareAppInviteLink() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Duplicate code, same as in AboutActivity
@Override | ||
public void onSaveInstanceState(Bundle outState) { | ||
super.onSaveInstanceState(outState); | ||
outState.putString(INVITE_SENDER, inviteSender); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
.getPerson(inviteSender) | ||
.enqueue(new Callback<Person>() { | ||
@Override | ||
public void success(Person sender) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we fail to load the sender then the invitation process stops here. We should still show the inviteContainer with a dummy image and a dummy name ("a friend" or so)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally makes sense. We should do it.
<string name="invitation_error_message">App Invite was unsuccessful.</string> | ||
<string name="thanks_for_feedback">Thank you for your Feedback!</string> | ||
<string name="retry">Retry</string> | ||
<string name="title_event_series_greetings">New events coming up!</string> | ||
<string name="event_series_greetings">Learn more about %1$s and see events around the world!</string> | ||
<string name="cd_invitation_sender_profile_image">Invitation sender profile image</string> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you upload the new strings to crowdin?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strings are already upload/updated in crowdin.
Show empty invite UI if we have the invite but the sender is not found
@friedger comments are addressed. 👍 |
import android.os.Parcel; | ||
import android.os.Parcelable; | ||
|
||
class Invite implements Parcelable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you add this class? Is this "just" for giving String a meaning? Then sender
should be gplusIdOfSender
or so
I did that to both provide a meaningful name and to handle 3 cases in 1 On Fri, Sep 23, 2016, 22:31 Friedger Müffke notifications@github.com
|
First of all, this turned out bigger than I expected. Sorry for that. 🙏
This PR adds new Firebase Invites API. Firebase Invites uses Dynamic Links internally which is where the magic occurs.
App Invite, on top of that, a UI where you can choose your friends to share. But it only includes sharing via SMS and Email. I really didn't like it. I think it is too restrictive.
Instead, I create dynamic links directly. Which then, user can choose whatever app they want to share.
I'm using https://gdg.events/invite/?sender=gplusid as link structure. This link will be opened if it is clicked in a computer.
If the app is already installed, it just opens the app.
If not, it installs the app and then in the login page, I put the picture and a welcome message there.
Since we have more info there now, I quickly add landscape version of the page.
Screenshots
Fixes #637