Knockout-Validation
Knockout-Validation copied to clipboard
Problems with .isValid() on required checkboxes
Hey guys! I experienced some problems when making checkboxes required (for example terms and conditions).
See here for reference: http://jsfiddle.net/v6d5y3or/
Expected behaviour: Form .isValid() should reflect the checkbox value: checkbox.true = form.true and checkbox.false = form.false
Actual behaviour: Form .isValid() is always true: checkbox.true = form.true and checkbox.false = form.true
I'm not really sure why this happens and one would think that required: true works together with the checked binding.
A temporary solution to me is to implement a custom validator to make it work as expected:
ko.observable(false).extend({
validation: [{
validator: (value, params) => Boolean(value)
}]
})
Can I consider this project dead when, after 2 weeks, nobody answers or even just labels this issue?
my best guess is, you have misused ko.validatedObservable. Namely, you did not extend it. Please see this example. http://jsfiddle.net/ichepurnoy/evxca44L/ I'm no pro, just trying to figure things out, and it's a pity that the docs not contain many things like this, that are already in the library. If i'm right, pls , try to use correct syntax for validatedObservable. Maybe get it from library itself. The project's not dead, and if it even was, we are pretty much stuck with it, as I see no replacement for it currently )
I now realised that you have used validatedObservable like in 'Getting Started' .. But check inside the library, maybe docs mislead you on this. Another thing, check http://jsfiddle.net/ichepurnoy/7brjnewm/, I replaced your input with input type = text. It works (!), although I also had to change the way the viewmodel is instantiated. It may be something in knockout itself (not ko.validation plugin), or the way its 'checked' binding works.
I extended the VM correctly in the jsfiddle above. Also I know it works on a text input - but not on a checkbox/radio. That's why I started this issue.
@krnlde The required rule converts the input value to a string and checks its length. For a boolean value the resulting string will always have a length greater than 0, thus making the validation to always pass. (https://github.com/Knockout-Contrib/Knockout-Validation/blob/2.0.3/src/rules.js#L45)
A simple fix would be to check for false to skip the conversion to string (that is a change to https://github.com/Knockout-Contrib/Knockout-Validation/blob/2.0.3/src/rules.js#L27).
Do you want to make a PR to fix this?
But L27 is a shortcut and will return with false if required === true. I'd rather prefer to do another type check as in L32:
if (typeof (val) === 'boolean'`) return val;
What do you think @crissdev
@krnlde Adding false to the existing checks will fail validation if the field is required - which is the desired behaviour. http://jsfiddle.net/v6d5y3or/2/
Alright thanks, I'll do a PR. Should I also build the dist?
Any news here?
Please check your PR. It seems the tests are falling.
Not anymore.
Looks good. Thanks.
Hello,
I don't think that the value false should be treated as null or undefined .
Example: I have two radio buttons: "Yes" and "No". Selecting yes, the underlying observable is set to true, selecting no, is set to false, and with no selection, the observable is set to null.
The form is valid when the value is either true or false, but invalid when it's null.
With your change the form is invalid even if I have selected No, because the observable is set to false.
Here is the JSFiddle: https://jsfiddle.net/989d6tbb/
Let me know your thoughts.
Thank you