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

ng-show stopped working for SVG elements in 1.2.0rc1 #3858

Closed
rbonvall opened this issue Sep 3, 2013 · 7 comments
Closed

ng-show stopped working for SVG elements in 1.2.0rc1 #3858

rbonvall opened this issue Sep 3, 2013 · 7 comments
Assignees
@btford
Labels
frequency: moderate PRs plz! severity: regression
Milestone
1.2.0-rc3

Comments

@rbonvall
Copy link
Contributor

rbonvall commented Sep 3, 2013

In Angular 1.1.5, you can hide SVG elements using ng-show. Example:

115

It doesn't work anymore in 1.2.0rc1. Example:

120

Tested on Chrome 25.0.1364.172 Linux x86_64.
@petebacondarwin
Copy link
Member

This is a regression. The problem is that our current addClass method is not able to attach classes to SVG rect elements. Before it used to update the rect element's style.

@petebacondarwin
Copy link
Member

jqLite.addClass, at this line https://github.com/angular/angular.js/blob/master/src/jqLite.js#L302, is not updating the CSS class information because element.className is an SVGAnimatedString rather than a straight string.

@petebacondarwin
Copy link
Member

Notably this doesn't work with jQuery either.

@petebacondarwin
Copy link
Member

See http://stackoverflow.com/questions/8638621/jquery-svg-why-cant-i-addclass

@petebacondarwin
Copy link
Member

and in particular http://keith-wood.name/svg.html#dom

@petebacondarwin
Copy link
Member

As a workaround you can use jQuery 1.7.2 with Keith Woods SVG plugin: http://plnkr.co/edit/AgFXoE?p=preview

@petebacondarwin
Copy link
Member

Not sure how much of this we would want to include in jqLite though!

@btford btford mentioned this issue Sep 24, 2013
Closed
@ghost ghost assigned btford Sep 24, 2013
btford added a commit to btford/angular.js that referenced this issue Sep 24, 2013
@btford
fix(jqLite): use get/setAttribute so that jqLite works on SVG nodes
3a515dc 
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. :'(

Closes angular#3858
@btford btford closed this as completed in c785267 Sep 27, 2013
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@btford @jamesdaily
fix(jqLite): use get/setAttribute so that jqLite works on SVG nodes
991bece 
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 angular#3858
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this issue Jan 27, 2014
@btford @jamesdaily
fix(jqLite): use get/setAttribute so that jqLite works on SVG nodes
b7effd5 
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 angular#3858
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@btford btford

Labels
frequency: moderate PRs plz! severity: regression
Projects
None yet
Milestone
1.2.0-rc3
Development

Successfully merging a pull request may close this issue.

fix(jqLite): use get/setAttribute so that jqLite works on SVG btford/angular.js
3 participants
@rbonvall @petebacondarwin @btford