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

Remove class="icon" from output SVG #25

Closed
wants to merge 3 commits into from
Closed

Remove class="icon" from output SVG #25

wants to merge 3 commits into from

Conversation

ingalls
Copy link

@ingalls ingalls commented Oct 5, 2022

Context

The icon class is used by several CSS frameworks for this own custom icons. Unfortunately this results in a poor display of tabler icons when used in conjunction.

This PR automatically parses and removes the icon class from output SVGs to avoid this clash.

I don't expect this to be merged (though it would be awesome -- although breaking -- if it was)

cc/ @alex-oleshkevich - Also really appreciate the work that went into this library. Really neat!

@alex-oleshkevich
Copy link
Owner

Fixed in master, thanks

ingalls added a commit to openaddresses/batch that referenced this pull request Oct 6, 2022
@esl51
Copy link

esl51 commented Nov 8, 2022

The .icon class is used by default in the Tabler admin panel, from which these icons were later separated. And .icon can be found in many places in the css file. Now, to use these components, you have to add class="icon" to each. This is not very convenient. It seems to me that tabler icons should match the tabler framework in the first place. Perhaps we need some parameter when registering the plugin to set the name of the class for all the icons.

@alex-oleshkevich
Copy link
Owner

I see. The optional parameter should be ok. .icon is very generic and causes troubles in any other project that reserves .icon for their needs.

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