-
Notifications
You must be signed in to change notification settings - Fork 887
feat(site): add descriptions for each auth method to the selection menu #9252
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
Conversation
"password", | ||
form.values.login_type === "password" | ||
? "" | ||
: "No password required for this login type", |
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.
If the password is not required, why not hide this field?
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.
jitter, mostly. on some level, seeing another field after the auth method prepares the user for "there is another potential step" even if it's disabled. if it's entirely hidden, they might just jump straight to the submit button.
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.
Nit: I would do this a bit differently.
Maybe we should make the authMethods
required and make the parent display the loading state if it is not loaded since this field is required. So we could do something like:
const availableMethods = authMethods.filter(method => method.enabled).concat("none")
<TextField>
{availableMethods.map(method => {
const language = authMethodLanguage[method.id]
return (
<MenuItem key={value} id={"item-" + value} value={value}>
<Stack spacing={0} maxWidth={400}>
{language.displayName}
<span className={styles.labelDescription}>
{language.description}
</span>
</Stack>
</MenuItem>
)
})}
</TextField>
that's probably nicer than doing the weird not-quite-a-component stuff. I'll do a quick refactor. |
Mostly motivated by the fact that just saying "None" was really cryptic and confusing.