Conversation
think2011
commented
Dec 24, 2015
|
: ( |
|
Hi @think2011. Could you share the thinking behind this functionality? At the moment I'm leaning towards not merging since IMO adding css classes to elements is outside of the responsabilities of this library. Would love to hear your thoughts. |
|
I am against merging this PR. Let me explain why. I could see a use-case for these classes. Since the script detects the browser being run, the classes can be used to apply browser-specific CSS to remedy certain browser bugs and inconsistencies (I have dealt with that myself). It also follows the trend Modernizr helped set by classing the I am against this PR for a fundamental reason: the script "detects" the browser being run. In order to "detect" the browser we scan and extract details from the user-agent. The problem is the user-agent can be changed at will, making it 100% unreliable. Because the detection and classes would be dependent on an unstable value, one merely must change their UA to receive code/styling that did not apply to their browser. That is why I am against merging this PR. I can sympathize with the need to apply browser-specific styling (although that is dangerous now that browsers use an evergreen update model), but adding those classes via UA-sniffing is not the way to do it. (I actually wrote a small script to feature-detect browsers for this express purpose. Yes it is an anti-pattern (which is why I do not release it), but it is what it is.). |
|
Im with you @le717. Lets see what @think2011 thinks. |
#box {
margin-left:20px;
}
.ie8 #box {
margin-left:21px;
}Like Modernizr, It's useful for my present work. And thanks for you explanation @le717, I agree with this point :-) |
|
👍🏻 |
|
@think2011 If you are trying to target old IE, I recommend researching IE conditionals and separate your IE hacks into a separate stylesheet. :) |
|
@le717 Thanks :-) |