Opened 11 years ago
Closed 11 years ago
#10888 closed enhancement (fixed)
[docs][patch][ccla]Axis labels
Reported by: | Adam Peller | Owned by: | Eugene Lazutkin |
---|---|---|---|
Priority: | high | Milestone: | 1.5 |
Component: | Charting | Version: | 1.4.0 |
Keywords: | Cc: | [email protected]…, deanw | |
Blocked By: | Blocking: |
Description
Another patch from Xiang:
- Add rotate chart axis label feature
- Add skip chart axis label feature
- test case in test_labels2d.html
Attachments (6)
Change History (25)
comment:1 Changed 11 years ago by
Cc: | [email protected]… deanw added |
---|
comment:2 follow-up: 9 Changed 11 years ago by
Status: | new → assigned |
---|
comment:3 Changed 11 years ago by
Hi elazutkin, I can not see the change of this patch in dojo latest trunk, if there are any code concern or code clean work I should do, please let me know, thank you.
comment:4 Changed 11 years ago by
A sidenote: submitting a patch does not guarantee to be in the trunk.
I was working on refactoring themes and didn't have time to review the patch thoroughly. My concern is the tick label alignment and a rotation point. When I glanced over I didn't find how you handle these things. I want to test the patch thoroughly to understand the implemented behavior, and see if it is flexible enough --- we cannot afford to redo this feature. Every time we introduce new feature we have to support it and propagate it at least for several versions leaving users with potential problems.
comment:5 Changed 11 years ago by
I assume the tick label alignment is handled by the labelAlign variable and user always rotate at default point. Follow your concern I add a parameter to control rotation point and created a new patch, I've asked Adam to upload it for me, any comments on it please let me know.
offsetX, offsetY is new rotate point's offset from base rotate point. labelRotate: {offsetX: -20, offsetY: 20, rotateAngle: -30},
Changed 11 years ago by
Attachment: | charting_label_r_s.patch added |
---|
updated: axis label support with tests, from Zhou Xiang (IBM, CCLA)
comment:6 Changed 11 years ago by
updated the patch. Two concerns: there appears to be a double BOM at the start of the HTML test file, and I'm wondering if the browser checks for IE and Opera should be replaced with some sort of feature detection?
comment:7 Changed 11 years ago by
Milestone: | 1.5 → 1.6 |
---|
I reviewed the patch and, while it works well, I found following minor problems (all in dojox.charting.axis2d.Default
):
- New parameters should be added to the list of
optionalParams
, so they can be used from the markup too. render()
is updated, yetcalculate()
doesn't take rotated labels into account. For example it prevents dense labeling with 90 degree rotated labels. In general 90 degree rotation should cause horizontal axis to treat its labels the same way as the vertical axis.
There is one conceptual problem, which is important:
- Rotation is done around an anchor point of a text label plus some optional offset in pixels. It doesn't align rotated labels closer to their ticks, and doesn't work well with labels of wildly different sizes.
- It would be much better if we can reformulate the label placement in terms of attachment points, and make rotation work around them. IMHO, there is no need for offsets, which are technical in nature forcing users to guess their values.
The example of anchor points would be:
- Default attachment for unrotated label on the bottom axis: middle/top (see below) of the text.
- Default attachment for unrotated label on the left axis: right/middle of the text.
- Possible attachment for 90 degree rotated label on the bottom axis: left/middle.
- Possible attachment for 45 degree rotated label on the bottom axis: left/top.
It appears that instead of offsets in pixels we can introduce 8 anchor points:
*---------*---------* | | * Axis label text * | | *---------*---------*
In this picture *
denotes an anchor point. Going from left to right and from top to bottom:
- left/top
- middle/top
- right/top
- left/middle
- right/middle
- left/bottom
- middle/bottom
- right/bottom
In general it is a superposition of two values:
- Horizontal alignment: left, middle, right.
- Vertical alignment: top, middle, bottom.
...but without middle/middle. We can support it, but it doesn't look good no matter what the rotation value is --- it will intersect text with ticks and the axis line.
We can even select those anchor points automatically depending on the rotation value.
With this in mind I am postponing this ticket until 1.6 to give us time to prepare a better patch.
Regarding the "skipping labels" part of the patch. I am not sure if it is a really useful functionality --- similar effects can be achieved with custom major/minor tick steps. In any case if we are to specify skipping labels, we should be able to specify a label we use a starting point of the skipping. I suggest to separate this feature from the rotation patch.
comment:8 Changed 11 years ago by
Hi Eugene, Follow your suggestion, I've asked adam help me to create a new ticket which just for label skipping feature, so this ticket is only for rotation feature. Comparing to "custom major/minor tick steps", label skipping is only focus on labels which do not affect ticks, user can skip unnecessary label by setting the starting point and skip interval without get rid of the ticks which may be useful in chart, user can also skip the labels if the label is too wide which looks weird. Actually label skip is based on tick steps.
comment:10 Changed 11 years ago by
Hi Eugene, Follow your suggestion, I've implemented an algorithm to maintain the label not intersect with ticks and axis line, but I use middle/middle(0.4) as its rotation point. you can refer to the attachment: label_rotation.png
Changed 11 years ago by
Attachment: | charting_label_rotation.patch added |
---|
updated patch from zhouxiang, handles label rotation only
comment:11 Changed 11 years ago by
Milestone: | 1.6 → 1.5 |
---|
comment:12 Changed 11 years ago by
The patch has some problems. For example, for vertical labels in calculate()
labelWidth
is used before being defined leading to undefined/NaN values and so on. And it doesn't take into account different rotation points leading to unpleasant gaps.
I'll redo the patch.
Changed 11 years ago by
Attachment: | charting_label_rotation_patch.txt added |
---|
comment:13 follow-up: 14 Changed 11 years ago by
Hi Eugene, I create a new patch based on your latest change, sorry for the undefined/NAN issue, but I'm not quite sure about your second comment: "it doesn't take into account different rotation points leading to unpleasant gaps", my rotation points are all the center point of each label, no other different rotation.
comment:14 Changed 11 years ago by
Replying to zhouxiang:
... my rotation points are all the center point of each label, no other different rotation.
Yes, this is why it looks unpleasant in some conditions (rotation, label length).
comment:15 Changed 11 years ago by
but actually, I've not only rotate the label text but also adjust the position for each label text to make sure they have the same alignment, you can refer to label.png and label1.png attachment, I still not quite sure about the key issue.
comment:16 Changed 11 years ago by
comment:17 Changed 11 years ago by
Summary: | [patch][ccla]Axis labels → [docs][patch][ccla]Axis labels |
---|
Need docs.
comment:18 Changed 11 years ago by
comment:19 Changed 11 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Related to #10387 closed in favor of this ticket.