Skip to content

Handle dynamic list of identity providers #1799

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

Closed
robapaul88 wants to merge 3 commits into from
Closed

Handle dynamic list of identity providers #1799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6de2d73
Update README.md (#1769)
trietbui85 Mar 27, 2020
6d41fb2
Fix tiny typo: afforted -> afforded (#1785)
puf May 26, 2020
81875b8
Handle dynamic list of identity providers
robapaul88 Jun 22, 2020
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
2 changes: 1 addition & 1 deletion auth/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ dependencies {
implementation 'com.firebaseui:firebase-ui-auth:6.2.1'

// Required only if Facebook login support is required
// Find the latest Facebook SDK releases here: https://goo.gl/Ce5L94
// Find the latest Facebook SDK releases here: https://github.com/facebook/facebook-android-sdk/blob/master/CHANGELOG.md
implementation 'com.facebook.android:facebook-login:4.x'
}
```
Expand Down
23 changes: 20 additions & 3 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 @@ -51,7 +51,6 @@
import com.google.firebase.auth.EmailAuthProvider;
import com.google.firebase.auth.FacebookAuthProvider;
import com.google.firebase.auth.FirebaseAuth;
import com.google.firebase.auth.GithubAuthProvider;
import com.google.firebase.auth.GoogleAuthProvider;
import com.google.firebase.auth.PhoneAuthProvider;

Expand Down Expand Up @@ -229,14 +228,32 @@ private void populateIdpListCustomLayout(List<IdpConfig> providerConfigs) {
for (IdpConfig idpConfig : providerConfigs) {
final String providerId = providerOrEmailLinkProvider(idpConfig.getProviderId());

if (!providerButtonIds.containsKey(providerId)) {
Integer buttonResId = providerButtonIds.get(providerId);
if (buttonResId == null) {
throw new IllegalStateException("No button found for auth provider: " + idpConfig.getProviderId());
}

@IdRes int buttonId = providerButtonIds.get(providerId);
@IdRes int buttonId = buttonResId;
View loginButton = findViewById(buttonId);
handleSignInOperation(idpConfig, loginButton);
}
//hide custom layout buttons that don't have their identity provider set
for (String providerBtnId : providerButtonIds.keySet()) {
if (providerBtnId == null) continue;
Copy link
Contributor

@samtstern samtstern Jun 22, 2020

Choose a reason for hiding this comment

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

nit: we try not to use single-line if statements

Suggested change
if (providerBtnId == null) continue;
if (providerBtnId == null) {
continue;
}

Copy link
Contributor Author

@robapaul88 robapaul88 Jun 23, 2020

Choose a reason for hiding this comment

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

Done.

boolean hasProvider = false;
for (IdpConfig idpConfig : providerConfigs) {
if (providerBtnId.equals(idpConfig.getProviderId())) {
hasProvider = true;
break;
}
}
if (!hasProvider) {
Integer resId = providerButtonIds.get(providerBtnId);
if (resId == null) continue;
Copy link
Contributor

@samtstern samtstern Jun 22, 2020

Choose a reason for hiding this comment

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

Suggested change
if (resId == null) continue;
if (resId == null) {
continue;
}

Copy link
Contributor Author

@robapaul88 robapaul88 Jun 23, 2020

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@samtstern samtstern Jun 22, 2020

Choose a reason for hiding this comment

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

Is this null condition even possible? How would we get into this state?

Copy link
Contributor Author

@robapaul88 robapaul88 Jun 23, 2020

Choose a reason for hiding this comment

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

Well, it's an unboxing, In theory I know that map should only have provider ids and layout res ids. But since that Map is passed around by reference, placing an Integer in it that's later nullified, you can end up with a null here. Better safe than sorry.

@IdRes int buttonId = resId;
findViewById(buttonId).setVisibility(View.GONE);
}
}
}

@NonNull
Expand Down
2 changes: 1 addition & 1 deletion firestore/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ FirestoreRecyclerAdapter adapter = new FirestoreRecyclerAdapter<Chat, ChatHolder

The `FirestorePagingAdapter` binds a `Query` to a `RecyclerView` by loading documents in pages.
This results in a time and memory efficient binding, however it gives up the real-time events
afforted by the `FirestoreRecyclerAdapter`.
afforded by the `FirestoreRecyclerAdapter`.

The `FirestorePagingAdapter` is built on top of the [Android Paging Support Library][paging-support].
Before using the adapter in your application, you must add a dependency on the support library:
Expand Down