Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
feat(ng:style): compatibility + perf improvements
Browse files Browse the repository at this point in the history
- better compatibility with 3rd party code - we clober 3rd party
  style only if it direcrtly collides with 3rd party styles
- better perf since it doesn't execute stuff on every digest
- lots of tests
  • Loading branch information
dmanek authored and IgorMinar committed Nov 8, 2011
1 parent 9f9ed4c commit e2663f6
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 31 deletions.
20 changes: 5 additions & 15 deletions src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -802,21 +802,11 @@ angularDirective("ng:hide", function(expression, element){
</doc:scenario>
</doc:example>
*/
angularDirective("ng:style", function(expression, element){
// TODO(i): this is inefficient (runs on every $digest) and obtrusive (overrides 3rd part css)
// we should change it in a similar way as I changed ng:class
return function(element){
var resetStyle = getStyle(element);
this.$watch(function(scope){
var style = scope.$eval(expression) || {}, key, mergedStyle = {};
for(key in style) {
if (resetStyle[key] === undefined) resetStyle[key] = '';
mergedStyle[key] = style[key];
}
for(key in resetStyle) {
mergedStyle[key] = mergedStyle[key] || resetStyle[key];
}
element.css(mergedStyle);
angularDirective("ng:style", function(expression, element) {
return function(element) {
this.$watch(expression, function(scope, newStyles, oldStyles) {
if (oldStyles) forEach(oldStyles, function(val, style) { element.css(style, '');});
if (newStyles) element.css(newStyles);
});
};
});
Expand Down
79 changes: 63 additions & 16 deletions test/directivesSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,35 +363,82 @@ describe("directive", function() {


describe('ng:style', function() {

it('should set', function() {
var scope = compile('<div ng:style="{height: \'40px\'}"></div>');
scope.$digest();
expect(element.css('height')).toEqual('40px');
});


it('should silently ignore undefined style', function() {
var scope = compile('<div ng:style="myStyle"></div>');
scope.$digest();
expect(element.hasClass('ng-exception')).toBeFalsy();
});

it('should preserve and remove previous style', function() {
var scope = compile('<div style="height: 10px;" ng:style="myStyle"></div>');
scope.$digest();
expect(getStyle(element)).toEqual({height: '10px'});
scope.myStyle = {height: '20px', width: '10px'};
scope.$digest();
expect(getStyle(element)).toEqual({height: '20px', width: '10px'});
scope.myStyle = {};
scope.$digest();
expect(getStyle(element)).toEqual({height: '10px'});
});
});

it('should silently ignore undefined ng:style', function() {
var scope = compile('<div ng:style="myStyle"></div>');
scope.$digest();
expect(element.hasClass('ng-exception')).toBeFalsy();
describe('preserving styles set before and after compilation', function() {
var scope, preCompStyle, preCompVal, postCompStyle, postCompVal;

beforeEach(function() {
preCompStyle = 'width';
preCompVal = '300px';
postCompStyle = 'height';
postCompVal = '100px';
element = jqLite('<div ng:style="styleObj"></div>');
element.css(preCompStyle, preCompVal);
jqLite(document.body).append(element);
scope = compile(element);
scope.styleObj = {'margin-top': '44px'};
scope.$apply();
element.css(postCompStyle, postCompVal);
});

afterEach(function() {
element.remove();
});


it('should not mess up stuff after compilation', function() {
element.css('margin', '44px');
expect(element.css(preCompStyle)).toBe(preCompVal);
expect(element.css('margin-top')).toBe('44px');
expect(element.css(postCompStyle)).toBe(postCompVal);
});


it('should not mess up stuff after $apply with no model changes', function() {
element.css('padding-top', '33px');
scope.$apply();
expect(element.css(preCompStyle)).toBe(preCompVal);
expect(element.css('margin-top')).toBe('44px');
expect(element.css(postCompStyle)).toBe(postCompVal);
expect(element.css('padding-top')).toBe('33px');
});


it('should not mess up stuff after $apply with non-colliding model changes', function() {
scope.styleObj = {'padding-top': '99px'};
scope.$apply();
expect(element.css(preCompStyle)).toBe(preCompVal);
expect(element.css('margin-top')).not.toBe('44px');
expect(element.css('padding-top')).toBe('99px');
expect(element.css(postCompStyle)).toBe(postCompVal);
});


it('should overwrite original styles after a colliding model change', function() {
scope.styleObj = {'height': '99px', 'width': '88px'};
scope.$apply();
expect(element.css(preCompStyle)).toBe('88px');
expect(element.css(postCompStyle)).toBe('99px');
scope.styleObj = {};
scope.$apply();
expect(element.css(preCompStyle)).not.toBe('88px');
expect(element.css(postCompStyle)).not.toBe('99px');
});
});
});


Expand Down

0 comments on commit e2663f6

Please sign in to comment.