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

outer tick not shown in radar chart when scale is reverse #3251

Closed
swosy opened this issue Sep 2, 2016 · 6 comments
Closed

outer tick not shown in radar chart when scale is reverse #3251

swosy opened this issue Sep 2, 2016 · 6 comments

Comments

@swosy
Copy link

swosy commented Sep 2, 2016

Found an issue as I needed a radar chart with reverse scale. Values ranges from 1 to 5.
With the reverse scale the outer tick is not shown.
canvas

As I drew into the LinearRadialScale I found in line 14213 (chart.bundle.js v.2.2.2) the folowing code
if (gridLineOpts.display && index !== 0 )
changing it to
if (gridLineOpts.display && index !== 0 || opts.reverse)
fixed the problem for me.

canvas2

I haven't tested this in other situations.
May this is usefull for others or it should get into the code with a pull request.

As I'm new to github and your project I didn't exactly know how to or if I should create a pull request. sry

@etimberg
Copy link
Member

etimberg commented Sep 2, 2016

It might actually be easier to simply reverse the ticks array if reversed because with your solution the inner grid line for the value '5' is drawn when it shouldn't be

@swosy
Copy link
Author

swosy commented Sep 4, 2016

Ah, ok I see what you mean.
so correctly the inner gridline should not be drawn.
In the normal scale this is the tick with index 0.
In reverse scale this is the tick with the last index.
Hmm, do you agree that can we get this with
if ( (gridLineOpts.display && index !== 0 &! opts.reverse ) || (gridLineOpts.display && opts.reverse && index !== me.ticks.length-1) )
Where the first condition is used for a normal scale and the second for a reversed scale.
I've checked this with my example and it seems to work fine.

May I didn't understand your suggestion to reverse the ticks array instead. Do you mean to reverse the ticks array in a callback function? Which callback I should use for this?

############

As I was checking with ticks length, I found an other issue you already have fixed for the linear scale 61ca178

#1890

example:
scale: {

                    reverse: true,
                    ticks: {

                        beginAtZero: false,
                        suggestedMin:1,
                        suggestedMax:5,
                        stepSize: 0.2,

                    },
                    pointLabels: {
                        fontSize: 13,
                        callback : function(label){
                            return label.substr(0,7)+'...';
                        }
                    },
                    gridLines:{
                        display: true,
                      }
                },

I set the stepSize to 0.2 to get more ticks. After that the output became:
canvas3

So may you can fix the floating problem in the LinearRadialScale to?
Thank you!

@etimberg
Copy link
Member

etimberg commented Sep 5, 2016

Your check will work for this case. My suggestion to reverse the ticks array was simply to always make the one at index 0 the inner one but that might already be done by the base class. I am happy to merge a PR with your updated check.

The floating issue has been mentioned before. The almostEquals part is actually already used for both the linear and radialLinear scales. The problem is that the linear scale has a different default callback which is what makes the difference https://github.com/chartjs/Chart.js/blob/master/src/scales/scale.linear.js#L10

swosy added a commit to swosy/Chart.js that referenced this issue Sep 9, 2016
@swosy
Copy link
Author

swosy commented Sep 9, 2016

My first time PR.
Big Thank you for your great work on charts.js

@SWGFL
Copy link

SWGFL commented May 10, 2017

I have the same issue, has this fix been merged yet?

@swosy
Copy link
Author

swosy commented May 15, 2017

As far as I know, it wasn't fixed. I fixed my issue with the code line:
if ( (gridLineOpts.display && index !== 0 &! opts.reverse ) || (gridLineOpts.display && opts.reverse && index !== me.ticks.length-1) )
LinearRadialScale, line 14213 (chart.bundle.js v.2.2.2)

But remember this was for version 2.2.2. !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants