Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,21 @@
import android.view.View;
import android.view.ViewGroup;
import android.widget.EditText;
import android.widget.TextView;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.R;
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.ui.ImeHelper;
import com.firebase.ui.auth.util.ui.PreambleHandler;
import com.firebase.ui.auth.util.ui.fieldvalidators.EmailFieldValidator;
import com.firebase.ui.auth.viewmodel.ResourceObserver;
import com.google.firebase.auth.EmailAuthProvider;

import java.util.List;

/**
* Fragment that shows a form with an email field and checks for existing accounts with that email.
* <p>
Expand Down Expand Up @@ -96,6 +101,7 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
}

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

@Override
Expand Down Expand Up @@ -175,4 +181,14 @@ private void validateAndProceed() {
mHandler.fetchProvider(email);
}
}

private void setUpTermsOfService(View view) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would be clearer if it accepted a TextView argument and the TextView was located by onViewCreated

List<AuthUI.IdpConfig> providers = getFlowParams().providerInfo;
if (providers.size() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe add a convenience method to FlowParams that does this logic for you since we'll need it in a few places? Something like flowParams.isSingleProviderFlow()?

PreambleHandler.setup(getContext(),
getFlowParams(),
R.string.fui_tos_and_pp,
view.<TextView>findViewById(R.id.email_tos_and_pp_text));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.RestrictTo;
import android.support.annotation.StringRes;
import android.support.design.widget.TextInputLayout;
import android.text.TextUtils;
import android.view.LayoutInflater;
Expand Down Expand Up @@ -140,11 +141,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 Down Expand Up @@ -173,6 +169,18 @@ public void onViewCreated(@NonNull View view, @Nullable Bundle savedInstanceStat
}
}

private @StringRes int getTermsStringResource() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This method is unused. Did you mean to use or remove it?

boolean hasTos = !TextUtils.isEmpty(getFlowParams().termsOfServiceUrl);
boolean hasPp = !TextUtils.isEmpty(getFlowParams().privacyPolicyUrl);
if(hasTos && hasPp) {
return R.string.fui_create_account_preamble_tos_and_pp;
} else if(hasTos) {
return R.string.fui_create_account_preamble_tos_only;
} else {
return R.string.fui_create_account_preamble_pp_only;
}
}

private void safeRequestFocus(final View v) {
v.post(new Runnable() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package com.firebase.ui.auth.ui.idp;

import android.app.Activity;
import android.arch.lifecycle.ViewModelProvider;
import android.arch.lifecycle.ViewModelProviders;
import android.content.Context;
Expand All @@ -28,6 +29,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 +44,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.ui.PreambleHandler;
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 +109,8 @@ protected void onFailure(@NonNull Exception e) {
}
}
});

setUpTermsOfService();
}

private void populateIdpList(List<IdpConfig> providerConfigs,
Expand Down Expand Up @@ -204,6 +209,14 @@ public void onClick(View view) {
}
}

private void setUpTermsOfService() {
TextView view = (TextView) findViewById(R.id.main_tos_and_pp);
PreambleHandler.setup(AuthMethodPickerActivity.this,
getFlowParams(),
R.string.fui_tos_and_pp,
view);
}

@Override
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
super.onActivityResult(requestCode, resultCode, data);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 @@ -83,7 +82,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,7 +91,6 @@ public View onCreateView(@NonNull LayoutInflater inflater, @Nullable ViewGroup c
setupCountDown(RESEND_WAIT_MILLIS);
setupSubmitConfirmationCodeButton();
setupResendConfirmationCodeTextView(phoneNumber);
setUpTermsOfService();
return v;
}

Expand Down Expand Up @@ -221,13 +218,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
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import android.widget.EditText;
import android.widget.TextView;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.R;
import com.firebase.ui.auth.data.model.CountryInfo;
import com.firebase.ui.auth.data.model.FlowParameters;
Expand All @@ -42,11 +43,14 @@
import com.firebase.ui.auth.util.GoogleApiUtils;
import com.firebase.ui.auth.util.data.PhoneNumberUtils;
import com.firebase.ui.auth.util.ui.ImeHelper;
import com.firebase.ui.auth.util.ui.PreambleHandler;
import com.firebase.ui.auth.viewmodel.RequestCodes;
import com.google.android.gms.auth.api.credentials.Credential;
import com.google.android.gms.auth.api.credentials.CredentialPickerConfig;
import com.google.android.gms.auth.api.credentials.HintRequest;
import com.google.firebase.auth.PhoneAuthProvider;

import java.util.List;
import java.util.Locale;

/**
Expand All @@ -55,7 +59,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 +110,13 @@ 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);
setUpTermsOfService(view);
}

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

private void setUpTermsOfService(View view) {
List<AuthUI.IdpConfig> providers = getFlowParams().providerInfo;
if(providers.size() > 1) {
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);
} else {
PreambleHandler.setup(getContext(),
getFlowParams(),
R.string.fui_verify_phone_number,
R.string.fui_sms_terms_of_service_extended,
view.<TextView>findViewById(R.id.send_sms_tos));
}
}

void showError(String e) {
mPhoneInputLayout.setError(e);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ public class PreambleHandler {
private static final String BTN_TARGET = "%BTN%";
private static final String TOS_TARGET = "%TOS%";
private static final String PP_TARGET = "%PP%";
private static final int NO_BUTTON = -1;

private final Context mContext;
private final FlowParameters mFlowParameters;
Expand All @@ -41,12 +42,30 @@ private PreambleHandler(Context context, FlowParameters parameters, @StringRes i
R.color.fui_linkColor));
}

private PreambleHandler(Context context, FlowParameters parameters) {
mContext = context;
mFlowParameters = parameters;
mButtonText = NO_BUTTON;
mLinkSpan = new ForegroundColorSpan(ContextCompat.getColor(mContext,
R.color.fui_linkColor));
}

public static void setup(Context context,
FlowParameters parameters,
@StringRes int buttonText,
@StringRes int textViewText,
TextView textView) {
PreambleHandler handler = new PreambleHandler(context, parameters, buttonText);
handler.setupCreateAccountPreamble();
handler.setupPreamble(textViewText);
handler.setPreamble(textView);
}

public static void setup(Context context,
FlowParameters parameters,
@StringRes int textViewText,
TextView textView) {
PreambleHandler handler = new PreambleHandler(context, parameters);
handler.setupPreamble(textViewText);
handler.setPreamble(textView);
}

Expand All @@ -55,8 +74,9 @@ private void setPreamble(TextView textView) {
textView.setText(mBuilder);
}

private void setupCreateAccountPreamble() {
String withTargets = getPreambleStringWithTargets();
private void setupPreamble(@StringRes int textViewText) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: don't really like this method name because setupPreamble and setPreamble are so similar.

Copy link
Contributor

Choose a reason for hiding this comment

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

Bump.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

String withTargets = (mButtonText == NO_BUTTON) ? getPreambleStringWithTargetsNoButton(
textViewText) : getPreambleStringWithTargets(textViewText);
if (withTargets == null) {
return;
}
Expand Down Expand Up @@ -92,24 +112,40 @@ private void replaceUrlTarget(String target, @StringRes int replacementRes, Stri
}

@Nullable
private String getPreambleStringWithTargets() {
private String getPreambleStringWithTargets(@StringRes int textViewText) {
boolean hasTos = !TextUtils.isEmpty(mFlowParameters.termsOfServiceUrl);
boolean hasPp = !TextUtils.isEmpty(mFlowParameters.privacyPolicyUrl);
if (hasTos && hasPp) {
return mContext.getString(textViewText,
BTN_TARGET, TOS_TARGET, PP_TARGET);
} else if (hasTos) {
return mContext.getString(textViewText,
BTN_TARGET, TOS_TARGET);
} else if (hasPp) {
return mContext.getString(textViewText,
BTN_TARGET, PP_TARGET);
}
return null;
}

@Nullable
private String getPreambleStringWithTargetsNoButton(@StringRes int textViewText) {
boolean hasTos = !TextUtils.isEmpty(mFlowParameters.termsOfServiceUrl);
boolean hasPp = !TextUtils.isEmpty(mFlowParameters.privacyPolicyUrl);
if (hasTos && hasPp) {
return mContext.getString(R.string.fui_create_account_preamble_tos_and_pp,
BTN_TARGET, TOS_TARGET, PP_TARGET);
return mContext.getString(textViewText,
TOS_TARGET, PP_TARGET);
} else if (hasTos) {
return mContext.getString(R.string.fui_create_account_preamble_tos_only,
BTN_TARGET, TOS_TARGET);
return mContext.getString(textViewText,
TOS_TARGET);
} else if (hasPp) {
return mContext.getString(R.string.fui_create_account_preamble_pp_only,
BTN_TARGET, PP_TARGET);
} else {
return null;
return mContext.getString(textViewText,
PP_TARGET);
}
return null;
}


private class CustomTabsSpan extends ClickableSpan {
private final String mUrl;
private final CustomTabsIntent mCustomTabsIntent;
Expand Down
10 changes: 10 additions & 0 deletions auth/src/main/res/layout-land/fui_auth_method_picker_layout.xml
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,16 @@
android:id="@+id/btn_holder"
style="@style/FirebaseUI.AuthMethodPicker.ButtonHolder" />

<TextView
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe you meant to put this out of the scroll view, right? If not, you can't put more than one view in a ScrollView so we'll need a LL.

android:id="@+id/main_tos_and_pp"
style="@style/FirebaseUI.Text.BodyText"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/fui_field_padding_vert"
android:textColor="?android:textColorTertiary"
android:textIsSelectable="false"
app:layout_constraintTop_toBottomOf="@+id/btn_holder" />

</ScrollView>

</android.support.constraint.ConstraintLayout>
20 changes: 16 additions & 4 deletions auth/src/main/res/layout/fui_auth_method_picker_layout.xml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
android:clipChildren="false"
android:clipToPadding="false">


<ImageView
android:id="@+id/logo"
style="@style/FirebaseUI.AuthMethodPicker.Logo"
Expand All @@ -26,12 +27,23 @@
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintTop_toTopOf="parent"
app:layout_constraintBottom_toBottomOf="parent"
app:layout_constraintVertical_bias="0.9">
app:layout_constraintVertical_bias="0.7">

<LinearLayout
android:id="@+id/btn_holder"
style="@style/FirebaseUI.AuthMethodPicker.ButtonHolder" />

style="@style/FirebaseUI.AuthMethodPicker.ButtonHolder"/>
</ScrollView>

<TextView
android:id="@+id/main_tos_and_pp"
style="@style/FirebaseUI.Text.BodyText"
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/fui_field_padding_vert"
android:textColor="?android:textColorTertiary"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the color change if you remove this? Either way, we need to use some AppCompat version, not the system one. I'll get back to you if the color is wrong when you remove it.

Copy link
Contributor

Choose a reason for hiding this comment

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

My comment above addresses this I think.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to attr/textColorTertiary

android:textIsSelectable="false"
android:gravity="center"
app:layout_constraintTop_toBottomOf="@id/container"/>
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you tested this with all numbers of buttons? IMO it would be more natural for this to be constrained Bottom_toBottomOf="parent" so it's always exactly on the bottom.

Also make sure it looks OK in landscape.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, I'll push these changes soon.

</android.support.constraint.ConstraintLayout>



Loading