Skip to content
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

♻️ refactor: Login and Registration component Improvement #2716

Merged
merged 15 commits into from
May 28, 2024

Conversation

ohneda
Copy link
Contributor

@ohneda ohneda commented May 15, 2024

Summary

I've made some improvements to the Login and Registration components.

  1. Previously, the Login and Registration displayed a blank screen while fetching StartupConfig data from the API. I've added a blinking logo that appears while the forms wait for the API to return theStartupConfig data. No worries, if you don't like this effect, you can simply remove the BlinkAnimation wrapper.
1.mov
  1. I've also implemented error messaging to inform users whenever they fail to access the API during the login or registration processes.

image

  1. Additionally, I've optimized the Login component only to make one API call for StartupConfig, even if the component re-renders after a failed login attempt.
  2. Previously, if a user failed to log in, then successfully logged in, and then logged out, the error message would still be displayed. I've made sure that the error message is cleared after a successful login.
  3. I've refactored the code to reduce duplication between the login and registration components.

I initially planned to make just a small improvement, but it turned into a bit of a larger change. However, the functionality of the Login and Registration components remains the same.

Please let me know if you have any concerns. I hope these changes can help improve the user experience, even if just a little bit.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • Local unit tests pass with my changes

* display error message when API is down
* add loading animation to Login form while fetching data
* optimize startupConfig to fetch data only on initial render to prevent unnecessary API calls
@danny-avila
Copy link
Owner

Awesome, looks great. Thank you for consolidating the logic, too. I'll be testing this soon.

@danny-avila
Copy link
Owner

@ohneda can you make me a contributor? I made some changes so we can optimize this a bit further. If you add me, I can avoid closing this PR and pushing to your branch.

Also could use your help on aligning request/reset password pages with the changes I made, which will make sense once you see them

@ohneda
Copy link
Contributor Author

ohneda commented May 22, 2024

@danny-avila Thanks, I have added you as a contributor to my repository.

@danny-avila
Copy link
Owner

Was hoping to get to this today along with a few other PRs but ran out of time. Will try to be back on it Sunday/Monday

@ohneda
Copy link
Contributor Author

ohneda commented May 24, 2024

Thanks for letting me know. All my PR are not urgent, so no need to rush

@danny-avila
Copy link
Owner

@ohneda my changes are all wrapped up, can you confirm it's all working as intended on your end?

@ohneda
Copy link
Contributor Author

ohneda commented May 27, 2024

@danny-avila Thanks, Everything is working as intended and it's become more clear! I see that the request/reset password pages have been updated too. Is there anything else I need to work on?

@danny-avila
Copy link
Owner

@danny-avila Thanks, Everything is working as intended and it's become more clear! I see that the request/reset password pages have been updated too. Is there anything else I need to work on?

Thanks for checking!

@danny-avila danny-avila merged commit 9f2538f into danny-avila:main May 28, 2024
2 checks passed
@ohneda ohneda deleted the refactor/login-form-improvement branch May 31, 2024 07:19
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.

None yet

2 participants