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

Add use load flag + isLoading status #1512

Merged
merged 6 commits into from
Jan 10, 2023
Merged

Add use load flag + isLoading status #1512

merged 6 commits into from
Jan 10, 2023

Conversation

timarney
Copy link
Member

@timarney timarney commented Jan 10, 2023

Summary | Résumé

  • Updates swr to latest package version
  • Updates useFlag to useLoadFlag which allows us to check the loading state for the API call

Prior to adding the isLoading variable the pages would briefly show the content as if the flag was off i.e. Registration is closed at this time.

With the updated flag check we can show a spinner prior to getting the status back.

Before After
https://user-images.githubusercontent.com/62242/211567350-05a3b08a-b012-46e9-ba65-6423dcdadfde.mp4 https://user-images.githubusercontent.com/62242/211567421-3ea5fc06-6e7f-4c93-846a-467c89051c8e.mp4

Test instructions | Instructions pour tester la modification

  • Visit /signup/register --- watch the loading progress

Unresolved questions / Out of scope | Questions non résolues ou hors sujet

@bryan-robitaille if we're happy with this approached I will update the other files that are using useFlag. I also opted not to pass the suspense true option for now ... some interesting discussion here: vercel/swr#1906

Pull Request Checklist

Please complete the following items in the checklist before you request a review:

  • Have you completely tested the functionality of change introduced in this PR? Is the PR solving the problem it's meant to solve within the scope of the related issue?
  • The PR does not introduce any new issues such as failed tests, console warnings or new bugs.
  • If this PR adds a package have you ensured its licensed correctly and does not add additional security issues?
  • Is the code clean, readable and maintainable? Is it easy to understand and comprehend.
  • Does your code have adequate comprehensible comments? Do new functions have docstrings?
  • Have you modified the change log and updated any relevant documentation?
  • Is there adequate test coverage? Both unit tests and end-to-end tests where applicable?
  • If your PR is touching any UI is it accessible? Have you tested it with a screen reader? Have you tested it with automated testing tools such as axe?

@github-actions
Copy link
Contributor

@timarney timarney marked this pull request as ready for review January 10, 2023 14:02
@bryan-robitaille
Copy link
Contributor

👍 I love the approach.

Copy link
Contributor

@craigzour craigzour left a comment

Choose a reason for hiding this comment

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

Nice work :)

@timarney
Copy link
Member Author

Will merge this and create a follow-up PR to update the remaining files that useFlag

@timarney timarney merged commit 2bd0d8c into develop Jan 10, 2023
@timarney timarney deleted the use-load-flag branch January 10, 2023 15:43
@timarney timarney mentioned this pull request Jan 11, 2023
8 tasks
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

3 participants