Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pull Request for Issue #191 #208

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

bonzani
Copy link

@bonzani bonzani commented Jul 29, 2017

Pull Request for Issue #191.

Problem

The backUpFromSmall function gets unnecessarily often called because the if condition is always true.
The code with the count variable and if condition was introduced in 1a55eb4 . The count variable was then shadowed in b179044 with a parameter. The original count variable was then removed in b615b21, since it was not used anymore.
According to the jquery docs the second argument of each is the value and not the count.

Summary of Changes

Rename the second argument of each from count to value and move the backUpFromSmall call to the outer loop.

@dbox
Copy link
Contributor

dbox commented Nov 13, 2017

Looks good to me. Can anyone else confirm?

@bonzani
Copy link
Author

bonzani commented Feb 6, 2018

It would be very good if someone else could confirm, since it is the only thing preventing multiple times per site usage. I am using it with the above changes on my site and did not encounter any issues.

@bonzani
Copy link
Author

bonzani commented Aug 26, 2021

ping @dbox

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants