search

brainfoolong / form-data-json

A zero dependency, cross browser library to easily get or set/manipulate form input values as/from a json object.
https://brainfoolong.github.io/form-data-json
MIT License
60 stars 10 forks source link

setInputValue on checkboxes that have initial value false to true doesn't work. #3

Closed imp-dance closed 4 years ago

imp-dance commented 4 years ago

Describe the bug FormDataJson.setInputValue() seems to work like this in the case of checkboxes:

inputElement.checked = value === inputElement.value

So, if the initial value is false, then true === false would return false.

Expected behaviour

FormDataJson.setInputValue(input, true); should set the input value to true.

Actual behaviour

FormDataJson.setInputValue(input, true); sets the input value to false.

To Reproduce https://codepen.io/schart/pen/gOaYOGQ

Proposed fix

Changing line 57 from

inputElement.checked = value === inputElement.value

to

inputElement.checked = value

Is this here on purpose, or is it a bug?

brainfoolong commented 4 years ago

Thank you for your pull request. I came across this specific case in my tests and the current behaviour is exactly that what i wanted. It should only set the checkbox to true when the actual value is the same as the provided one.

The reason is simple: The data what you can get from .formToJson should set exactly the same with .fillFormFromJsonValues.

Imagine when you swap checkboxes in dom and they have no specific array key like name=ids[] than you definitely not want to set to true when the value doesnt match.

brainfoolong commented 4 years ago

Maybe special value like boolean true/false could work as check/uncheck in this case but i'm not sure to add different behaviour depending on the value.

imp-dance commented 4 years ago

Fair enough, I just figured I'd let you know incase the behaviour wasn't expected. I saw that there were no checkboxes in the demo either, so I though it might've slipped between the gaps.

brainfoolong commented 4 years ago

Nice catch. Thank you, i should add checkboxes to the demo as well. In the real TestSuite at https://brainfoolong.github.io/form-data-json/tests/test.html there are all possible inputs.