Styles should merge (or do they?)

[chekoopa]

Current Flame's behaviour on styles is overwriting, so the latter style or even styleAttr property only matters. On other hand, Hedwig and Elm merge style attributes, giving us a single style list.

So, we should have that too. The most rational implementation is utilizing style module of Snabbdom. It also would enable us delayed, destroy and remove properties for basic animation features (not transitions, it would require more tinkering). And also, a challenge about CSS variables (such as --colorPrimary).

The only concerns are:

• styleAttr behaviour on module-based styles (would it still overwrite these styles?)
• static style rendering, like in tests

I'm on trying style module implementation. The current test case for this feature looks like:

test "style merging" do
        let html = HE.a [ HA.style { mystyle: "test", mylife: "good" }, HA.style { mystyle: "check" } ] [HE.text "TEST"]
        html' <- liftEffect $ FRS.render html
        TUA.equal """<a style="mystyle:check;mylife:good">TEST</a>""" html'

        let html2 = HE.a [HA.style { width: "23px", display: "none" }, HA.style { height: "10px" } ] [HE.text "TEST"]
        html2' <- liftEffect $ FRS.render html2
        TUA.equal """<a style="width:23px;display:none;height:10px">TEST</a>""" html2'

[chekoopa]

Damn it. :(

/nix/store/.../node_modules/snabbdom/modules/style.js:4
var raf = (typeof window !== 'undefined' && (window.requestAnimationFrame).bind(window)) || setTimeout;
                                                                           ^

TypeError: Cannot read property 'bind' of undefined
    at Object.<anonymous> (/nix/store/.../node_modules/snabbdom/modules/style.js:4:76)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/home/user/projects/purescript-flame/output/Flame.Renderer/foreign.js:7:2)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
[error] Tests failed: exit code: 1

UPD: a dirty hack into Renderer.js resolves this, though I'm not sure about that going into production.

if (typeof window.requestAnimationFrame === "undefined") {
    window.requestAnimationFrame = setTimeout;
}

[easafe]

Hi @chekoopa ,

On other hand, Hedwig and Elm merge style attributes, giving us a single style list.

Is there a reason why the style attribute has special treatment? I think all the attributes/properties are overwritten -- is this a source of bugs, or?

styleAttr behaviour on module-based styles (would it still overwrite these styles?)

I am not sure what this means.

static style rendering, like in tests

Is this related to string rendering?

For the error in the tests, I am guessing you can add requestAnimationFrame to the global object in test/Main.js.

[chekoopa]

Is there a reason why the style attribute has special treatment?

Yes, sometimes we need join some styles, like overall text style (somebody likes putting it under a distinct property textStyle = HA.style { ... }) and some additional styles based on the model, like availability, so we have { opacity = if value then "1" else "0.5" }, which we need to append somehow.

I think all the attributes/properties are overwritten -- is this a source of bugs, or?

Singleton attrs are OK. Singleton styles are OK if they are static and singleton. But when we need to calculate them dynamically and merge, it sure is a nest. We can get away with writing own boilerplate code to join styles and putting it into styleAttr, but there are many way to shoot ours own leg.

The biggest caveat here is homogeneous records usage. They are native and easier for basic usage, than tuple arrays, but when we need to join styles, we can only rely on purescript-record with merge function, which is left-biased.

Also, same story can be applied to classes, but (oh boy, another tight bind to our VDOM library), but not that critical. But wait, current implementation already uses a compatible type,

Actually, the most lazy option is to explictly state that these particular properties don't merge, but that might be an impact on UX.

styleAttr behaviour on module-based styles (would it still overwrite these styles?)

I am not sure what this means.

I'm talking about how styleAttr will behave with styles put in separate property of VNodeData. I'll experiment with that.

Is this related to string rendering?

Yes. BTW, it works, but inserts spaces after colons and semicolons. I'm not sure whether we can't implement style value generation by ourselves, but we'll surely stumble on styleAttr.

In "VNode creation":
  In "style/class name case":
    Error: expected "<element style=\"border-box:23;s:34;border-left-top-radius:20px\"></element>", got "<element style=\"border-box: 23; s: 34; border-left-top-radius: 20px\"></element>"

For the error in the tests, I am guessing you can add requestAnimationFrame to the global object in test/Main.js.

Thanks, done that.

[easafe]

I get it now, thanks. Besides extending VNodeData, what do you propose changing if we were to leverage snabbdom's style module?

[chekoopa]

Actually, style module isn't even API-breaking, all changes are done under the hood. I'm setting PR up right now.

And, yes, I've tried in REPL, style property overrides styleAttr.

I've had a little doubt about style resetting, but we still can somehow reproduce old behavior if we need it.