-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[gatsby-plugin-nprogress]: change default show delay #3368
Comments
After taking a look at the source code, I may have found the piece of code which hardcodes the gatsby/packages/gatsby/cache-dir/production-app.js Lines 77 to 83 in c1d47bb
And here's the plugin's code which listens to the gatsby/packages/gatsby-plugin-nprogress/src/gatsby-browser.js Lines 4 to 7 in 571bb13
If this was to be configurable, how would you go about doing so? Happy to contribute with some guidance. Thanks 👍 |
Any particular reason why 600ms vs. 1000ms? ~1000ms is the well-tested point at which people start to think something is wrong if there's not feedback https://www.nngroup.com/articles/response-times-3-important-limits/ which is why that's the value that's hard-coded. |
I've been looking for an article like this, plus I love the research made by NN/g. Thanks for this 👍 However while testing my website I had the feeling that 1000ms with no feedback was a bit too long, probably because I fade in the content, making it seem even slower? I can take a look and see if I can improve it on my side. Looking at websites such as Medium and YouTube I have the feeling that the Page Loading Bars are displayed sooner than 1000ms. Additionally Pace suggests ~500ms. It seems to me that although 1000ms is a good rule of thumb, it's also subjective, which makes me question if Gatsby should allow the user to set a custom Would be happy to hear about what other users think of this too 🙂 |
Subjective???? This is SCIENCE!!! :-P Fair points :-) I'd be up for a PR that lets you override the default. Probably should be done via gatsby-browser.js so plugins can use it. Perhaps gatsby-plugin-nprogress could then take an option for changing the timeout. We should also make a proper API for calling plugins when the page load is delayed instead of using the internal emitter. |
@KyleAMathews Cool, I'll take a look then! 🙂 I agree that it feels weird to use the internal event for plugins, but should this refactor be done as a separate PR? |
Might as well do it at the same time. |
@KyleAMathews Ok, so I've forked and cloned the Gatsby repo. Bootstrap is done and test have passed. As I've never worked on Gatsby before, I'm struggling to find my way around. You mentioned the If this is a global setting, where should the user define this option? Is there any sort of customisation like this in place? I'll appreciate any guidance 🙂 Thanks! |
Sure! There's a function in production-app.js named
You'll want to create a new API called Use that to replace the current event emitter stuff https://github.com/gatsbyjs/gatsby/search?utf8=%E2%9C%93&q=onDelayedLoadPageResources&type= Then for the delay time, add another API called
|
good stuff ya'll 👏👍 just adding my $.02 - hopefully not a stick in the spokes :) it would be awesome if we had an event / hook / whatever that let us show something at the start of navigation. today we have the notions of "delayed" and "done" but not "started" (unless i'm derping 🤪) |
@busticated I agree... It's like a navigation lifecycle API. Some ideas below:
These events could then be used by plugins. It may be better to have a separate task to uniform all the internal event naming convention too as well as introducing new ones - so PRs are easier to review. |
Yeah 💯 to creating standard lifecycle APIs |
@KyleAMathews
Is this intentional? (Perhaps because they’re fired from Sorry for the stupid questions 😳 |
Not a stupid question :-) First yes, they'll only show up in "production" i.e. you need to build the site and then load it with Second, those APIs don't exist yet :-) So you'll also need to add those by calling |
@KyleAMathews Ah, that explains it. Ok, I've been having some thoughts about how this work could be done in smaller PRs. This is my first contribution to Gatsby and I think as an initial PR it's a bit overwhelming to start refactoring internal events and creating a navigation lifecycle API. I'd like to propose the following:
Does that sound reasonable for you? |
Sure! |
So is it possible to reduce the delay from 1000 to 0 ? |
Hello,
I've just implemented
gatsby-plugin-nprogress
. Easy to set up and works out of the box, love it.However I was wondering if it's possible to change the default show delay. What I mean is by default Gatsby displays the
NProgress
component after 1 second of a link being clicked - I'd like to change this to something shorter, perhaps 600ms.Thanks a lot
The text was updated successfully, but these errors were encountered: