ryanve / verge

get viewport dimensions...detect elements in the viewport...trust in <!DOCTYPE html>
https://ryanve.dev/verge
MIT License
695 stars 51 forks source link

inViewport returns true if element is hidden #19

Open LucidityDesign opened 8 years ago

LucidityDesign commented 8 years ago

If a element is hidden via display: none verge.inViewport returns always true. Can you add a test if a element is visible or not?

englishextra commented 7 years ago

Got the same issue, and with js cannot get img object style.display = "" It wont show block none or whatever

if (verge.inY(e)) { also returns true

@LucidityDesign @ryanve

display:none will remove the element from the document's normal flow and set the values for position/height/width to 0 on the element and its children.

so the workaround:

if (verge.inViewport(e) && 0 !== e.offsetHeight) {

and also:

if (verge.inX(e) && 0 !== e.offsetHeight) {
if (verge.inY(e) && 0 !== e.offsetHeight) {

or you may try to add in original lib:

in .inX ... &&r.left<=viewportW()&&(0!==el.offsetHeight);
in .inY ... &&r.left<=viewportW()&&(0!==el.offsetHeight);
in .inViewport ... &&r.top<=viewportH()&&(0!==el.offsetHeight);

but I didn't tested possible consequences, only I tested in vertical scroll

ryanve commented 7 years ago

To be safe I think inViewport should stay the same. But we could add a separate isVisible method. Do you think that fits within the scope of the library or is it better handled by jQuery?

jQuery(elem).is(':visible') versus adding

function isVisible(elem) {
  return elem.offsetWidth > 0 || elem.offsetHeight > 0 
}

for use like verge.inViewport(elem) && verge.isVisible(elem)

Opinions?

englishextra commented 7 years ago

@ryanve My case was lazyloading using two libs:

the one of yours: https://github.com/ryanve/verge and https://github.com/bfred-it/image-promise

I could have used https://github.com/vvo/lazyload

but it doesn't provide fallback, if the image is not found or broken, and thus in Firefox the image box dissapears, it gets no height, and I needed that height so that data image placeholder should be visible.

So I did my own implementation, where having a visibility check was meaningful, because I had hidden images that should show on mobile only. So when testing on desktop, the display none images got loaded because verge says they are in the viewport.

So I hardcoded &&(0!==el.offsetHeight) for the time being, but still not sure, perhaps there should be other cases where &&(0!==el.offsetHeight) could damage the lib's logic.

But I'd be happy to have two checks verge.inViewport(elem) && verge.isVisible(elem) rather than hardcoded visibility check right into inViewport check.

separate property