[dashboard] fix <Loading /> spinners disappear too early #6283
Conversation
Recently I noticed that loading spinners on dashboards would disappear too early.
| @@ -176,7 +176,7 @@ class Chart extends React.PureComponent { | |||
| > | |||
| {this.renderTooltip()} | |||
|
|
|||
| {isLoading && <Loading size={50} />} | |||
| {chartStatus !== 'rendered' && <Loading size={50} />} | |||
There was a problem hiding this comment.
How about updating the definition of isLoading on line 164?
from
const isLoading = chartStatus === 'loading';to
const isLoading = chartStatus !== 'rendered';There was a problem hiding this comment.
That's what I tried at first, but that broke things, looks like it has to do with the skipChartRendering-related logic
There was a problem hiding this comment.
Variable-naming-wise isLoading = chartStatus === 'loading' makes more sense than isLoading = chartStatus !== 'rendered', so I want to stick with it.
The confusion is more around the component being called <Loading /> which should be shown while the component is still either loading (async call) or rendering (waiting for browser's attention).
There was a problem hiding this comment.
you could possibly add a new variable that makes this more readable const showLoader = chartStatus !== 'rendered'.
This is an interesting observation tho, I think it just shows that the chart rendering is actually really slow, esp for certain vis types and when you are loading a dashboard because there are a HUGE amount of DOM changes/additions. Theoretically the rendering is starting at the same time we were previously hiding the Loader ...
Codecov Report
@@ Coverage Diff @@
## master #6283 +/- ##
=========================================
+ Coverage 76.91% 77.01% +0.1%
=========================================
Files 64 64
Lines 9508 9508
=========================================
+ Hits 7313 7323 +10
+ Misses 2195 2185 -10
Continue to review full report at Codecov.
|
|
@mistercrunch @kristw |
|
Oh... |
|
Let me do another take on this. |
Addressing post-merge comments in apache#6283
Addressing post-merge comments in #6283
Recently I noticed that loading spinners on dashboards would disappear too early.
Addressing post-merge comments in apache#6283
mistercrunch
added
🏷️ bot
Recently I noticed that loading spinners on dashboards would disappear
too early. @kristw @williaster