elm-explorations / test

Write unit and fuzz tests for Elm code.
https://package.elm-lang.org/packages/elm-explorations/test/latest
BSD 3-Clause "New" or "Revised" License
236 stars 40 forks source link

Don't render boolean attributes with a False value to match behavior of HTML property false #212

Closed dillonkearns closed 1 year ago

dillonkearns commented 1 year ago

I believe I found an error in the HTML to String code for boolean attributes here:

https://github.com/elm-explorations/test/blob/2bc02f7405622a08215c2acb4303a083d056d0aa/src/Test/Html/Internal/ElmHtml/ToString.elm#L137-L141

In Elm's virtual DOM code, boolAttribute is setting a property (not an attribute) of a boolean, true or false: https://github.com/elm/html/blob/94c079007f8a7ed282d5b53f4a49101dd0b6cf99/src/Html/Attributes.elm#L167-L169.

However, rendering these boolean attributes as key="true" or key="false" is not equivalent to these boolean properties.

For example, take the HTML attribute hidden. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden

Setting <div hidden>, <div hidden="true">, and <div hidden="false"> will all result in a hidden div. "false" has no special meaning for this attribute, and in fact there is no way for the hidden attribute to be present yet set to the equivalent of the property false. It needs to be absent to get that behavior.

You can see this in action in the live code demo in the MDN hidden example: https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden. If you try changing the <p hidden> <p> you will see that the element becomes visible, whereas changing it to either <p hidden="true"> or <p hidden="false"> results in it being hidden.

So I believe this code should be changed to this:

boolAttributes =
    Dict.toList facts.boolAttributes
        |> List.filterMap
            (\( k, v ) ->
                if v then
                    Just k

                else
                    Nothing
            )
        |> String.join " "
        |> Just

While this code isn't exactly the same, since the underlying Elm code for these boolAttributes uses properties this is the only way to represent that when rendering to a String since HTML properties only exist in JavaScript-land™️, not HTML-string-land™️.