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

Filler fix #6989

Merged
merged 2 commits into from
Jan 21, 2020
Merged

Filler fix #6989

merged 2 commits into from
Jan 21, 2020

Conversation

kurkle
Copy link
Member

@kurkle kurkle commented Jan 19, 2020

Fixes: #6985

The target boundary is built using the line points after update.
Those values are the view values, not the model values. This causes the bug when element is moved in the indexScale direction.

This PR adds reference to the points from where the boundary property is gotten from and updates those values before filling.

Fiddle

@kurkle kurkle added this to the Version 3.0 milestone Jan 19, 2020
etimberg
etimberg previously approved these changes Jan 19, 2020
@benmccann
Copy link
Contributor

I want to understand the issue a little better. What do you mean by "when element is moved in the indexScale direction."?

@kurkle
Copy link
Member Author

kurkle commented Jan 21, 2020

I want to understand the issue a little better. What do you mean by "when element is moved in the indexScale direction."?

Let's take a usual financial chart, index scale is the x-scale. So in that case I mean x-coordinate change .
In the default animation, x-coordinate is not changed, but is initialized (reset) to the final value. When index scale range changes for any reason, then the coordinates change and that change is animated. This is where the filler falls behind (before this fix) to the x-coordinate at update time.

Now to the why: the reference line, when we are filling against origin or chart area edge, is build to match the segments of the actual line. That is done to make the path around the fill area straight forward.

And now I realized, the if (target.segments) matches to datasets too (when filling between datasets), and in that case the loop is just excess processing (doing nothing, there is no _prop or _ref in those points). Will push an update.

@etimberg etimberg merged commit b76dd46 into chartjs:master Jan 21, 2020
@kurkle kurkle deleted the fill-fix branch February 19, 2020 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[V3] Line not correctly filled after updating dataset
3 participants