Skip to content
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

Improve label capacity calculation in time scale #6297

Closed
wants to merge 2 commits into from

Conversation

nagix
Copy link
Contributor

@nagix nagix commented May 24, 2019

Problem 1

Currently getLabelCapacity uses the displayFormats for 'millisecond' if unit is not specified. This results in less label capacity than actual (and this often happens). The example below shows that the unit is automatically set to 'month' when there are two labels ['2015-01-01', '2015-02-01'].

Problem 2

#6265 changed to compute the capacity using the major unit if applicable, but me._majorUnit is not set when getLabelCapacity is called. Furthermore, it is not always true that the label width of a major tick is larger than that of a minor tick or vise versa.

Solution

This PR takes into account the label capacity of both minor and major ticks in getLabelCapacity. And, determineUnitForAutoTicks checks the label capacity for each unit.

Potential problem of this change is that more labels will be rotated because getLabelCapacity calculates the capacity using rotation. But I think this should be controlled by the maxRotation option.

Master: https://jsfiddle.net/nagix/zdkcaerb/
Screen Shot 2019-05-24 at 5 15 28 PM

This PR: https://jsfiddle.net/nagix/5kersouy/
Screen Shot 2019-05-24 at 5 15 39 PM

Fixes #5093

kurkle
kurkle previously approved these changes May 24, 2019
Copy link
Member

@kurkle kurkle left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@nagix
Copy link
Contributor Author

nagix commented Jun 20, 2019

Rebased and reverted some changes for size reduction.

kurkle
kurkle previously approved these changes Jun 22, 2019
etimberg
etimberg previously approved these changes Jun 23, 2019
@benmccann
Copy link
Contributor

@nagix I finished my other work on the time scale and auto-skipping. Thanks for letting me get those PRs in first. I really appreciate it. This one can be rebased now

@benmccann
Copy link
Contributor

@nagix I'm going to close this since we haven't heard anything on it for awhile, but please feel free to reopen if you're still interested in pursuing

@benmccann benmccann closed this Jan 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time scales labels not displayed if min & max are specified unless a unit is specified
4 participants