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

Improve ng style #640

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 7 additions & 14 deletions src/directives.js
Original file line number Diff line number Diff line change
Expand Up @@ -789,25 +789,18 @@ angularDirective("ng:hide", function(expression, element){
</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);
this.$watch(expression, function(scope, newVal, oldVal){
var styleToAdd = newVal || {},
styleToRemove = oldVal || {},
key;
for(key in styleToRemove)
Copy link
Contributor

Choose a reason for hiding this comment

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

wrap for loops into {} just like in google3

element.css(key, '');
Copy link
Contributor

Choose a reason for hiding this comment

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

indent with 2 spaces

element.css(styleToAdd);
});
};
});


/**
* @ngdoc directive
* @name angular.directive.ng:cloak
Expand Down
78 changes: 62 additions & 16 deletions test/directivesSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -363,37 +363,83 @@ 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'});
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');
});
});
});

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('ng:show', function() {
it('should show and hide an element', function() {
Expand Down