While using the flag module to flag search results I noticed that with every flag ajax call an extra 'Show more' link was added to all the facet blocks.

Adding a once() in Drupal.facetapi.applyLimit() (line 40; facetapi.js) fixed this.

$ git diff -w sites/all/modules/contrib/facetapi/facetapi.js
diff --git a/sites/all/modules/contrib/facetapi/facetapi.js b/sites/all/modules/contrib/facetapi/facetapi.js
index d05cf23..7a12c57 100644
--- a/sites/all/modules/contrib/facetapi/facetapi.js
+++ b/sites/all/modules/contrib/facetapi/facetapi.js
@@ -37,7 +37,7 @@ Drupal.facetapi.applyLimit = function(settings) {
     // Adds "Show more" / "Show fewer" links as appropriate.
     $('ul#' + settings.id).filter(function() {
       return $(this).find('li').length > settings.limit;
-    }).each(function() {
+    }).once().each(function() {
       $('<a href="#" class="facetapi-limit-link"></a>').text(Drupal.t('Show more')).click(function() {
         if ($(this).prev().find('li:hidden').length > 0) {
           $(this).prev().find('li:gt(' + limit + ')').slideDown();
CommentFileSizeAuthor
#5 facetapi-1242566-5.patch665 bytescpliakas
show_more.png10.51 KBmrharolda

Comments

cpliakas’s picture

Status: Active » Needs review

Confirmed. Marking as "needs review".

cpliakas’s picture

Status: Needs review » Needs work

Great job finding this bug, and the attached fix works as described. The one piece that is missing is that if the list is expanded and a bookmark is selected, the list collapses itself again.

Instead of using once(), how about adding an additional condition next to the if (settings.limit > 0 testing if the "facetapi-processed" class is set? If it is not set, set it via something like $('ul#' + settings.id).addClass('facetapi-processed');. If it is set, skip processing that entire function which will ultimately take no action when a flag is selected.

Great job discovering this,
Chris

cpliakas’s picture

Issue tags: +low hanging fruit

Putting this on the low hanging fruit list. Really looking to have MrHaroldA or someone else drive this home.

mrharolda’s picture

@cpliakas: I might have some time this week to look into it. Can't promise it though... ;)

cpliakas’s picture

Status: Needs work » Fixed
StatusFileSize
new665 bytes

The attached patch is what I was thinking. Committed at b486e61.

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