Knockout-Validation icon indicating copy to clipboard operation
Knockout-Validation copied to clipboard

Problems with .isValid() on required checkboxes

Open krnlde opened this issue 9 years ago • 13 comments
trafficstars

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)
  }]
})

krnlde avatar Dec 22 '15 18:12 krnlde

Can I consider this project dead when, after 2 weeks, nobody answers or even just labels this issue?

krnlde avatar Jan 05 '16 15:01 krnlde

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 )

ichepurnoy avatar Jan 06 '16 12:01 ichepurnoy

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.

ichepurnoy avatar Jan 06 '16 16:01 ichepurnoy

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 avatar Jan 06 '16 18:01 krnlde

@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?

crissdev avatar Jan 06 '16 20:01 crissdev

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 avatar Jan 06 '16 21:01 krnlde

@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/

crissdev avatar Jan 06 '16 22:01 crissdev

Alright thanks, I'll do a PR. Should I also build the dist?

krnlde avatar Jan 07 '16 10:01 krnlde

Any news here?

krnlde avatar Jan 22 '16 14:01 krnlde

Please check your PR. It seems the tests are falling.

crissdev avatar Jan 22 '16 14:01 crissdev

Not anymore.

krnlde avatar Jan 22 '16 15:01 krnlde

Looks good. Thanks.

crissdev avatar Jan 22 '16 15:01 crissdev

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

crushjz avatar Jul 25 '17 09:07 crushjz