Invisible elements should never considered as "in the viewport"

[tzi]

Hi @vvo!

We currently have a problem with layload/in-viewport: it fires image lazyloading on hidden images. So we propose a fix to not fire isInViewport on hidden elements.

This come from the getBoundingClientRect() method. It always returns 0, 0, 0, 0 on hidden elements (display: none or a parent with display: none). So all hidden elements are considered as "is in the viewport" when the page is loaded.

This could be a breaking change, because there is no simple method to detect an hidden element. We propose to handle an element with no height and no width as an hidden element.

What do you think about it?

Cheers, Thomas.

[tzi]

PR updated with a fix.

Cheers, Thomas.

[vvo]

Hello @tzi thanks for the awesome PRS. Only things I am against are:

• stylistic changes not related to the bugfix, even if legit, makes it hard to see what really changed
• no need to add the builded library in your commit, you can remove it
• no need to put a gitignore. I have a global gitignore so my modules does not requires them

Thanks a lot because you added an example and a test! This library is a bit old so tell me what you thought of the develop process?

[tzi]

Hi @vvo!

1/ I used to left a clean things when I'm coding. Don't know if it's a good or bad habit, but I understand it's confusing.

2/ The builded library is already commited in the master branch. Why should I not updated it?

3/ The .gitignore file could be helpful for the (new) contributors, event if you have a global one.

What do you want me to do about it? Do I revert the stylistic changes, the build and the .gitignore?

This library is really useful! Big thanks for sharing it. The code is pretty easy to read and do the job :) But for example, it will need a bit more of re-organization to fix the issue like #7.

Cheers, Thomas.

[vvo]

The builded library is already commited in the master branch. Why should I not updated it?

I only generate it when releasing

What do you want me to do about it? Do I revert the stylistic changes, the build and the .gitignore?

Only the build and gitignore need to be removed, otherwise looks very good! Will then test it myself and see if everything goes well with the lazyloader also.

Then Ill merge and publish etc.

[tzi]

Great plan!

My part is done ;)

Cheers! Thomas.

[vvo]

Sorry it took me so long @tzi now merged with some test modifications, available as 3.1.0 and lazyload upgraded to to 3.1.0

[tzi]

Great! Thank you a lot ;)

Thomas.