Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

StyleAttr.cssFloat should use the float css property name #187

Open
jfilby opened this issue Feb 18, 2021 · 5 comments
Open

StyleAttr.cssFloat should use the float css property name #187

jfilby opened this issue Feb 18, 2021 · 5 comments
Labels

Comments

@jfilby
Copy link

jfilby commented Feb 18, 2021

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

@ajusa
Copy link
Collaborator

ajusa commented Apr 11, 2021

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.

@jfilby
Copy link
Author

jfilby commented Apr 11, 2021

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

@ajusa ajusa added the bug label May 2, 2021
@timotheecour
Copy link
Collaborator

@Araq is there any situation where StyleAttr is preferable to toCss (introduced in #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?

@Araq
Copy link
Collaborator

Araq commented May 17, 2021

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.

@timotheecour
Copy link
Collaborator

timotheecour commented May 17, 2021

You probaly need to make toCss a compiletime operation.

then maybe it's time to merge nim-lang/Nim#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 nim-lang/RFCs#257 in any way; see timotheecour/Nim#553 for some pre-RFC)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants