crysalead-js / dom-layer

Virtual DOM implementation.
MIT License
30 stars 1 forks source link

SVG attributes #27

Closed ghost closed 9 years ago

ghost commented 9 years ago

I see you now fixed all attributes / properties. But what about SVG attributes?? There are some SVG properties etc that has to be set as an attribute. https://github.com/facebook/react/blob/0.13-stable/src/browser/ui/dom/SVGDOMPropertyConfig.js#L20-L40

jails commented 9 years ago

Afaik SVG doesn't have any property related. If you want to set "cx" for example you will need to do it using .setAttribute(). As I said in a previous post, it's imo up to the developper to know what must be set as property and what must be set as an attribute. So if something doesn't work on the DOM (like setting "cx" as a property), in won't in the VDOM either.

And if you were talking about namespaced attributes (like "xlink:href"), they must be defined inattrsNS. So I don't see anymore issues on that topic.

ghost commented 9 years ago

The only problem as I see here, and as we discussed before is the class and className. I see both Virtual DOM, Deku, ReativeJS, ReactJS etc. have a workaround on this. Example if namespace, REACT are forcing use of setAttribute. This to avoid the script code from throwing. I also notice when it comes to class and classNames that they can be set as an object / array as well - both in Virtual DOM, Deku, but in REACT this have been moved in to a separate plugin now.

jails commented 9 years ago

I think it's the same as working with the DOM directly. should I set the class name by using element.className = ... or by using element.setAttribute("class",...) ? So imo it's not to a virtual dom implementation to decide which way to go. The virtual dom should just be able to transcribe DOM operations at a virutal level and be able to apply them back to the DOM.

ghost commented 9 years ago

I get your point and the other I mention with classes I guess can be done on a higher level.

jails commented 9 years ago

Yup it can, but imo using attrs is the way to go. props should only be used for some very specific stuff related to input elements.