Skip to content
This repository has been archived by the owner on Dec 7, 2017. It is now read-only.

Add option ''modernizr' to 'Unused Classes' #7

Closed
wants to merge 1 commit into from

Conversation

oskarjakiela
Copy link

I know, it could be easy reach by overriding rule, but i think that there are 2 issues about:

  • Modernizr is popular and uses as basic tool to build modern web apps, so many developers would have to list classes on their own to avoid flood of "Unused Classes" warns
  • Modernizr adds its classes to HTML tag, what isn't to simple override

Thank you and I'm sorry if something goes wrong - it's just my first pull request ;)

@philipwalton
Copy link
Owner

I'll have to think about this a bit more, but my initial inclination is to not include these classes in the whitelist. Modernizr provides a really easy way to prefix all its class names, which makes extending the "unused-classes" rule trivial. Furthermore, most of those classes are pretty safe, but I could see something like .flexbox being widely used as a helper class by a lot of people, so I wouldn't necessarily want it to be whitelisted everywhere.

At my company we use Modernizr and we just prefix every class name with the string supports- to mimic the native CSS @supports conditional rules. As you probably noticed, the /supports\-/ RegExp is included in the whitelist out of the box.

But either way, I think you bring up a good point that most people who use both Modernizr and HTML Inspector with their default configs will be annoyed with all those errors. At a minimum I think it deserves a mention in the README.

@oskarjakiela
Copy link
Author

If I'm not mistaken in Modernizr 2.6 only way to prefix every class is using custom builder, is not? It could be some obstacle for existing projects.

But I agree with you - prefix Modernizr is more "elegant" than hardcode every possibility - now i realize it ;) Update doc and little gist with whitelisted class names should solve the case.

@philipwalton
Copy link
Owner

Yeah, you do need to use a custom build to get a prefix, but as far as I know, must people do that. There's even an awesome Web interface for it.

The only downside of a custom prefix (that I can think of) is for large, existing sites that don't want to or aren't able to change their existing CSS that already uses the default Modernizr classes. But perhaps for those sites, simply excluding the HTML element is better.

I'm still thinking about this.

@isimmons
Copy link

After taking a look at your "CSS Architecture" article, it makes good sense to prefix with supports or js so this works well. I think the addition of the ability to exclude DOM elements would cover older sites and those who for what ever reason may not want to add a prefix. Plus people who use the cdnjs version which is not prefixed.

@philipwalton
Copy link
Owner

I updated the README with an FAQs section that specifically mentions how to deal with the Modernizr case. I'm going to close this issue for now, but definitely let me know if you think it needs to be presented more clearly or accomplished more easily.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants