Skip to content

Conversation

yos1p
Copy link
Contributor

@yos1p yos1p commented Oct 22, 2018

I'm trying to propose API (and later, implementation) for issue #1477

My idea is:

  • Defining the layout with AuthLayout, where developers can set up main layout, and add button IDs for each provider if needed.
  • Show the Layout, and use OnClickListener on those button ID to direct the app to sign-in with selected Auth provider.

What do you guys think?

@yos1p yos1p requested a review from samtstern as a code owner October 22, 2018 07:10
Copy link
Contributor

@samtstern samtstern left a comment

Choose a reason for hiding this comment

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

@iamyaoxi thanks so much for this! Functionally it looks great, I just have some minor API comments.

Also could you change the target branch to version-4.3.0-dev? That's the next feature release. If you need any help with conflicts that come up just let me know, I am happy to deal with that.

Bundle buttonsBundle = in.readBundle(getClass().getClassLoader());
this.providersButton = new HashMap<>();

for (String key : buttonsBundle.keySet()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep a static list somewhere of the accepted String keys here? Maybe it's the same as @SupportedProvider. I just want to make sure we're validating data.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think if we just use AuthUI.SUPPORTED_PROVIDERS to validate data?

@yos1p yos1p changed the base branch from master to version-4.3.0-dev October 22, 2018 11:21
@yos1p
Copy link
Contributor Author

yos1p commented Oct 23, 2018

In the latest commit, I separated sign-in handler to a different function, to reduce duplicated code between populateIdpList and populateIdpListCustomLayout.

I'm still thinking there might be more efficient or safer than this, rather than just to make sure it works. Any suggestions? @samtstern @lsirac @SUPERCILEX

@samtstern
Copy link
Contributor

@iamyaoxi let me know if you need help with any of these conflicts!

@yos1p
Copy link
Contributor Author

yos1p commented Oct 27, 2018

@samtstern Oh, turns out I can!

@samtstern
Copy link
Contributor

@iamyaoxi thanks for handling that! I will hopefully be able to merge this soon, but it may take a few days due to the Firebase Summit.

Very excited about this feature!

@samtstern
Copy link
Contributor

@iamyaoxi thanks for your patience! Took a final look at this and it's great. I will probably follow this change with some small tweaks to the sample app, but this will launch in 4.3.0.

@samtstern samtstern merged commit eb031c7 into firebase:version-4.3.0-dev Oct 31, 2018
@samtstern samtstern added this to the 4.3.0 milestone Oct 31, 2018
@yajur96
Copy link

yajur96 commented Sep 6, 2019

Hi @samtstern @iamyaoxi How do we implement this in Android? Can't see documentation anywhere.

@samtstern
Copy link
Contributor

@yajur96 here are the docs about using custom layouts:
https://github.com/firebase/FirebaseUI-Android/tree/master/auth#custom-layout

The sample app in the app folder of this repo also shows an example.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants