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

feat: Add global loading indicator to top nav #176

Merged
merged 2 commits into from
Feb 5, 2024

Conversation

gustavoguichard
Copy link
Contributor

@gustavoguichard gustavoguichard commented Feb 5, 2024

According to @brophdawg11 's suggestion and issue #163 , I added this POC to the root.tsx.

In the example below I added a await sleep(2000) to the blog loaders and the theme switch action to see the complete animation and the places where the progressbar stops in both slow actions and loaders.

This was taken from my blog post and I've been using this code successfully in almost all my Remix apps.
I appreciate any feedback if rejected and willing to make any modifications requested. ;)

I've chosen a gradient from bg-blue-brand to bg-aqua-brand I think works good on both dark and light modes.

Preview:

screen

@gustavoguichard
Copy link
Contributor Author

gustavoguichard commented Feb 5, 2024

I noticed the GIF preview got a lil cut off the top. The actual bar is slightly taller (h-1 = 4px):
Screenshot 2024-02-05 at 09 14 50

Copy link
Contributor

@brookslybrand brookslybrand left a comment

Choose a reason for hiding this comment

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

Thanks @gustavoguichard for taking this on! Mostly little suggestions/code nits

@@ -0,0 +1,42 @@
import * as React from "react";
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit: can we import the hooks from React. It's not a big thing, just trying to get more uniform into this codebase to that pattern

Suggested change
import * as React from "react";
import { useRef, useState, useEffect } from "react";

app/ui/global-loading.tsx Outdated Show resolved Hide resolved
app/ui/global-loading.tsx Outdated Show resolved Hide resolved
@brookslybrand
Copy link
Contributor

@brophdawg11 since you added the suggestion I wanted to bring this to your attention in case you wanted to provide feedback

@gustavoguichard
Copy link
Contributor Author

@brookslybrand , applied the suggestions ;)

@brookslybrand brookslybrand merged commit f0c65a2 into remix-run:main Feb 5, 2024
4 checks passed
@gustavoguichard gustavoguichard deleted the gg/loading-indicator branch February 5, 2024 23:02
takagimeow pushed a commit to takagimeow/remix-website that referenced this pull request Feb 7, 2024
* feat: Add global loading indicator to top nav

* chore: Apply some of Brook's suggestions to global-loading code
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.

2 participants