| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | javascript |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
Within #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries), jpetso brought up an issue that it's difficult to alter an array list for the dependencies. Should we switch to an associative list here instead?
Not sure about the format of "dependencies", see this piece of code that was committed to system.api.php:
<?php
'dependencies' => array(
// Require jQuery UI core by System module.
array('system' => 'ui'),
// Require our other library.
array('my_module', 'library-1'),
// Require another library.
array('other_module', 'library-3'),
),
?>Note the associative arrow "=>" in the "system" example and the "," in the other ones. The documentation also hasn't got the clearest of all wordings on this matter:
<?php
/**
* - 'dependencies': An array of libraries that are required for a library. Each
* element is an array containing the module and name of the registered
* library. Note that all dependencies for each dependent library will be
* added when this library is added.
*/
?>I think this error in the example might highlight a source of potential errors, other developers could easily make the same mistake. Maybe a format like the following one would be more appropriate?
<?php
'dependencies' => array(
// Require jQuery UI core by System module.
'system' => array('ui'),
// Require our other library.
'my_module' => array('library-1'),
// Require another library.
'other_module' => array('library-3'),
),
?>
Comments
#1
Unless we go with class objects and properties, this is probably the next best thing.
#2
Also looks like my patch from #315100-285: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) was not committed (which at least fixed the docs).
#3
@sun, the #315100: Allow to add JS/CSS libraries (sets of files, settings, and dependent libraries) was committed in it's #281.
In it's #291, Rob Loach says "Thanks for giving that a look, Gabor.... http://drupal.org/update/modules/6/7#drupal_add_library . I also updated http://drupal.org/update/modules/6/7#jquery_ui ."
Is there anything left to work on for this bug?
I don't see anything to review...
Mike
#4
#5
Re-attaching here.
#6
The last submitted patch, drupal.library-api.patch, failed testing.
#7
It would be nice if this was an associative array, but unfortunately it's not. I think we should just fix the documentation here, and possibly open up a new issue if we want to change the API later on. I'm setting this to RTBC.
#8
Committed to CVS HEAD. Thanks.
#9
Automatically closed -- issue fixed for 2 weeks with no activity.