Refactor login/registration API

[nbarbettini]

Based on today's standup:

• [ ] Remove showRegistration, rename existing showLogin() to show()

[ ] Add a higher-level component/controller to manage the state of the entire widget (should consume LoginComponent, RegistrationComponent, etc) (out of scope)

[robertjd]

I just came across an issue with this. A story we want to support is: if I visit /register on a framework integration, I want to have the widget load into a registration-specific mode.

Some solutions:

1. Leave showRegistration([renderTo]) for exactly this purposes. It should construct only the registration form, and the framework will define a container to put it in.

2. Be more explicit. Remove showRegistration() and add showRegistrationForm(container), which requires a container to be defined.

3. Don't support this story, remove the /register view from the frameworks, and have /login show the widget, and the widget gives the user the ability to register

[nbarbettini]

Of those options, I think 1 makes the most sense. (3 is tempting, but would be more breaking changes for the frameworks.)

We could go with 1 and make it so calling showRegistration is the same thing as calling showLogin and then clicking on the "Sign in" tab or whatever it ends up being.

[lhazlewood]

What about show({view: "register"})? This reflects the show method rename, but they can provide context to that method if they need to default to a different starting view for route-specific scenarios.

Thoughts?

[lhazlewood]

FWIW, I think we'll always have to support route-specific scenarios - marketing teams will often have people land directly on the /register page via marketing campaigns, etc. Being able to start the widget in a relevant context seems pretty necessary...

[lhazlewood]

Oh, don't forget /change too - it's often necessary to land on that url directly from password reset emails. So the 'load but show this context first' seems like a 'requirement' unless I'm missing something.

[nbarbettini]

Oh, don't forget /change too - it's often necessary to land on that url directly from password reset emails.

Agreed, that should be out of scope of this change. Based on what we've discussed so far there will be a different method for that.

For context @lhazlewood: we were discussing that showLogin() and showRegistration() (our original design) had some overlap, especially when talking about social login where those things are basically identical. We thought maybe we could reduce it to one show() method, but @robertjd brought up a good point above.

IMHO, I think the cleanest design might be:

• show() shows the default view
• showRegistration() displays and jumps to the registration view
• showForgot() displays and jumps to the forgot password view etc.

[typerandom]

I think it's better to be explicit here. If we want to have show(), I still think we should keep showLogin. I.e:

• show(): Shows login/registration tabbed.
• showLogin(): Shows login form only.
• showRegistration(): Shows registration form only.

[lhazlewood]

Out of curiosity, why do you favor multiple show methods instead of one that receives a context object? I'm just trying to learn your preferences here - I've no strong opinion either way. I thought JS developers loved context objects ;)

[typerandom]

@lhazlewood Haha many do. From a pure DX perspective, I think it's easier to understand what showLogin/showRegistration does by reading their names, instead of what show does. Even though documentation is crucial, I'm all for the "code is documentation" approach. I.e. APIs should be so simple that you'd just be able to look at them and understand what they do and get a "feel" of how to use them.

[robertjd]

jQuery plugin creators like magic objects that do all the things :)

Decided this morning: explicit function names with a single hash parameter (cuz we still need some magic) so usage is now like this:

showLogin() showRegistration() showRegistration({ container: <dom node reference>}) showEmailVerification({ sptoken: 'foo' })

[typerandom]

During todays meeting we actually decided to abolish the show API and just go with showLogin and showRegistration.

[nbarbettini]

+++1 for code as documentation. Signed, a sometimes-JS guy. <3

[nbarbettini]

Based on our discussion, this ends up being a wont-fix.