karax icon indicating copy to clipboard operation
karax copied to clipboard

StyleAttr.cssFloat should use the float css property name

Open jfilby opened this issue 3 years ago • 5 comments

When I use StyleAttr.cssFloat the resulting CSS style name is css-float when it should be float.

jfilby avatar Feb 18 '21 06:02 jfilby

Hm, with

include karax / prelude
import karax/vstyles

proc createDom(): VNode =
    result = buildHtml(tdiv(style = style((StyleAttr.cssFloat, kstring"right")))):
        text "Hello World!"

setRenderer createDom

I get

<div id="ROOT" style="float: right;">Hello World!</div>

on the current master branch of Karax. However, if you are using it just to generate HTML, then this might be a bug as https://www.w3schools.com/jsref/prop_style_cssfloat.asp says cssFloat is only used for the JS property to set it, despite the actual name being float. So when using the Karax DSL directly on the non JS backend, I get

<div style="css-float: right; ">Hello World!</div>

which seems to match your issue. Not sure what the best fix here is. An easy one is just a hack to check and see which backend is being used, and translate accordingly. My guess is that this is the only CSS property with this issue, I glanced through the other ones that Karax provides and those seemed fine.

Suggested workaround:

proc createDom(): VNode =
    result = buildHtml(tdiv(style = "float: right;".toCSS)):
        text "Hello World!"

It is possible to directly set the style using Karax.

ajusa avatar Apr 11 '21 01:04 ajusa

Yes, I'm using Karax to generate HTML in this case.

jfilby avatar Apr 11 '21 03:04 jfilby

@araq is there any situation where StyleAttr is preferable to toCss (introduced in https://github.com/karaxnim/karax/pull/158) ? toCss is easier to use IMO because it's easier to go back and forth between css vs karax (and doesn't have this issue);

if there's no such situation, should we migrate the code in karax to use toCss and promote it in docs?

timotheecour avatar May 17 '21 08:05 timotheecour

Can't remember, but what I do remember: The style handling code is most performance critical and can easily dominate the performance profile in a real-world app, not just in benchmarks. You probaly need to make toCss a compiletime operation.

Araq avatar May 17 '21 08:05 Araq

You probaly need to make toCss a compiletime operation.

then maybe it's time to merge https://github.com/nim-lang/Nim/pull/15528 (const now works with ref types) ? There are so many use cases for it (including table at CT, jsonNode at CT etc);

(so that proc toCss*(a: static string): VStyle overload can be defined)

(note that addressable const should not use const but let a {.rom.} = [1,2,3], so it doesn't clash with https://github.com/nim-lang/RFCs/issues/257 in any way; see https://github.com/timotheecour/Nim/issues/553 for some pre-RFC)

timotheecour avatar May 17 '21 08:05 timotheecour