Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I learned about this module in a training and think it is fun. I noticed that there is a check to make sure that the respond.js file is not being included by an @import so it nags the user to aggregate the javascript.
That is great, but if the user has the conditional style sheet module enabled they get this kind of included javascript:
<script type="text/javascript" src="/sites/all/libraries/respondjs/respond.min.js?m0oy5u"></script>
I think that is *good to go* so I added a patch to check if the user has conditional styles installed and not nag them in that case.
Comment | File | Size | Author |
---|---|---|---|
#2 | respondjs_requirements_not_nag.patch | 1.56 KB | populist |
conditional-styles-option.patch | 758 bytes | populist |
Comments
Comment #1
ruplIt's not dependent on the respond.js
<script>
tag, which will always be included that way. The crucial requirement is having stylesheet<link>
tags that should be used in lieu of@import
ed styles. Here's the passage on the project page currently:Reading it again, I think I'm wording the documentation poorly. Maybe it should read:
Comment #2
populist CreditAttribution: populist commentedThanks for the clarification here.
Now allow me to pivot the issue to discuss the hook_init() provided user nag. How would you feel about moving that into hook requirements instead? It seems to make more sense in there (like the other two hook_requirement considerations) than on every admin screen. I made a patch.
Comment #3
ruplYeah I can see doing that. This tiny bit of config has accounted for probably 80% of support requests, so when I added the warning, I went for full-annoyance mode :) Patch looks good on visual inspection, will review/commit during Denver. Thanks, Matt!
Comment #4
populist CreditAttribution: populist commentedMaybe we can make the "nag" the default for first time users, but can be easily hide "Dont Nag Me Anymore!" and then we just use hook_requirements() for the real check?
Comment #5
ruplAh yes, like the Advanced Help message in Views, that's a good idea.
Comment #6
ruplThe move to hook_requirements() was committed to dev and will be released as part of 1.1 very shortly. Thanks for the patch!
I opened up #1487030: Allow admins to disable the severity of CSS aggregation warning as a follow up to your other suggestion.
Comment #7.0
(not verified) CreditAttribution: commentedWrapped HTML in
so it displays in the summary.