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!
Comments
Comment #2
venutip CreditAttribution: venutip commentedSigh. OK, re-rolled against dev.
Comment #4
venutip CreditAttribution: venutip commentedPath issue.
Comment #5
idebr CreditAttribution: idebr commentedAnimating 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:
<ul>
element instead of<div>
to keep the markup W3C validComment #6
cpliakas CreditAttribution: cpliakas commentedThanks 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
Comment #7
jefuri CreditAttribution: jefuri as a volunteer and commentedRerolled the patch, and fixed the changes between them. Also update to correct javascript coding standards.
Comment #8
jefuri CreditAttribution: jefuri as a volunteer and commentedSmall bug in the code, the list is not default hidden. Causing the show more/show fewer to be reversed.