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

Points plotted outside scale min and max on line chart #6540

Closed
benmccann opened this issue Sep 26, 2019 · 4 comments
Closed

Points plotted outside scale min and max on line chart #6540

benmccann opened this issue Sep 26, 2019 · 4 comments

Comments

@benmccann
Copy link
Contributor

benmccann commented Sep 26, 2019

Expected Behavior

Points outside of scale min and max should not be plotted

Current Behavior

Points are showing up below the scale

Possible Solution

We could have the scale return NaN in getPixelForValue for numbers outside its range. Probably the best way to do this is add a check in getPixelForDecimal to ensure the value is in the range 0-1. The line controller would then skip those points

Another option would be to have the line controller check the point relative to the scale bounds and not attempt to update the element. I'm not sure if I like that option as much because we'd have to be sure to update every individual controller for each chart type.

Steps to Reproduce (for bugs)

https://codepen.io/benmccann/pen/QWLPdrv

Context

Causes zoom plugin not to work with line charts

Environment

  • Chart.js version: 2.8.0
@benmccann
Copy link
Contributor Author

Looks like it's more complicated especially for the bar chart

The bar chart has calculateBarValuePixels doing:

var start = value.start === undefined ? 0 : value.max >= 0 && value.min >= 0 ? value.min : value.max;
...
base = scale.getPixelForValue(start);

So it assumes the bar starts at 0, but 0 might be outside the min and max range

@kurkle
Copy link
Member

kurkle commented Jan 1, 2020

#6642 fixed this, right?

@benmccann
Copy link
Contributor Author

Seems like it might have. I'm on mobile right now so can't update the codepen to use the latest version of Chart.js to test. Maybe this weekend I can try

@benmccann
Copy link
Contributor Author

Yes, it looks fixed. https://codepen.io/benmccann/pen/KKwyZjP

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.

2 participants