Skip to content

Fix small issues left in version 4.2.1 #1495

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

Merged
merged 9 commits into from
Oct 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 6 additions & 0 deletions auth-github/build.gradle.kts
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
android {
lintOptions {
disable("UnknownNullness") // TODO fix in future PR
}
}

dependencies {
compileOnly(project(":auth")) { isTransitive = false }
compileOnly(Config.Libs.Firebase.auth) { isTransitive = false }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,13 @@
* Array adapter used to display a list of countries with section indices.
*/
final class CountryListAdapter extends ArrayAdapter<CountryInfo> implements SectionIndexer {

// Map from first letter --> position in the list
private final HashMap<String, Integer> alphaIndex = new LinkedHashMap<>();

// Map from display name --> position in the list
private final HashMap<String, Integer> countryPosition = new LinkedHashMap<>();

private String[] sections;

public CountryListAdapter(Context context) {
Expand Down Expand Up @@ -67,6 +72,11 @@ public void setData(List<CountryInfo> countries) {
notifyDataSetChanged();
}

@Override
public int getCount() {
return countryPosition.size();
}

@Override
public Object[] getSections() {
return sections;
Expand All @@ -92,6 +102,16 @@ public int getPositionForSection(int index) {

@Override
public int getSectionForPosition(int position) {
if (sections == null) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since we were using "fast scrolling" this method had to be implemented to get the scroll bar to work correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

I tested this, and it seems a bit jumpy. If you scroll all the way down, and go back up, it immediately jumps to a quarter of the way up. If you can fix that, it'd be perfect. Otherwise, still a big improvement over what we had.

Copy link
Contributor

Choose a reason for hiding this comment

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

The scrollbar also doesn't show up unless we start scrolling - is that the intended behavior?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah won't show up till you start scrolling, that's normal. There's nothing I can figure out to fix the jumpiness, will maybe look into it again later if people report it.

return 0;
}

for (int i = 0; i < sections.length; i++) {
if (getPositionForSection(i) > position) {
return i - 1;
}
}

return 0;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<?xml version="1.0" encoding="utf-8"?>
<layer-list xmlns:android="http://schemas.android.com/apk/res/android">
<item>
<shape android:shape="rectangle">
<corners android:radius="3dp" />
<solid android:color="@color/fui_bgAnonymous" />
</shape>
</item>
</layer-list>
6 changes: 2 additions & 4 deletions auth/src/main/res/drawable/fui_ic_anonymous_white_24dp.xml
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
<vector
xmlns:android="http://schemas.android.com/apk/res/android"
xmlns:tools="http://schemas.android.com/tools"
android:width="24dp"
android:height="24dp"
android:viewportHeight="24.0"
android:viewportWidth="24.0"
tools:ignore="InvalidVectorPath">
android:viewportWidth="24.0">
<path
android:fillColor="#FFFFFF"
android:pathData="M12 5.9c1.16 0 2.1.94 2.1 2.1s-0.94 2.1-2.1 2.1S9.9 9.16 9.9 8s0.94-2.1 2.1-2.1m0 9c2.97 0 6.1 1.46 6.1 2.1v1.1H5.9V17c0-0.64 3.13-2.1 6.1-2.1M12 4C9.79 4 8 5.79 8 8s1.79 4 4 4 4-1.79 4-4-1.79-4-4-4zm0 9c-2.67 0-8 1.34-8 4v3h16v-3c0-2.66-5.33-4-8-4z" />
android:pathData="M12,5.9c1.16,0 2.1,0.94 2.1,2.1s-0.94,2.1 -2.1,2.1S9.9,9.16 9.9,8s0.94,-2.1 2.1,-2.1m0,9c2.97,0 6.1,1.46 6.1,2.1v1.1L5.9,18.1L5.9,17c0,-0.64 3.13,-2.1 6.1,-2.1M12,4C9.79,4 8,5.79 8,8s1.79,4 4,4 4,-1.79 4,-4 -1.79,-4 -4,-4zM12,13c-2.67,0 -8,1.34 -8,4v3h16v-3c0,-2.66 -5.33,-4 -8,-4z" />
</vector>
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,8 @@
android:layout_height="wrap_content"
android:layout_marginTop="@dimen/fui_field_padding_vert"
android:text="@string/fui_trouble_signing_in"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintEnd_toStartOf="@+id/button_done"
app:layout_constraintStart_toStartOf="parent"
app:layout_constraintTop_toBottomOf="@+id/password_layout" />

<Button
Expand All @@ -75,7 +75,8 @@
android:id="@+id/email_footer_tos_and_pp_text"
style="@style/FirebaseUI.PrivacyFooter"
app:layout_constraintTop_toBottomOf="@+id/button_done"
app:layout_constraintBottom_toBottomOf="parent" />
app:layout_constraintBottom_toBottomOf="parent"
tools:text="Terms of Service and Privacy Policy" />

</android.support.constraint.ConstraintLayout>

Expand Down
2 changes: 2 additions & 0 deletions auth/src/main/res/values/dimens.xml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@
<dimen name="fui_button_inset_bottom">3dp</dimen>
<dimen name="fui_button_inset_left">1dp</dimen>
<dimen name="fui_button_inset_right">2dp</dimen>

<dimen name="fui_min_height_target">48dp</dimen>
</resources>
5 changes: 4 additions & 1 deletion auth/src/main/res/values/styles.xml
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,8 @@
</style>

<style name="FirebaseUI.Text.Link">
<item name="android:minHeight">@dimen/fui_min_height_target</item>
<item name="android:gravity">center_vertical</item>
<item name="android:textColor">@color/fui_linkColor</item>
</style>

Expand Down Expand Up @@ -256,7 +258,8 @@
<item name="android:layout_height">wrap_content</item>
<item name="android:layout_marginTop">@dimen/fui_field_padding_vert</item>
<item name="android:layout_marginBottom">@dimen/fui_field_padding_vert</item>
<item name="android:gravity">end</item>
<item name="android:minHeight">@dimen/fui_min_height_target</item>
<item name="android:gravity">end|center_vertical</item>
<item name="android:textIsSelectable">false</item>
</style>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import android.content.Context;
import android.content.res.Resources;
import android.util.Log;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.AuthUI.IdpConfig;
Expand All @@ -34,6 +35,7 @@

import org.robolectric.RuntimeEnvironment;

import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
Expand All @@ -44,13 +46,17 @@
import static org.mockito.Mockito.when;

public final class TestHelper {

private static final String TAG = "TestHelper";
private static final String DEFAULT_APP_NAME = "[DEFAULT]";

public static final FirebaseApp MOCK_APP;

static {
FirebaseApp app = mock(FirebaseApp.class);
when(app.get(eq(FirebaseAuth.class))).thenReturn(mock(FirebaseAuth.class));
when(app.getApplicationContext()).thenReturn(RuntimeEnvironment.application);
when(app.getName()).thenReturn(FirebaseApp.DEFAULT_APP_NAME);
when(app.getName()).thenReturn(DEFAULT_APP_NAME);
MOCK_APP = app;
}

Expand Down Expand Up @@ -134,7 +140,7 @@ public static FlowParameters getFlowParameters(Collection<String> providerIds,
}
}
return new FlowParameters(
FirebaseApp.DEFAULT_APP_NAME,
DEFAULT_APP_NAME,
idpConfigs,
AuthUI.getDefaultTheme(),
AuthUI.NO_LOGO,
Expand All @@ -146,5 +152,40 @@ public static FlowParameters getFlowParameters(Collection<String> providerIds,
false);
}

/**
* Set a private, obfuscated field of an object.
* @param obj the object to modify.
* @param objClass the object's class.
* @param fieldClass the class of the target field.
* @param fieldValue the value to use for the field.
*/
public static <T, F> void setPrivateField(
T obj,
Class<T> objClass,
Class<F> fieldClass,
F fieldValue) {

Field targetField = null;
Field[] classFields = objClass.getDeclaredFields();
for (Field field : classFields) {
if (field.getType().equals(fieldClass)) {
if (targetField != null) {
throw new IllegalStateException("Class " + objClass + " has multiple fields of type " + fieldClass);
}

targetField = field;
}
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: extra space & I would name f "field" instead

if (targetField == null) {
throw new IllegalStateException("Class " + objClass + " has no fields of type " + fieldClass);
}

targetField.setAccessible(true);
try {
targetField.set(obj, fieldValue);
} catch (IllegalAccessException e) {
Log.w(TAG, "Error setting field", e);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import com.firebase.ui.auth.testhelpers.TestConstants;
import com.firebase.ui.auth.testhelpers.TestHelper;
import com.firebase.ui.auth.viewmodel.phone.PhoneProviderResponseHandler;
import com.google.firebase.auth.AuthCredential;
import com.google.firebase.auth.AuthResult;
import com.google.firebase.auth.FirebaseAuth;
import com.google.firebase.auth.FirebaseAuthUserCollisionException;
Expand Down Expand Up @@ -104,9 +105,13 @@ public void testSignIn_autoUpgradeAnonymousEnabledWithExistingUser_expectMergeFa
mHandler.getOperation().observeForever(mResponseObserver);
setupAnonymousUpgrade();

FirebaseAuthUserCollisionException ex =
new FirebaseAuthUserCollisionException("foo", "bar");
TestHelper.setPrivateField(ex, FirebaseAuthUserCollisionException.class,
AuthCredential.class, mCredential);

when(mMockAuth.getCurrentUser().linkWithCredential(mCredential))
.thenReturn(AutoCompleteTask.<AuthResult>forFailure(
new FirebaseAuthUserCollisionException("foo", "bar", mCredential)));
.thenReturn(AutoCompleteTask.<AuthResult>forFailure(ex));

IdpResponse response = new IdpResponse.Builder(new User.Builder(
PhoneAuthProvider.PROVIDER_ID, TestConstants.EMAIL).build())
Expand Down
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// This empty file along with settings.gradle help Android Studio recognize the project
// as a gradle project, despite the use of .gradle.kts scripts.
16 changes: 8 additions & 8 deletions buildSrc/src/main/kotlin/Config.kt
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ object Config {
}

object Plugins {
const val android = "com.android.tools.build:gradle:3.2.0"
const val android = "com.android.tools.build:gradle:3.2.1"
const val kotlin = "org.jetbrains.kotlin:kotlin-gradle-plugin:$kotlinVersion"
const val google = "com.google.gms:google-services:4.0.2"

Expand All @@ -36,7 +36,7 @@ object Config {
const val cardView = "com.android.support:cardview-v7:$version"
const val customTabs = "com.android.support:customtabs:$version"

const val constraint = "com.android.support.constraint:constraint-layout:1.1.2"
const val constraint = "com.android.support.constraint:constraint-layout:1.1.3"
}

object Arch {
Expand All @@ -51,15 +51,15 @@ object Config {
}

object Firebase {
const val core = "com.google.firebase:firebase-core:16.0.3"
const val auth = "com.google.firebase:firebase-auth:16.0.3"
const val firestore = "com.google.firebase:firebase-firestore:17.1.0"
const val core = "com.google.firebase:firebase-core:16.0.4"
const val auth = "com.google.firebase:firebase-auth:16.0.5"
const val firestore = "com.google.firebase:firebase-firestore:17.1.1"
const val database = "com.google.firebase:firebase-database:16.0.3"
const val storage = "com.google.firebase:firebase-storage:16.0.2"
const val storage = "com.google.firebase:firebase-storage:16.0.3"
}

object PlayServices {
const val auth = "com.google.android.gms:play-services-auth:16.0.0"
const val auth = "com.google.android.gms:play-services-auth:16.0.1"
}


Expand Down Expand Up @@ -106,7 +106,7 @@ object Config {
}

object Lint {
private const val version = "26.2.0-alpha17"
private const val version = "26.2.1"

const val api = "com.android.tools.lint:lint-api:$version"
const val tests = "com.android.tools.lint:lint-tests:$version"
Expand Down
2 changes: 2 additions & 0 deletions settings.gradle.kts → settings.gradle
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
rootProject.buildFileName = 'build.gradle.kts'

include(
":app", ":library",

Expand Down