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

Doesn't take into account className #125

Closed
deadcoder0904 opened this issue Apr 28, 2020 · 5 comments
Closed

Doesn't take into account className #125

deadcoder0904 opened this issue Apr 28, 2020 · 5 comments

Comments

@deadcoder0904
Copy link

The UX is incredible but it doesn't take into account className

For example,

<svg
	className="w-4 h-4 fill-current text-red-500"
	stroke-linecap="round"
	stroke-linejoin="round"
	stroke-width="2"
	stroke="currentColor"
	viewBox="0 0 24 24"
>
	<path d="M11.049 2.927c.3-.921 1.603-.921 1.902 0l1.519 4.674a1 1 0 00.95.69h4.915c.969 0 1.371 1.24.588 1.81l-3.976 2.888a1 1 0 00-.363 1.118l1.518 4.674c.3.922-.755 1.688-1.538 1.118l-3.976-2.888a1 1 0 00-1.176 0l-3.976 2.888c-.783.57-1.838-.197-1.538-1.118l1.518-4.674a1 1 0 00-.363-1.118l-3.976-2.888c-.784-.57-.38-1.81.588-1.81h4.914a1 1 0 00.951-.69l1.519-4.674z"></path>
</svg>

gets converted to

import React from "react";

function Icon() {
  return (
    <svg
      stroke="currentColor"
      strokeLinecap="round"
      strokeLinejoin="round"
      strokeWidth="2"
      viewBox="0 0 24 24"
    >
      <path d="M11.049 2.927c.3-.921 1.603-.921 1.902 0l1.519 4.674a1 1 0 00.95.69h4.915c.969 0 1.371 1.24.588 1.81l-3.976 2.888a1 1 0 00-.363 1.118l1.518 4.674c.3.922-.755 1.688-1.538 1.118l-3.976-2.888a1 1 0 00-1.176 0l-3.976 2.888c-.783.57-1.838-.197-1.538-1.118l1.518-4.674a1 1 0 00-.363-1.118l-3.976-2.888c-.784-.57-.38-1.81.588-1.81h4.914a1 1 0 00.951-.69l1.519-4.674z"></path>
    </svg>
  );
}

export default Icon;

Notice, the missing className property

@balajmarius
Copy link
Owner

Hey @deadcoder0904,

Make sense as className it's an invalid attribute for SVGs, should be class.

<svg class="w-4 h-4 fill-current text-red-500" ...

Work as it should.

@deadcoder0904
Copy link
Author

deadcoder0904 commented Apr 29, 2020

@balajmarius I'm using React. In React, we don't use class as it collides with ES6's class so we use className.

There are some other attributes that collide as well like for, so in React we use it as htmlFor.

So it's totally valid in React. Not sure if you want to add this one as its framework-specific :)

@deadcoder0904
Copy link
Author

Also, just tried

<svg
	className={`${!menuOpen ? 'hidden' : 'block'} h-6 w-6`}
	stroke="currentColor"
	fill="none"
	viewBox="0 0 24 24"
>
	<path
		stroke-linecap="round"
		stroke-linejoin="round"
		stroke-width="2"
		d="M6 18L18 6M6 6l12 12"
	/>
</svg>

And it gives an error

@balajmarius
Copy link
Owner

balajmarius commented Apr 29, 2020

Yes, it should throw an error. What I'm trying to say it's that your SVG is invalid, the SVG2JSX takes valid SVG and outputs JSX.

This is not valid SVG className={`${!menuOpen ? 'hidden' : 'block'} h-6 w-6`}, same for className="foo".

@deadcoder0904
Copy link
Author

Cool, next time I'd add the class instead of className I guess. Then it works perfectly fine.

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

2 participants