jhamlet / svg-react-loader

Webpack SVG to React Component Loader
MIT License
559 stars 82 forks source link

SVG styles override each other #21

Closed kinetifex closed 8 years ago

kinetifex commented 8 years ago

If I have a few SVGs I want to use on the same view, the styles from the last svg will override the others.

If the SVG files look something like this: (actual svgs I'm using are much more complicated, these are just to demonstrated the issue)

// logo_one.svg
<svg viewBox="0 0 24 24">
    <style>.a{fill:#FF0000;}</style>
    <circle class='a' cx="12" cy="12" r="12"/>
</svg>

// logo_two.svg
<svg viewBox="0 0 24 24">
    <style>.a{fill:#00FF00;}</style>
    <circle class='a' cx="12" cy="12" r="12"/>
</svg>

// logo_three.svg
<svg viewBox="0 0 24 24">
    <style>.a{fill:#0000FF;}</style>
    <circle class='a' cx="12" cy="12" r="12"/>
</svg>

and if I use them into my react app like this:

const LogoOne = require('babel!svg-react!./logo_one.svg');
const LogoTwo = require('babel!svg-react!./logo_two.svg');
const LogoThree = require('babel!svg-react!./logo_three.svg');

function SomeComponent() {
    return (
        <div>
            <LogoOne />
            <LogoTwo />
            <LogoThree />
        </div>
    )
}

then all SVGs end up rendering with same color because style class .a gets overridden by the last.

Is there a known solution for this using inline SVGs with this loader?

jhamlet commented 8 years ago

@kinetifex I don't think svg-react-loader can fix this. Illustrator (and I'm guessing inkscape and other vector editors) export style information in a predetermined way which ends up clashing with other instances on the same page.

The overhaul I plan on doing (eventually) in #11 should allow for modifying the style information before it gets turned into a React component/class.

jhamlet commented 8 years ago

Not closing -- going to investigate pulling out generating styles and allowing a processing step on them before inlining them into the react component...

kinetifex commented 8 years ago

Thanks for the update. If there's something I could help you with let me know.

jhamlet commented 8 years ago

I wonder if prefixing the given style class names (and/or ids) with the name of the component (BEM style) would work?

// logo_one.svg
<svg viewBox="0 0 24 24">
    <style>.logo_one__a{fill:#FF0000;}</style>
    <circle class='logo_one__a' cx="12" cy="12" r="12"/>
</svg>

// logo_two.svg
<svg viewBox="0 0 24 24">
    <style>.logo_two__a{fill:#00FF00;}</style>
    <circle class='logo_two__a' cx="12" cy="12" r="12"/>
</svg>
kinetifex commented 8 years ago

Yes, infact that would be preferred. I did a PR for sairion/svg-inline-loader#30 with a feature exactly like this awhile back. I would preferred to move to using your loader once this fix is available, since yours creates actual SVG React components rather using dangerouslySetInnerHTML.

I'm interested in collaborating with you on this if you'd like.

jhamlet commented 8 years ago

Ok.

I'm trying to a get a re-factor in (as this whole thing started as a quick afternoon project) and one of the things I wanted to do (since there are a lot of issues that stem from having to reformat the xml to work with React) is build a pluggable visitor for node traversal.

Maybe after that, I can hand this to you?

kinetifex commented 8 years ago

Sure thing, sounds great.

jhamlet commented 8 years ago

@kinetifex I'm working on the processor for this and I've hit a usage issue.

We've got the situation where you are using an svg exported through illustrator or inkscape which creates the class names dynamically, and in other situations you might have a hand-crafted svg file where the class names were written by hand.

I was hoping to get a way with a general configuration setting:

var Icon = require('svg-react?prefixCssClassNames=foobar!../my-icon.svg');
// or
var Icon = require('svg-react?prefixCssClassNames!../my-icon.svg');

The first would result in prefixing class names where the rule has only a class name selector with foobar__, and the second would do the same, but result in my-icon__ (the files name).

However you get into the situation where some svgs are hand-crafted and others are exported from various svg editors, and then you have to start configuring them on a case-by-case basis.

Do you see any other use-cases?

kinetifex commented 8 years ago

Hmm, I don't know that you'll be able to have a generic configuration without going into case-by-case configurations. I imagine the user will know the nature of the svg content and can configure the loader as needed. I'd suggest a simpler interface, with just a classPrefix query param. For example:

// Using default hashed prefix
var logoOne = require('svg-react?classPrefix!./logo_one.svg');

// Using custom string
var logoTwo = require('svg-react?classPrefix=my-prefix-!./logo_two.svg');

// Using custom string and hash
var logoThree = require('svg-react?classPrefix=__prefix-[sha512:hash:hex:5]__!./logo_three.svg');

// No prefix
var logoFour = require('svg-react!./logo_four.svg');

Hashing should be done using https://github.com/webpack/loader-utils#interpolatename

jhamlet commented 8 years ago

Thanks for the heads-up on the loaderUtils.interpolatename() utility method.

My thoughts ended up very close to yours. Except if you just use a bare cssClassIdPrefix I would use the name generated for the displayName of the component (either the svg's basename, or the supplied name=MyComponent query parameter.

Thanks, ;-j

jhamlet commented 8 years ago

Fixed with v0.4.0-beta.2

jhamlet commented 8 years ago

@kinetifex If you have the bandwidth can you see if the current beta version handles your needs?

npm install svg-react-loader@next or npm install svg-react-laoder@0.4.0-beta.2 should do grab the right version.

You can peruse the documentation in the README-0.4.0.md file (npm registry has a problem with pulling the wrong version of READMEs). It's not 100% accurate, nor finished.

Any feedback would be appreciated.