Skip to content

Fix element classNames/ids for objects when using classIdPrefix property #79

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

Merged
merged 1 commit into from
Aug 26, 2017
Merged

Conversation

samshelley
Copy link
Contributor

Hi @jhamlet, thanks for making this loader!

I'm taking advantage of the classIdPrefix property as I have multiple auto-generated svgs with conflicting class names being loaded into the same file and ran into an issue where the class styles are not showing up in my application. I think the issue is because the "." is included in both the style names and in the actual class property for the rendered element.

This PR seemed to fix the issue for me -- although not sure if this is actually the right way to go about fixing it. (it assumes the styles are calculated sequentially prior to the elements, but I think that's already assumed given the cache?) Let me know if there's a better way to do this.

I also updated the test to match the new output.

The classIdPrefix would previously add the '.className' version of the
class to both the style class and the rendered react component. Instead
only include the prefix (. or #) for the style, not the actual elements.
@agonbina
Copy link

agonbina commented Aug 22, 2017

I had the same issue as well and this fixes it. It would be great to merge it(if it doesn't break anything else), until then I'll use the fork from @samshelley.

Thanks.

@jhamlet
Copy link
Owner

jhamlet commented Aug 23, 2017

Duoh! Dumb mistake on my part.

I'll rev a patch version in a day, or so.

@jhamlet jhamlet merged commit 2db5221 into jhamlet:master Aug 26, 2017
@jhamlet
Copy link
Owner

jhamlet commented Aug 26, 2017

Merged and published v0.4.5

@samshelley
Copy link
Contributor Author

Awesome, thanks @jhamlet

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.

3 participants