jamesmartin / inline_svg

Embed SVG documents in your Rails views and style them with CSS
MIT License
709 stars 72 forks source link

Feature request: Add a class to all SVG elements on a page #48

Open jamesmartin opened 8 years ago

jamesmartin commented 8 years ago

Requested in #44 by @Undistraction.

Additionally I need to add a class to all SVG elements. Obviously I could pass the same class every time I use inline-svg, but this feels like unnecessary repetition. I solved this by writing a transform that pulls a default class from config, but this would be a nice built-in config option.

Undistraction commented 7 years ago

Sorry I missed this. Thanks for adding.

meowsus commented 7 years ago

@Undistraction I'm just being curious here, but do you need to add a class to every SVG? If you were trying to style every SVG couldn't you just style that base class accordingly?

At work we use a bastard mutation of SMACSS and ITCSS. The older I get, the more I realize the need for the styling of a reasonable number of base classes. Things like table for consistent tabular layouts, or h1 through h6 because I don't want to have to write heading heading--1 on every single h1 element, just to achieve a style that could easily be done in a base layer.

This is where my curiosity comes in... if you're interested in styling every SVG in some consistent way, what's the difference between:

svg { width: 100px; height: 100px; }

and

.svg { width: 100px; height: 100px; }
Undistraction commented 7 years ago

@meowsus What happens when you use a third-party lib that does something different from what you're doing? You've now clobbered it by styling the base element. It does make sense to style base elements sometimes, but not with something as specific as dimensions. Some flexible defaults might make sense (for example width:100%; height:auto; for responsive images. If you look at ITCSS it does the bare minimum with the base classes. Just sets flexible, sensible defaults on top of normalisation.

Actually, even your h1 example is something I avoid. h1-6 have semantic meaning (the importance of the header) but this should be decoupled from the styling as otherwise you bake in the way the header looks with what it means. This might make sense in simple documents, but often not in a complex webpage or application and not on a responsive site where often on mobile, sizes are not indicative of importance.

It is also much easier to remove styles by removing a class than by overriding base styles - How can you remove a style on a base element?

There are numerous other arguments but I think these are the main ones.

meowsus commented 7 years ago

I see your point. Especially regarding the third-party library.

Undistraction commented 7 years ago

I think the rule is that if you are normalising, resetting or setting defaults, do it on the element (as long as they are genuinely defaults and you don't end up overriding them everywhere) Otherwise functionality should be added by classes. The classes should contain as few rules as possible and you shouldn't be afraid to have many classes on an element.

On Wed, 31 May 2017, 20:15 Curt Howard, notifications@github.com wrote:

I see your point. Especially regarding the third-party library.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jamesmartin/inline_svg/issues/48#issuecomment-305288579, or mute the thread https://github.com/notifications/unsubscribe-auth/AAJ20SCAUHDP9ROIKXApEipc95xNf4weks5r_bw0gaJpZM4JyO9c .

elliottregan commented 6 years ago

Extendable defaults for all options would be nice, actually. 'aria' and 'nocomment' are things that are generally a good idea to have everywhere.