Skip to content

/ Chart.js

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New time scale `ticks.bounds` option #4556

Merged
merged 1 commit into from Jul 25, 2017
Merged

New time scale `ticks.bounds` option #4556

merged 1 commit into from Jul 25, 2017
+361 −129

Conversation

@simonbrunel
Copy link
Member

simonbrunel commented Jul 24, 2017 •
edited

Edit: renamed scale.ticks.bounds to scale.bounds in #4595

--

bounds ('data' (default)|'ticks'): data preserves the data range while ticks ensures that all labels are visible. This option is bypassed by the min/max time options.

Remove the useless time scale _model object containing private members: instead, make these members private (prefixed by _) part of the scale.

image

Fixes #2249
Fixes #2513
Fixes #2599
Fixes #2684
Fixes #2774
Fixes #3297
Fixes #3654
Fixes #3671
Fixes #4263
Fixes #4264
Fixes #4529
Fixes #4550
@simonbrunel simonbrunel added this to the Version 2.7 milestone Jul 24, 2017
@simonbrunel simonbrunel requested a review from etimberg Jul 24, 2017
@simonbrunel

This comment has been minimized.

Sign in to view
Copy link
Member Author

simonbrunel commented Jul 24, 2017

image

Interesting ...
@simonbrunel

This comment has been minimized.

Sign in to view
Copy link
Member Author

simonbrunel commented Jul 24, 2017

The master implementation is equivalent to ticks.bounds: 'labels' but based on the many reported issues regarding the trailing and leading gaps, I think it's better to change it for ticks.bounds: 'data'.
@benmccann

This comment has been minimized.

Sign in to view
Copy link
Collaborator

benmccann commented Jul 24, 2017

Wow, amazing to close so many tickets!
@benmccann
benmccann reviewed Jul 24, 2017
View changes
src/scales/scale.time.js Outdated

/**
* Ticks distribution along the scale:
* - 'linear': ticks are spread according to their time (distances can vary),

This comment has been minimized.

Sign in to view
Copy link
@benmccann

benmccann Jul 24, 2017

Collaborator

I think it's more accurate to say data rather than ticks here

This comment has been minimized.

Sign in to view
Copy link
@benmccann

benmccann Jul 24, 2017

Collaborator

Or rather maybe more informative to say "ticks and data"

When source:auto mode, we try to place ticks spread evenly regardless of mode so I think it helps explain it better if we mention data as well
@benmccann

This comment has been minimized.

Sign in to view
Copy link
Collaborator

benmccann commented Jul 24, 2017

looks good to me
@simonbrunel
New time scale `ticks.bounds` option
Loading status checks…
92809e2 
`ticks.bounds` (`'data'`(default)|`'label'`): `data` preserves the data range while `labels` ensures that all labels are visible. This option is bypassed by the min/max time options.

Remove the useless time scale `_model` object containing private members: instead, make these members private (prefixed by `_`) part of the scale.
@simonbrunel simonbrunel force-pushed the simonbrunel:ticks-bounds branch from a91a1f7 to 92809e2 Jul 24, 2017
@simonbrunel

This comment has been minimized.

Sign in to view
Copy link
Member Author

simonbrunel commented Jul 24, 2017

And I'm sure I missed some tickets
@etimberg
etimberg approved these changes Jul 24, 2017
View changes
@simonbrunel simonbrunel merged commit 3aa7a20 into chartjs:master Jul 25, 2017
3 checks passed
3 checks passed
codeclimate All good!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.003%) to 94.604%
Details
@simonbrunel simonbrunel deleted the simonbrunel:ticks-bounds branch Jul 25, 2017
@simonbrunel simonbrunel mentioned this pull request Jul 27, 2017
Closed
@dabrave

This comment has been minimized.

Sign in to view
Copy link

dabrave commented Jul 27, 2017

Hi Simon and everyone!

First of all congrats to you all for such great community! 🥇 🥇 🥇

Tried with your last update for this issue (is it included on ChartJS 2.6 right? Or do I have to upgrade the code manually on src/scales/scale.time.js ?)

Maybe I'm implementing the solution wrong... Below you can see how (tried with just one tick.bounds on xAxis and both, but nothing...) As you can see there are still some dates not included on the chart.

2017-07-27_1851

2017-07-27_1854

Hope one day I could contribute in any code here, Cheers!
@benmccann

This comment has been minimized.

Sign in to view
Copy link
Collaborator

benmccann commented Jul 27, 2017

This isn't included in 2.6.0. You have to checkout the latest code from github, build it (npm install & gulp build), and then use the version you compiled
@simonbrunel

This comment has been minimized.

Sign in to view
Copy link
Member Author

simonbrunel commented Jul 27, 2017 •
edited

It's not released yet, you will have to wait the next release (2.7). However you can try the latest build from master (for testing only - do not pay attention to the version number in this file) and see if it works as you expect. bounds is only applicable to the time scale right now (so will have no effect on your y-axis).
@simonbrunel

This comment has been minimized.

Sign in to view
Copy link
Member Author

simonbrunel commented Jul 27, 2017

Also, I don't think beginAtZero is used by the time scale.
@dabrave

This comment has been minimized.

Sign in to view
Copy link

dabrave commented Jul 27, 2017

Ok cool, thanks a lot for such quick answer guys! :)
@etimberg etimberg mentioned this pull request Jul 27, 2017
Closed
nagix added a commit to nagix/Chart.js that referenced this pull request Jul 28, 2017
@simonbrunel @nagix
New time scale `ticks.bounds` option (chartjs#4556)
011baa3 
`ticks.bounds` (`'data'`(default)|`'label'`): `data` preserves the data range while `labels` ensures that all labels are visible. This option is bypassed by the min/max time options.

Remove the useless time scale `_model` object containing private members: instead, make these members private (prefixed by `_`) part of the scale.
@simonbrunel simonbrunel mentioned this pull request Aug 1, 2017
Merged
@benmccann benmccann mentioned this pull request Aug 2, 2017
Open
@simonbrunel simonbrunel mentioned this pull request Aug 4, 2017
Closed
@benmccann benmccann mentioned this pull request Aug 5, 2017
Merged
yofreke added a commit to yofreke/Chart.js that referenced this pull request Dec 30, 2017
@simonbrunel @yofreke
New time scale `ticks.bounds` option (chartjs#4556)
da417d8 
`ticks.bounds` (`'data'`(default)|`'label'`): `data` preserves the data range while `labels` ensures that all labels are visible. This option is bypassed by the min/max time options.

Remove the useless time scale `_model` object containing private members: instead, make these members private (prefixed by `_`) part of the scale.
@simonbrunel simonbrunel mentioned this pull request May 16, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benmccann benmccann

@etimberg etimberg

Assignees
No one assigned
Labels
type: bug type: enhancement
Milestone
  Version 2.7
Linked issues

Successfully merging this pull request may close these issues.

Time axes creates overlapping labels when data doesn't match standard interval
Line-chart not using full width
Last tick overlapping when using "time scale"
Dangling label on time scale still :(
Last tick label/width different from the rest
Time scale bugs in bar and line charts type
Xaxis time range does not start and end according to time data
I want not show min/max label on time scale
[BUG] Min and Max from Time Scale not working properly
Month Lengths Misaligned with time series month view
4 participants
@simonbrunel @benmccann @dabrave @etimberg
You can’t perform that action at this time.