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] Tick text extents not calculated correctly #4502
Comments
I could not see anything in the diffs that would cause the symptom either. But that is where my binary search regression tests lead me. The relevant from the following is:
2 of the fiddles are referencing chartjs builds from those 2 commits, and that is where I see the difference show up. Unless of course I managed to mess up my checkout, build, copy that created the .js files used with the fiddles.
|
Looks like my process was good.
The only thing I saw that was at all suspicious, was the |
Ok, this problem isn't the scale label it's the tick strings. There must be some longer string that is measured and then not used but is used when calculating the length of the longest tick label. If I force the rotation to 0 then it goes away https://jsfiddle.net/w9c5ta7u/ |
That should narrow where to look, but it still seems to have been introduced by this PR. If I change the #4268 looks like it is doing the calculations, again assuming a 45° degree placement, but as you say, with an inappropriate string length. I see references to maximum string length. For the horizontal axis, and the left margin, that should really be the length of the first tick string. That is the one that extends the furthest to the left. |
That unknown long string is probably why my fiddles were rotating the tick labels in the first place. The actual tick label text for the new 2d7c1f0 format is actually shorter than for 3a2884f, and I had to increase the css div width from Without that |
I see why this is happening now. It's very very subtle. On two spots in the core scale we measure the longest tick: With #4268, the ticks array contains objects instead of strings so now we measure each tick as being the length of @benmccann @hurskiy-andriy any thoughts on this? |
I see two ways tolve this problem:
|
Perhaps an orthogonal issue, but it looks to me like
|
It may make sense to have all scales use an object inside of |
If no one else is picking this up, I can have a closer look. Now that I know what to look for and at. It might take some time though, depending. |
Where is the tick text origin point that lines up with the end of the leader line? I found an easy place to fix the main tick text length calculation. As was suggested by @hurskiy-andriy , in helpers.longestText. Implementing that reduced the extra blank space, but did not fully eliminate it. While tracing where the rest of the padding is coming from, I found this TODO. I can see a better formula for calculating the height, but I need a little input on where the intended attachment point is between a rotated tick text entry, and the (padded) end of the tick leader line. It seems tricky to accurately describe the context in just text, so I created a fiddle with labels that can be referenced. To improve that To accurately calculate the width needed for the tick text, I will also need to know which of the vertical axis lines, M-Q matches the position of the tick leader line. Which is the same as the grid line. Reading the code, it looks like the answer is currently none of those. It looks like it is (F,O), but offset straight upward by the top padding size. From the In that image:
|
Expected Behaviour
No space should be included in the chart layout for scaleLabel axis content, when no scaleLabel is being displayed.
Current Behaviour
Since PR #4268 (Time axis ticks formatting for base and senior unit) from @hurskiy-andriy, the chart body is being positioned on the canvas as if axis scaleLabels where being displayed. Untested, but this may only occur when using a time type axis.
Possible solution
I have looked at the code changes introduced by the PR, but so far have not seen anything obvious that introduced the new result.
Steps to reproduce
Context
2d7c1f0 #4268 introduced a problem with layout for scale labels on Jun 15 14:20:16 2017 . The space needed to place them is being reserved, even when they are not displayed. This is true if no Axis scaleLabel option information is included, or if that is explicitly set to display: false. It looks correct if a scaleLabel is displayed, and seems to use exactly the same amount of space that is being left blank without the scaleLabel.
Environment
The text was updated successfully, but these errors were encountered: