weavejester / hiccup

Fast library for rendering HTML in Clojure
http://weavejester.github.io/hiccup
Eclipse Public License 1.0
2.68k stars 174 forks source link

Allow keywords in style maps #140

Closed pesterhazy closed 2 years ago

pesterhazy commented 7 years ago

Reagent allows keywords as CSS attribute values such as

{:style {:color :red}}

Hiccup already supports this for top-level DOM node attributes. This commit extends this feature to style maps.

pesterhazy commented 7 years ago

I should mention the current behavior is to generate style="color::red"

davidperrenoud commented 5 years ago

@pesterhazy To match Reagent's convention, should integers be rendered as pixels by default?

For example:

(str (html [:div {:style {:margin 10}}]))

To:

<div style="margin:10px;"></div>

Note 1: this might be due to React's rendering engine and not Reagent's: https://reactjs.org/docs/dom-elements.html#style

Note 2: unitless values for incorrect attributes (such as margin) are often recognised as pixels anyway by browsers.

Note 3: some attributes do expect unitless values (such as line-height).

pesterhazy commented 5 years ago

Yes, I think so

On Fri 26. Jul 2019 at 09:32, David Perrenoud notifications@github.com wrote:

@pesterhazy https://github.com/pesterhazy To match Reagent's convention, should integers be rendered as pixels by default?

For example:

(str (html [:div {:style {:margin 10}}]))

To:

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/weavejester/hiccup/pull/140?email_source=notifications&email_token=AAAZ6WET5JLEEHR5OPJJJGLQBKR7BA5CNFSM4C4P3QPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD23YFKA#issuecomment-515343016, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAZ6WBNXKCSI6ZHQMNRAPTQBKR7BANCNFSM4C4P3QPA .

metasoarous commented 4 years ago

@weavejester Is there a reason not to merge this? This would be very helpful for me with Oz.

weavejester commented 4 years ago

This PR seems to have fallen through the cracks. It should be fine to merge, but the commit message body should be removed, as I think the subject is sufficient explanation.

metasoarous commented 4 years ago

Thanks @weavejester! Do you need @pesterhazy to edit the commit history before it gets merged then?

shhivam commented 2 years ago

Hey @weavejester! Could I take this? I would like to take this to the finish line.

Edit: if I can take this up, I will make a new PR and add @pesterhazy as co-author

weavejester commented 2 years ago

Sure.

weavejester commented 2 years ago

Closed by #194