bigbite / macy.js

http://macyjs.com/
MIT License
1.3k stars 156 forks source link

[bug] recalculate( true, true ) - breaks container height randomly #29

Closed VlooMan closed 6 years ago

VlooMan commented 7 years ago

Trying to implement a filter, so we need to use "recalculate( true, true );" to re-order all visible items.

We only call the recalculate on click and we do not hide anything. On some page reloads and a consequent click the items act normally, but on others all items jump to the bottom of the page. Basically it does not replace the container height, but adds the "new height", which is the same as the previous, to the already existing one.

Any idea how to avoid this issue?

jrmd commented 7 years ago

Hi @VlooMan , Could you provide me with a demo that demonstrates the bug? so I can look in to this

VlooMan commented 7 years ago

@jrmd Absolutely.

Here is the link SEE DEMO.

Try reloading the page normally or with hard reload "CMD SHIFT R" or "CTR SHIFT R" and also opening it in new tab. Then when clicking on the filter it only calls recalculate( true, true );. Sometimes the items jump down.

Happens both on MAC and WIN in latest Chrome. Are you able to reproduce it?

Cheers

VlooMan commented 7 years ago

Here is a second link with more functionality on click SEE DEMO 2.

In this case it happens even more often.

On click - we fade out all items, show only the filtered ones but they are not visible to humans, we call Macy recalculate & then we fade the visible items in.

jrmd commented 7 years ago

Hey @VlooMan

I did manage to reproduce the bug but only once, which I thought was strange. I’m not really sure how to fix this but maybe as a suggestion you could try doing macy_instance.recalculate( true ) this would in turn force the images to recalculate but wouldn’t add the loaded attribute. Which doesn’t seem to be needed in your example.

Let me know if this resolves it.

Thanks, Jerome

wottpal commented 7 years ago

Hi, I have exactly the same problem on window-resizes :( I do the following to prevent it, which is unfortunately super performance-costly. (Tried it with throttling/debouncing -> didn't work)

   window.addEventListener('resize', event => {
      macy.reInit()
    });
jrmd commented 7 years ago

Hi @dkerzig

Is there a reason you are reIniting or recalculating on resize?

Macy will auto recalculate on resize so maybe that's the issue. As they maybe conflicting.

Thanks

jrmd commented 7 years ago

So, I think i figured out the issue with this; and I have came up with a fix for this. I am currently doing some more testing just in case. But I should hopefully have it pushed soon

VlooMan commented 7 years ago

Let us know if we can test as well ;)

jrmd commented 6 years ago

hey @VlooMan

I have just pushed version 2.2.0 to npm, this issue is now fixed.

One thing to note, I altered the image loading script so it now requires a promise polyfill for IE11. I chose this solution as it was much more reliable, especially since promises are supported quite well now.