Closed (fixed)
Project:
Zen
Version:
7.x-5.x-dev
Component:
CSS/HTML markup
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
15 Aug 2011 at 00:57 UTC
Updated:
11 Dec 2011 at 09:20 UTC
Jump to comment: Most recent file
Comments
Comment #1
KrisBulman commented+1 for modernizr
Comment #2
johnalbinYes, Modernizr is nice. But a custom build is the proper way to add it to a theme. And its MIT, not GPL licensed. We can definitely add some docs about how to override Zen's default HTML5 shim + Respond.js with a custom Modernizr script.
Comment #3
johnalbinI've added respond.js to the end of the html5.js and renamed it to html5-respond.js. http://drupalcode.org/project/zen.git/commit/3c62c50
Its pretty likely that someone using Modernizr would want to remove the HTML5 shim and Respond.js at the same time, so it makes sense for them to be pre-aggregated and easier to remove by a Zen sub-theme. (They just need to override html.tpl.php and remove the conditional comments in
<head>.)Comment #4
KrisBulman commentedFor the life of me I cannot get media queries to to work in ie6, ie7 or ie8 using respond.js (stock, or html5-respond.js)
Does it perhaps not support the @import method for stylesheets that drupal 7 uses?
Comment #5
KrisBulman commentedComment #6
KrisBulman commentedok, fiddled around with this some more. I am able to get respond working if i switch to respond.min.js in the IE conditional comment and aggregate css, however it does not work with non aggregated css or with html5-respond.js
Comment #7
KrisBulman commentedthe problem appears to be with the monkey patch (all js files run fine until I hit this one)
Comment #8
KrisBulman commentedOK, so this patch should resolve the problem, the shiv/innershiv/monkey patch must be loaded before the scripts variable, and the respond.min.js loaded after. I used the html5.js from the html5 tools module which includes the shiv/innershiv/monkey patch, and switched to the minified version of respond.js
If html5.js is loaded after the scripts variable, "'Drupal.ajax.prototype' is null or not an object" errors appear in IE
If respond.js is loaded before the scripts variable, mediaqueries have no effect. I tested with a simple mediaquery css file loaded in zen's .info file containing the following:
which turns the background black after the 480px breakpoint.
NOTE: CSS still must be compressed in order for respond.js to actually work.
Comment #9
KrisBulman commentedThis may be something related to my setup, if others could test the existing implementation that would be great.. I'd like to isolate it.. if you find it's not working as expected, please try the patch.
I wonder.. how are we getting around the 'no 3rd party libraries on Drupal.org' policy by adding these gpl scripts? http://drupal.org/node/422996
Comment #10
ayalon commentedThanks for the patch. I applied it and the annoying error in IE 7 & 8 disappeared!
Comment #11
KrisBulman commentedas stated on the html5shiv page: "for the sake of performance, it would make better sense to include the CSS first then this script.", so the previous patch is out the window. I also think my dev box had something to do with the error, I was fiddling with the html5 tools module and it may have introduced some unexpected results.
instead, I added html5-respond.js as a theme setting, and a simple variable to html.tpl.php which adds the conditional if checked in theme settings, and adds nothing if not. I created this solution to avoid dirtying html.tpl.php with conditional js markup. It also does not produce any errors in IE.
I haven't included this, but additional checkboxes or radios could potentially be added for just html5shiv + innershiv, and respond.js in the event the user does not want to use html5 or respond.js together. thoughts on this are appreciated.
Comment #12
johnalbinThat was the issue! For some reason, running uglifyjs on the monkey patch actually breaks it. I also figured out I need to add a
if (Drupal.ajax) {around the monkey patch so that it doesn't error out if Drupal's ajax isn't loaded (like on a regular page for anonymous users.)I've committed just those minimal changes for now.
The larger question of should we make this a setting can be restated as…
How do we make it easy for sub-themers to remove Zen's HTML5 shim and Respond.js and replace it with Modernizr/whatever?
And I think we need to debate that question a bit. Up until now I've been taking the strategy of just letting sub-themers override html.tpl.php and remove the PHP snippet that adds the script in. Is this the easiest?
Comment #13
KrisBulman commenteda conditional javascript option in theme.info would be easiest of course, however due to #865536: drupal_add_js() is missing the 'browsers' option I'm pretty sure that's not possible. The approach I took uses a simple var to use it, or not. But I suppose it's less code if you just hand remove it while inserting something else.
There are also 2 modules to consider in the debate, that add the shim, or modernizr.
Comment #15
johnalbinYeah, there's no consistent way to add a conditional javascript for Drupal 7. :-\
Ok. I've been thinking about this for a bit and a theme setting would make it even easier to remove Zen's html5-respond.js. However, I've implemented it slightly different than the patch in #11.
Comment #16
KrisBulman commentedwhich commit is this? in the 7.x-5.x branch, I only see js fixes, but no addition of theme settings yet.
f4fe7cb - Update html5 shim to use iepp v2.2.
4a6b4e7 - Fix monkeypatch in js/src/html5-innershiv.js.
c5888f7 - Move unused javascripts into js/src directory.
Comment #17
johnalbinSorry laptop ran out of juice before I could push the commit. Pushed now. :-)
Comment #18
KrisBulman commentedvery clean solution to this problem; kudos! :)
Comment #20
tsi commentedIs this really working for you guys ? I've done a lot of testing of this implementation (7.x-5.x-dev) as well as the one by html5 tools module while working on my own theme (965) but none of them could get even a simple media query to respond in IE7-8.
Am I missing something ? can you really get something like :
to work under IE7/8 ?
Comment #21
jwillard commentedHave you had much luck with respond when using @font-face? Using @font-face in combination with respond can cause hard crashes of IE 7 & 8. I haven't tested the zen implementation yet, but I'd be curious to know if anyone has tested that use case.
Comment #22
johnalbin@tsi: Ouch! https://github.com/scottjehl/Respond/commit/fb7d75a5a01ff85c9d39385e6229...
It appears the easy work-around for that is to not include a @font-face inside a media query. Instead just put that @font-face outside any media queries. And then everything should work fine.
From the Respond.js README:
Comment #23
johnalbinI just realized that some people may not want to do a responsive design. "Some people may not want to do a responsive design" being a polite way of saying "my stupid client still wants to do a fixed-width design".
So we need a way to add the HTML5 shim without the Respond.js. And we should make the HTML5 shim toggle-able too, in case they want to use Modernizr. Oh, and the meta tags we are specifically adding for responsive design should be toggle-able too.
ok. one more try.
Comment #24
tsi commentedDon't want to create an argue here, but this is something I was dealing with myself in the past week and I got to the same conclusion - many projects/clients will never have a modified UX for mobile devices. So I made that optional too.
the thing is, I don't think this makes them stupid, I think today's mobile devices deal really well with the "real" (not-mobile) web, yes you need to zoom, so what ? is that really a good enough reason to invest all those resources on a mobile design ?
This article, even if taking a bit drastic approach, sums it up pretty well.
Don't get me wrong, I'm all for responsive layouts and many times they can (and should) be achieved very easily, but in the end, many of our big projects, and we're not talking blogs here, don't want to make that investment in design, development and maintenance, just to save the user one double-tap.
Furthermore, the mobile-first approach made me waste time not on design for mobile but on, you guessed it, supporting IE 7-8, because a mobile first layout without respond.js (couldn't get it to work, sometimes it's drupal's @import, sometimes @font-face as you found out, maybe something else too) is a no-layout-at-all for oldIEs. this was a deal breaker for me - I have decided to lose the mobile first approach and give users optional viewport meta tag and media queries.
Bottom line, I believe every project should be able to choose their target viewport. I believe most of the times, and this is not going to change IMO, this target viewport will not be smaller than 960px (desktops + tablets) so this is where the design should aim, while keeping the design at least functional for other devices.
Web-devs many times get caught in a hype, saying stuff like "[some_technology] is the future/right way forward/only way to go/my stupid client doesn't want to pay for that" etc. history tought us that if you believe one way is the only true way, you're probably wrong :)
Comment #25
johnalbinFair enough, Tsachi!
My comment was really meant just as a joke. But I do feel strongly that a mobile-first, responsive design is the best solution for all clients at this point.
However, as I hope the rest of my comment in #23 hinted at, I feel its important that Zen 7.x-5.x support both responsive and fixed-width designs. We're in a transitional phase in web development and we have to accommodate the traditional and the emerging styles of development at the same time.
Comment #26
tsi commentedNo problem, I got the joke :)
Just wanted to say that I too decided to allow opting-out of the responsiveness (in 965) and that maybe all the hype around responsive sites is a tiny bit exaggerated with todays mobile devices and so, maybe a client not investing in a mobile design, actually knows what he is doing better than us when we can't even imagine a site not responding to smaller devices, just because it can.
Anyway, +1 from me for the optional viewport meta tag etc. I totally agree.
you rock ;)
Comment #27
johnalbinOk. Here's the new options to disable Respond.js, HTML5 shim and the meta tags if you don't need them.
http://drupalcode.org/project/zen.git/commitdiff/3a6e01f