Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Remove clearing as numbers of `min`, `max` and `range` rules #528

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
3 participants
Contributor

hazzik commented Oct 12, 2012

These rules are fully suitable to check any comparable JavaScript objects such as strings or dates. Also clearing prevents HTML5 date input fields to work when these rules are coming from HTML (fixes #455).

Collaborator

jzaefferer commented Nov 23, 2012

Thanks for the contribution. Could you extend the unit tests to address #455?

Contributor

hazzik commented Nov 26, 2012

Yes, sure, but a little bit later, as now I do not have a good internet connection. Sorry for the delay

Collaborator

jzaefferer commented Nov 26, 2012

Well, at least you can work offline with git :-)

Contributor

hazzik commented Nov 26, 2012

... if I have cloned copy of reposirory...

hazzik added some commits Nov 29, 2012

@hazzik hazzik Add tests for `min` and `max` html attributes with dates (#455) 9e48f7b
@hazzik hazzik Add tests for `min` and `max` html attributes with strings (#455) f9d957b
@hazzik hazzik Remove clearing as numbers of `min`, `max` and `range` rules (#455)
These rules are fully suitable to check any comparable JavaScript objects such as strings or dates. Also clearing prevents HTML5 date input fields to work when these rules are coming from HTML (#455).
7ad1adf
Contributor

hazzik commented Nov 29, 2012

Ok, I've added tests, but some of them are failing in Safary and Node.js (as I understand on travis-ci tests are running by Node.js), It happens because Safary (and Node.js, I think) think that '2012-12-21' is incorrect date format (new Date('2012-12-21') returns incorrect date) and validation fails for that value.

Collaborator

jzaefferer commented Dec 2, 2012

Can we replace that with a value that works for those browsers as well? As for Travis, that's running tests using PhantomJS, which is comparable to Safari. You can also run that locally, see https://github.com/jzaefferer/jquery-validation/blob/master/CONTRIBUTING.md

@nschonni nschonni referenced this pull request in wet-boew/wet-boew Dec 3, 2012

Closed

Form validation - input date - min and max issue #799

Collaborator

jzaefferer commented Jan 31, 2013

I tried replacing the dashes with slashes, e.g. 2012/11/21. Got it to pass in Safari that way, but now its failing in Chrome. I guess anything relying on the crippled new Date constructor is prone to failure.

Not sure yet how to deal with that. Focus on #581 (using Globalize)? Merge the fix without the tests? @nschonni @hazzik looking for your input on this.

Collaborator

nschonni commented Jan 31, 2013

I don't have Safari to test on, what happens when you pass an ISO date to the constructor in Safari?

Shot it the dark: an approach like what crockford did https://github.com/douglascrockford/JSON-js/blob/master/json.js#L211-L222 then go back to the regex on the string.

Collaborator

jzaefferer commented Jan 31, 2013

> new Date("2012-11-20")
Invalid Date

then go back to the regex on the string.

I'm not following.

Collaborator

nschonni commented Jan 31, 2013

Sorry, I miss understood the Safari Issue. So I'm guess it's failing on this line https://github.com/hazzik/jquery-validation/blob/7ad1adf78cb89bbf793f066928cdacbcd4036749/jquery.validate.js#L1097

Nothings coming to mind that doesn't involve a browser sniffing hack that would replace the dash with a slash for safari before testing.

Contributor

hazzik commented Feb 1, 2013

> new Date("2012-11-20T00:00:00Z")
Tue Nov 20 2012 13:00:00 GMT+1300 (New Zealand Daylight Time)

Adding T00:00:00Z solves the problem, but I dont know how to integrate this

@jzaefferer jzaefferer closed this in 69831a0 Feb 4, 2013

Collaborator

jzaefferer commented Feb 4, 2013

I've solved this by making these inputs type="text", instead of "date". That way the data method is skipped and the tests pass.

The date method is still pretty much broken. Unless someone wants to provide a better implementation, I'll defer to #581 to address that.

@hazzik hazzik deleted the unknown repository branch Feb 4, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment