Just a note that I am aware of the Libraries API.
When I introduced the dependency on jQuery Timers I thought about integrating with Libraries API. I did not understand, though, what Libraries API currently does, so for simplicity I just released without it. I should have put the jQuery Timers library in sites/all/libraries instead of directly in the clock module directory, but that's too late now.
I am thinking about writing some code to nudge people in the UI to move the library, but I not convinced that it's actually worth it, without true Libraries API integration.
I appreciate any information, help, comments, etc. in this direction!
Marking postponed on a stable release of Libraries API for now.
Comment | File | Size | Author |
---|---|---|---|
#14 | libraries-level2.patch | 4.05 KB | tstoeckler |
#13 | libraries-level2.patch | 4.52 KB | tstoeckler |
#10 | libraries_d7.patch | 1.19 KB | tstoeckler |
#8 | libraries_level1forreal.patch | 4.08 KB | tstoeckler |
#5 | libraries_level1.patch | 3.56 KB | tstoeckler |
Comments
Comment #1
tstoecklerLet's join the crowd
Comment #2
tstoecklerI think we should do three releases of Clock:
1. nudge people that /libraries is a better directory on /admin/block/configure/clock/clock with a 'warning' message telling them to move because the next release will not support it. Also tell them to not rename the file anymore.
2. tell people that /clock is WRONG! on /admin/block/configure/clock/clock with an 'error' message telling them that this is unsupported also make the JS-part not work (maybe hide the checkbox). Also show a similar error if the file is in /libraries but they have renamed it.
3. Remove all handling of the duplicate directories. Just include the file at /libraries and be done with it. People that haven't moved their file yet, get some weird PHP error probably. This will allow #720376: Add a Drush Make file.
I'm also thinking that this should go hand in hand with dependecy on Libraries API. Maybe in the following order (the numbers are the same as above).
1. In the warning message (and the release notes) include a note that Clock will depend on Libraries API in the future, so it's best to go get it now.
2. Do not do a
dependencies[] = libraries
in the .info file, but show a warning if Libraries API is not installed regardless where the library is.3.
dependencies[] = libraries
Will upload a patch for 1 soon.
Comment #3
tstoecklerWorked on 1 and got it working, but there is one problem.
jQuery Timers is distributed as a .js.txt files for whatever reasons.
drupal_add_js() didn't work for me with that file, only when renaming it to .js. I will investigate whether
1. jQuery Timers people are weird for letting us download a .js.txt file
2. Drupal is weird for not handling .txt files as JS.
3. We will have to rename the file until the end of time.
Comment #4
tstoecklerSo, I clicked around a little.
It seems core does rename the files from .js.txt to .js. I could find no documentation whatsoever about why jQuery will only allow .js.txt downloads, but it seems consistent with all plugins I looked at. I'm guessing security of their website, in case the JS is corrupt. I also could not find any source code of jQuery Timers or any form of Version control it is in.
I do not know whether it is actually a bug, for Drupal not to handle .js.txt files. Actually know that I think of it. The file was added to the scripts, I just got an error, maybe that was even browser-related. I don't know.
Anyways, I don't think it can be considered best practice to have a JS file with .txt ending.
Therefore we need to rename it.
In the past we also removed the version string from the filename. I looked and it seems, this is an inconsistency in jQuery Timers. Other jQuery libraries do not include the version string in their filename.
Therefore:
I hereby proclaim that is the Right Thing (c) to rename jquery.timers-1.2.js.txt to jquery.timers.js and will be of this belief until proven otherwise.
Working on a patch now.
Comment #5
tstoecklerWill commit this in a minute.
Comment #6
tstoecklerhttp://drupal.org/cvs?commit=344722
"Back" to active.
When Clock-6.x-1.2 is out we will do part 2.
In the meantime I will commit a patch to Drupal 7 which simply introduces a dependency on Libraries API.
Will update the project page then.
Comment #7
tstoecklerNooooo!!!
Writing that code for D7 I just realized that for Libraries API jQuery Timers will need to live in a 'jquerytimers' or 'timers' folder. I actually think 'timers' is the better name, because it is not 'namespaced', but the title of the project page of jQuery Timers is "jQuery Timers", so 'jquerytimers' is the correct folder name.
Will upload a patch to correct that.
...And then on to D7.
Comment #8
tstoecklerThis is how it should have been.
Comment #9
tstoecklerhttp://drupal.org/cvs?commit=344726
Committed untested. Remember to run tests before release. (Also: remember to update the translation template before release)
Now on to D7.
Comment #10
tstoecklerLet's see if it works...
Comment #11
tstoecklerYup. The patch file contains the wrong info file, but I didn't upload a new patch for that.
http://drupal.org/node/706594
Back to active and D6
Comment #12
tstoecklerNow that 1.2 is out, we can approach level 2.
Comment #13
tstoecklerUntested.
Comment #14
tstoecklerWell, I guess removing the form is just not that nice...
Comment #15
tstoecklerMissing trailing period.
This will lead to WSOD for people upgrading, who don't have Libraries yet. Should be wrapped in a function_exists().
Powered by Dreditor.
Comment #16
tstoecklerhttp://drupal.org/cvs?commit=371560
I must have multiple personalities...
Marking postponed, until the next release, where we will add: dependencies[] = libraries and get rid of the cruft.
Comment #17
tstoecklerIn #853000: Use only 3 variables instead of 4. I accidentally committed "true" Libraries API integration, so that Clock now depends on Libraries >7.x-alpha1. Will need to document that in the info file.
Comment #18
tstoecklerComment #19
tstoecklerSee #918734: Remove dependency on jQueryTimers