Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

when min option is set for datetime axis and min value is exactly divisible by day, the format given in dateTimeLabelFormats is not respected. #1649

Closed
tgvrssanthosh opened this Issue · 5 comments

2 participants

@tgvrssanthosh
  1. Access the following Fiddle http://jsfiddle.net/tgvrs_santhosh/t4DhY/
  2. the dateTimeLabelFormats contain's a format specific to day.
  3. min value is set to timeepoch of Date.UTC(2010, 0, 1).
  4. the first label should be shown as 1 of jan, rather it is showing 00:00.
  5. removing min value is displaying label in proper format given in dateTimeLabelFormats.
@tgvrssanthosh tgvrssanthosh referenced this issue from a commit in tgvrssanthosh/highcharts.com
@tgvrssanthosh tgvrssanthosh fix for the issue #1649
if min value is exactly equal to 12:00 AM, then it was not pushed to higherRanks Array as we are adding interval time before checking whether it is 12:00 AM.
deducting min by 1 to make sure it will be pushed.
ee3a8f4
@highslide-software highslide-software closed this issue from a commit
@highslide-software Fixed issue with the first label on a datetime axis not taking a high…
…er rank, like day format on an axis based on hours. Closes #1649.
b630a38
@tgvrssanthosh

Hi,

The above provided patch fixed the min issue.. but it doesn't fix all issues..

  1. now, As we are updating time value after the if condition, it does not satisfy if we set max value, as max value will not be pushed to higherRanks Array..
    a. Access the following fiddle.. http://jsfiddle.net/tgvrs_santhosh/t4DhY/2/
    b. I included your fix as gist. by default it is uncommented.
    c. I have set max value which is exactly divisible by day.
    d. the dateLabelFormat is not respected as it is not pushed to higherRanks array.

  2. this can be further improved by changing the check 'time < max' to 'time <= max'. This will fix the problem
    a. Access the following fiddle.. http://jsfiddle.net/tgvrs_santhosh/t4DhY/2/
    b. I included this fix as gist. by default it is commented (third gist). you can comment the second gist and uncomment third gist
    c. I have set max value which is exactly divisible by day.
    d. the dateLabelFormat is respected now.
    e. Even this has issues, let's say i have given max value which is not divisible by day (i deducted it by 1 second.). now give endOnTick and showLastLabel as true.
    f. with this fix, that case will fail as it will not be added to higherRanks array.
    g. you can access the above mentioned fiddle and uncomment the max, showLastLabel and endOnTick options..

@tgvrssanthosh

Also, the pull request #1616 contains fixes for multiple bugs.. But it has been closed..

@tgvrssanthosh

Please reopen the issue..

@highslide-software

Yes I suspected that one coming... By moving the check for higherRanks to before the time is added, we miss higherRanks for the max value. But under any circumstance, the min value is more important in this regard.

I don't think it's a good idea to actually change the min and max - it may have unforeseen consequences. Like in your first pull request, but substracting 1 from the min, it would break an axis that were showing actual milliseconds.

Also, the pull request #1616 contains fixes for multiple bugs.. But it has been closed..

The other issue was your request on individual formatters for each series. We haven't acknowledged this as a bug. One thing is that the formatter in many cases applies to multiple series (when tooltip.shared is true). Another thing is that you can already taylor the formatter for each series by checking for the series name or similar within the formatter.

@tgvrssanthosh

does changing the condition 'time < max' to 'time <= max' break the things? by changing, it will work for max value which is divisible by date atleast.

yes, it's good catch.. i missed the milliseconds part.. i wonderi if higherRanks array is required at all? can't we directly check whether it's divisible by day/hour/.. while plotting the labels itself?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.