Skip to content
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

class attribute does not work for SVG nodes #47

Closed
binary-koan opened this issue Apr 27, 2016 · 4 comments
Closed

class attribute does not work for SVG nodes #47

binary-koan opened this issue Apr 27, 2016 · 4 comments

Comments

@binary-koan
Copy link

Hi,

I've been playing around rendering SVG with maquette (awesome that it handles namespacing automatically btw!) and have discovered that setting the class attribute on an SVG node doesn't work, but setting classes in the vnodeSelector or using the classes attribute does.

For example:

h("g", { class: "test" }) // => <g></g>
h("g.test", "") // => <g class="test"></g>
h("g", { classes: { test: true } }) // => <g class="test"></g>

It looks like this is because SVG nodes support updating the classList attribute (set from the vnodeSelector and classes) but not the className attribute (set from the class attribute). Is there a reason that className is used here? - could it be changed to classList or have an exception added for SVG nodes?

Thanks!

@RickHoving
Copy link
Contributor

Hi,

For as far as I know, the problem you are having is not specific to SVG but to all type elements,
In JavaScript, class is a reserved keyword, therefore Maquette does not support using it as a property name.
Luckily you have already found the alternatives to setting the class on an element (your second and third example).
We use classList instead of className because of performance reasons, you can look at this short video to see the difference it can make in performance.
https://www.youtube.com/watch?v=hZJacl2VkKo

Kind regards,

@johan-gorter
Copy link
Contributor

Hello,

First of all, maquette did not support the class attribute until version 2.2, because we wanted to encourage people to use the hyperscript notation (your second line). Because we wanted to support JSX in version 2.2, we stopped blacklisting 'class'.

It seems we missed supporting SVG here. In SVG, className is not a string, so we have to switch to classList as you pointed out.

Thanks for pointing this out.

@johan-gorter
Copy link
Contributor

One not of warning: Using SVG classes currently does not work IE11, see #48

@binary-koan
Copy link
Author

I see, thanks for the clear explanation and the quick fix! Much appreciated :)

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

No branches or pull requests

3 participants