Twipped / Kalendae

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

'Today' does not respect current timezone #90

Closed danhodos closed 11 years ago

danhodos commented 11 years ago

Hello! I noticed this problem with Kalendae's notion of what today is, and wanted to bring it up here.

Steps to reproduce:

Expected behavior: the day indicated as today (underlined, with the CSS class of k-today) is today (in my case: Wed, May 29, 2013).

Actual behavior: the day indicated as today (underlined, with the CSS class of k-today) is tomorrow (in my case: Thu, May 30, 2013).

roryf commented 11 years ago

The build on http://chipersoft.com/Kalendae/ is v0.2 which is out of date, should re-test this with latest from repo.

magnars commented 11 years ago

This is a problem for us too. In 0.4.1.

yogendra689 commented 11 years ago

Date should change according to System timezone. Jquery Datepicker changes according to System timezone. This is not happening in Kalendae js.

tagliala commented 11 years ago

The date is taken from system timezone but there is a strange calculation of "dayyear" that is rounded up to the next day under certain circumstances.

I suppose the best way to do it is always convert time to UTC at midnight. I was making some tests but I wasn't able to find a fast fix

yogendra689 commented 11 years ago

I fixed it using a variable .. check_today = moment(new Date()).format("YYYY-MM-DD"); and using condition if (day.format("YYYY-MM-DD") === check_today) klass.push(classes.dayToday);

This gives me local date according to system timezone. This solved my problem. Now it is showing correct local date as underlined.

yogendra689 commented 11 years ago

I think problem is with dayyear() function, it extracts current date using epoch. I think in this case we get current UTC date instead of local date

tagliala commented 11 years ago

@yogendra689 you should be right. I'm going to take a look right now

Twipped commented 11 years ago

@tagliala If you find a solution for this, go ahead and push out a new release.

tagliala commented 11 years ago

@ChiperSoft sure. I just created another branch with a fix.

Maybe when you started Kalendae there was no startOf('day') function in moment.js so you had to write your own one? (stripTime)

@yogendra689 @danhodos @magnars please download the build from timezone-fix branch and check if it's ok for you. Please also check other features (blackout, directions).The index page seems fine

Twipped commented 11 years ago

Yup, that's exactly what happened. That's good, sounds like we can remove some stuff from the moment extensions.

magnars commented 11 years ago

I've pushed a new version of our app with the timezone-fix branch. Will report back.

tagliala commented 11 years ago

@ChiperSoft sorry I'm not being so useful here. Can I merge the timezone-fix branch or it was already merged?

@magnars any issue to report?

Twipped commented 11 years ago

Go ahead and merge it, I think we can assume silence is acceptance.

magnars commented 11 years ago

No complaints after installing this patch.

On 24. okt. 2013, at 16:45, Geremia Taglialatela notifications@github.com wrote:

@ChiperSoft sorry I'm not being so useful here. Can I merge the timezone-fix branch or it was already merged?

@magnars any issue to report?

— Reply to this email directly or view it on GitHub.

tagliala commented 11 years ago

@ChiperSoft merged in my local repository. I think we should bump the version number to 0.5.0 instead of 0.4.2 because of #112 and #111

Can I proceed?

Twipped commented 11 years ago

Sounds good, go ahead.