Skip to content

Form.Validator.test now should recognize the ignoreValidation flag. #242

Closed
wants to merge 1 commit into from

6 participants

@kiyoto
kiyoto commented May 12, 2011

It was looking at the data-validators field, although ignoreField adds 'ignoreValidation' to the field's class.

kiyoto Form.Validator.test looks for 'ignoreValidation' in a field's class.
It was looking at the data-validators field, although ignoreField
adds 'ignoreValidation' to the field's class.
fe5c81a
@arian
MooTools member
arian commented May 12, 2011

It should support both. I think ignoreField should add this to data-validators.

@kiyoto
kiyoto commented May 12, 2011

arian: the only place in the code where ignoreValidation is added to an element is in the Form.Validator.ignoreField function, where it is added through the 'addClass' method. Also, in Form.Validator.Inline.js, looking at the getAllAdviceMessages method, it is only testing if the field has 'ignoreValidation' in its class. In my opinion, it is more consistent just to change it in Form.Validator.test as done in my patch.

@anutron
MooTools member
anutron commented May 17, 2011

The main issue for not doing both here is the backwards compat that we offer in supporting the data- attrib and the class name style for validators. As I wrote to Kiyoto:

If you look at line 71, you'll see the getter for validators looks at data-validators first and then at the class name if that property isn't set. The issue here is that we deprecated using classes in favor of a data- property. This introduces a complexity that presents a problem.

If the developer using the class uses class names, this will work because get('validators') will not find a data-validators property. If they are using the data-validators property it'll get ignored. If we change the method to use the data-property and the developer uses the class names, we'll have the opposite effect, our data-property will be found and the classes ignored.

As I see it, there's only one solution here. Line 246 should not check for hasValidator(field, 'ignoreValidation') but instead should check if it has that class. There's no other place in the class where we are writing to the list of what validators are in use. We shouldn't.

Arian, do you object?

@arian
MooTools member
arian commented May 17, 2011

No I agree, do you wanna pull this?

@anutron
MooTools member
anutron commented Jun 5, 2011

sorry, missed this comment; I'll pull it.

@arian
MooTools member
arian commented Jul 31, 2011

@anutron: did you fix/pull this already?

@restlessdesign

Bump? :suspect:

@kulbakin
kulbakin commented Sep 8, 2013

adding my voice for the need of a fix for ignoreField

@SergioCrisostomo
MooTools member
@anutron
MooTools member
anutron commented May 1, 2014

Apparently, I suck. Pulling now.

@anutron
MooTools member
anutron commented May 1, 2014

Couldn't pull from this repo as it's no longer there. Pushed it:

anutron@f25930a

@anutron anutron closed this May 1, 2014
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.