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

Fixed issue #346 (Timeline start and end options are not taken into account) #351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ova2
Copy link
Contributor

@ova2 ova2 commented Jun 10, 2015

Fix for #346 Please approve this pull request.

@josdejong
Copy link
Contributor

Thanks for your PR Oleg.

I think though that the function setVisibleChartRangeAuto is not the right place for this logic: the function will then no longer (always) do what it's supposed to do: fit all items.

@ova2
Copy link
Contributor Author

ova2 commented Jun 11, 2015

It was working for me. What is the right place then? Somewhere here?

if (this.firstDraw) {
    // logic for start / end options
   ...
    this.setVisibleChartRangeAuto();
}

Currenty, the function setVisibleChartRangeAuto() is only called in this if (this.firstDraw). I tested all use cases and it was fine, so I can not see any problems.

@josdejong
Copy link
Contributor

It will indeed work fine, but setVisibleChartRangeAuto is a public method which then will no longer behave as advertised.

I guess a more robust solution would be to create a new (private?) helper function for this, something like _setVisibleChartRangeInitial.

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.

2 participants