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

SVGLoader: fixed multiple css classes #23191

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

vojty
Copy link
Contributor

@vojty vojty commented Jan 10, 2022

Multiple CSS classes don't work with SVGLoader.

Demo

https://codesandbox.io/s/three-js-svg-multiple-classes-wdxhd

Expected

expected

Actual

actual

Problem

The attributes of the first class are overridden by empty strings from the next ones

Object.assign({ fill: "red" }, { fill: '' }) // produces {fill: ''}

Clearing the attributes with empty strings should be sufficient

@vojty vojty force-pushed the fix-multiple-classes-svg-loader branch from f69d886 to 898b01b Compare January 10, 2022 09:49
@mrdoob mrdoob added this to the r137 milestone Jan 10, 2022
@mrdoob mrdoob requested a review from yomboprime January 10, 2022 21:00
Copy link
Collaborator

@yomboprime yomboprime left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To my knowledge the PR is correct, though I didnt code the CSS parsing part.

@mrdoob mrdoob merged commit 07c0651 into mrdoob:dev Jan 10, 2022
@mrdoob
Copy link
Owner

mrdoob commented Jan 10, 2022

Thanks!

@mrdoob
Copy link
Owner

mrdoob commented Jan 13, 2022

@vojty Is it possible to implement this without creating a unused var?

Screen Shot 2022-01-12 at 7 58 45 PM

@vojty
Copy link
Contributor Author

vojty commented Jan 13, 2022

@mrdoob ah, sure #23222

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