Skip to content

Commit

Permalink
Fixed state of the checkbox if no value is provided
Browse files Browse the repository at this point in the history
  • Loading branch information
@mahagr
mahagr committed Dec 22, 2020
1 parent d73953c commit bff6b9d
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# v4.3.1
## mm/dd/2020

1. [](#bugfix)
* Fixed state of the checkbox if no value is provided

# v4.3.0
## 12/14/2020

Expand All @@ -8,7 +14,6 @@
1. [](#bugfix)
* Fix admin access check [#463](https://github.com/getgrav/grav-plugin-form/pull/463)


# v4.2.0
## 12/02/2020

Expand Down
2 changes: 1 addition & 1 deletion templates/forms/fields/checkbox/checkbox.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
name="{{ (scope ~ field.name)|fieldName }}"
value="{{ field.value ?? '1' }}"
type="checkbox"
{% if value == field.value ?? '1' %} checked="checked" {% endif %}
{% if value == (field.value ?? '1') %} checked="checked" {% endif %}

{# input attribute structures #}
{% block input_attributes %}
Expand Down

3 comments on commit bff6b9d

@NicoHood
Copy link
Contributor

@NicoHood NicoHood commented on bff6b9d Jan 8, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is still not correct when the form submission contains errors and the form is rendered again.

This code will set value to true. And the twig check true == 'fun' will always return true as well. So the whole statement is useless it seems, as simple check against true should be sufficient. Or maybe a is same as() check is required here?
https://github.com/getgrav/grav-plugin-form/blob/develop/classes/Form.php#L841-L843

agree_to_terms:
        type: checkbox
        label: "Agree to the terms and conditions"
        default: false
        value: 'fun'

Or maybe it would make sense to also patch the Form.php code to return the value instead of true. Because if this does not get patched, the same as check will fail.

Edit:
I think this template also need to be fixed:
https://github.com/getgrav/grav-plugin-form/blob/develop/templates/forms/default/data.html.twig#L29

@mahagr
Copy link
Member Author

@mahagr mahagr commented on bff6b9d Jan 11, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code doesn't set anything. It just marks checkbox to be checked if value equals field.value.

The issue is that #472 (review) hack breaks the logic in here as it basically means that checkboxes inside regular forms can not have any other values than true and false -- which is not true outside regular form. Basically that also makes your value: 'fun' to be non-valid expression.

@NicoHood
Copy link
Contributor

@NicoHood NicoHood commented on bff6b9d Jan 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the 'hack' is still good to have, but requires a fix to also support the value option. I know, that this template fix (this commit) is correct by itself, I just wanted to note, that the feature is still incomplete.

I've tracked it here real quick: #476

Please sign in to comment.