Opened 10 years ago

Closed 9 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: xiangxz@…, deanw
Blocked By: Blocking:

Description

Another patch from Xiang:

  1. Add rotate chart axis label feature
  2. Add skip chart axis label feature
  3. test case in test_labels2d.html

Attachments (6)

charting_label_r_s.patch (6.3 KB) - added by Adam Peller 10 years ago.
updated: axis label support with tests, from Zhou Xiang (IBM, CCLA)
label_rotation.png (3.5 KB) - added by JayZ(zhouxiang) 10 years ago.
label_rotation
charting_label_rotation.patch (7.2 KB) - added by Adam Peller 10 years ago.
updated patch from zhouxiang, handles label rotation only
charting_label_rotation_patch.txt (9.1 KB) - added by JayZ(zhouxiang) 9 years ago.
label.png (3.0 KB) - added by JayZ(zhouxiang) 9 years ago.
label
label1.png (2.9 KB) - added by JayZ(zhouxiang) 9 years ago.
label1

Download all attachments as: .zip

Change History (25)

comment:1 Changed 10 years ago by Adam Peller

Cc: xiangxz@… deanw added

comment:2 Changed 10 years ago by Eugene Lazutkin

Status: newassigned

Related to #10387 closed in favor of this ticket.

comment:3 Changed 10 years ago by JayZ(zhouxiang)

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 10 years ago by Eugene Lazutkin

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 10 years ago by JayZ(zhouxiang)

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 10 years ago by Adam Peller

Attachment: charting_label_r_s.patch added

updated: axis label support with tests, from Zhou Xiang (IBM, CCLA)

comment:6 Changed 10 years ago by Adam Peller

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 10 years ago by Eugene Lazutkin

Milestone: 1.51.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, yet calculate() 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 10 years ago by JayZ(zhouxiang)

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:9 in reply to:  2 Changed 10 years ago by JayZ(zhouxiang)

please refer to #11088 for label skipping feature

Changed 10 years ago by JayZ(zhouxiang)

Attachment: label_rotation.png added

label_rotation

comment:10 Changed 10 years ago by JayZ(zhouxiang)

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 10 years ago by Adam Peller

updated patch from zhouxiang, handles label rotation only

comment:11 Changed 9 years ago by Eugene Lazutkin

Milestone: 1.61.5

comment:12 Changed 9 years ago by Eugene Lazutkin

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 9 years ago by JayZ(zhouxiang)

comment:13 Changed 9 years ago by JayZ(zhouxiang)

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 in reply to:  13 Changed 9 years ago by Eugene Lazutkin

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 9 years ago by JayZ(zhouxiang)

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.

Changed 9 years ago by JayZ(zhouxiang)

Attachment: label.png added

label

Changed 9 years ago by JayZ(zhouxiang)

Attachment: label1.png added

label1

comment:16 Changed 9 years ago by Eugene Lazutkin

Robot missed the commit: (In [22397]) charting: added rotated labels based on a patch by zhouxiang, thx ibm!, !strict, refs #10888.

comment:17 Changed 9 years ago by Eugene Lazutkin

Summary: [patch][ccla]Axis labels[docs][patch][ccla]Axis labels

Need docs.

comment:18 Changed 9 years ago by Eugene Lazutkin

(In [22406]) charting: bugfix for some label rotation values, !strict, refs #10888.

comment:19 Changed 9 years ago by Eugene Lazutkin

Resolution: fixed
Status: assignedclosed

Robot missed: (In [22411]) charting: fixed typos, cleaned up, !strict, fixes #10888.

Note: See TracTickets for help on using tickets.