Comments

I agree with that feature!
Subscribing...

I second that. At least it should be possible to choose the layout.

Agreed.
Often various modules utilised in a site each using different instances of Colorbox can cause each other to malfunction.

Subscribing

Title:Use colobox module settingUse colorbox module setting

Subscribe

Subscribing

I created a patch that should make it possible to use most of the ColorBox module settings. It is based on media_gallery beta 4 and you need to install the latest version of the ColorBox module (http://drupal.org/project/colorbox).

There are still problems with the styles that are shipped along the ColorBox library, especially example3 (e.g. wrong background color). I'm still working on a better way to get the description displayed in the overlay. For now, I set a fixed height in the CSS file (scrollbar is shown if the description is too long), however you should still be able to override these settings in your theme if needed.

Please test it and let me know if you run into other issues.

Hello, I'm new in Drupal and I need just it, gerrit.k youcan say me in wich file/module the patch is apply.

Thanks a lot.

Disculpen por el idioma,es que no domino el ingles

Put the patch in your drupal/sites/all/modules/media_gallery/ directory and apply it. http://drupal.org/patch/apply has some more information on how to do this (I only used git to test it).

Version:7.x-1.0-beta3» 7.x-1.0-beta4
Status:Active» Needs work
StatusFileSize
new7.15 KB

I made some smaller changes to the patch and tested it with the ColorBox example1-5 styles. Compared to the original beta 4 I moved the description away from the right side because example3 looks horrible. Example3 has the navigation buttons on top of the image and not above or below, so it would be rendered on top of the text otherwise. However when loading flash videos (e.g. media_youtube), the buttons are unusable with that theme.

Maybe some CSS wizard can have a look at the stylesheet and figure out a way to make all styles work equally good by default, without "fixing" them manually.

Status:Needs work» Needs review
StatusFileSize
new12.56 KB

I've updated the patch to have fewer changes in the PHP files but many more in the CSS and JavaScript ones.
According to the selected style in the ColorBox module settings a class is added to the body tag on creation of the ColorBox (e.g. example2 is selected, .cbox-example2 is added). Obviously this allows you to have completely different CSS code for each ColorBox style. I included somewhat OK-looking ones for all styles shipping with the ColorBox library and module. Example1 is mostly unchanged compared to the original, all other styles have the description field moved below the image / video.

Feedback would be really appreciated.

Thanks its looking good !

StatusFileSize
new12.23 KB

Same as patch 3 (comment #12) with fixes for whitespaces and new line at end of file.

when will this be committed as I have not hope of applying patches.

Title:Use colorbox module settingIntegrate with colorbox module
Status:Needs review» Needs work

Renaming to hopefully better describe the issue.

Patch at #14 is looking great. I've found the following problems though with Firefox 3.6.17:

  1. When using Stockholm Syndrome the close button appears at the very top of the viewable page. If the page is scrolled up to the very top and the colorbox is opened only haft the close button is seen.
  2. Example 1 theme is taking up extra whitespace to the right (320 px).
  3. Using Example 4 the "Next" text is slightly overlapping the "x of y" counter.

Other browsers have not been tested.

Good points you made there and I can confirm most of them. However I'm not so sure that these issues are specific to this patch:

  1. Stockholm Syndrome is a style that ships with the ColorBox module itself and doesn't work that great for me at all.
    I created a custom content type with a multiple image upload field that is displayed in a ColorBox (and therefore not using media_gallery at all) and the problem still exists while having a bunch of other issues like creating inline scroll bars for the image etc.. This could probably still be fixed inside this patch, but it might be better if this is fixed in the ColorBox style itself in the ColorBox module.
  2. Do you mean there is additional white space to the image or to the description? Before the patch the style example1 would be used exclusively and the description field was on the right side of the image (extra space to the right). I didn't want to alter this look, so this shouldn't have changed. I currently don't have Firefox 3.6 to test this, but in Firefox 4 and other browsers it looks fine for me. However if there is even more space added to the right in addition to that, it is definitely a bug that should be fixed. Can you confirm this?
  3. Example4 is a style that ships with the ColorBox library. On the demonstration page you can see that they use shorter labels ("next"/"previous" instead of "« Previous"/"Next »" (ColorBox module defaults)). The style doesn't reserve enough space for longer labels. However this is not an issue specific to media_gallery or this patch. Again I think this should rather be fixed in the original style or in ColorBox module where other uses of the style would also be covered.

StatusFileSize
new274.79 KB

Attaching a screenshot to better describe the extra whitespace issue.

I guess you have enabled "Show title and description" for your gallery but didn't enter a description for the image? If that is the case then your screenshot would look the same as if you used media_gallery beta 4 without the patch. I tested it on my installation with both versions and "Show title and description" enabled and they both reserve that extra white space even if a description for that particular image is missing. However that would mean that it isn't a regression of the patch, right?

Thank you for the patch, with it the colorbox display is well structured. I have two questions regarding the output:
First: Is it possible to change the order of fields, so that I can display the title (cboxTitle) before the Textfield (mg-lightbox-description)?
Second: I have seen, that the div of the textfield is apperaring in the code of the displayed colorbox even if it is empty. So it is showing the following: <div class="mg-lightbox-description"></div>
Would it be possible to leave it from being printed when its empty?
Best Regards,

Version:7.x-1.0-beta4» 7.x-1.0-beta5
StatusFileSize
new12.32 KB

Update patch for 1.0-beta5 release.

Any hope that the patch will be rolled into a release soon?

Thanks!

subscribe

+1. patch in #21 seems to be working well for me. Thanks.

Status:Needs work» Needs review
StatusFileSize
new12.32 KB

patch in #23 fails to apply through drush make due to a file permissions issue.

warning: media_gallery.info has type 100644, expected 100755

Attached patch resolves.

Status:Needs review» Needs work

This patch (#25) is failing on beta6.

bash-$ git apply ../../hacks/1053674-Use_colorbox_module_setting-4_0_0.patch
error: patch failed: media_gallery.info:3
error: media_gallery.info: patch does not apply
bash-$

I think this is a pretty important feature and I'd like to see this go into the dev branch if possible. This module should really be using the colorbox module, colorbox is pretty stable at this point.

Also I think the small css bugs are not important, given that the current media_gallery module's colorbox looks more messed up, and you can't configure it.

Media Gallery is a great module and a great way forward.

FYI this is a related thread: http://drupal.org/node/1088738

Version:7.x-1.0-beta5» 7.x-1.0-beta6
Status:Needs work» Needs review
StatusFileSize
new12.33 KB

Update patch for 1.0-beta6 release.

subscribe.
Thanks for your great work

StatusFileSize
new348.56 KB

I installed media_gallery beta-6 and I use patch in #25 but the whitespace problem not resolved by patch!
What I did i do wrong?

This was my fault, I tried it again, I did it.

I use the patch in #28 (media_gallery beta-6) but I see no difference. Colorbox which always has the same size independent of the screen resolution.

Now I see the difference. Thanks!
Just a pity that the Colorbox the screen resolution does not adapt.

Version:7.x-1.0-beta6» 7.x-1.x-dev
Status:Needs review» Reviewed & tested by the community

Patch from #28 works perfectly on 1.x-dev :)

Pls submit it to core!!!

subscribe

Version:7.x-1.x-dev» 7.x-1.0-beta7
Assigned:Unassigned» Joenet

Hello,

Will there be a patch for 7.x-1.0-beta7?

Version:7.x-1.0-beta7» 7.x-1.x-dev
StatusFileSize
new12.37 KB

Update patch for 1.x-dev (works on 1.0-beta7)

Yes, it works. Thank you!

Subscribe

Version:7.x-1.x-dev» 7.x-1.0-beta7
Assigned:Joenet» chules

I am running version 7.x-1.0beta7 and just applied patch integrate_with_colorbox_module-1053674-37.patch. I am using the Colorbox default style and everything seems to work fine. I have tested a few of the Colorbox styles as well.

The one issue I see on my site is that I am now missing the slideshow feature that was present before the patch where a site visitor can click it and watch the slideshow.

Is there something I missed in configuration? Does anyone know how I can activate this feature.

Thank you - chules

Hi chules,

You can re-activate it through the Colorbox configuration (admin/config/media/colorbox). Under Options, click Custom, and navigate down to Slideshow Settings.

This appears to be in stable enough state to be committed. Is it expected to be checked in soon? Thanks

Assigned:chules» Unassigned
Status:Reviewed & tested by the community» Needs work

Notes to patch in #37:

1. If you add a dependency to colorbox, you should also change the README.txt file (the installation guide - requirements).

2. Coding Standards: Always use a space between the dot and the concatenated parts to improve readability.
+    '<div class="mg-lightbox-wrapper clearfix '.$media_desc_class.'">' .
replace with
+    '<div class="mg-lightbox-wrapper clearfix ' . $media_desc_class . '">' .

Status:Needs work» Needs review
StatusFileSize
new13.88 KB

Here's my try to improve the patch in #37 based on the suggestions of Moloc in the previous comment - it just fixes the coding style in the mentioned line and adds the necessary information to the README.

I've tested this patch with the 1.3.19 version of the Colorbox plugin, hence the "tested" line in the README. Please modify it as necessary.

subscribe

I've set patch #44 and got rid of all the white space on the sides, but there is still a large white space at the bottom, where a description might be. I tried hiding the description field in the content type, but still have the white space. Any ideas?

Thanks
Sharon

Hi. I have applied patch #44 to Media Gallery 7.x-1.0-beta7 + colorbox 7.x-1.2 and it all seems to be working as expected, great work, thanks! I am not getting the large white space at the bottom mentioned in #46.

This is a very useful feature and it looks like many people have got this working, any plans for this to be commited soon?

I had another look at this. When I applied patch #44 on Media Gallery 7.x-1.0-beta7, the patch applied successfully but the following change did not apply for some reason:

-   '<div class="mg-lightbox-wrapper clearfix">' . "
+    '<div class="mg-lightbox-wrapper clearfix ' . $media_desc_class . '">' .

Without this change applied, everything worked fine. When I made this change manually to the media_gallery.theme.inc, I was getting the large white space at the bottom, as also reported by Sharon in #46. I guess that this is where the description is supposed to be, but even if I have setup descriptions for the images these are not shown and there is this white space which should be removed, does anybody know how to show the descriptions if required?

Apart from this problem, which I think that should be resolved, everything else seems to be working ok so far.

My patch made the change as mentioned was missing in #48 and my descriptions show up, but still a large white space underneath.

Sharon

As per #47 and #48, the integration with colorbox seems to be working ok with patch #44, apart from the large white space underneath. Does anybody have any idea on how to fix this? Assuming this is resolved, are you happy to get this functionality commited? Thank you.

Hi all, does anybody have any thoughts on #50? I hope this can be progressed and commited, thank you!

Will someone please commit this?

Assigned:Unassigned» Babich

Hello, I applied the patch # 44, but the problem persists, what could be the reason?

Assigned:Babich» Unassigned

As per my post at #50, the only thing that is stopping #44 from being RTBC is the large white space underneath. Can somebody please review so that we can get this commited?

@Babic, please do not assign to yourslef unless you are planning to work on this :)

I just wanted to clarify why the patch may not work #44

I second that!

Subscribe - that should go into the main code.

Could any of you make a recap of this issue and modify the patch, so we can get this to RTBC? The issue queue in media_gallery is moving again, and things are getting committed now.

+1

please use the green button to follow instead of subscribe comments

@lsolesen - it is great to see that finally things are moving again with this project. I think that patch at #44 is almost providing all the functionality required, but please see the outstaning issue described in #48 and #49, hopefully somebody can identify what the problem is and update the patch appropriately.

If someone could give me a hint on what to test exactly I would be happy to help with this issue.

write a recap of the issue

In the current release, we assume, that the colorbox-library is available. As there is also another feature request (#1311568: Integration with other lightbox alternatives), to use alternative lightboxes, it would be a bad idea to make the colorbox-module a dependency. We should make that optional. If there is a lightbox module, the user should be able to select a installed lightbox.

agreed. so remove existing integration and make it optional to have a lightbox module.

Status:Needs review» Closed (won't fix)

We should be working on this one instead: #1311568: Integration with other lightbox alternatives

Status:Closed (won't fix)» Needs review

For me, integration with other lightboxes means, that we provied some way (hook, api,...) to use other lightboxes. This does not exclude the core of this patch - using colorbox settings. If we do not provide colorbox support, a lot of sites may get broken. So i think this patch is usefull.

Personally, I think patch number 44 should get commited since its already there rather than waiting for the whole lightbox alternative integration.

Thats my 2 cents.

As far as I can see #1311568: Integration with other lightbox alternatives will take some time to deliver a somewhat complete lightbox solution - IMHO more than implementing this properly would. Still, it would be useful to have support for different lightboxes of course.

@Moloc. We need to come to a conclusion here.

a) Some people do not like us to force a dependency on the colorbox module. However, they are already forced to use the colorbox library.
b) We need to add more settings, making it easy to just use the colorbox module.

We have two options:

1) Integrate the colorbox module if it is available. Otherwise use the existing functionality.
2) Ditch the existing functionality and only use the colorbox module.

Seems to me that we should go with option 2, but make sure to write good documentation with the upgrade where this happens. Your thoughts?

Move on to Option 2.

Any thoughts about this?

Move to option 2! Patch#44 worked great for me, using media_gallery 7.x-1.0-beta8. Please commit.

I'm late to this thread.

Will this fix the problem of Media Galleries colorbox windows being fixed in size?

I find that it doesn't work well for mobile, yet where I use colorbox elsewhere, the modal windows scale nicely to the available screen size.

Is a version of the patch #44 available for Media Gallery 7.2 ?

I would also suggest that #44 is committed to 1.x and also implemented for 2.x (for D7) until and when #1311568: Integration with other lightbox alternatives is completed. Many thanks for the great work.

Status:Needs review» Reviewed & tested by the community

I've installed and tested the patch in #44 quite a bit, and don't see the white space issue mentioned above.

Colorbox: 7.x-2.4
Colorbox Library: 1.4.21
Media Gallery Version : 7.x-1.0-beta8

Here is a version of the patch working with 7.2

Issue summary:View changes
Status:Reviewed & tested by the community» Needs work
  1. +++ b/media_gallery.css
    @@ -514,57 +514,197 @@ a.media-gallery-thumb img,
    +.cbox-example1 .lightbox-title,

    Can we move all the colorbox related css into its own file?

  2. +++ b/media_gallery.install
    @@ -1303,3 +1303,61 @@ function media_gallery_update_7010() {
    +function media_gallery_update_7012() {

    This is from another patch and was already committed.

  3. +++ b/media_gallery.module
    @@ -749,35 +749,6 @@ function media_gallery_block_save($delta = '', $edit = array()) {
    - * Implements hook_library().

    This I love to see :)

  4. +++ b/media_gallery.info
    @@ -3,6 +3,7 @@ description = A flexible gallery of media.
    +dependencies[] = colorbox

    Rather than making this a hard dependency, it would be best to just leave it available as an option. There will be no harm to sites that choose not to use colorbox.

Edit: upgraded colorbox library and all is good in the world

---
thanks for this. I like being able to use the colorbox module since I already have it installed.

But I am getting 'Cannot call method 'toString' of undefined:

On the edit page:
Uncaught TypeError: Cannot call method 'toString' of undefined colorbox-display.js?n0jftm:62
(anonymous function) colorbox-display.js?n0jftm:62
n jquery.min.js?v=1.7.1:2
o.fireWith jquery.min.js?v=1.7.1:2
e.extend.ready jquery.min.js?v=1.7.1:2
c.addEventListener.B

Here is the line:
var cboxTheme  = (Drupal.settings.colorbox.__drupal_alter_by_ref).toString();