Skip to content

Conversation

@KoyoSE
Copy link
Contributor

@KoyoSE KoyoSE commented Dec 26, 2016

The following problem was solved.
1.Rotation of scale label when scale is right
2.Scale position at rotation when scale is top.

Before

2016-12-26 15 _li
jsFiddle

After

2016-12-26 18
jsFiddle

1.Rotation of scale label when scale is right.
2.Scale position at rotation when scale is top.
Copy link
Member

@etimberg etimberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than my comment on the vertical label, I think this looks good. Not sure if there is a way to write a test for this or not

scaleLabelX = isLeft ? me.left + (scaleLabelFont.size / 2) : me.right - (scaleLabelFont.size / 2);
scaleLabelY = me.top + ((me.bottom - me.top) / 2);
rotation = isLeft ? -0.5 * Math.PI : 0.5 * Math.PI;
rotation = -0.5 * Math.PI;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this change. I, personally, find it easier to read in the current orientation on the right side. CC @chartjs/maintainers thoughts?

Copy link
Contributor Author

@KoyoSE KoyoSE Dec 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please compare these two examples.
2016-12-30 2 _li
2016-12-30 1 _li

I think that the second figure is easier to read when I look at the four scale labels.
Because It is a personal opinion, I think that it is OK to cancel this change.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's also consistent with the label on the left.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I wrote it the current way is that to me it felt more natural to turn my head to the right on the right side and to the left on the left side.

I don't know what most chart packages do though. If everyone else has the proposed rotation then we can change it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that the old version is better, and the labels should have their bottom along the side of the chart.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for considering it. I will cancel this change.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KoyoSE

@etimberg etimberg merged commit 83c5419 into chartjs:master Jan 5, 2017
@simonbrunel simonbrunel added this to the Version 2.5 milestone Jan 14, 2017
exwm pushed a commit to exwm/Chart.js that referenced this pull request Apr 30, 2021
Fix Scale position at rotation when scale is top.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants