search

elm / virtual-dom

The foundation of HTML and SVG in Elm.
https://package.elm-lang.org/packages/elm/virtual-dom/latest
BSD 3-Clause "New" or "Revised" License
209 stars 80 forks source link

Use `setProperty` to apply styles rather than directly assigning #95

Closed zwilias closed 7 years ago

zwilias commented 7 years ago

The current applyStyles implementation directly assigns to the style key on the domNode. CSSOM exposes an alternative method, setProperty which behaves the same way for our intents (i.e. not setting invalid properties, removing style properties set to the empty string) while enabling one further feature we currently don't support: custom CSS properties.

Performance-wise, this changes nothing in the set of browsers I tested (Chrome, Chrome beta, Firefox, Firefox beta, IE9, IE10, IE11, IE edge, Safari, Safari Mobile, and a bunch of other mobile browsers) validating time-spent on applyStyles using @jinjor's vdom-dev, specifically the Animation.elm example which changes styles every 1000/60 ms.

Closes elm-lang/html#129

process-bot commented 7 years ago

Thanks for the pull request! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

johanalkstal commented 7 years ago

Any chance this will get merged in the foreseeable future?

evancz commented 7 years ago

I responded to this feature request over here https://github.com/elm-lang/html/issues/129#issuecomment-313608729

From what I have seen of this feature, I am not convinced it is a good idea. I would want to see some evidence that it leads to nice outcomes at all, and that those outcomes outweigh any downsides. That all comes before making changes to this library!