Skip to content

Commit

Permalink
fix(StyleObserver): set style properties individually
Browse files Browse the repository at this point in the history
Fixes #290
  • Loading branch information
z-brooks committed Jan 23, 2016
1 parent 86722fa commit 8e9a2eb
Showing 1 changed file with 29 additions and 13 deletions.
42 changes: 29 additions & 13 deletions src/element-observation.js
Expand Up @@ -51,31 +51,47 @@ export class StyleObserver {
constructor(element, propertyName) {
this.element = element;
this.propertyName = propertyName;

this.styles = [];
}

getValue() {
return this.element.style.cssText;
}

setValue(newValue) {
if (newValue instanceof Object) {
newValue = this.flattenCss(newValue);
}
this.element.style.cssText = newValue;
this.clearStyles( (newValue instanceof Object ? this.objectUpdate(newValue) : this.stringUpdate(newValue)) );
}

subscribe() {
throw new Error(`Observation of a "${this.element.nodeName}" element\'s "${this.propertyName}" property is not supported.`);
}

flattenCss(object) {
var s = '';
for(var propertyName in object) {
if (object.hasOwnProperty(propertyName)){
s += propertyName + ': ' + object[propertyName] + '; ';
}
clearStyles(styles) {
this.styles.forEach( (v) => {
if ( !styles.includes(v) ) { this.element.style[v] = ''; }
});
this.styles = styles;
}

stringUpdate(css) {
let pairs = css.split(/(?:[:;]\s*)/).filter( v => v.length );
let props = [];
for( let i = 0, length = pairs.length; i < length; i++ )
{
props.push(pairs[i]);
this.element.style[pairs[i]] = pairs[++i];
}
return props;
}

objectUpdate(object) {
let props = Object.getOwnPropertyNames(object);
for( let propertyName of props )
{
this.element.style[propertyName] = object[propertyName];
}
return s;
return props;
}
}

Expand Down

5 comments on commit 8e9a2eb

@stsje
Copy link

@stsje stsje commented on 8e9a2eb Jan 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more appropiate to use a map instead of an array to store the styles?
clearStyles(styles) could get rid of the includes method call, if a map was used instead. That would probably perform better?

I don't agree with the naming clearStyles(styles). The name is misleading, as the method isn't clearing the styles provided as argument.

@jdanyow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@stsje we'll do some performance tuning on this in a future release to minimize array allocations and closures. Take a look at ClassObserver for more info.

@dev-zb
Copy link
Contributor

@dev-zb dev-zb commented on 8e9a2eb Jan 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdanyow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow that was quick :) PR please!

@dev-zb
Copy link
Contributor

@dev-zb dev-zb commented on 8e9a2eb Jan 26, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 done.

Please sign in to comment.