-
-
Notifications
You must be signed in to change notification settings - Fork 216
Improve login form #788
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
Improve login form #788
Conversation
| ); | ||
| } | ||
|
|
||
| if (isLoading && !user) return null; |
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.
this caused the login page to re-render on switching tabs which resulted in reset input fields
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.
I think removing this might cause the login screen to "flash" every refresh, since there will be a render where user has not loaded yet but the user has localstorage token
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.
Yeah I just verified this behavior from this change
|
|
||
| if (isLoading && !user) return null; | ||
|
|
||
| if (!user || error) return <Login />; |
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.
This suggestion should fix
| if (!user || error) return <Login />; | |
| if (error) return <Login />; | |
| if (!user) return null; |
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.
Overall it looks good, I want to include it in 1.19.2 so I'm just going to make and make the small change I suggested myself and it still should fix that issue
* 1.19.2-dev-0 * deploy 1.19.2-dev-1 * Add option to make run command detachable (#766) * improve missing files log to include the missing paths * bump mungos for urlencoding mongo creds * Update permissioning.md - typo: "priviledges" -> "privileges" (#770) * Add support for monaco-yaml and docker compose spec validatiaon (#772) * deploy 1.19.2-dev-2 * on delete user, remove from all user groups * fix Google login issues around `picture` * unsafe_unsanitized_startup_config * improve git provider support re #355 * should fix #468 * should fix exit code re #597 * deploy 1.19.2-dev-3 * fix container ports sorting (#776) * missing serde default * deploy 1.19.2-dev-4 * ensure git tokens trimmed in remote url * Add link to Authentik support docs * Fix incorrect commit branch when using linked repo re #634 * Better display container port ranges (#786) * ensure build and sync also commit to correct branch. re #634 * deploy 1.19.2-dev-5 * Improve login form (#788) * Use proper form for login, add autocomplete and names to input fields * Do not return null if loading * Remove unused function * Cleanup and streamline * improve login screen flash on reload * first builder given same name as first server * 1.19.2 --------- Co-authored-by: mbecker20 <[email protected]> Co-authored-by: Brian Bradley <[email protected]> Co-authored-by: Ravi Wolter-Krishan <[email protected]> Co-authored-by: Christopher Hoage <[email protected]> Co-authored-by: jack <[email protected]>
Solves #403
and maybe #652 but I couldn't reproduce everything from that issue so unsure. I tested with iOS, safari and bitwarden, protonpass
this PR improves the login page form implementation. Main changes are replacing local state for credentials with native form submission and improving accessability. I also looked at https://www.chromium.org/developers/design-documents/create-amazing-password-forms/ for some tips.
also removed the unused
WithLoadingcomponent fromutil.tsx(i couldnt find any other references of it)