Skip to content
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

Remove Modernizr #1541

Merged
merged 1 commit into from
Oct 10, 2015
Merged

Remove Modernizr #1541

merged 1 commit into from
Oct 10, 2015

Conversation

retlehs
Copy link
Sponsor Member

@retlehs retlehs commented Sep 15, 2015

No description provided.

@retlehs
Copy link
Sponsor Member Author

retlehs commented Sep 15, 2015

modernizr finally came out with a new version (years later) and instead of updating to the new ver, i think it's probably better if we just remove it.

we've discussed this internally a few times recently. see this comment from @QWp6t:

im in favor of getting rid of modernizr. i personally don't use it at all anymore. if there's anything i need to test for, i just write a small test for it. just look at the code for modernizr tests. most are self-contained. don't need the whole library for it. i guess if i was testing for a lot of stuff, i'd probably load it for convenience.

we're also not supporting less than ie9 out of the box, so the html5shiv isn't necessary.

@JulienMelissas
Copy link
Sponsor Contributor

More often than not, I remove this at the end or beginning of the project. Totally in favor of removing Modernizr - it's obviously not very hard to add.

retlehs added a commit that referenced this pull request Oct 10, 2015
@retlehs retlehs merged commit 2be91c1 into master Oct 10, 2015
@retlehs retlehs deleted the remove-modernizr branch October 10, 2015 17:52
@retlehs retlehs mentioned this pull request Oct 14, 2015
@AustinGil
Copy link

I agree with removing Modernizr, but Im curious how you guys test for Javascript being enabled? I noticed base.php no longer has class="no-js" in it.

@QWp6t
Copy link
Sponsor Member

QWp6t commented Apr 7, 2016

By default, we don't. If you need it, just add the class back and then write some js to remove it.

If you want further support, please post on discourse.roots.io

@AustinGil
Copy link

Gotcha. Well that's not too bad. Just wanted to make sure I wasnt missing something. Thanks for the reply and Ill make sure to post there in the future.

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

Successfully merging this pull request may close these issues.

None yet

4 participants