Support from Acquia helps fund testing for Drupal Acquia logo

Comments

camdarley’s picture

I agree with that feature!
Subscribing...

KirstenLangholz’s picture

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

BarwonHack’s picture

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

BenK’s picture

Subscribing

sw3b’s picture

Title: Use colobox module setting » Use colorbox module setting
Thomas Bosviel’s picture

Subscribe

anavarre’s picture

Subscribing

gerrit.k’s picture

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.

jamc17’s picture

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

gerrit.k’s picture

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).

gerrit.k’s picture

Version: 7.x-1.0-beta3 » 7.x-1.0-beta4
Status: Active » Needs work
FileSize
7.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.

gerrit.k’s picture

Status: Needs work » Needs review
FileSize
12.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.

sw3b’s picture

Thanks its looking good !

gerrit.k’s picture

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

chaloum’s picture

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

JimmyAx’s picture

Title: Use colorbox module setting » Integrate 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.

gerrit.k’s picture

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.
JimmyAx’s picture

Attaching a screenshot to better describe the extra whitespace issue.

gerrit.k’s picture

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?

brenes’s picture

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,

Thomas Bosviel’s picture

Version: 7.x-1.0-beta4 » 7.x-1.0-beta5
FileSize
12.32 KB

Update patch for 1.0-beta5 release.

kirilius’s picture

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

Thanks!

good_man’s picture

subscribe

mrfelton’s picture

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

mrfelton’s picture

Status: Needs work » Needs review
FileSize
12.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.

glass.dimly’s picture

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.

glass.dimly’s picture

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

Thomas Bosviel’s picture

Version: 7.x-1.0-beta5 » 7.x-1.0-beta6
Status: Needs work » Needs review
FileSize
12.33 KB

Update patch for 1.0-beta6 release.

jczuo’s picture

subscribe.
Thanks for your great work

segi’s picture

FileSize
348.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?

segi’s picture

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

Joenet-dupe’s picture

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.

Joenet-dupe’s picture

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

axe312’s picture

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!!!

bkosborne’s picture

subscribe

Joenet-dupe’s picture

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

Hello,

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

Thomas Bosviel’s picture

Version: 7.x-1.0-beta7 » 7.x-1.x-dev
FileSize
12.37 KB

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

Joenet-dupe’s picture

Yes, it works. Thank you!

franzkewd’s picture

Subscribe

chules’s picture

Version: 7.x-1.x-dev » 7.x-1.0-beta7
Assigned: » 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

cjgriffin’s picture

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.

marktheshark’s picture

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

Moloc’s picture

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 . '">' .

scorchio’s picture

Status: Needs work » Needs review
FileSize
13.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.

midmood’s picture

subscribe

SharonD214@aol.com’s picture

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

dimitriseng’s picture

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?

dimitriseng’s picture

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.

SharonD214@aol.com’s picture

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

Sharon

dimitriseng’s picture

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.

dimitriseng’s picture

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

flightrisk’s picture

Will someone please commit this?

Babich’s picture

Assigned: Unassigned » Babich

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

dimitriseng’s picture

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 :)

Babich’s picture

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

von_stirliz’s picture

I second that!

Subscribe - that should go into the main code.

lsolesen’s picture

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.

selinav’s picture

+1

lsolesen’s picture

please use the green button to follow instead of subscribe comments

dimitriseng’s picture

@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.

scorchio’s picture

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

lsolesen’s picture

write a recap of the issue

Moloc’s picture

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.

lsolesen’s picture

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

lsolesen’s picture

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

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

Moloc’s picture

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.

webadpro’s picture

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.

scorchio’s picture

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.

lsolesen’s picture

@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?

webadpro’s picture

Move on to Option 2.

Any thoughts about this?

kenheim’s picture

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

MrPaulDriver’s picture

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.

sahaj’s picture

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

dimitriseng’s picture

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.

jrreid’s picture

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

camdarley’s picture

Here is a version of the patch working with 7.2

saltednut’s picture

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.

SocialNicheGuru’s picture

I am getting this error:
colorbox 7.x-2.6
media_gallery 7.x.dev https://drupal.org/node/1855276 from 2/26/14
colorbox library 1.5.9

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();

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

mrfelton’s picture

Status: Needs work » Needs review
FileSize
18.82 KB

I have similar issues as @SocialNicheGuru in #77. I wasn't able to find much reference to this __drupal_alter_by_ref property and it was breaking things, so I removed it from the patch.

Additionally, in previous patches colorbox settings for maxWidth and maxHeight were not carried over from colorbox module, so I have added those.

Also, media_gallery_update_7012 has nothing todo with this patch and already exists in media_gallery, so I've removed that too.

mrfelton’s picture

For reference, I'm using this with the following and things seem to be working okish

projects[colorbox][version] = 2.8


projects[media_gallery][version] = 2.x-dev
projects[media_gallery][download][type] = git
projects[media_gallery][download][url] = http://git.drupal.org/project/media_gallery.git
projects[media_gallery][download][branch] = 7.x-2.x
projects[media_gallery][download][revision] = 65cc2120b66929809b4804704c0298e54312a34b
projects[media_gallery][patch][2325539] = https://www.drupal.org/files/issues/2325539-media_gallery-media-dependency.patch
projects[media_gallery][patch][1938138] = https://www.drupal.org/files/1938138-media_gallery-no-menu-item.patch
projects[media_gallery][patch][1053674] = https://www.drupal.org/files/issues/integrate_with_colorbox_module-1053674-79.patch


projects[colorbox_lib][type] = library
projects[colorbox_lib][download][type] = get
projects[colorbox_lib][download][url] = https://github.com/jackmoore/colorbox/archive/1.4.37.zip
projects[colorbox_lib][directory_name] = colorbox
projects[colorbox_lib][subdir] = .
SocialNicheGuru’s picture

Status: Needs review » Needs work

I think #79 also incorporates this patch: enable-gallery-collections-1037002-73.patch
http://drupal.org/node/1037002

brenes2’s picture

Hello I'am trying to insert the title content in the cboxTitle div. I assume that the following Jquery of the patched colorbox-display.js should be responsible for this:

 // Put the title in the correct div container
    jQuery(document).bind('cbox_complete', function() {
                var cboxTitle = jQuery(".lightbox-title").html();
                jQuery("#cboxTitle").html(cboxTitle);
    });

But there is no effect at all. Please have a look at the website
http://itg-bau.de/leistungen
Any Ideas how I am able to get it running?

I am using colorbox library 1.3.16, media gallery 7.x-1.0-beta7, colorbox module 7.x-2.9

ivnish’s picture

Status: Needs work » Closed (outdated)