feat(query-on-demand): loading indicator #28562
Conversation
…akes over a second this will show both for client side navigation and first render
gatsbot
bot
added
the
status: triage needed
pieh
added
topic: query on demand*
and removed
status: triage needed
…isable get request
…f loading indicator is not enabled, disable by default in cypress env
…et precedent that would add configuration options to flags and we might regret it later
| if (!window.___gatsbyDidShowLoadingIndicatorBefore) { | ||
| // not ideal to this in render function, but that's just console info | ||
| debugLog( | ||
| `You might have seen "gatsby develop" loading indicator in the bottom left corner.\n\nIf you want to disable it you can visit ${window.location.origin}/___loading-indicator/disable (will disable it for current session).` |
There was a problem hiding this comment.
Slight edit to the message:
A loading indicator is displayed in-browser whenever content is being requested upon navigation (Query On Demand). \n\nYou can disable the loading indicator for your current session by visiting ${window.location.origin}/___loading-indicator/disable
There was a problem hiding this comment.
@pieh - what do you think of the minor edit? Do you think it misses any detail, or creates confusion?
There was a problem hiding this comment.
We do show loading indicator only if request doesn't finish within 1000 miliseconds - this is some "perceived performance" trick that I've read about somewhere that showing indicator immediately make it feels as less performant than it actually is (or maybe rather delaying showing indicator make it feel more performant) - I don't know if it's important to capture it in the message. So we show it when request is taking longer than what could be described as "almost instant" (maybe 1000 miliseconds is too high threshold for that, but we already do have precedent in using this value for some APIs that allow users to integrate their own custom loaders)
There was a problem hiding this comment.
But overall - I think this is improvement over my message, so I will push update soon ;) And we can always iterate more if needed
| if (!window.___gatsbyDidShowLoadingIndicatorBefore) { | ||
| // not ideal to this in render function, but that's just console info | ||
| debugLog( | ||
| `A loading indicator is displayed in-browser whenever content is being requested upon navigation (Query On Demand).\n\nYou can disable the loading indicator for your current session by visiting ${window.location.origin}/___loading-indicator/disable` |
There was a problem hiding this comment.
Maybe we should also add a link to query on demand umrella here? @pragmaticpat @pieh
| indicatorEnabled = true | ||
| writeVirtualLoadingIndicatorModule() | ||
| } else if (req.params.method === `disable` && indicatorEnabled !== false) { | ||
| indicatorEnabled = false |
There was a problem hiding this comment.
Does it make sense to disable it for all develop sessions (i.e. save to cache)? Otherwise, users will have to disable it on every gatsby develop run?
There was a problem hiding this comment.
For permanent solution I would want to be able to configure this in gatsby-config - I even had this before, but it was setting precedence that we would have to stick with and it didn't feel right to do it in haste.
0a180e5 removed permanent setting
I did put note about this in description:
[x] permenently (gatsby-config)I backed out of it, because it sets precedent for configuration in flags which we might regret - it doesn't mean something like won't be added in the future - just that it needs more thinking and discussions
I don't like using this method to save permanently, because it's only thing that act this way and it's not clear to users how to re-enable it later.
Disabling for current session is mostly here to address potential issues of adding "foreign UI elements" to user sites - like iterating on e2e tests (i.e. cypress) in develop or running some other forms of automated audits with browser extensions etc.
This shows loading indicator in
gatsby developwhen using query on demand. It will show fixed element in bottom left if first render or navigation takes over 1s and dismiss it just before first render or when navigation completes.TODO:
[x] permenently (gatsby-config)I backed out of it, because it sets precedent for configuration in flags which we might regret - it doesn't mean something like won't be added in the future - just that it needs more thinking and discussions[ ] e2e testsindicator being shown in shadow-dom make it not feasible to test with our version of cypress - it seems like newer versions of cypress do support it, but upgrading cypress version is bigger lift and should be out of scope for this PRRelated Issues
closes #28182
[ch19091]