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

#261: Improve Cold Start requests by adding timeout and blocking user input #264

Merged
merged 7 commits into from
Oct 24, 2024
Merged

#261: Improve Cold Start requests by adding timeout and blocking user input #264

merged 7 commits into from
Oct 24, 2024

Conversation

gigigimay
Copy link
Contributor

@gigigimay gigigimay commented Oct 18, 2024

Description

Improve Cold Start requests by adding timeout and blocking user input in login and register page

Commit type

  • feat: indicates the addition of a new feature or functionality to the project.

Issue

#261

Solution

Proposed Changes

I made these changes in these requests: login, auto login, register:

  • show loading circle and disable form inputs and buttons
  • handle login + auto login + register request timeout by showing an error popup and cancelling the http request
  • the cold start timeout duration (5 seconds) was also moved to global config

Potential Impact

--

Screenshots

(in the screen record, I changed the timeout to 2 secs to make it faster)
ezgif-2-394271b5a1

Additional Tasks

--

Assigned

@AntonioMrtz

@AntonioMrtz
Copy link
Owner

AntonioMrtz commented Oct 18, 2024

Hi @gigigimay , thanks for your time. I will review this PR ASAP :). I runned the pipelines and saw ESlint Check / run-frontend-lint failing, can you take a look?

@gigigimay
Copy link
Contributor Author

Thanks, I have fixed the failed lint.

Copy link
Owner

@AntonioMrtz AntonioMrtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have tests that validate the new feature behaviour?

Electron/src/pages/StartMenu/RegisterMenu.tsx Outdated Show resolved Hide resolved
Electron/src/pages/StartMenu/StartMenu.tsx Show resolved Hide resolved
@gigigimay gigigimay requested a review from AntonioMrtz October 18, 2024 15:27
@gigigimay
Copy link
Contributor Author

Unit tests added. It would be easier to review while ignoring white-spaces since I wrapped all the tests in RegisterMenu.test.tsx into a describe function.

@gigigimay
Copy link
Contributor Author

Hi @AntonioMrtz , All the requested changes were done. (Unit tests and moving loading into the buttons) Please let me know if there's anything else I should change. :)

Copy link
Owner

@AntonioMrtz AntonioMrtz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only this little adjustment :). Code looks nice, this has to be one of the best PRs I have received in the project, I enjoyed checking and testing the changes as well as the implementation. Don't worry about the pipeline, the failing one has a dependency on a enviroment variable provided to the github repo, I will get it solve soon

Electron/src/pages/StartMenu/startMenu.module.css Outdated Show resolved Hide resolved
@gigigimay gigigimay requested a review from AntonioMrtz October 24, 2024 05:08
@gigigimay
Copy link
Contributor Author

gigigimay commented Oct 24, 2024

@AntonioMrtz I'm glad you enjoyed reviewing my PR. I'm participating in the Hacktoberfest event, and for this PR to count, could you please label it as 'hacktoberfest-accepted' or add the 'hacktoberfest' topic to this repository? You can find more details about the rules here: https://hacktoberfest.com/participation/. Thank you very much! :)

@AntonioMrtz
Copy link
Owner

@AntonioMrtz I'm glad you enjoyed reviewing my PR. I'm participating in the Hacktoberfest event, and for this PR to count, could you please label it as 'hacktoberfest-accepted' or add the 'hacktoberfest' topic to this repository? You can find more details about the rules here: https://hacktoberfest.com/participation/. Thank you very much! :)

Yeah, I was planning on adding the hacktoberfest-accepted tag :). I hope you enjoyed your experience colaborating in our project, there's also more open issues with the hacktoberfest tag in the repo, take a look at them if you we're thinking about contributing :). Let my know if you need anything

@AntonioMrtz AntonioMrtz merged commit 93bf0d4 into AntonioMrtz:master Oct 24, 2024
@gigigimay gigigimay deleted the feat/261-Improve-Cold-Start-requests-by-adding-timeout-and-blocking-user-input branch October 29, 2024 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve Cold Start requests by adding timeout and blocking user input
2 participants