Skip to content

Initial label alignment option#3233

Closed
etimberg wants to merge 11 commits intomasterfrom
vertical-tick-alignment
Closed

Initial label alignment option#3233
etimberg wants to merge 11 commits intomasterfrom
vertical-tick-alignment

Conversation

@etimberg
Copy link
Member

Initial work on fixing issues like #2478

Adds a new tick option consistentAlignment that if true sets the alignment of horizontal axis labels to be all the same. If false, the first tick uses left alignment and the last uses right alignment.

@simonbrunel @zachpanz88 I'm not entirely sure about the name of this option. I think I still need to do some work to make the auto skip work better for these new cases. I also don't know if we should default this true or false

@AlexDar this is the issue I mentioned to you.

@simonbrunel
Copy link
Member

Same, I'm not sure about the option name but that's maybe because it looks like a workaround option? What if we provide a new tick.alignment (or tick.align) option instead and for both axis orientation:

alignment: 'start'  // horizontal axis: 'left', vertical axis: 'top'
alignment: 'end'    // horizontal axis: 'right', vertical axis: 'bottom'
alignment: 'center' // horizontal axis: 'center', vertical axis: 'center'
alignment: 'auto'   // first tick: 'left' or 'top', last tick: 'right' or 'bottom' else 'center'

By default it would be auto and for the case you implemented, it would be center.

@etimberg
Copy link
Member Author

I do like the idea of 'alignment'. I will prototype that out and then also work on the auto-skip with rotation. We can just default to alignment: 'center' which matches the old behaviour then change the default to 'auto' in a major version release

@simonbrunel
Copy link
Member

Sounds good, I thought that the current implementation was auto and people asked for the center option (consistentAlignment). Then agree that switching to auto should be done for a major release.

@etimberg
Copy link
Member Author

@simonbrunel I changed the option to alignment and got it working for both horizontal and vertical axes. I haven't updated the auto skip code yet to handle this better.

This graph has both axes set to auto
screen shot 2016-08-31 at 6 27 21 pm

@simonbrunel
Copy link
Member

Looks really good

@etimberg
Copy link
Member Author

etimberg commented Sep 6, 2016

@simonbrunel I got the tests passing. I still need to figure out how to make the auto skipper work better with these new settings, but they should really help with the issues users have seen.

…vertical implementation because it's easier to deal with as the label widths do not matter. The first and last tick are always shown. Any other label that overlaps the first and last ticks is not shown. It is also compared against the last label that was shown in the middle to see if it overlaps.
@etimberg
Copy link
Member Author

etimberg commented Sep 7, 2016

@simonbrunel @zachpanz88 @tannerlinsley @derekperkins

I got started on improving the auto skipper. It now works for the vertical axes but I haven't re-enabled it for the horizontal axes. The following series of screenshots shows what it looks like during resizing.

screen shot 2016-09-06 at 9 21 24 pm

screen shot 2016-09-06 at 9 21 38 pm

screen shot 2016-09-06 at 9 21 54 pm

screen shot 2016-09-06 at 9 22 20 pm

screen shot 2016-09-06 at 9 22 29 pm

@etimberg
Copy link
Member Author

etimberg commented Sep 7, 2016

Another screenshot that better shows the behaviour
screen shot 2016-09-06 at 9 47 31 pm

…ck use of the autoSkipPadding for vertical axes and added some rounding to improve the stability.
@simonbrunel
Copy link
Member

Looks great, not sure about the last screenshot which seems weird (I would expect 25 and 250 or less ticks). Some space between the tick label and grid line would be nice (~2px).

We should really start to split these huge methods in small functions, scale.draw() is now more than 300 lines (82 statements) and starts to be difficult to understand (such as many other methods).

@etimberg
Copy link
Member Author

etimberg commented Sep 7, 2016

Agreed about the function length. For the last image, only 25 and 250 go because of the new alignment on the first and last labels.

@etimberg
Copy link
Member Author

etimberg commented Sep 7, 2016

The last pic is also a bit weird because I forced a step size of 25 to make sure that the auto skipper ran.

@etimberg
Copy link
Member Author

I did some more work on this for the horizontal auto skip. It does not work well. I think before we can really do anything with this we need to brainstorm how this will work.

@tannerlinsley @derekperkins @zachpanz88 @simonbrunel any ideas?

@KoyoSE
Copy link
Contributor

KoyoSE commented Dec 20, 2016

@etimberg Hello I came from #3707.
About auto skipper.
Just now I'm making a scale extension for Chart.js.
Will this be a reference for something?

https://jsfiddle.net/KoyoSE/c7orw1ka/
https://github.com/KoyoSE/chartjs-extension-finescale

In this case, Auto skipper may not be needed. (but not sure 😄 )

@KoyoSE
Copy link
Contributor

KoyoSE commented Dec 22, 2016

I created an example.
2016-12-22

https://jsfiddle.net/KoyoSE/w25hjah1/
First chart: Original linear scale.
Second chart: Using the draw method for fineScale on the original linear scale.
Third chart: fineScale.

How about other methods instead of auto skipper like this sample?

@etimberg
Copy link
Member Author

@KoyoSE other options instead of the autoskipper are definitely a possibility. I'm not sure what the implications of that are

@KoyoSE
Copy link
Contributor

KoyoSE commented Dec 22, 2016

@etimberg
(My English is something bad. 😓 so I will use simple English.)

2016-12-22 1

https://jsfiddle.net/KoyoSE/hj4g8zgm/

Common setting

min: -42500,
max: 45000,
stepSize: 2500

First chart

type : 'linear',
autoSkip : true

Second chart

type: 'fineLinear',
autoSkip: false
(In this sample too less ticks...)

Plan (Proposal)

Controlling the display of tick text, lines are displayed.
In this way, auto skiper may be unnecessary.
Finescale uses maxTicks to limit the number of display ticks.
And Finescale uses three tick levels and tick text display flags.

Other

solution of issue #3707
https://github.com/KoyoSE/chartjs-extension-finescale/blob/master/src/scale.finelinear.js#L71-L74
The tick creation number by stepSize is limited to 1000 or less.
May I PR for original?

@etimberg
Copy link
Member Author

@KoyoSE please open a PR with your changes. I'd love to see how they work and get the other @chartjs/maintainers thoughts on them

@KoyoSE
Copy link
Contributor

KoyoSE commented Dec 23, 2016

@etimberg All right. Please wait a little.

@etimberg
Copy link
Member Author

Closing as out of date

@etimberg etimberg closed this Feb 11, 2017
@etimberg etimberg deleted the vertical-tick-alignment branch March 7, 2020 18:47
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.

3 participants