Closed (won't fix)
Project:
HTML5 Tools
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
18 Sep 2010 at 23:35 UTC
Updated:
30 Jul 2011 at 23:05 UTC
Jump to comment: Most recent file
Comments
Comment #1
alanburke commentedPatch attached would let people remove the code from the Modernizr.s that adds the js class.
Comment #2
alanburke commentedDrush to the resuce... possibly.
I made an attempt on Drush integration that would download the Modernizr js file.
But I can't find a direct download link for the compressed js file.
Drush integration only seems to work for modules, so the drush integration lives in the html5_tools module,
but downloads to the html5_base theme.
The file downloaded is straight from the github site, while I look for a direct link to the minimized file.
Anyway, not sure how much use this is, but nonetheless, patch attached.
Comment #3
alanburke commentedComment #4
paul.irish commentedHaving 'js' twice in the className attribute shouldn't be a problem.
There is no additional performance hit nor do any styles or selectors work any differently with 1 'js' or 2.
So I would say you can ignore the double class.
The drush patch seems unrelated, but is pretty cool. Though you probably want to link to something that won't change whenever I commit something new. :)
Here's the tagged 1.5 release:
http://github.com/Modernizr/Modernizr/raw/2ec8bca2efef3638270ecfb02cea5b...
Comment #5
alanburke commentedHi Paul,
Thanks for the direct link, and for the useful info that the double js class is harmless.
Is there a direct link to the minified file anywhere?
Updated patch anyway.
I might try and get the drush command to patch the file as it downloads it,
just for the sake of removing that double class.
Comment #6
mason@thecodingdesigner.com commentedI'm confused about this patch. Are you patching against the HTML5_base theme or the HTML5_tools module?
Comment #7
alanburke commentedBoth :-)
The Drush integration is against HTML5 tools [drush integration only works for modules, AFAIK].
The other patch can be applied against modernizr.js to stop it producing a duplicate 'js' class.
I'll move this to HTML 5 tools.
I think I'll adjust it to download to sites/all/libraries.
Maybe HTML5 base can look there for the file instead?
Comment #8
ericduran commentedI like the idea, not sure it'll work.
If we set up drush to dl the modernizr to sites/all/libraries then html5_base will need to be hard coded to that url, or do some logic to get it from any libraries folder (ex. sites/example.com/libraries).
It'll be nice if we could just used the libraries module but that's no possible (from the theme that is).
I don't really have a recommendation but I did bring this issue up in #917728: Fixing default settings value, I think for now we can leave it checking the js folder but fix the validation as mention in the other issue :-/
Comment #9
ericduran commentedany other comments on this?
Switch it to feature request as it's not really a bug.
Comment #10
ericduran commentedDon't remember the status of this.
Comment #11
alanburke commentedI'll have a look at this again when the mod is ported to D7
Comment #12
adrinux commentedFor the record I think automated drush integration is a bad idea. Especially now we have Modernizr 2 beta where you can tailor the library you download to only those features you're actually using. At the very least it would need to be configurable and default to off.
This would also really mess with people like me that are building platforms with drush_make. What happens if I've already had drush_make install a tailored version of the library for me?
Comment #13
ericduran commentedWe're only using html5shiv in html5_tools so no need for use to provide a way to download Modernizr.