Skip to content

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 angular#16860
  • Loading branch information
gkalpak committed Apr 27, 2019
1 parent c8d985a commit 9e5ac74
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 9e5ac74

Please sign in to comment.