-
Notifications
You must be signed in to change notification settings - Fork 250
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
Scss version does not handle vendor prefixes #211
Comments
@stefanoverna Ah, you're right. I didn't notice because I use Autoprefixer all the time but this should be built into Jeet. Would you mind submitting a PR? |
@corysimmons done! |
Is there no sass autoprefixer plugin? I don't really think this should be built into jeet... |
@Jenius There is, but that puts dependency on that particular plugin whereas Jeet's core functionality should work with or without those plugins. I think the rule here is to not start adding crap like resets and boilerplates into the equation. As long as it's still within the context that we're coding in (Stylus/SCSS) then it makes sense to me. |
I think the rule should be to make jeet a grid system, not anything else. As soon as you start saying, "well, we can put in some prefixing", what exactly is the justification for saying no to a reset or boilerplate? You have to draw a line somwhere, an easy place to draw it is "we only include code that makes this a grid. if you have other issues with browsers, resets, boilerplates, etc, you can use other libraries with jeet to solve this." You're walking a dangerous line by including this stuff. Just make sure to think about justification before you do so. |
I have, and feel like this is a decent justification. I won't let it get out of hand and if someone is using autoprefixer then it will get rid of these. Tbh, I might re-write this without the extra prefixes and the mixin itself, but I stand behind allowing broken stuff to be prefixed to save users the confusion. |
So here's my line of thought: Now that you have prefixes, you need to decide how far back you support. All prefixes that ever have been around? If not, you might not support some situations, and people will complain. And what if someone doesnt want any prefixes? If you are injecting all prefixes because you want to support anything, what if I don't want my code to have that extra bloat by default? So solve that problem, now do you need more config options to turn them on or off? Or are you going to offer a custom range of browsers like autoprefixer? At that point you might as well embed autoprefixer into your library. But then if someone wants to use autoprefixer on their code as well, you are loading 2 versions of autoprefixer for no reason. Is what you have now just a halfhearted solution, or does it really solve the problem? And that being said, does it really even need to solve the problem? Also, now that you have done sass, you also need to prefix the stylus library, correct? I entirely understand your viewpoint of "well this is just a small change and it will save new users some time" but you also must understand that it is not technically the right decision. You are letting your scope slip, it's becoming less modular, and you are offering a half-hearted fix for prefixing, something that other tools are entirely dedicated to solving better, as part of the core of your grid library. I would rather put a line in the readme that says "if you are using this with older browsers, you should make sure you also are using a prefixing library like autoprefixer" to save confusion than throw extra things in core to solve the problem. As this library becomes more popular, you will find that more and more newer and beginning developers will be using it, and filing issues. Many of them will not have read the readme at all. Some of them might have just copy-pasted code straight from your API examples without understanding what it does. (Not accusing Stefano of this, I just mean in general). You cannot allow this to sway you into making Jeet worse for the sake of 'beginner-friendliness'. Your goal here, I assume, is to make excellent software, not to make software that anyone can use without reading instructions or thinking about it. All I'm saying here is these things that may seem like small and simple decisions at the time mean a lot, especially with a library so narrow in scope as jeet. So really make sure you have taken time to think about the further reaching consequences. As a quick example, if the stylus version of jeet starts pumping out 5 browser prefixes on half the lines, I'd rather fork it to remove the functionality to use on our projects because I don't need it. It's easy to add functionality, but a whole lot harder to remove it. |
Tbh, I did have a specific point in mind, IE9+ without any external libs at all. I think the mixin approach isn't great so I'll fix it and do manual autoprefixing. Is this really this big of a deal? Especially to your workflow where you'll never see meaningless prefixes as they'll be compiled and overridden by Autoprefixer? I'm just making this a tiny bit more accessible at no real cost to anyone, and IE9 prefixing was my cutoff. Don't be so worried I'm going to make this monolithic again. |
Yes, it is. I have said all I need to say here though, the rest is up to you. |
Also on this ticket: I'd also propose killing all browser prefixes and possibly new ones and fall back to W3C (Draft) properties which should then be taken care of �by IMO, prefixing is not in Jeet's scope and since we are discussing a lightweight library, let's keep it that way. |
👍 ✨ 💖 ✨ 👍 |
The problem is present in the
align
mixin, for example, where the-webkit
prefix is missing:The
align
mixin, as it is, won't work on most browsers.The text was updated successfully, but these errors were encountered: