Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

It doesn't show up as visible if height is 0 #16

Open
cusspvz opened this issue Mar 18, 2016 · 13 comments
Open

It doesn't show up as visible if height is 0 #16

cusspvz opened this issue Mar 18, 2016 · 13 comments

Comments

@cusspvz
Copy link

cusspvz commented Mar 18, 2016

It should show an element as visible independently of its height, but instead, it doesn't.

PoC

captura de ecra 2016-03-18 as 14 57 36

captura de ecra 2016-03-18 as 14 57 20

@vvo
Copy link
Owner

vvo commented Mar 21, 2016

Hi, thanks for the bug report!

Can you create a new failing test and submit a PR with a fix if needed? Everything needed is available in the repo to do so.

Let me know.

@cedricdelpoux
Copy link

Thank you for reporting this bug. I was looking for what was happening :D
My solution was to set min-height: 1px;. But I'm not very happy with this solution

@tzi
Copy link
Collaborator

tzi commented Apr 17, 2017

Yes if an element has no height, it is not visible.
This behavior is intentional and ISO with the $(':hidden') selector of jQuery

Here is the line that is responsible of this feature : https://github.com/vvo/in-viewport/blob/master/in-viewport.js#L143

is it a problem?
In which use case?

Cheers,
Thomas.

@cedricdelpoux
Copy link

cedricdelpoux commented Apr 17, 2017 via email

@tzi
Copy link
Collaborator

tzi commented Apr 18, 2017

Thanks for the details!

This lib is made to detect "when an element becomes visible in a viewport". Perhaps we should simplify and detect only when an element is in a viewport, because this lib is uses for lazy loading most of the time.

It would be simpler et prevent some issues with developers. Do you agree @vvo?

⚠️ This would mean to create a major version.

@vvo
Copy link
Owner

vvo commented Apr 20, 2017

I am not sure to get the distinction and your proposal. For sure I don't mind doing breaking changes but can you detail a bit more what do you mean?

@niksy
Copy link
Contributor

niksy commented May 14, 2017

As @tzi mentioned, it could prevent common development issues since elements which are tested for the viewport presence are usually ones which should be lazy loaded and their placeholders usually don’t have any content, therefore, height with 0px.

Maybe this could also be an option? Or just cater to that common use case by default without any option?

@cusspvz
Copy link
Author

cusspvz commented May 14, 2017

@vvo what if the detection of 0px heighted elements was implementad as a method option?

@TehShrike
Copy link

This prompted me to copy the visibility function, remove some of the functions I didn't need, and comment out these lines:

in-viewport/in-viewport.js

Lines 153 to 155 in 04b22a6

if (!elt.offsetWidth || !elt.offsetHeight) {
return false;
}

Works dandily.

@cusspvz
Copy link
Author

cusspvz commented Jun 19, 2017

I agree with @TehShrike solution, it would return a0 instead of a false, and wouldn't break most of use cases (except typed-check based).

@vvo
Copy link
Owner

vvo commented Jun 20, 2017

I am down for changing the behavior of the library, send a PR and we will discuss it!

@TehShrike
Copy link

I think the confusion came from the fact that the library is named "in-viewport", but it internally uses a function named "isVisible". Those two things aren't always the same.

@rulrok
Copy link

rulrok commented Jun 16, 2018

I would appreciate if this behaviour could be passed in an options parameter.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants