elastic-charts icon indicating copy to clipboard operation
elastic-charts copied to clipboard

onRenderChange is not called if chart displays "No data to display"

Open flash1293 opened this issue 2 years ago • 2 comments

Describe the issue Debatable whether this is a bug or not, but it caught me off-guard. Open for suggestions.

When a chart doesn't render because there is no data, the onRenderChange callback is not triggered with a truthy value. This is problematic for consumers relying on this callback to check when things are "done".

To Reproduce Steps to reproduce the behavior:

  1. Configure an arbitrary chart and watch onRenderChange calls - it's called with false, then with true
  2. Set data to an empty array
  3. onRenderChange is called with false, but not with true

Expected behaviour onRenderChange is called with true even if the "rendering" is just displaying the default No data to display message. I didn't find any official documentation of onRenderChange (it says undocumented in the API docs https://github.com/elastic/elastic-charts/blob/bcf65663ef338d668c186d3a6596f143c75bc911/packages/charts/api/charts.api.md#L2385) , but IMHO every form of displaying information on the display is considered rendering from the users perspective and this should be the semantic of the callback

Version (please complete the following information): Tested with 46.12.0

flash1293 avatar Jul 08 '22 08:07 flash1293

Thanks @flash1293, you are right that's a bug, the onRenderChange is not triggered in the case the chart is empty. We should fix that.

I will investigate also about the onRenderChange triggering 2 times with false/true. I'm pretty sure this was the wanted behaviour, because we want to give you the state before and after the rendering (false -> nothing rendered, true -> something rendered) but I can see how this could be just reduced to a better renderingUpdate() call that notify that something on the chart has changed.

markov00 avatar Jul 08 '22 09:07 markov00

No strong opinion on the false/true case. We do a check for true in all places we consume it so it's not relevant for our use case. The "loading" state management is dependent on data fetching anyway so it happens much earlier.

flash1293 avatar Jul 08 '22 10:07 flash1293