Skip to content

set className with setAttribute for SVG compat #1072

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

jaredly
Copy link
Contributor

@jaredly jaredly commented Feb 13, 2014

Fixes SVG bug where changes to the className prop are not changed in the svg element.

Fixes SVG bug where changes to the className prop are not changed in the svg element.
@sophiebits
Copy link
Collaborator

If we're going to use the attribute, perhaps we can just mark it MUST_USE_ATTRIBUTE and get rid of the custom mutation method; DOMPropertyOperations.setValueForProperty will call removeAttribute for a null value.

@jaredly
Copy link
Contributor Author

jaredly commented Feb 13, 2014

cool, didn't realize that. Sounds great.

@sophiebits
Copy link
Collaborator

@jaredly
Copy link
Contributor Author

jaredly commented Feb 14, 2014

:( hmm. Is there a way to just use setattribute for svg elements?
On Feb 13, 2014 7:22 PM, "Ben Alpert" notifications@github.com wrote:

Hmm, looks like this will break things in IE actually:

http://stackoverflow.com/questions/2490627/how-can-i-reliably-set-the-class-attr-w-javascript-on-ie-ff-chrome-etc

Reply to this email directly or view it on GitHubhttps://github.com//pull/1072#issuecomment-35049774
.

@plievone
Copy link
Contributor

Do you know which IE versions would be are affected, perhaps it's less than IE8?
If it would would lead to feature testing IE vs others and DOM vs SVG, then one simple solution might be to use multiple setters at the same time, extra attribute would just be ignored? (Idea from discussion in Dojo bugtracker)

@sophiebits
Copy link
Collaborator

My idea was:

if (typeof node.className === "string") {
  node.className = value;
} else {
  node.setAttribute('className', value);
}

@zpao
Copy link
Member

zpao commented Feb 28, 2014

Any updates on what we're going to do here?

@forresto
Copy link

forresto commented Mar 3, 2014

Firefox gets upset (TypeError: setting a property that has only a getter), and working around this is ugly. Looking forward to taking out those.

@sophiebits
Copy link
Collaborator

Closing in favor of #1264.

@sophiebits sophiebits closed this Mar 30, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants