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

Updating the chart after adding data and label doesn't render the dataset properly #9092

Closed
KucaKun opened this issue May 14, 2021 · 4 comments · Fixed by #9105
Closed

Updating the chart after adding data and label doesn't render the dataset properly #9092

KucaKun opened this issue May 14, 2021 · 4 comments · Fixed by #9105
Milestone

Comments

@KucaKun
Copy link

KucaKun commented May 14, 2021

Expected Behavior

When a chart is updated with a data point and a label, it should render itself properly.
It shouldn't matter if the datapoint is added before the label as in:
chart.data.datasets.forEach((dataset) => { dataset.data.push(data); }); chart.data.labels.push(label); chart.update();
Or after the label:
chart.data.labels.push(label); chart.data.datasets.forEach((dataset) => { dataset.data.push(data); }); chart.update();

Current Behavior

Instead the chart doesn't render properly when the datapoint is pushed to the array before the label.

Possible Solution

There are two ways to fix that:

  • State in the documentation clearly that data should be added after the label and the order of operations matter,
  • Change it so the order of adding data and labels before update doesn't matter (here I can't be of any help, but I noticed there's a listener on the chart.data.dataset[n].data array.)

Steps to Reproduce

Here's a codepen for this issue. Top chart is rendered properly, while at the bottom chart there is no line, sometimes it shows up (for example when you resize the page), there are no errors or notices in the console.
https://codepen.io/KucaKun/pen/oNZxEjK

Environment

  • Chart.js version: 3.2.1 and latest master at the time of creating this issue
  • Browser name and version: Firefox 88.0.1 and Chrome 90.0.4430.93 (Both on UBUNTU 18)
@LeeLenaleee
Copy link
Collaborator

It seems to work half, if you add the label first and then the datapoint it gets animatited correctly, if you add the data first and then the label it has some kind of a delay without animation. If you set the delay on your interval to a bit higher you will see that both charts will update with the data so it is regestering the data.

My guess is that it somewhere in the animation it doesnt like that the data is added first, if you set animation duration to 0 it will also work fine with smaller interval

Larger interval pen: https://codepen.io/leelenaleee/pen/QWpNQVx
0 duration animation pen: https://codepen.io/leelenaleee/pen/PopNQxK

@KucaKun
Copy link
Author

KucaKun commented May 14, 2021

Oh, let me add to that, both ways work when you use chart.update("none")

@kurkle
Copy link
Member

kurkle commented May 14, 2021

Not sure how it relates to animations, but I think the behavior comes from the hooks that are attached to the data array. The hooks are used to detect changes, and I think we are prosesing the changes to some level right away (parsing the data). Ideal fix would probably be to postpone the parsing to the actual update call. I think we'd need to record the dirty zones that need to be parsed. (or maybe the opposite, have clean zones on record)
@etimberg thoughts?

@etimberg
Copy link
Member

I think it is the hooks and the parsing during the hook. Now that we used the parsed variables for everything, we notice the problem. Marking as dirty could be ok, but I'm not sure if the update order needs to be kept. I think it does, otherwise the metadata array gets out of sync. Another option is to keep the hooks as is, but also mark the spots dirty and then do a final update/parse when update() is called

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants