search

mattdesl / dom-css

fast dom CSS styling
MIT License
152 stars 13 forks source link

Take string argument #12

Closed dy closed 8 years ago

dy commented 8 years ago

Regarding #11

mattdesl commented 8 years ago

Thanks, pushed!

I am considering whether this should be augmented at all with auto-prefixing... Otherwise it's not really doing anything other than changing cssText which begs the question why a module is necessary.

dy commented 8 years ago

Agreed, but we have to do sort of easy parsing then. I’ll give a try.

hollowdoor commented 8 years ago

I'm curious. The usual method of setting with properties is adding styles. This new string method is replacing the style attribute value. I'm wondering if there is something lost in this break in consistent behavior.

mattdesl commented 8 years ago

Yes, after more thought it may be worth reverting since it seems a bit out of place.

dy commented 8 years ago

@hollowdoor the usual method is not actually adding, but setting styles, in that for absent property it is added and for existing property it is replaced, and the logic for string is the same for the whole style set. But I guess having that done in per-property fashion would be more meaningful.

hollowdoor commented 8 years ago

@mattdesl I wouldn't say it's out of place. It's that as it's implemented it's not consistent with adding sets of properties like the current discrete property setter. This could just be me being nitpicky, but I think the string interface can benefit from being discrete like the property setter. Especially as it's being passed through an overloaded parameter.

The only problem is parsing which might be too much if one of the purposes of this module is to stay lightweight. As far as css parsing itself goes there are parsers available. Then again like I said it could be that I'm just being nitpicky.

@dfcreative I should have said discrete instead of just adding. Discrete to style attributes/properties.