When there are a large number of results for a facet (in my case, 250+), there is a several-second delay (10 or more!) when clicking either "Show more" or "Show fewer". This patch tries to address that performance issue in a few ways:

1. Switch from using li:gt() to li.slice(), which is more performant, according to the jQuery docs:

Because :gt() is a jQuery extension and not part of the CSS specification, queries using :gt() cannot take advantage of the performance boost provided by the native DOM querySelectorAll() method. For better performance in modern browsers, use $("your-pure-css-selector").slice(index) instead. (http://api.jquery.com/gt-selector/)

2. Switch from using li:hidden to li.filter(':hidden'), which is more performant, according to the jQuery docs:

Because :hidden is a jQuery extension and not part of the CSS specification, queries using :hidden cannot take advantage of the performance boost provided by the native DOM querySelectorAll() method. To achieve the best performance when using :hidden to select elements, first select the elements using a pure CSS selector, then use .filter(":hidden"). (http://api.jquery.com/hidden-selector/)

3. Run slideUp() / slideDown() on a div that wraps all the list items to be shown / hidden, instead of running slideUp() / slideDown() on every list item. Running that animation on ~50 list items is OK, but when run on 250+ items, causes a massive slowdown.

4. Cache some references to frequently called elements.

Thanks!

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Status: Needs review » Needs work

The last submitted patch, facetapi-show-more-show-fewer-performance.diff, failed testing.

venutip’s picture

Version: 7.x-1.3 » 7.x-1.x-dev
Status: Needs work » Needs review
FileSize
2.28 KB

Sigh. OK, re-rolled against dev.

Status: Needs review » Needs work

The last submitted patch, 2: facetapi-show-more-show-fewer-performance.patch, failed testing.

venutip’s picture

Status: Needs work » Needs review
FileSize
2.01 KB

Path issue.

idebr’s picture

Animating individual facet items has a nice effect, but should probably not be the default because the performance implications described in #1. The patch in #4 optimizes the existing code with faster selectors and improves the animation performance significantly.

I made a few changes to the patch in #4 to extend the improvements to the 'Show more' / 'Show fewer' performance:

  1. Instead of creating a container on the fly when the 'Show more'-link is clicked, create a container on page load. This keeps the markup consistent during interaction with the user and makes the markup easier to theme for developers
  2. Remove the showing/hiding of superfluous facets items and showing/hiding the container instead
  3. The container is now a <ul> element instead of <div> to keep the markup W3C valid
cpliakas’s picture

Status: Needs review » Needs work

Thanks for the contribution! I am all for improving the performance of the javaScript. Marking as needs work since the patch has to be re-rolled as a result of the following changes:

#2193621: Show more/fewer links not configurable
#2131773: "Show More" link JS is too tightly coupled to markup

Thanks,
Chris

jefuri’s picture

Status: Needs work » Needs review
FileSize
9.95 KB

Rerolled the patch, and fixed the changes between them. Also update to correct javascript coding standards.

jefuri’s picture

Small bug in the code, the list is not default hidden. Causing the show more/show fewer to be reversed.