-
Notifications
You must be signed in to change notification settings - Fork 26
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
Client login service #2678
base: development
Are you sure you want to change the base?
Client login service #2678
Conversation
…clientLoginService
…clientLoginService
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, should make a ticket to remove all the old SignIn/SignUp models once this is tested
|
src/foam/nanos/auth/login/SignIn.js
Outdated
required: true, | ||
validationTextVisible: false, | ||
label: 'Email or Username', | ||
preSet: function(_, n) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't you just say trim: true ?
src/foam/nanos/auth/login/SignUp.js
Outdated
inputValidation: /\S+@\S+\.\S+/, | ||
restrictedCharacters: /^[^\s]$/, | ||
displayMode: foam.u2.DisplayMode.RW | ||
}; | ||
}, | ||
validateObj: function(email) { | ||
validateObj: function(email, emailAvailable) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we make these validationPredicates instead? For all of them?
}, | ||
{ | ||
name: 'wizardVerifyEmail', | ||
code: async function(x, email, username, password) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we just use this verifyEmail flow across the board? Dont see why we need to maintain two of them
move signin signup logic to client login service