Opened 6 years ago

Last modified 2 years ago

#17381 assigned enhancement

[cla][partial pr] Incorrect (asymmetric) margins with rotation option set for labels

Reported by: dgerhardt Owned by: cjolif
Priority: undecided Milestone: 1.15
Component: Charting Version: 1.9.1
Keywords: Cc:
Blocked By: Blocking:

Description

The chart's margins are calculated incorrectly if tick labels are rotated. Sample code demonstrating the issue is attached.

http://jsfiddle.net/vyQsU/

Attachments (1)

chart-rotated-labels.js (692 bytes) - added by dgerhardt 6 years ago.
Code sample demonstrating the issue

Download all attachments as: .zip

Change History (7)

Changed 6 years ago by dgerhardt

Attachment: chart-rotated-labels.js added

Code sample demonstrating the issue

comment:1 Changed 6 years ago by cjolif

Owner: set to cjolif
Status: newassigned
Type: defectenhancement

The reason for this behavior is that the margins are computed for the worst case. And int he worst case scenario that margins that appears to be useless are useful. See: http://jsfiddle.net/nMVHM/ . Base on this I don't think this is a defect. This certainly could be improved but this looks more like a enhancement request to me.

comment:2 Changed 5 years ago by dgerhardt

I now looked into the issue again and noticed that it is not solely related to rotated labels. With rotated labels the incorrect offsets are just more visible since the chart is no longer centered.

This offset is not caused by the worst case scenario calculation for the longest label. Here are two additonal samples with equal length labels: http://jsfiddle.net/vyQsU/8/ (rotated) http://jsfiddle.net/vyQsU/9/

The incorrect offsets are occuring when the first tick is not at the origin which is the case for Columns/Bars? plot types as long as includeZero is not set true. The offset of the first tick is not taken into account for the axis' offset calculation.

I dug into the code and did some adjustments to substract the offset from the first tick. https://github.com/dgerhardt/dojox/compare/issue-17381-offset-fix

I checked the testing pages and did not find any regressions, yet. But I'm not entirely sure if firstTickOffset = (s.major.start - s.bounds.from) * s.bounds.scale is correct for all cases.

comment:3 Changed 5 years ago by cjolif

looks like a possible fix indeed, maybe a good next step would be to sign the Dojo CLA if you have not and issue a pull request on dojox? Then we can review the fix?

comment:4 Changed 5 years ago by dgerhardt

Ok, I signed the CLA and issued a pull request.

https://github.com/dojo/dojox/pull/119

comment:5 Changed 3 years ago by dylan

Milestone: tbd1.12
Summary: Incorrect (asymmetric) margins with rotation option set for labels[cla][partial pr] Incorrect (asymmetric) margins with rotation option set for labels

Has a work in progress PR that is unfinished. Setting for 1.12.

comment:6 Changed 2 years ago by dylan

Milestone: 1.131.15

Ticket planning... move current 1.13 tickets out to 1.15 to make it easier to move tickets into the 1.13 milestone.

Note: See TracTickets for help on using tickets.