Add x-axis spacing options to make them more even #5499
Conversation
|
Tests have passed for this improvement. This fixes a recurrent issue that people have complained about regularly. |
Can you link to the issues? |
|
I like it that you didn't forget to update the docs |
|
No point adding functionality unless people know about it! |
src/core/core.scale.js
Outdated
|
|
||
| var i, tick; | ||
|
|
||
| // Specifiy the label spacing |
src/core/core.scale.js
Outdated
| var labelSpacing; | ||
| if (optionTicks.labelSpacing) { | ||
| labelSpacing = optionTicks.labelSpacing; | ||
| // If not an even number, reject |
There was a problem hiding this comment.
Why this restriction? We should mention in the documentation if we keep the restriction
There was a problem hiding this comment.
The restriction is because the ticks are integers, updated to say 'integer' instead of 'even number'
| var labelSpacing; | ||
| if (optionTicks.labelSpacing) { | ||
| labelSpacing = optionTicks.labelSpacing; | ||
| // If not an integer, reject |
There was a problem hiding this comment.
Either expand the comment here to explain why labelSpacing must be an integer and document the restriction in linear.md or remove the restriction. I'm not sure your assumption that the ticks must be integers is true?
| delete tick.label; | ||
| } | ||
| } else if ((i !== tickCount - 1) && ( // Always show last tick | ||
| (skipRatio > 1 && i % skipRatio > 0) || |
There was a problem hiding this comment.
I would indent this line and the next one more tab so that it doesn't line up with the content inside the curly braces (assuming that doesn't cause the linter to fail)
|
Should we really make |
| @@ -12,6 +12,8 @@ The following options are provided by the linear scale. They are all located in | |||
| | `min` | `Number` | | User defined minimum number for the scale, overrides minimum value from data. [more...](#axis-range-settings) | |||
| | `max` | `Number` | | User defined maximum number for the scale, overrides maximum value from data. [more...](#axis-range-settings) | |||
| | `maxTicksLimit` | `Number` | `11` | Maximum number of ticks and gridlines to show. | |||
| | `evenLabelSpacing` | `Boolean` | | To be used with `maxTicksLimit`. If true, forces ticks to be evenly spaced, which is different to the default behaviour of unevenly spacing and forcing the display of the final point. | |||
| | `labelSpacing` | `Number` | | This setting determines the spacing between labels on the x-axis. | |||
There was a problem hiding this comment.
What's the difference between labelSpacing and stepSize? Is it that labelSpacing is used exactly whereas a multiple of stepSize may be used? If so, it may be better to reuse stepSize and see if we can do something like autoSkip: false instead
|
It was discussed previously and rejected (I think) however, I think that
default to even spacing would be best. Let's quickly get back a reply on
this idea before I clean up the other points.
…On Sun, 8 Jul 2018 at 04:17, Ben McCann ***@***.***> wrote:
Should we really make evenLabelSpacing an option? I would instead say
that the current behavior is a bug and we should always apply
evenLabelSpacing. Thoughts @simonbrunel <https://github.com/simonbrunel>
@etimberg <https://github.com/etimberg> ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#5499 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AE4c_lAN2SLIKPGouWIEXS9nPgvPtKcvks5uEPsdgaJpZM4UGcKs>
.
|
|
@JamesJansson sorry for the slow reply on this. I discussed with Simon. Let's not try to fit the last point. I think the current behavior is a bug and should be changed. We don't need a new option to maintain the old behavior |
|
So even spacing will become the default behavior? I think that this is best, and will stop a lot of heartache. How/when will the be implemented? |
|
Would you be able to update this PR to make that change? |
|
@JamesJansson I wanted to check if you think you'll be able to update this to make even spacing the default |
|
@JamesJansson do you think you'll be updating this PR? If not, we may close it as inactive, but you're always welcome to reopen it after making the suggested changes. Thanks! |
|
I'm closing this PR as inactive for now since I haven't gotten any response. Please feel free to reopen after making the suggested changes |
Currently, spacing on the x-axis is uneven to allow for the final point to be displayed. This change gives two new options:
evenLabelSpacing - ensures even spaces between the labels (possibly ignoring the final point)
labelSpacing - allows the user to choose the space between the labels