glhd / aire

Modern form builder for Laravel
https://airephp.com
MIT License
542 stars 36 forks source link

defaultValue does not work with checkbox elements #93

Closed Bbansjkl closed 3 years ago

Bbansjkl commented 3 years ago

Describe the bug If the function defaultValue is called on a checkbox-element, the element's checked-attribute is not set, instead the value attribute receives the provided default value, which does nothing.

Have you published the Aire config file? Yes

Have you added custom Aire views? No

What version does this affect?

To Reproduce Steps to reproduce the behavior:

  1. Create a checkbox element with the following code: Aire::checkbox('test', 'test')->defaultValue(true)
  2. Open the view where said element is used
  3. Note that the checkbox is not checked
  4. Inspect the checkbox element with browser's debug-tools
  5. Note that instead the "value" attribute is set to "1"

Expected behavior If a default value is provided for a checkbox element via defaultValue-method, the checkbox's checked-attribute should be set according to the value provided in said method.
If it is intentional that the defaultValue does not work properly on checkboxes, the documentation should be updated to reflect this and the function removed from the Galahad\Aire\Elements\Checkbox-class.

inxilpro commented 3 years ago

For checkboxes you need to use defaultChecked() rather than defaultValue()

Bbansjkl commented 3 years ago

Out of curiosity, what is the purpose of the "defaultValue" function on checkbox elements then? I read up a bit more on the value attribute and I don't really see a reason to use defaultValue over value on them.

Also, shouldn't the "defaultChecked" function be mentioned in the documentation then? I read the entire website before making this issue to see if it was adressed somewhere, and re-reading it and later searching each page for defaultC didn't return any result.

inxilpro commented 3 years ago

Checkboxes just inherit the method because they have a value. I get that it's confusing. Maybe we can change the behavior in a future major release.

I have added defaultChecked() to the docs. Thanks!