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

Fix bug #4202 #4971

Closed
wants to merge 1 commit into from
Closed

Fix bug #4202 #4971

wants to merge 1 commit into from

Conversation

leonfed
Copy link

@leonfed leonfed commented Nov 19, 2017

Fix bug #4202
Problem: line on the edge get cut
Example: https://jsfiddle.net/2tqmf2bg/
Pull request: https://jsfiddle.net/onnbzLuu/1/

@etimberg
Copy link
Member

@leonfed is there a way to unit test this?

@leonfed
Copy link
Author

leonfed commented Nov 19, 2017

@etimberg I don't know how to make a unit test, because it's changes in visual look of charts.

@etimberg etimberg added this to the Version 2.8 milestone Nov 21, 2017
@etimberg
Copy link
Member

Closing due to inactivity

@etimberg etimberg closed this Dec 31, 2017
@simonbrunel simonbrunel removed this from the Version 2.8 milestone Jan 13, 2018
@Hainesy
Copy link

Hainesy commented Jan 31, 2018

Why has this been closed? This is a real issue that a lot of users experience. For example, I have many charts that show scores between 0 and 100. If the score reaches 100, the line gets clipped. I don't want to increase my y-axis to show a higher score, as no scores higher than 100 are possible.

@sunknudsen
Copy link

Adding my vote to reopen this pull request

@theunreal
Copy link

@etimberg Why close a real bug?

@lethosor
Copy link

@etimberg @simonbrunel Could you merge this, or at least review it? #4202 is still unfixed in 2.7.2. As @leonfed said, there isn't a clear way to unit test this, but that's no reason to close this without any further discussion.

@lethosor
Copy link

Okay, for anyone looking at this, #5321 was merged after 2.7.2, and claims to fix #4202. I still don't think this PR was handled appropriately. However, http://www.chartjs.org/dist/master/Chart.min.js as of right now still exhibits #4202. I don't know if that's outdated or if #4202 is still not entirely fixed (that file is different from https://cdnjs.cloudflare.com/ajax/libs/Chart.js/2.7.2/Chart.min.js, at least).

@simonbrunel
Copy link
Member

These changes have been merged as part of #5321 but are not released yet.

@lethosor it works with master (https://jsfiddle.net/e8n4xd4z/8951/)

@lethosor
Copy link

Like I said, I tried 2.7.2 and master and saw the same issue in both. It's possible that it's not exactly the same issue as #4202. I'll try to get it to reproduce in your fiddle and follow up in #4202.

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

7 participants