17.0.4 generated class name is not added when there's also a set class name

tesk9 commented 2 years ago

(Example available at https://ellie-app.com/gnGZhWfMXcMa1.)

When there's just css, the styles work as expected:

import Css exposing (..)
import Html.Styled exposing (..)
import Html.Styled.Attributes exposing (class, css)

main =
    div
        [ css
            [ height (Css.px 100)
            , width (Css.px 100)
            , backgroundColor (rgb 100 0 0)
            , color  (rgb 100 0 0)
            ]
        ]
        [ text "This text should be hidden!" ]
        |> toUnstyled

And the generated class name is attached to the element:

However, when a class attribute is also added,


import Css exposing (..)
import Html.Styled exposing (..)
import Html.Styled.Attributes exposing (class, css)

main =
    div
        [ css
            [ height (Css.px 100)
            , width (Css.px 100)
            , backgroundColor (rgb 100 0 0)
            , color  (rgb 100 0 0)
            ]
        , class "some-custom-class"
        ]
        [ text "This text should be hidden!" ]
        |> toUnstyled

The generated class name is not attached to the element:

Which results in the styles not being applied.

rtfeldman commented 2 years ago

@Confidenceman02 - any other ideas besides the class approach in https://github.com/rtfeldman/elm-css/pull/562?

tesk9 commented 2 years ago

@Arkham and I noticed that :

Html.Styled.Attributes.class currently uses property to set the "className".

If manually use Html.Styled.Attributes.attribute "class" instead, the class is correctly included in the rendered code: https://ellie-app.com/gnJc3ttWvnga1

Should we change the class implementation to use an attribute instead?

rtfeldman commented 2 years ago

Worth a try! It might break other things, but at this point it seems to be down to finding these things the hard way. 😅

tesk9 commented 2 years ago

I started working on a PR to implement this, and found another problem when I went to add tests to Styled.elm

bug564 : Test
bug564 =
    describe "Github Issue #564: https://github.com/rtfeldman/elm-css/issues/564"
        [ Test.test "generated class is included when there's no custom class" <|
            \_ ->
                div
                    [ css [ color (rgb 0 0 0) ]
                    ]
                    []
                    |> toUnstyled
                    |> Query.fromHtml
                    |> Query.has [ Selector.class "_5dc67897" ]
        , Test.test "custom class is included when there's no generated class" <|
            \_ ->
                div [ class "some-custom-class" ]
                    []
                    |> toUnstyled
                    |> Query.fromHtml
                    |> Query.has [ Selector.class "some-custom-class" ]
        , Test.test "custom class is included as well as generated class" <|
            \_ ->
                div
                    [ css [ color (rgb 0 0 0) ]
                    , class "some-custom-class"
                    ]
                    []
                    |> toUnstyled
                    |> Query.fromHtml
                    |> Query.has [ Selector.classes [ "_5dc67897", "some-custom-class" ] ]
        ]

Before I made any other changes, I expected only the test "custom class is included as well as generated class" to fail. However, "generated class is included when there's no custom class" also fails!

I believe that elm-explorations/test only supports testing properties, not attributes.

I made a quick test module with elm/html demonstrating the behavior:

module Example exposing (..)

import Html exposing (..)
import Html.Attributes exposing (attribute, class, property)
import Json.Encode
import Test exposing (..)
import Test.Html.Query as Query
import Test.Html.Selector as Selector

suite : Test
suite =
    describe "class attribute assertions"
        [ describe "Using Html.Attribute.class" <|
            classAssertions <|
                div [ class "some-custom-class" ] []
        , describe "Using Html.Attribute.property" <|
            classAssertions <|
                div [ property "className" (Json.Encode.string "some-custom-class") ] []
        , describe "Using Html.Attribute.attribute" <|
            classAssertions <|
                div [ attribute "class" "some-custom-class" ] []
        ]

classAssertions : Html msg -> List Test
classAssertions html =
    [ test "Test.Html.Selector.class finds the class" <|
        \_ ->
            html
                |> Query.fromHtml
                |> Query.has [ Selector.class "some-custom-class" ]
    , test "Test.Html.Selector.classes finds the class" <|
        \_ ->
            html
                |> Query.fromHtml
                |> Query.has [ Selector.classes [ "some-custom-class" ] ]
    , test "Test.Html.Selector.exactClassName finds the class" <|
        \_ ->
            html
                |> Query.fromHtml
                |> Query.has [ Selector.exactClassName "some-custom-class" ]
    ]

With these tests, these are my results:

Running 9 tests. To reproduce these results, run: elm-test --fuzz 100 --seed 267504658862962

↓ Example
↓ class attribute assertions
↓ Using Html.Attribute.attribute
✗ Test.Html.Selector.class finds the class

    ▼ Query.fromHtml

        <div class="some-custom-class">
        </div>

    ▼ Query.has [ class "some-custom-class" ]

    ✗ has class "some-custom-class"

↓ Example
↓ class attribute assertions
↓ Using Html.Attribute.attribute
✗ Test.Html.Selector.classes finds the class

    ▼ Query.fromHtml

        <div class="some-custom-class">
        </div>

    ▼ Query.has [ classes "some-custom-class" ]

    ✗ has classes "some-custom-class"

↓ Example
↓ class attribute assertions
↓ Using Html.Attribute.attribute
✗ Test.Html.Selector.exactClassName finds the class

    ▼ Query.fromHtml

        <div class="some-custom-class">
        </div>

    ▼ Query.has [ attribute "className" "some-custom-class" ]

    ✗ has attribute "className" "some-custom-class"

TEST RUN FAILED

Duration: 121 ms
Passed:   6
Failed:   3

All of which is a long-winded way of saying: changing property -> attribute for class in elm-css will mean that class assertions using elm-explorations/test will no longer work.