-
Notifications
You must be signed in to change notification settings - Fork 574
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
Stale data and labels due to merging #1320
Comments
@Ondrashx the reason it is done that way is to support Chart.js animations, which require the JS object ref to stay the same. The solution is definitely not perfect yet, I'll add some tests to check animations still work when not merging labels and datasets. |
@santam85 We're getting a very weird behavior where we have a time serie which data is bound to an Observable collection. If we change the values of the the existing points everything seems to work correctly, but as soon as we reduce or add more points the graph behaves really weirdly, keeping the previous data points and messing up the time scale. Is this related to this issue? |
@Gimly Yep, that seems exactly like the issue caused by merging. The fix is simple though ... you can tap into the stream of data and reset the datasets and labels, like I did in the client code. |
@Ondrashx could you try the latest RC? I believe I got this fixed... |
@santam85 Yep, it seems to work now. Sorry for late answer, not going here that much ... . Thanks. |
Reproduction of the problem
First of all thanks for making your work public.
There is one problem I run into
The merging of the config on ngChanges is not ok. New data and labels are merged. When input of baseChart directive of data and labels changes only those new data and labels should be used. Instead they are merged together. The merging happens here:
What I have done in my client code to overcome this issue, is that I clear the data and labels before assigning to inputs of baseChartDirective new data, but it is just hack.
I think the merging should not be employed at all, but maybe there is some reason I do not see.
Anyway, just wanted to let you know.
The text was updated successfully, but these errors were encountered: