-
Notifications
You must be signed in to change notification settings - Fork 23
If element height is 0, return it instead of false
#30
base: master
Are you sure you want to change the base?
Conversation
If I try to summarise it (sorry I am not actively using my own lib). The current behavior (before this PR) is to consider element not in the viewport when their height and width is 0. With your change, they will now be considered in the viewport as soon as they are in the viewport, no matter their height. If I get this right, I am down for the change and we can move forward, let me know |
For me, you summarized well the problem. 👍 to merge, and to release a major version |
Tests are failing now, I cannot really understand why but there's something to dig here (run them locally with npm run dev) |
It's because Travis doesn't expose the Saucelabs credentials to branches created by people who don't own the repo. Either you'll have to run them yourself directly, or you can push this branch up to another branch in this repository and open a new pull request, at which point Travis should pass the credentials along happily. |
I did run them myself and they are failing, just added one fix (cc @tzi we broke this contains method) |
@vvo You totally right. I didn't make the link... Sorry 😞 In theory, this library should not test visibility, but only test if an element is in the viewport. That's a problem. We should actually replace the old Cheers, |
Change proposed by @TehShrike on #16.
This changes the behavior on cases where the height of the element is
0
.Instead of returning
false
, it will return0
instead.This happens because the element is on screen but, as it has a height of 0, it is not displayed.
The result is a
falsy
one as before, but now people can check if it is indeed on screen by checking by type (boolean
ornumber
)