diff options
| author | Brian Ford | 2013-09-23 17:29:51 -0700 |
|---|---|---|
| committer | Igor Minar | 2013-09-27 12:38:27 -0700 |
| commit | c785267eb8780d8b7658ef93ebb5ebddd566294d (patch) | |
| tree | 46fa85051d7aa768719b8c4572aec63ed30d52e2 | |
| parent | 6aaae062171bfc8e5046c3eae99bc9d63037120a (diff) | |
| download | angular.js-c785267eb8780d8b7658ef93ebb5ebddd566294d.tar.bz2 | |
fix(jqLite): use get/setAttribute so that jqLite works on SVG nodes
jqLite previously used `elt.className` to add and remove classes from a DOM Node, but
because the className property is not writable on SVG elements, it doesn't work with
them. This patch replaces accesses to `className` with `get/setAttribute`.
`classList` was also considered as a solution, but because only IE10+ supports it, we
have to wait. :'(
The JqLiteAddClass/JQLiteRemoveClass methods are now also used directly by $animate
to work around the jQuery not being able to handle class modifications on SVG elements.
Closes #3858
| -rw-r--r-- | src/jqLite.js | 18 | ||||
| -rw-r--r-- | src/ng/animate.js | 8 | ||||
| -rw-r--r-- | test/helpers/matchers.js | 9 | ||||
| -rw-r--r-- | test/jqLiteSpec.js | 14 | ||||
| -rw-r--r-- | test/ng/animateSpec.js | 12 |
5 files changed, 52 insertions, 9 deletions
diff --git a/src/jqLite.js b/src/jqLite.js index 1e1d025c..d3110788 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -279,17 +279,17 @@ function JQLiteData(element, key, value) { } function JQLiteHasClass(element, selector) { - return ((" " + element.className + " ").replace(/[\n\t]/g, " "). + return ((" " + (element.getAttribute('class') || '') + " ").replace(/[\n\t]/g, " "). indexOf( " " + selector + " " ) > -1); } function JQLiteRemoveClass(element, cssClasses) { if (cssClasses) { forEach(cssClasses.split(' '), function(cssClass) { - element.className = trim( - (" " + element.className + " ") + element.setAttribute('class', trim( + (" " + (element.getAttribute('class') || '') + " ") .replace(/[\n\t]/g, " ") - .replace(" " + trim(cssClass) + " ", " ") + .replace(" " + trim(cssClass) + " ", " ")) ); }); } @@ -297,11 +297,17 @@ function JQLiteRemoveClass(element, cssClasses) { function JQLiteAddClass(element, cssClasses) { if (cssClasses) { + var existingClasses = (' ' + (element.getAttribute('class') || '') + ' ') + .replace(/[\n\t]/g, " "); + forEach(cssClasses.split(' '), function(cssClass) { - if (!JQLiteHasClass(element, cssClass)) { - element.className = trim(element.className + ' ' + trim(cssClass)); + cssClass = trim(cssClass); + if (existingClasses.indexOf(' ' + cssClass + ' ') === -1) { + existingClasses += cssClass + ' '; } }); + + element.setAttribute('class', trim(existingClasses)); } } diff --git a/src/ng/animate.js b/src/ng/animate.js index fbd17848..b3fb08de 100644 --- a/src/ng/animate.js +++ b/src/ng/animate.js @@ -156,7 +156,9 @@ var $AnimateProvider = ['$provide', function($provide) { className = isString(className) ? className : isArray(className) ? className.join(' ') : ''; - element.addClass(className); + forEach(element, function (element) { + JQLiteAddClass(element, className); + }); done && $timeout(done, 0, false); }, @@ -177,7 +179,9 @@ var $AnimateProvider = ['$provide', function($provide) { className = isString(className) ? className : isArray(className) ? className.join(' ') : ''; - element.removeClass(className); + forEach(element, function (element) { + JQLiteRemoveClass(element, className); + }); done && $timeout(done, 0, false); }, diff --git a/test/helpers/matchers.js b/test/helpers/matchers.js index 57bf35c7..14430b37 100644 --- a/test/helpers/matchers.js +++ b/test/helpers/matchers.js @@ -31,7 +31,14 @@ beforeEach(function() { } function isNgElementHidden(element) { - return angular.element(element).hasClass('ng-hide'); + // we need to check element.getAttribute for SVG nodes + var hidden = true; + forEach(angular.element(element), function (element) { + if ((' ' +(element.getAttribute('class') || '') + ' ').indexOf(' ng-hide ') === -1) { + hidden = false; + } + }); + return hidden; }; this.addMatchers({ diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 79c0d0c6..e6e3a2ac 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -479,6 +479,20 @@ describe('jqLite', function() { describe('class', function() { + it('should properly do with SVG elements', function() { + // this is a jqLite & SVG only test (jquery doesn't behave this way right now, which is a bug) + if (!window.SVGElement || !_jqLiteMode) return; + var svg = jqLite('<svg><rect></rect></svg>'); + var rect = svg.children(); + + expect(rect.hasClass('foo-class')).toBe(false); + rect.addClass('foo-class'); + expect(rect.hasClass('foo-class')).toBe(true); + rect.removeClass('foo-class'); + expect(rect.hasClass('foo-class')).toBe(false); + }); + + describe('hasClass', function() { it('should check class', function() { var selector = jqLite([a, b]); diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js index 79115c1e..e982862a 100644 --- a/test/ng/animateSpec.js +++ b/test/ng/animateSpec.js @@ -40,6 +40,18 @@ describe("$animate", function() { expect(element).toBeHidden(); })); + it("should add and remove classes on SVG elements", inject(function($animate) { + if (!window.SVGElement) return; + var svg = jqLite('<svg><rect></rect></svg>'); + var rect = svg.children(); + $animate.enabled(false); + expect(rect).toBeShown(); + $animate.addClass(rect, 'ng-hide'); + expect(rect).toBeHidden(); + $animate.removeClass(rect, 'ng-hide'); + expect(rect).not.toBeHidden(); + })); + it("should throw error on wrong selector", function() { module(function($animateProvider) { expect(function() { |
