Use the proper boolean attributes in HTML form tags. #1598
Conversation
| if field.param == checked_value | ||
| html_options = merge_options(html_options, {"checked" => "true"}) | ||
| attrs = attrs | [:checked] |
There was a problem hiding this comment.
this one could technically call <<, but I just kept it consistent with the other forms in this file.
| @@ -80,7 +81,7 @@ module Lucky::InputHelpers | |||
| def checkbox(field : Avram::PermittedAttribute(Bool), attrs : Array(Symbol), **html_options) : Nil | |||
| unchecked_value = "false" | |||
| if field.value | |||
| html_options = merge_options(html_options, {"checked" => "true"}) | |||
| attrs = attrs | [:checked] | |||
There was a problem hiding this comment.
Can't use << here because arguments are passed by reference. Which means that when we pass EMPTY_BOOLEAN_ATTRIBUTES, it would pass that to other methods causing ALL radios/checkboxes to be marked as checked
| @@ -211,6 +212,7 @@ module Lucky::InputHelpers | |||
| "value" => input_value(field), | |||
| }.merge(input_overrides) | |||
| update_array_id_counter!(field) | |||
| attrs.uniq! | |||
There was a problem hiding this comment.
just in case so you don't accidentally send over duplicate attrs...
There was a problem hiding this comment.
HTML is pretty fast and loose with it's attributes, especially in the age of React and other front end frameworks which make up their own elements...are we sure there is no use case for passing the same attribute more than once to a tag?
There was a problem hiding this comment.
This would only prevent a tag being rendered like <input ... checked checked required required />. I'm sure browsers would just ignore it, so it's probably not required, but in the case of some random browser that breaks due to it, I figured why not? 🤷♂️
Purpose
Fixes #1560
Description
We had been using
checked="true"orselected="true"which according to these docs is invalid.Checklist
crystal tool format spec src./script/setup./script/test