cee-chen / object-fit-polyfill

A Javascript polyfill for browsers that don't support the object-fit CSS property.
ISC License
496 stars 93 forks source link

Make styles !important. #8

Closed nextgenthemes closed 7 years ago

nextgenthemes commented 8 years ago

I would be glad if you would pull this, otherwise I have to maintain my own copy.

I am aware that !important styles are bad practice but the thing is I am dealing with 3rd party code so my CSS needs to have !important rules in it to make sure elements are styled the way I define them.

With the native native browsers object-fit there is no problem because as object fit does not work like this polyfill, meaning it does not calculate and does this with margin and stuff so this values are ignored even if set with !important. One rule where this polyfill fails is margin: 0 !important; for example. I rather tried to make everything stick because I think when using this polyfill one really wants it to apply its styles no matter what. I think its not a bad Idea to do this even for other users so !important styles do never get into the way.

setProperty is IE9+ so it would fit.

This is currently untested and I only done this to the basic version only to see what you say.

cee-chen commented 8 years ago

Hey, sorry it took a while to get back to you - I was out of the country for the last week.

Unfortunately, I would incredibly strongly prefer not to do this. While I understand some 3rd party plugins do this kind of thing, I think the preferable solution would be to open a PR on the plugin (if it's open source), removing their dirty CSS or !important rules with flat namespaced classes, or to personally tweak the plugins in your projects/repo.

Either way, I think the solution would be to encourage cleaner code on all sides rather than introducing a hacky solution to fix a hacky practice. I also don't think that this polyfill should necessarily completely un-overridable (since JS-inserted inline CSS with !important is literally impossible to override in just CSS) - the .object-fit-polyfill class hook was added to it so people could personally tweak settings as necessary.

For alternative workarounds, if you provide me an example of the CSS you are trying to override, I might be able to offer some tips or solutions to overriding them without !important (if the third-party CSS itself does not use !important, you can likely override by IDs or more specific nesting).

As a second alternative, if your specific issue is with margin only, I would suggest a workaround that works with IE9+ (when autoprefixed) - you could look for the CSS class .object-fit-polyfill and add transform: translateX(-50%) translateY(-50%);, which simulates the negative margin positioning.

nextgenthemes commented 7 years ago

I don't think you understood me correctly. I do code for the wordpress world, its not about my project its about their sites that may use one or multiple of millions of themes and plugins. So no your "preferable" solution to do "a PR on THE plugin" is not acceptable. I won't wait for my customers to come one by one report issues, then try to figure out where the issues comes form and them go and try to contact the author of a theme or plugin to get then to change their code. Thats a task that is unfinishable. While most WordPress plugins and themes are actually open source, often they are outdated and often authors cant be reached, ofter understandably authors have higher priorities then to fix their gratis stuff ... people use stuff modified by themselves or some freelancer ... I also not want to end up coding one horrible workaround after the other. When I tell people to edit their plugins/themes for better CSS it will be gone after the next update.

So no in this world I play in I can cant encourage people to do better code, I need my product to work, and as I said for this case, as native browsers function would work and it simply does not. Native browsers to not care about what margin is set just for this one example, but I can also predict more issues coming because of CSS so I just made everything important and have peace of mind.

You speak of

flat namespaced classes

no idea what you mean. If have to deal with styles added to html elements directly or added with !important classes wont get me anywhere.

I might be able to offer some tips or solutions to overriding them without !important (if the third-party CSS itself does not use !important, you can likely override by IDs or more specific nesting)

sounds to me like horrible hacky workarounds, except that I have to do them one by one and actually wait for a customer to have issues to then code a horrible workaround up for the specific issue one by one. Why is that better then just do !important in the first place? To me its much worse. I also don't like IDs. I have adopted this form Bootstrap (World most famous CSS framework) not to use them. Made so much sense to me and I never looked back.

And thanks but I don't think I need any tips from you how CSS works, I know CSS pretty well, much better then JS.

I don't think it makes sense to show you a example because its, as I said not about this one plugin, one project, one issue. I want it to be future proof and just work. And again native browsers object-fit works. No matter what margin is set, not matter if its important or not. I also fail to get what a user would want to change about the CSS this project does. Its supposed to emulate what a browser would do and Its working fine I guess and I ended up here because on paper this solution with actually positioning the image seems far superior to me then do some weird stuff with background images like other polyfills do. I also did used another polyfill that I also had issues with so I was very happy that I found this.

I think this is a special case and I think for a polyfill like this where you target a specific object only and you actually want it to do its thing would not be that bad to have !important added like for other general CSS. I think its could actually be beneficial.

Maybe this can be a optional switch?

nextgenthemes commented 7 years ago

Sorry you are absolutely right and I am a idiot. Looks like I unlearned CSS or something, I now increased specificity and feel way more comfortable to get rid of horrible importants.