Skip to content

Added main ToS/PP #1300

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
May 17, 2018
Merged

Added main ToS/PP #1300

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
dac52e6
Added main ToS/PP, WIP
lsirac May 11, 2018
f5b0b75
Adds footer required for GDPR compliance. Also made changes based on …
lsirac May 15, 2018
420da68
Fixes footer ToS/Pp links. Also made changes based on review feedback.
lsirac May 15, 2018
5793bd8
Minor fixes before merge
lsirac May 16, 2018
670b363
Fixes lint errors and auth method picker landscape issues.
lsirac May 16, 2018
21df8b3
Move ToS/PP into scrollviews (#1302)
samtstern May 16, 2018
3cedd37
Fix lint errors
samtstern May 17, 2018
302c94c
Applies code review suggestions
lsirac May 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions auth/src/main/java/com/firebase/ui/auth/data/model/FlowParameters.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import android.support.annotation.Nullable;
import android.support.annotation.RestrictTo;
import android.support.annotation.StyleRes;
import android.text.TextUtils;

import com.firebase.ui.auth.AuthUI.IdpConfig;
import com.firebase.ui.auth.util.ExtraConstants;
Expand Down Expand Up @@ -128,4 +129,16 @@ public FlowParameters[] newArray(int size) {
return new FlowParameters[size];
}
};

public boolean isSingleProviderFlow() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you use this in SignInKickstarter too? I like your method much better.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

return providerInfo.size() == 1;
}

public boolean isTermsOfServiceUrlProvided() {
return !TextUtils.isEmpty(termsOfServiceUrl);
}

