Twipped / Kalendae

A javascript date picker that just works.
MIT License
1.99k stars 285 forks source link

Draw function should start at first of month, causes previousMonthDisabled class errors #162

Closed coreyzev closed 8 years ago

coreyzev commented 9 years ago

Currently I had a 3 month calendar up with an end-limit of 3 months (the end-limit is code I added myself re: the pull request I added, though I do not believe it affects this issue).

When the calendar was redrawn with a start date near the end of the first month, and the first do ... while loop finished with an extra month.add(1, 'month') the resulting diff in the directional section was over 3 months.

So my suggestion is that at the start of draw(), the month variable should be defined as:

var month = moment(this.viewStartDate).startOf('month').add(12, 'hours'), //force middle of the day to avoid any weird date shifts

Because it should be starting at the start of the month anyway, and you still have the VSD stored anyway.

Twipped commented 8 years ago

Could you produce an example of what you're describing here? I'm having trouble visualizing it.

coreyzev commented 8 years ago

(See edit at the bottom of this first, this MAY have all been for naught)

I'm going to try and re-explain it, since I dont have my project public at the moment. And I cant build an example right now. Also not sure why you closed the issue immediately after asking.


So, looking at https://github.com/ChiperSoft/Kalendae/blob/master/src/main.js#L530

var month = moment(this.viewStartDate).startOf('day').add(12, 'hours'),

This sets the start of your draw loop on VSD, instead of on the first of the month. When, in my opinion, since this is just the draw, and you're always drawing the whole month, it should be set to .startOf('m').

The error I encountered, was in the resulting diff - used here: https://github.com/ChiperSoft/Kalendae/blob/master/src/main.js#L629-L668

example, existing

Using TODAY, February 25th as an example. I think I just learned that the bug depends on moment() being at the end of the month.

// simplified: main.js

// spoofing "today" for the sake of proving the bug
var today = moment('2016-02-25'); 

var viewStartDate = moment(today); // a chosen date near the end of the month

var month = moment(viewStartDate).startOf('day').add(12, 'hours'), // line 530
    i = 0,
    c = 3; // calendars.length of 3

do { // line 548
    //stuff, lines 549 - 624

    month.add(1, 'months');
} while (++i < c);

// if direciontal scrolling
var diffCompare = moment(today).startOf('day').hours('12'); // line 630
diff = month.diff(diffCompare, 'months', true);

// -----
console.log(diff); //  3

example, updated w/ my change

// simplified: main.js

// spoofing "today" for the sake of proving the bug
var today = moment('2016-02-25'); 

var viewStartDate = moment(today); // a chosen date near the end of the month

var month = moment(viewStartDate).startOf('month').add(12, 'hours'), // replace 'day' with 'month'
    i = 0,
    c = 3; // calendars.length of 3

do { // line 548
    //stuff, lines 549 - 624

    month.add(1, 'months');
} while (++i < c);

// if direciontal scrolling
var diffCompare = moment(today).startOf('day').hours('12'); // line 630
diff = month.diff(diffCompare, 'months', true);

// -----
console.log(diff); //  2.1724137931034484

What this ends up affecting is on line 596, you compare against opts.months, it is greater than, and even though you are in the VSD month, it sets disablePreviousMonth to false.

ex:

// with current code, using a VSD of today, feb 25th
// diff = 3
// opts.direction = 'future'
// opts.months = 3

if (diff > opts.months) { // line 596
    this.disablePreviousMonth = false;
    util.removeClassName(this.container, classes.disablePreviousMonth);
}
// thus enabling clicking back a month when there is no reason to. 

Seems I typo'd my title, I meant disablePreviousMonth.

There. that should do it. You end up with the ability to navigate back a month, when you shouldnt be able to.

EDIT:

So I went to your demo page to see what was and wasnt happening. And i'm seeing that self.viewStartDate gets set to vsd.date(1) on line 58.

I'll be honest, I remember seeing the issue and having to write this bug. But seeing self.VSD get set to date(1)... i'm wondering what happened. And since this was over 4 months ago and I havent worked on the project. I have no idea. I cant focus on it any more at the moment. And now cause of that there are contextual errors w/ my code above.

IS there any possibility that this.viewStartDate on line 530 wouldnt have been the first date of the month?