I noticed an issue with galleryformatter in that it binds itself to the window.load event (which is obviously done to make sure images are loaded before building the gallery). This is fine for normal page loads, but it has the sideeffect that it won't build a gallery for content that is loaded through ajax (in my case I have the issue that on a product display in drupal commerce the image gallery wouldn't load when using the dropdown to select a different product variation).

I have no real fix for this, but if you take out the (jQuery)(window).bind('load')function(){ and matching }); in galleryformatter.js, it will work for content that is loaded through ajax. (i have not seen this myself, but i imagine this change will give issues when your images are slow to load in some browsers, so someone who knows more about how Drupal handles ajax could find a better fix).

Drupal.behaviors.galleryformatter = {
  attach: function (context) {
      (jQuery)('.galleryformatter:not(.gallery-processed)', context).each(function(){
        Drupal.galleryformatter.prepare(this);
      }).addClass('gallery-processed');
  }
};
CommentFileSizeAuthor
#9 interdiff.txt890 bytesManuel Garcia
#9 galleryformatter_binds-2006016-9.patch1.82 KBManuel Garcia
#3 galleryformatter-ajax_enable-2006016-3.patch1.82 KBweseze
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
CreditAttribution: kscheirer commented

Hmm, so it sounds like we should either document that galleryformatter won't work with ajax image loads, or come up with something that handles both cases. Could we also bind to an additional event that's triggered by ajax content?

rich.3po’s picture

rich.3po CreditAttribution: rich.3po commented

Hi,

I have the same problem, which i also managed to fix by removing the (jQuery)(window).bind('load')function(){ stuff.

JS is not my strong area, but is there any reason why this cannot just be removed anyway? Its my understanding that code within the attach: property only runs when the DOM is ready, and my current build is not suffering any issues with regards to image sizes being miscalculated...?

cheers

weseze’s picture

weseze CreditAttribution: weseze commented
Assigned: Unassigned » weseze
Issue summary: View changes
Status: Active » Needs review
FileSize
galleryformatter-ajax_enable-2006016-3.patch1.82 KB

Attached patch will work on ajax calls also., without losing the condition on waiting for images to load.

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia commented
Version: 7.x-1.3 » 7.x-1.x-dev

Thanks for the patch @weseze!

Is the problem only with lazy loading image modules or?
Can anyone let me know specific steps to reproduce the problem so that I can properly test the patch on #3?

weseze’s picture

weseze CreditAttribution: weseze commented

I experienced the issue when using commerce module. Configuring a product would load an imagefield for that specific configuration via an ajax request. The applied galleryformatter would not work because the gallery JS is not applied against new DOM elements.

But basically every case were a field using the galleryformatter is rendered through ajax calls would give you this issue.

The commerce setup I have is a bit complex to easily reproduce, but maybe someone else can give a better (easier) reproducible setup?

gregoryshearer’s picture

gregoryshearer CreditAttribution: gregoryshearer commented

Thanks too for the patch @weseze!

Fixed the same issue with the ajax loading of product variants in Drupal Commerce. This patch should be highly considered for inclusion in this project but I concur that I cannot easily reproduce outside a fairly complex commerce install.

kscheirer’s picture

kscheirer
Primary language English
Location Vallejo
CreditAttribution: kscheirer commented
Status: Needs review » Reviewed & tested by the community

Based on #6

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commented
Status: Reviewed & tested by the community » Needs work 
  1. +++ b/theme/galleryformatter.js
@@ -1,20 +1,27 @@
+      $('.galleryformatter img', context).each(function () {

    We are doing this selection twice, can we cache it? 

  2. +++ b/theme/galleryformatter.js
@@ -1,20 +1,27 @@
+          loaded_images++

    Missing semicolon

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commented
Status: Needs work » Needs review
FileSize
galleryformatter_binds-2006016-9.patch1.82 KB
interdiff.txt890 bytes

Went ahead and made the changes myself, pls test =)

  • Manuel Garcia committed c759783 on 7.x-1.x authored by weseze 
    Issue #2006016 by weseze: galleryformatter binds to the wrong event
Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commented

Fixing #2087213: use as formatter in Views - nothing shows allowed me to actually see the bug, so this is now committed, thanks a lot!

Manuel Garcia’s picture

Manuel Garcia CreditAttribution: Manuel Garcia as a volunteer commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.