public boolean isPrivacyPolicyUrlProvided() {
return !TextUtils.isEmpty(privacyPolicyUrl);
}
}
3 changes: 1 addition & 2 deletions auth/src/main/java/com/firebase/ui/auth/data/remote/SignInKickstarter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,8 @@ public void onComplete(@NonNull Task<CredentialRequestResponse> task) {

private void startAuthMethodChoice() {
List<AuthUI.IdpConfig> idpConfigs = getArguments().providerInfo;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we inline getArguments().providerInfo below? (Trick: use Ctrl + Shift + N)

Copy link
Contributor Author

@lsirac lsirac May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure what you mean. Ctrl + Shift + N lets me search for files.

I changed it so that there is no local idpConfigs list, we just do getArguments().providerInfo.get(0) instead. Is that what you meant?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Lol, my B. I meant Ctrl + Alt + N. 😊 You should try it, I'm always impressed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But yeah, LGTM


// If there is only one provider selected, launch the flow directly
if (idpConfigs.size() == 1) {
if (getArguments().isSingleProviderFlow()) {
AuthUI.IdpConfig firstIdpConfig = idpConfigs.get(0);
String firstProvider = firstIdpConfig.getProviderId();
switch (firstProvider) {
Expand Down
16 changes: 16 additions & 0 deletions auth/src/main/java/com/firebase/ui/auth/ui/email/CheckEmailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,14 @@
import android.view.View;
import android.view.ViewGroup;
import android.widget.EditText;
import android.widget.TextView;

import com.firebase.ui.auth.R;
import com.firebase.ui.auth.data.model.FlowParameters;
import com.firebase.ui.auth.data.model.User;
import com.firebase.ui.auth.ui.FragmentBase;
import com.firebase.ui.auth.util.ExtraConstants;
import com.firebase.ui.auth.util.data.PrivacyDisclosureUtils;
import com.firebase.ui.auth.util.ui.ImeHelper;
import com.firebase.ui.auth.util.ui.fieldvalidators.EmailFieldValidator;
import com.firebase.ui.auth.viewmodel.ResourceObserver;
Expand Down Expand Up @@ -96,6 +99,19 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
}

view.findViewById(R.id.button_next).setOnClickListener(this);

TextView termsText = view.<TextView>findViewById(R.id.email_tos_and_pp_text);
Copy link
Contributor

Choose a reason for hiding this comment

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

optional style nit:

I normally see this:

TextView termsText = (TextView) view.findViewById(R.id.email_tos_and_pp_text);

Not this:

TextView termsText = view.<TextView>findViewById(R.id.email_tos_and_pp_text);

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah, I forgot to mention something else about this. You don't need those casts! Notice how the IDE greys them out? That's cuz findViewById was updated to use generics in API 26.

TextView footerText = view.<TextView>findViewById(R.id.email_footer_tos_and_pp_text);
FlowParameters flowParameters = getFlowParams();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we get rid of flowParameters? (It's just as long as getFlowParams() and is only used once.)

Copy link
Contributor Author

@lsirac lsirac May 17, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I changed it to pass flowParameters instead of making 2 more calls to getFlowParams() - unless you want me to change them all to getFlowParams()?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, SGTM

if (flowParameters.isSingleProviderFlow()) {
PrivacyDisclosureUtils.setupTermsOfServiceAndPrivacyPolicyText(getContext(),
getFlowParams(),
termsText);
} else {
PrivacyDisclosureUtils.setupTermsOfServiceFooter(getContext(),
getFlowParams(),
footerText);
}
}

@Override
Expand Down
24 changes: 12 additions & 12 deletions auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
import com.firebase.ui.auth.data.model.User;
import com.firebase.ui.auth.ui.FragmentBase;
import com.firebase.ui.auth.util.ExtraConstants;
import com.firebase.ui.auth.util.data.PrivacyDisclosureUtils;
import com.firebase.ui.auth.util.data.ProviderUtils;
import com.firebase.ui.auth.util.ui.ImeHelper;
import com.firebase.ui.auth.util.ui.PreambleHandler;
import com.firebase.ui.auth.util.ui.fieldvalidators.BaseValidator;
import com.firebase.ui.auth.util.ui.fieldvalidators.EmailFieldValidator;
import com.firebase.ui.auth.util.ui.fieldvalidators.NoOpValidator;
Expand Down Expand Up @@ -80,7 +80,9 @@ public void onCreate(@Nullable Bundle savedInstanceState) {
@Override
protected void onSuccess(@NonNull IdpResponse response) {
startSaveCredentials(
mHandler.getCurrentUser(), response, mPasswordEditText.getText().toString());
mHandler.getCurrentUser(),
response,
mPasswordEditText.getText().toString());
}

@Override
Expand Down Expand Up @@ -140,11 +142,6 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
// Only show the name field if required
nameInput.setVisibility(requireName ? View.VISIBLE : View.GONE);

PreambleHandler.setup(
getContext(),
getFlowParams(),
R.string.fui_button_text_save,
view.<TextView>findViewById(R.id.create_account_text));
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && getFlowParams().enableCredentials) {
mEmailEditText.setImportantForAutofill(View.IMPORTANT_FOR_AUTOFILL_NO);
}
Expand All @@ -171,6 +168,9 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
} else {
safeRequestFocus(mEmailEditText);
}

TextView footerText = view.<TextView>findViewById(R.id.email_footer_tos_and_pp_text);
PrivacyDisclosureUtils.setupTermsOfServiceFooter(getContext(), getFlowParams(), footerText);
}

private void safeRequestFocus(final View v) {
Expand Down Expand Up @@ -233,11 +233,11 @@ private void validateAndRegisterUser() {
boolean nameValid = mNameValidator.validate(name);
if (emailValid && passwordValid && nameValid) {
mHandler.startSignIn(new IdpResponse.Builder(
new User.Builder(EmailAuthProvider.PROVIDER_ID, email)
.setName(name)
.setPhotoUri(mUser.getPhotoUri())
.build())
.build(),
new User.Builder(EmailAuthProvider.PROVIDER_ID, email)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is something wrong with the formatting settings?

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't worry about this. Equally legible either way.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, we just made an indentation change to the styling a long time ago and never bothered running it over the whole codebase. Like @samtstern said, we're not that nit picky. 😆 Thanks for checking though! 👍

I'll take another look at this tomorrow.

.setName(name)
.setPhotoUri(mUser.getPhotoUri())
.build())
.build(),
password);
}
}
Expand Down
7 changes: 7 additions & 0 deletions auth/src/main/java/com/firebase/ui/auth/ui/idp/AuthMethodPickerActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import android.view.View;
import android.view.ViewGroup;
import android.widget.ImageView;
import android.widget.TextView;
import android.widget.Toast;

import com.firebase.ui.auth.AuthUI;
Expand All @@ -42,6 +43,7 @@
import com.firebase.ui.auth.data.remote.PhoneSignInHandler;
import com.firebase.ui.auth.data.remote.TwitterSignInHandler;
import com.firebase.ui.auth.ui.AppCompatBase;
import com.firebase.ui.auth.util.data.PrivacyDisclosureUtils;
import com.firebase.ui.auth.viewmodel.ResourceObserver;
import com.firebase.ui.auth.viewmodel.idp.ProviderSignInBase;
import com.firebase.ui.auth.viewmodel.idp.SocialProviderResponseHandler;
Expand Down Expand Up @@ -106,6 +108,11 @@ protected void onFailure(@NonNull Exception e) {
}
}
});

