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

[BUG] Incorrect chart offset for yAxis labels #4441

Closed
instant1100 opened this issue Jun 29, 2017 · 6 comments · Fixed by #4942
Closed

[BUG] Incorrect chart offset for yAxis labels #4441

instant1100 opened this issue Jun 29, 2017 · 6 comments · Fixed by #4942

Comments

@instant1100
Copy link

Expected Behavior

Show full yAsix label.

Current Behavior

Shows only the fractional part.
First off all Chart.js calculation L1 width (in my case used longestText for all tick), but after that chart.js add several ticks.
[0, 1, 2, 3] => [0, 0.5, 1, 1.5, 2, 2.5, 3]

Steps to Reproduce (for bugs)

jsfiddle

@etimberg
Copy link
Member

@instant1100 where in the code did you notice the extra ticks being added?

@mMerlin
Copy link

mMerlin commented Jun 30, 2017

For that particular graph a workaround (not fix) is to specify an empty y axes scale label.

"yAxes": [{
        "stacked": true,
        "scaleLabel": {
          "display": true,
          "labelString": ""
        },
        "ticks": {
          "fontSize": 10,
          "beginAtZero": true
        }
      }]

That adds enough padding to make room for the ticks that are being truncated. Still a bug though. If a real scaleLabel is used, it ends up overlapping with the tick labels. There may be some better way to explicitly add padding to the tick labels, but that would still not be a fix for the problem. The initial calculation of the yAxes tick column width (using longestText) for range 0 … 3 (beginAtZero to maximum 3) is less than the final column width, once the starting integer tick values are converted to floating.

I do not know where the extra ticks are added, but I assume that once the initial layout was done, chart.js decided it wanted more than 4 yAxes ticks. The change to use 0.5 step size causes the tick length to be longer than the initial maximum calculation.

@sylfabre
Copy link

A workaround that works for me:

yAxes: [{
	ticks: {
		min: 0,
		stepSize: 1
	}
}]

@mMerlin I assumed the same about the 0.5 step. The algo should keep an integer (1 for instance) for step size if all data values are integers. @etimberg Should I open a separate issue?

@mMerlin
Copy link

mMerlin commented Jul 26, 2017

An even better workaround, keeping the the generated (re-generated?) steps, is to specify the stepSize. Once it is known what it will end up being.

"yAxes": [{
        "ticks": {
                "stepSize": 0.5
        }
}]

Telling it up front that the step size is 0.5 convinces chartjs to include the extra space for the text to start with. Messes up any automatic recalculation for a responsive, resizeable chart. For a fixed width chart like the example fiddle though, it works fine.

I am looking at the code causing #4502, and I think this will get caught by that one. Now that I know about this one. Unless the extent calculation for numeric ticks is totally separate from that for time values. Some if it should be common, and current symptoms are leading me to believe that part of the problem there is also changes between initial layout calculations, and actual final size of the text. Just in the reverse case there. It is leaving too much space instead of not adding enough extra.

An option to limit to integer tick labels with only integer data would be separate.

@pietrofxq
Copy link

pietrofxq commented Aug 8, 2017

The stepSize solution isn't viable at all. It forces the values to increase 0.5 all the way up, causing the following issue if the values on the yAxes can be high:

image

It does fix the title issue though

@jcopperfield
Copy link
Contributor

The problem is caused by the layout service Step 4 being initially trying a chart area height of 50% of the canvas height. When in the initial case there's only enough space for integer ticks, then the y-axis will reserves only the width for integer tick labels. Later in Steps 5 & 6 this reserved space isn't allowed to increase, even when the allowed height is increased in this step.
A simple solution would be to increase the allowed height in the initial computation step to be maxChartAreaHeight instead of chartAreaHeight.

diff --git a/src/core/core.layoutService.js b/src/core/core.layoutService.js
index 5649f11..df28f97 100644
--- a/src/core/core.layoutService.js
+++ b/src/core/core.layoutService.js
@@ -194,7 +194,7 @@ module.exports = function(Chart) {
 					minSize = box.update(box.fullWidth ? chartWidth : maxChartAreaWidth, horizontalBoxHeight);
 					maxChartAreaHeight -= minSize.height;
 				} else {
-					minSize = box.update(verticalBoxWidth, chartAreaHeight);
+					minSize = box.update(verticalBoxWidth, maxChartAreaHeight);
 					maxChartAreaWidth -= minSize.width;
 				}

Issue v2.7
issue_y-axis_labels_partially_hidden
Proposed fix
fix-issue_y-axis_labels_partially_hidden

etimberg pushed a commit that referenced this issue Nov 25, 2017
…nitial fitting. (#4942)

* Fix issue 4441:
 - y-axis labels partially hidden due to restrictive initial fitting.

* Add regression test to linear scale

* Moved regression test from linear scale to core layout service
@etimberg etimberg added this to the Version 2.8 milestone Nov 25, 2017
yofreke pushed a commit to yofreke/Chart.js that referenced this issue Dec 30, 2017
…ctive initial fitting. (chartjs#4942)

* Fix issue 4441:
 - y-axis labels partially hidden due to restrictive initial fitting.

* Add regression test to linear scale

* Moved regression test from linear scale to core layout service
exwm pushed a commit to exwm/Chart.js that referenced this issue Apr 30, 2021
…ctive initial fitting. (chartjs#4942)

* Fix issue 4441:
 - y-axis labels partially hidden due to restrictive initial fitting.

* Add regression test to linear scale

* Moved regression test from linear scale to core layout service
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants