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

Commit

Permalink
fix(ngStyle): correctly remove old style when new style value is invalid
Browse files Browse the repository at this point in the history
Since d6098ee, old styles were not removed if `newStyles` specified an
invalid value for the style (e.g. `false`). The assumption was that the
new style would overwrite the old style value, but using an invalid
value made browsers ignore the new value and thus keep the old style.
This would typically happen when guarding a style with a boolean flag;
e.g.: `ng-style="{backgroundColor: isError && 'red'}"`

This commit essentially revers commit d6098ee, whose main purpose was
to work around jquery/jquery#4185. The jQuery issue has been fixed in
3.4.0, so that should not be a problem any more.

Fixes #16860

Closes #16868
  • Loading branch information
gkalpak committed May 9, 2019
1 parent 019dded commit 5edd253
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
9 changes: 1 addition & 8 deletions src/ng/directive/ngStyle.js
Expand Up @@ -54,14 +54,7 @@
var ngStyleDirective = ngDirective(function(scope, element, attr) {
scope.$watchCollection(attr.ngStyle, function ngStyleWatchAction(newStyles, oldStyles) {
if (oldStyles && (newStyles !== oldStyles)) {
if (!newStyles) {
newStyles = {};
}
forEach(oldStyles, function(val, style) {
if (newStyles[style] == null) {
newStyles[style] = '';
}
});
forEach(oldStyles, function(val, style) { element.css(style, ''); });
}
if (newStyles) element.css(newStyles);
});
Expand Down
11 changes: 11 additions & 0 deletions test/ng/directive/ngStyleSpec.js
Expand Up @@ -143,6 +143,17 @@ describe('ngStyle', function() {
expect(element.css(postCompStyle)).not.toBe('99px');
});

it('should clear style when the value is false', function() {
scope.styleObj = {'height': '99px', 'width': '88px'};
scope.$apply();
expect(element.css(preCompStyle)).toBe('88px');
expect(element.css(postCompStyle)).toBe('99px');
scope.styleObj = {'height': false, 'width': false};
scope.$apply();
expect(element.css(preCompStyle)).not.toBe('88px');
expect(element.css(postCompStyle)).not.toBe('99px');
});

it('should set style when the value is zero', function() {
scope.styleObj = {'height': '99px', 'width': '88px'};
scope.$apply();
Expand Down

0 comments on commit 5edd253

Please sign in to comment.