TextView termsText = findViewById(R.id.main_tos_and_pp);
PrivacyDisclosureUtils.setupTermsOfServiceAndPrivacyPolicyText(AuthMethodPickerActivity.this,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: do you need AuthMethodPickerActivity.this or could it be just this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll change to this

getFlowParams(),
termsText);
}

private void populateIdpList(List<IdpConfig> providerConfigs,
Expand Down
15 changes: 10 additions & 5 deletions auth/src/main/java/com/firebase/ui/auth/ui/phone/PhoneActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,8 @@ protected void onCreate(@Nullable Bundle savedInstanceState) {
mPhoneNumber = savedInstanceState.getString(KEY_VERIFICATION_PHONE);

if (savedInstanceState.getSerializable(KEY_STATE) != null) {
mVerificationState = (VerificationState) savedInstanceState.getSerializable(KEY_STATE);
mVerificationState = (VerificationState) savedInstanceState.getSerializable(
KEY_STATE);
}
return;
}
Expand Down Expand Up @@ -165,7 +166,8 @@ public void submitConfirmationCode(@NonNull String confirmationCode) {
// See: https://github.com/firebase/FirebaseUI-Android/issues/922
Log.w(PHONE_VERIFICATION_LOG_TAG,
String.format("submitConfirmationCode: mVerificationId is %s ; " +
"confirmationCode is %s", TextUtils.isEmpty(mVerificationId) ? "empty" : "not empty",
"confirmationCode is %s",
TextUtils.isEmpty(mVerificationId) ? "empty" : "not empty",
TextUtils.isEmpty(confirmationCode) ? "empty" : "not empty"));
return;
}
Expand Down Expand Up @@ -311,7 +313,8 @@ private void showAlertDialog(@StringRes int messageId,
String s = getString(messageId);
mAlertDialog = new AlertDialog.Builder(this)
.setMessage(s)
.setPositiveButton(R.string.fui_incorrect_code_dialog_positive_button_text, onClickListener)
.setPositiveButton(R.string.fui_incorrect_code_dialog_positive_button_text,
onClickListener)
.show();
}

Expand Down Expand Up @@ -351,7 +354,8 @@ public void onFailure(@NonNull Exception e) {
R.string.fui_incorrect_code_dialog_body,
new DialogInterface.OnClickListener() {
@Override
public void onClick(DialogInterface dialog, int which) {
public void onClick(DialogInterface dialog,
int which) {
getSubmitConfirmationCodeFragment()
.setConfirmationCode("");
}
Expand All @@ -362,7 +366,8 @@ public void onClick(DialogInterface dialog, int which) {
R.string.fui_error_session_expired,
new DialogInterface.OnClickListener() {
@Override
public void onClick(DialogInterface dialog, int which) {
public void onClick(DialogInterface dialog,
int which) {
getSubmitConfirmationCodeFragment()
.setConfirmationCode("");
}
Expand Down
25 changes: 12 additions & 13 deletions auth/src/main/java/com/firebase/ui/auth/ui/phone/SubmitConfirmationCodeFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@
import com.firebase.ui.auth.ui.FragmentBase;
import com.firebase.ui.auth.util.CustomCountDownTimer;
import com.firebase.ui.auth.util.ExtraConstants;
import com.firebase.ui.auth.util.data.PrivacyDisclosureUtils;
import com.firebase.ui.auth.util.ui.BucketedTextChangeListener;
import com.firebase.ui.auth.util.ui.ImeHelper;
import com.firebase.ui.auth.util.ui.PreambleHandler;

/**
* Display confirmation code to verify phone numbers input in {{@link VerifyPhoneNumberFragment}}
Expand All @@ -56,7 +56,6 @@ public class SubmitConfirmationCodeFragment extends FragmentBase {
private Button mSubmitConfirmationButton;
private CustomCountDownTimer mCountdownTimer;
private PhoneActivity mVerifier;
private TextView mAgreementText;
private long mMillisUntilFinished;

public static SubmitConfirmationCodeFragment newInstance(FlowParameters flowParameters,
Expand All @@ -73,8 +72,10 @@ public static SubmitConfirmationCodeFragment newInstance(FlowParameters flowPara

@Nullable
@Override
public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup container, @Nullable
Bundle savedInstanceState) {
public View onCreateView(@NonNull LayoutInflater inflater,
@Nullable ViewGroup container,
@Nullable
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: formatter kinda nuked this one. Can we put the Bundle savedInstanceState back on the line with @Nullable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep

Bundle savedInstanceState) {
View v = inflater.inflate(R.layout.fui_confirmation_code_layout, container, false);
FragmentActivity parentActivity = getActivity();

Expand All @@ -83,7 +84,6 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
mResendCodeTextView = v.findViewById(R.id.resend_code);
mConfirmationCodeEditText = v.findViewById(R.id.confirmation_code);
mSubmitConfirmationButton = v.findViewById(R.id.submit_confirmation_code);
mAgreementText = v.findViewById(R.id.create_account_tos);

final String phoneNumber = getArguments().getString(ExtraConstants.PHONE);

Expand All @@ -93,10 +93,16 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
setupCountDown(RESEND_WAIT_MILLIS);
setupSubmitConfirmationCodeButton();
setupResendConfirmationCodeTextView(phoneNumber);
setUpTermsOfService();
return v;
}

@Override
public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
TextView footerText = view.<TextView>findViewById(R.id.email_footer_tos_and_pp_text);
PrivacyDisclosureUtils.setupTermsOfServiceFooter(getContext(), getFlowParams(), footerText);
}

@Override
public void onStart() {
super.onStart();
Expand Down Expand Up @@ -221,13 +227,6 @@ private void cancelTimer() {
}
}

private void setUpTermsOfService() {
PreambleHandler.setup(getContext(),
getFlowParams(),
R.string.fui_continue_phone_login,
mAgreementText);
}

@VisibleForTesting(otherwise = VisibleForTesting.NONE)
CustomCountDownTimer getCountdownTimer() {
return mCountdownTimer;
Expand Down
30 changes: 23 additions & 7 deletions auth/src/main/java/com/firebase/ui/auth/ui/phone/VerifyPhoneNumberFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.firebase.ui.auth.util.ExtraConstants;
import com.firebase.ui.auth.util.GoogleApiUtils;
import com.firebase.ui.auth.util.data.PhoneNumberUtils;
import com.firebase.ui.auth.util.data.PrivacyDisclosureUtils;
import com.firebase.ui.auth.util.ui.ImeHelper;
import com.firebase.ui.auth.viewmodel.RequestCodes;
import com.google.android.gms.auth.api.credentials.Credential;
Expand All @@ -55,7 +56,6 @@
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
public class VerifyPhoneNumberFragment extends FragmentBase implements View.OnClickListener {
public static final String TAG = "VerifyPhoneFragment";

private Context mAppContext;

private CountryListSpinner mCountryListSpinner;
Expand Down Expand Up @@ -107,15 +107,14 @@ public void onDonePressed() {
parentActivity.setTitle(getString(R.string.fui_verify_phone_number_title));
setupCountrySpinner();
setupSendCodeButton();
setupTerms();

return v;
}

private void setupTerms() {
final String verifyPhoneButtonText = getString(R.string.fui_verify_phone_number);
final String terms = getString(R.string.fui_sms_terms_of_service, verifyPhoneButtonText);
mSmsTermsText.setText(terms);
@Override
public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceState) {
super.onViewCreated(view, savedInstanceState);
TextView footerText = view.<TextView>findViewById(R.id.email_footer_tos_and_pp_text);
setupPrivacyDisclosures(footerText);
}

@Override
Expand Down Expand Up @@ -273,6 +272,23 @@ private void setCountryCode(PhoneNumber phoneNumber) {
}
}

private void setupPrivacyDisclosures(TextView footerText) {
final String verifyPhoneButtonText = getString(R.string.fui_verify_phone_number);
final String multipleProviderFlowText = getString(R.string.fui_sms_terms_of_service,
verifyPhoneButtonText);
FlowParameters flowParameters = getFlowParams();
PrivacyDisclosureUtils.setupTermsOfServiceAndPrivacyPolicySmsText(getContext(),
flowParameters,
mSmsTermsText,
multipleProviderFlowText);

if (!flowParameters.isSingleProviderFlow()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So if this is a single provider flow then we'll have both the big text and the footer? If that's not the case (maybe the previous method has some internal switching) let's make that clearer here by doing if / else so only one of the PrivacyDisclosureUtils classes runs.

Copy link
Contributor Author

@lsirac lsirac May 16, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, SMS is special because we warn them either way (about the SMS cost). I'll change this

PrivacyDisclosureUtils.setupTermsOfServiceFooter(getContext(),
flowParameters,
footerText);
}
}

void showError(String e) {
mPhoneInputLayout.setError(e);
}
Expand Down
Loading