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

Not displaying tick zero value in callback for radar chart with scale's min below zero #11503

Closed
Megaemce opened this issue Sep 15, 2023 · 5 comments

Comments

@Megaemce
Copy link
Contributor

Megaemce commented Sep 15, 2023

Expected behavior

By default when creating radar chart the tick's zero value is not displayed.
However when the scale's min value is changed it's shown again. When user set a tick's callback, the zero should be handled and displayed just like any other value.

           r: {
                    min: -0.3,
                    max: 1,
                    ticks: {
                        autoSkip: false,
                        stepSize: 0.1,
                        // callback to display only few values
                        callback: function (value) {
                            if (value === 0.9) return "very strong";
                            if (value === 0.7) return "strong";
                            if (value === 0.4) return "moderate";
                            if (value === 0.2) return "weak";
                            if (value === 0) return "no"; // this should be displayed just like any other value
                        },
                    },
                },
         

image

Current behavior

When tick's callback is set the zero value will not be shown. The only possible (and dirty) workaround is to return all the values like so:

callback: function (value) {
           if (value === 0.9) return "very strong";
           if (value === 0.7) return "strong";
           if (value === 0.4) return "moderate";
           if (value === 0.2) return "weak";
           if (value === 0) return "no";
           return ""; // dirty workaround showing all the values
},

image

Reproducible sample

https://codepen.io/kowal66b/pen/vYvJryv

Optional extra steps/info to reproduce

No response

Possible solution

No response

Context

I want show only limited amount of ticks with zero value as well

chart.js version

4.4.0

Browser name and version

No response

Link to your project

www.correlations.world

@finnegantdewitt
Copy link

Hi Megaemce, I attempted to fix this as my first contribution to Chart.js and found the issue in the file src/scales/scale.radialLinear.js. The code skips ticks[0] at lines 581 and 648, which appears to always be the lowest value tick. They have included this to skip the 0th tick of auto generated ticks in radial graphs to not have a label at the center of the graph. I've made a fix for it in my own forked repo, but find it unlikely that they will merge the pull request because it may break other people's projects. You can check out my fix at https://github.com/finnegantdewitt/Chart.js. A work around I'd recommend is including another if in the callback for the lowest value. That should stop it generating all those extra lines that the return without an if makes.

@Megaemce
Copy link
Contributor Author

Hi Megaemce, I attempted to fix this as my first contribution to Chart.js and found the issue in the file src/scales/scale.radialLinear.js. The code skips ticks[0] at lines 581 and 648, which appears to always be the lowest value tick.

Thank you for you support @finnegantdewitt. There should be a logic here that if the min is set below 0 then this tick should be displayed. I am not so sure about using tickOpts.autoSkip. Something like this:

if (index !== 0 || (index === 0 && this.min < 0)) {
     offset = this.getDistanceFromCenterForValue(tick.value);
     const context = this.getContext(index);
     const optsAtIndex = grid.setContext(context);
     const optsAtIndexBorder = border.setContext(context);

     drawRadiusLine(this, optsAtIndex, offset, labelCount, optsAtIndexBorder);

Can you give it a try and see if there is no errors (for example from this.getDistanceFromCenterForValue(tick.value))?

I've made a fix for it in my own forked repo, but find it unlikely that they will merge the pull request because it may break other people's projects.

That is why we use github and versioning bro!
Don't be shy and create a merge request (just test it correctly and give my version a try).

@finnegantdewitt
Copy link

Maybe it would be best to not display based off the this.getDistanceFromCenterForValue(tick.value) because the if is supposed to prevent the center of the graph from having a value. What do you think?

@Megaemce
Copy link
Contributor Author

Megaemce commented Nov 2, 2023

Maybe it would be best to not display based off the this.getDistanceFromCenterForValue(tick.value) because the if is supposed to prevent the center of the graph from having a value. What do you think?

I just mentioned getDistance... as a reference to thorough code review before creating a merge request. The getDistance actually should make no harm here.

I just briefly checked getContex(index) and setContext and in my undestand this should make no issues at all.

Just try out my solution on your own device and if it works - make a merge request

@Megaemce
Copy link
Contributor Author

Megaemce commented Feb 24, 2024

@finnegantdewitt you were right. I changed both lines and did pull request (#11682) which should fix this issue.

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

No branches or pull requests

2 participants