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

inViewport returns true if element is hidden #19

Open
LucidityDesign opened this issue Jun 23, 2016 · 3 comments
Open

inViewport returns true if element is hidden #19

LucidityDesign opened this issue Jun 23, 2016 · 3 comments

Comments

@LucidityDesign
Copy link

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
Copy link

englishextra commented Mar 10, 2017

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
Copy link
Owner

ryanve commented Mar 11, 2017

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
Copy link

englishextra commented Mar 11, 2017

@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

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

No branches or pull requests

3 participants