[Patch][CLA] Add dojox.chart centered axis

[youngho]

Hello,

I hope to add cented axis for math graph which generated like dojox.calc.Grapher.

[youngho]

math graph drawing with centered axis

[Eric Durocher]

Hi @youngho, this looks nice, the only thing I would say is that maybe it would be better to use strings like "leftOrBottom", "center" and "rightOrTop" (or shorter names if you have ideas) instead of -1/0/1 for the position value.

Could you please create a pull request on the the GitHub? dojo/dojox repository (see https://github.com/dojo/dojox/blob/master/CONTRIBUTING.md), so it is easier to discuss and track?

Thanks.

[Eric Durocher]

Another question is whether it would be much more complicated to allow any position, e.g. position could be a 0-1 float? Maybe more complicated due to limit cases, like axis close to the left side so labels must be taken care of...

[youngho]

Hello Eric,

Sorry to late respons.

Looks great! Please go ahead.

[Eric Durocher]

Hi,

Sorry but what are you agreeing with? Replacing -1/0/1 with string values as in my first comment? Or allowing any position as in my second comment? The second proposal would probably require more time to implement and test than what we have (one day remaining before code freeze). But on the other hand, if we do change this code, it seems a pity to limit the option to just centered axis...

Also, as I said it would have been easier to comment the code in a Github pull request. In particular, I think some of your code changes in getOffsets() are useless since you set the offsets to 0 later.

[youngho]

Hi Eric,

I was tring to set up a pull request. But because I am not familliar with github, I need a learning time. please understand me.

For the issue,

I think it is better to seperate between axis position notation and how far seperate the label positioned from axis.

for the axis position notation, my preference is -1, 0, +1 notaion. we can easily extends to other enhancement for example, please look at the #17711 we can set this kinds of labels as [-1, +1, +2]

Thanks,

Youngho

[Eric Durocher]

Replying to youngho:

Hi Eric,

I was tring to set up a pull request. But because I am not familliar with github, I need a learning time. please understand me.

Sure I understand.

For the issue,

I think it is better to seperate between axis position notation and how far seperate the label positioned from axis.

I don't understand this, I don't think I spoke about something like that...

for the axis position notation, my preference is -1, 0, +1 notaion.

Well as I said I am sure we can propose something easier to understand for the user.

we can easily extends to other enhancement for example, please look at the #17711 we can set this kinds of labels as [-1, +1, +2]

I don't understand, but that seems another story?

Anyway, all in all I don't think this is quite ready for committing it in 1.10. I would really like to add axis positioning, but it seems we still need discussions on how exactly to do it.

In addition, I think this needs more testing, in particular unit tests to check that it does not break anything together with things like rotated labels, etc.

[Eric Durocher]

As noted in the previous comment, I think this must be discussed more and reworked, and is not ready for committing in 1.10 as is. So I am moving this to the 1.11 milestone.

[youngho]

Hi Eric,

I added rotatedLabel test case patch.

Also please let me know there any potential breakable current feature exist from my patch.

And I didn't catch why you think about float value at the second your comment. can you explain more ?

[Eric Durocher]

Thanks for the test case for rotated labels.

About my second comment/float value, my point is that I think it would be useful to be able to place the axis not only at the center, but at any position like 1/4 of the width etc. For this, we would allow the position to be a float between 0 and 1; assuming a vertical axis, the values would mean 0 = left, 1 = right, 0.5 = center, 0.25 = 1/4 of the chart width, 0.75 = 3/4 of the width, etc.

So, what I would like to avoid is changing the API and deprecate a property, and then having to change again because the first change was not sufficient or general enough.

Is it clearer?

[Eric Durocher]

What we can do is do only centered axis for now, but change -1/0/+1 (which is my opinion are too misleading) to string values "leftOrBottom"/"center"/"rightOrTop". This way, we can decide later to also allow arbitrary float values as well, without breaking compatibility. If you are OK with that and you can provide a patch today, I can commit that for 1.10.

Thanks!

[Eric Durocher]

OK, never mind I did it myself.

Committed in: ​https://github.com/dojo/dojox/commit/6c2f3c03c5adb24d36d97d2cbeb15cf1cd04fd99

Thanks!

[Eric Durocher]

I also updated the release notes: ​http://livedocs.dojotoolkit.org/releasenotes/1.10#charting, please review.

If you could update the ref doc ​http://livedocs.dojotoolkit.org/dojox/charting#configuring-axis I would appreciate, thanks.