Y-m-d #34
Y-m-d #34
Conversation
|
Thank you for your collaboration, before I dd the merge, please add proper testing to your PR (do the changes on your branch and do a push to update in here). If you don't know how to start please look at https://github.com/hilios/jQuery.countdown/blob/master/test/countdown_test.js#L237 |
|
Is this what you meant? :) |
|
Yes it is goo job, but something went wrong, because the test are failing.... https://travis-ci.org/hilios/jQuery.countdown/builds/18123302 |
|
Do you have any idea why isn't it working? As I can see the regexp matches the given strings and it is working fine in my implementation. :S |
|
Indeed, but seens the PhantomJS (a Webkit headless browser) can't parse the 2014-02-03 Barta Ákos notifications@github.com:
Edson Hilios |
|
Hey man, did you gave up on this? I was looking that at this issue and this is the actual behaviour on Qt Webkit (used in Chrome Canary, Safari, iOS between others). So the approval of this PR would cause the script breaks in browsers that use it. But fear not, your request is still possible you just need to add a RegExp to replace the dash char What you think @bartaakos? Can we finish this PR ;) |
| @@ -31,7 +31,7 @@ | |||
| var instances = [], matchers = []; | |||
| matchers.push(/^[0-9]*$/.source); | |||
| matchers.push(/([0-9]{1,2}\/){2}[0-9]{4}( [0-9]{1,2}(:[0-9]{2}){2})?/.source); | |||
| matchers.push(/[0-9]{4}(\/[0-9]{1,2}){2}( [0-9]{1,2}(:[0-9]{2}){2})?/.source); | |||
| matchers.push(/[0-9]{4}([\/\-][0-9]{1,2}){2}( [0-9]{1,2}(:[0-9]{2}){2})?/.source); | |||
There was a problem hiding this comment.
At line #29 you could just add a conditional statement in parseDate function to replace dashes to slashes, simple like:
if(String(dateString).match(matchers)) {
// If looks like a millisecond value cast to number before
// final casting (Thanks to @msigley)
if(String(dateString).match(/^[0-9]*$/)) {
dateString = Number(dateString);
}
// Replace dashes to slashes
if(String(dateString).match(/^[0-9]*$/)) {
dateString = dateString.replace(/\-/g, '/');
}
return new Date(dateString);
}
|
Hi, Sorry for not replying for ages :/ So basicly you say, that we can get a workaround by simply replacing |
|
That's it @bartaakos, if everything is green in Travis this PR will LGTM and will be merge right away! |
|
I just got back to my computer and pushed in the changes :) |
|
Finally :) |
|
Great job, you are now a TDD master!!! |
|
Well I stayed quite long in the red but.. :D |
|
Oh btw I didn't update the .min file because I didn't know which minifier did you use. |
|
It's all bundled in the A little bit bellow at line #106 you can see that I have a |
|
But you don't need to worry by now, once I will do a proper release with tag |
|
Okay, cool :) |
I added support for Y-m-d (dashed) date format for parseDateString matchers