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

ui: Suspenseful loading #3440

Merged
merged 20 commits into from
Nov 14, 2023
Merged

ui: Suspenseful loading #3440

merged 20 commits into from
Nov 14, 2023

Conversation

mastercactapus
Copy link
Member

Description:
Updates the UI to enable suspense for all urql queries.

As components are migrated from apollo to urql, they will automatically get tracked by suspense.

Notably, boundaries were added:

  • Around the entire app for the Login switch, with a new RequireAuth component (instead of the redux mess)
  • Around the title bar text
  • Around the page content
  • Around groups of dialog boxes

The QuerySelect tool explicitly turns off suspenseful loading as it handles loading state internally and shouldn't trigger a page-level (or dialog-level) spin.

Which issue(s) this PR fixes:
Part of #2557

Out of Scope:

  • apollo queries will not trigger Suspense, so some pages still aren't as nice as they will be
  • fetching/isLoading logic will still need to be removed/cleaned up separately
  • Cleaning up redux usage around auth detection

Screenshots:

suspense-demo-3.mp4

Describe any introduced user-facing changes:

  • Application-wide spinner will resolve once config has been loaded, or the Login page is ready
  • Dialogs will not appear until their data has loaded (urql only)
  • Title bar will be blank while the name is fetched, if missing (no "User" -> "Abigail" flicker)
  • Pages using urql (and any nested components doing so) will spin until all content is loaded

Describe any introduced API changes:
N/A

Additional Info:
With suspense enabled, a useQuery will never return fetching: true you will always get data or error.

@mastercactapus mastercactapus marked this pull request as ready for review November 9, 2023 18:02
Forfold
Forfold previously approved these changes Nov 9, 2023
Copy link
Contributor

@Forfold Forfold left a comment

Choose a reason for hiding this comment

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

LGTM, let's test in CI

Forfold
Forfold previously approved these changes Nov 9, 2023
Forfold
Forfold previously approved these changes Nov 13, 2023
KatieMSB
KatieMSB previously approved these changes Nov 13, 2023
@mastercactapus mastercactapus dismissed stale reviews from KatieMSB and Forfold via acd57e2 November 13, 2023 22:19
@mastercactapus mastercactapus merged commit 57a61e8 into master Nov 14, 2023
@mastercactapus mastercactapus deleted the suspenseful-loading branch November 14, 2023 16:21
@Forfold Forfold mentioned this pull request Jan 9, 2024
76 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants