Closed (fixed)
Project:
Colorbox
Version:
8.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
9 Mar 2016 at 07:46 UTC
Updated:
29 Mar 2016 at 02:14 UTC
Jump to comment: Most recent, Most recent file
Problem/Motivation
I think either libraries needs to be made a hard dependency instead of a soft one or it needs to be removed from the equation in hook_requirements. Right now, without libraries installed you don't get any warnings that you aren't setup.
Proposed resolution
Either remove the soft dependency on libraries entirely or just in "hook_requirements".
Remaining tasks
Decide if this is a good or bad idea.
User interface changes
Hopefully a more accurate status page.
API changes
None.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2683947-7-detect-without-libraries.patch | 5.25 KB | frjo |
| #5 | 2683947-detect-without-libraries.patch | 4.23 KB | sam152 |
| #5 | Screen Shot 2016-03-11 at 5.04.33 PM.png | 12.01 KB | sam152 |
| #5 | Screen Shot 2016-03-11 at 5.04.19 PM.png | 19.19 KB | sam152 |
Comments
Comment #2
frjo commentedI agree that we should remove the dependency on the library module. I have kept it so far since it's has an easy way to retrieve a library version and check that it's installed. I have not found similar functionality in the Drupal 8 core library functions (colorbox.libraries.yml).
Maybe we simply should skip the version check and only implement a simpler check that "/libraries/colorbox/jquery.colorbox-min.js" exist?
Comment #3
sam152 commentedYeah, I think that's a good plan. To be honest I think the master package has never been broken before and I don't think there are multiple branches out there?
I don't mind looking at this.
Comment #4
sam152 commentedHm, the module suggests a minimum of 1.6.1, but I've been using v1.5.9 - 2014-04-25. So I'd guess any recent version isn't going to cause issues and the check for the file is fine as you suggest. Most people aren't getting the benefit of the version check, not having libraries installed.
Comment #5
sam152 commentedAttached is the patch and what you can see in the UI now.
Comment #6
sam152 commentedComment #7
frjo commentedWorks well! I edited the messages a bit so it has a value plus description. Also changed the download url in the Drush command and the README to be the same as in the requirement message.
Colorbox used to have two branches 1.x and 2.x. Colorbox on Drupal 7 with the default jquery version only worked with 1.x. Now it seems that development on 2.x has closed and master is the one to use just as you already noticed.
Comment #8
sam152 commentedLooks good to me!
Comment #9
frjo commentedCommitted.