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

Allow keywords in style maps #140

Closed

Conversation

pesterhazy
Copy link
Contributor

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.

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
Copy link
Contributor Author

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

@davidperrenoud
Copy link

davidperrenoud commented Jul 26, 2019

@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
Copy link
Contributor Author

pesterhazy commented Jul 26, 2019 via email

@metasoarous
Copy link

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

@weavejester
Copy link
Owner

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
Copy link

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

@weavejester weavejester added the waiting on dev Waiting for the PR opener to make changes label Jan 20, 2022
@shhivam
Copy link
Contributor

shhivam commented Jan 24, 2022

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
Copy link
Owner

Sure.

@weavejester
Copy link
Owner

Closed by #194

@weavejester weavejester removed the waiting on dev Waiting for the PR opener to make changes label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants