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

Fixed bug with bezier still being used with tension set to 0 #1826

Merged
merged 2 commits into from
Dec 23, 2015

Conversation

hallatore
Copy link
Contributor

According to the docs bezier curve should be disabled when tension is set to 0.

@etimberg
Copy link
Member

Not entirely sure that this is needed. A bezier tension of 0 will draw straight lines. Have you been running into scenarios where a tension of 0 is not giving straight lines?

@etimberg etimberg added the v2.x label Dec 23, 2015
@hallatore
Copy link
Contributor Author

@etimberg Bezier with tension set around 0 will cause artifacts.

tension set to 0
Original code: http://i.imgur.com/5ePYdRv.png
New code: http://i.imgur.com/E46Egko.png

@etimberg
Copy link
Member

Ok, I'll merge this as soon as the tests pass 😄

@hallatore
Copy link
Contributor Author

@etimberg You fix the tests or I?

I'm not sure where to get the options object from that part of the code if this._chart is not available.

@etimberg
Copy link
Member

If you could fix the tests, that'd be great.

I think you should have access to the tension as this._model.tension per https://github.com/nnnick/Chart.js/blob/v2.0-dev/src/controllers/controller.line.js#L90

If you want the animated tension, look in this._view.tension but I think you want the non animated one in this case. There are two possible correct solutions: Check the tension in this._model.tension or change the line controller so that the property is _tension (properties with an underscore at the start of the name are not animated) and then check this._view._tension

@hallatore
Copy link
Contributor Author

@etimberg Would this work?

@etimberg
Copy link
Member

Yup. The only thing that might be a problem is if the user animates the bezier curve tension to 0. That is highly unlikely however.

@etimberg
Copy link
Member

Thanks again @hallatore

etimberg added a commit that referenced this pull request Dec 23, 2015
Fixed bug with bezier still being used with tension set to 0
@etimberg etimberg merged commit 968a11a into chartjs:v2.0-dev Dec 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants