Dynamically added images #93
Conversation
| @@ -171,6 +171,10 @@ | |||
| return regex.test(element.href); | |||
| }); | |||
| var gallery = []; | |||
|
|
|||
| if(tagsNodeList.length === 0){ | |||
There was a problem hiding this comment.
Spaces after if and before the bracket.
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
5 times, most recently
from
388b6b4
to
a92ac4e
Compare
| unbind(imageElement, 'click', imagedEventHandlers[galleryID + '_' + imageElement]); | ||
| }); | ||
| imagesMap.pop(); | ||
| for(var selector in data) { |
There was a problem hiding this comment.
Please use a space after keywords like if, for, etc.
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
from
a92ac4e
to
1067976
Compare
|
@alexdrimbe: make sure there are not new JSHint issues in your branch. |
|
There are no JSHint issues introduced by the new code. |
|
@alexdrimbe: please fetch and rebase, |
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
from
1067976
to
ad3cd01
Compare
| @@ -148,50 +146,75 @@ | |||
| supports.svg = testSVGSupport(); | |||
|
|
|||
| buildOverlay(); | |||
| destroyGalleriesForSelector(selector); | |||
There was a problem hiding this comment.
Why do we destroy on run() ? I'm not sure I understand the function's responsibility.
There was a problem hiding this comment.
When we call run() function multiple times for the same selector we want to destroy any cached data for selector and unbind its listeners.
|
@alexdrimbe could you summarize this PR? I've already forgotten the essence of the change, besides significant code improvements (btw. great job!). |
|
@feimosi this PR could be summarize as follows: If you dynamically add or remove DOM elements from the gallery and then you call baguetteBox.run( someSelector ) again for the same selector the gallery works fine. Before, if you called 'run' function multiple times for the same selector, the click event was attached multiple times for the same elements and this is a source for memory leak; Now if you call run function multiple times it works properly. Before: With the change: |
… will replace it in foreach call
…leries, imagesMap imagedEventHandlers currentGallery becomes an array For the moment we delete unbindImageClickListeners
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
from
ad3cd01
to
9e5c800
Compare
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
from
9e5c800
to
b2606ef
Compare
alexdrimbe
force-pushed
the
dynamically-added-images-rebased
branch
from
b2606ef
to
4356ffd
Compare
|
Ok, we're good to go with this PR. Really good job @alexdrimbe, thank you! |
No description provided.