Issues with Selector.attribute and style

[ffigiel]

I'm trying to write a test checking for Selector.attribute "style" "...", but it seems to fail on me every time.

Test case

import Html
import Html.Attributes exposing (style)
import Test exposing (test)
import Test.Html.Query as Query
import Test.Html.Selector as Selector

case = test "Broken style attribute matching" <|
    \() ->
        Html.div
            [ style [ ( "width", "10px" ), ( "height", "10px" ) ] ]
            []
            |> Query.fromHtml
            |> Query.has [ Selector.attribute "style" "height:10px;width:10px" ]

The result (notice the lack of semicolon)

    ▼ Query.fromHtml

        <div style="height:10pxwidth:10px">
        </div>

    ▼ Query.has [ attribute "style" "height:10px;width:10px" ]

    ✗ has attribute "style" "height:10px;width:10px"

I tried writing a test with a single css property, but it fails as well...

    ▼ Query.fromHtml

        <div style="width:10px">
        </div>

    ▼ Query.has [ attribute "style" "width:10px" ]

    ✗ has attribute "style" "width:10px"

I hope I am doing something wrong here. What could be causing this?

[eeue56]

I don't think you want to be looking up style here. Try testing width or something instead.

[ffigiel]

Do you mean something like this? I wouldn't expect that to work because it's a different attribute than style, and it actually does fail.

|> Query.has [ Selector.attribute "width" "10px" ]

Do you have any ideas where the original error may be coming from? In the end, style should be a simple string attribute, right?

In my case, I could just use Expect.equal to compare the actual Html instances, but I'd like to figure this problem anyway

[eeue56]

I'll take a look at some time. I think what's happening is that the style function in elm-html doesn't do what you might expect it to do

[ffigiel]

A quick look at Html.Attributes.style led me to this part: https://github.com/elm-lang/virtual-dom/blob/master/src/Native/VirtualDom.js#L405-L413 It looks like the styles are being written directly into the dom node, so the style attribute is in fact missing.

That said, I think this should be handled somehow, because error message in the tests isn't really helpful. Should we handle the specific case where we see someone's using Selector.attribute "style" and fail instantly?

[knewter]

This issue is fascinating. I know that's not helpful, but it's true :)

[pablen]

Such a pity! :( Most packages dealing with inline styles would need to test style attribute. I think testing against text value of style attribute is not good though, it should test against a List (String, String).

Thanks for the great library anyway!

[rtfeldman]

Should we handle the specific case where we see someone's using Selector.attribute "style" and fail instantly?

I think there are three questions here.

1. Should it be possible to test what style an element has? (Seems like a clear Yes to me. A hasStyle selector which looks things up in Facts sounds useful.)
2. Should it be possible to separately test whether an element has had a style attribute set versus its style property changed? (I'm not convinced. On a real DOM node, if I set elem.styles.display = "none", that puts an entry in elem.attributes - there's no distinction there, so why should we make one in virtual DOM nodes?)
3. What should the behavior be when people write Query.has [ attribute "style" "width:10px" ]? (I could see an argument for translating Facts back into a style String and testing on that, but I think it would make for a better UX to (A) introduce Query.hasStyles and (B) have attribute "style" queries fail with a message that directs you to use Query.hasStyles instead.)

To sum up, the changes I propose to address this issue would be:

• Introduce Query.hasStyles which checks what styles have been specified in Facts
• Have attribute "style" queries automatically fail, with a message to use Query.hasStyles instead

Thoughts?

[rtfeldman]

Thinking about this more, I think there may be a better solution here:

Change attribute from taking a pair of strings to taking Attribute Never instead.

Current API

Query.has [ attribute "style" "width:10px" ]

Proposed API

Query.has [ attribute <| style [ ( "width", "10px" ) ] ]

Under the hood, this could be implemented as applying the given Attribute to a dummy div, then running through all the attributes/Facts on that div and checking to see if they are present on the target as well.

This way there's no need for another function, or for that matter an error message saying "use this other thing situationally instead, because what you reached for won't work the way you expect." There's one function and it Just Works, with no surprises.

As a bonus, this would mean a separate boolAttribute function would no longer be necessary, because you could do Query.has [ attribute (checked True) ] directly. Another bonus: for cases where you have your style defined in a separate variable, you could pass that into the test directly - e.g. Query.has [ attribute primaryButtonStyle ]

I'm inclined to go for this solution, but curious what others think

[eeue56]

I don't think that querying by style is objectively the right thing to be doing. Calculating computed styles is something that is best tested in real browsers. What is a valid use case for this?

[ffigiel]

The new API makes the previous Query.hasStyles look like a hack 😂 One drawback I can see is that we'll be relying on a (long) whitelist of supported attributes. But we could provide a backwards-compatible Attribute constructor like so:

Query.has [ attribute (miscAttribute "class" "foo")]

[ffigiel]

@eeue56 My use case was that I was writing a game which used inline styles (because of dynamic values). It seems nobody else ran into this issue since I reported it.

[eeue56]

@megapctr Why are you trying to test if something has styles, regardless of inline or not? This makes for an extremely brittle test suite.

[pablen]

@eeue56 My use case is that I'm writing a package that let you render toast notifications on the your view and I want to provide the user a way to define custom attributes for the list container and for individual items, to let her control things like positioning, animation and ARIA attrs however she wants (by adding classes or inline styles, etc).

[rtfeldman]

One drawback I can see is that we'll be relying on a (long) whitelist of supported attributes.

Why would that be? 🤔

I don't think there would need for a whitelist of supported attributes...any attribute that can be passed to an argument expecting Attribute Never should work fine as far as I can tell, including custom ones defined by Html.Attributes.attribute

[ffigiel]

@rtfeldman Oh, so the style function would come from Html.Attributes? I thought it would need to be imported from elm-html-test, but this is much better.

[rtfeldman]

That was my thought, yeah. More work to implement, but seems like it has several nice benefits - one of which would be making this issue moot.

[FRosner]

What is a valid use case for this?

I am testing a function that takes a boolean and generates an icon. Depending on the boolean this icon will be colored red or green. Since I am using Elm I don't need CSS but can abstract these things nicely, writing modular, testable and reusable code.

[eeue56]

A temp fix is in place in 4.1.0. API redesign is welcome, but also we have aworking solutionf or now.

[eeue56]

Should be fixed with this new PR: https://github.com/eeue56/elm-html-test/pull/35#issuecomment-315841519 published in 5.0.0

[andys8]

@eeue56 and @rtfeldman Looking at stlyes elm-css and elm-html-test are not playing well together, right? Is there a way to handle the inlined classes created by elm-css?

http://package.elm-lang.org/packages/eeue56/elm-html-test/latest#selecting-elements-by-html-attribute-msg-

Example

...
 |> toUnstyled
 |> Query.fromHtml
 |> Query.has [ attribute <| Attributes.style [ ( "font-size", "14px" ) ] ]

    ▼ Query.fromHtml

        <span class="_f3628a26">
            <style>
                ._f3628a26 {
            font-size: 14px;
            font-weight: lighter;
            line-height: 20px;
            border: 1px solid #009900;
            border-radius: 20px;
            padding: 5px 10px;
            margin-right: 10px;
        }
            </style>
            FINISHED
        </span>

    ▼ Query.has [ styles font-size:14px; ]

    ✗ has styles font-size:14px;