-
Notifications
You must be signed in to change notification settings - Fork 112
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
Add requireOffsetParent
option
#119
base: master
Are you sure you want to change the base?
Add requireOffsetParent
option
#119
Conversation
7fb5713
to
5994d08
Compare
amazing work with those tests! It would be great if you could add documentation about that new option to the readme. |
Hey, sure, happy to add to README. BTW #116 was a more trivial way of determining whether an element is within the viewport, but I opted against it because I didn't want to assume that people were never using inview with 0-width, 0-height elements. If you are happy with that restriction. then any element within a modal/tab or similar should also be picked up by that commit (though I haven't checked to be sure). That would make the tests I've written here more broadly application and you wouldn't need the flag at all. In summary:
|
Hmm I merged #116 too quickly then. I can in fact see use cases where 0-height elements might be useful to check for inview (ie: sentinels). I now understand the need of basically "waiting for initial render" before checking inview. However I think that a better, more retro-compatible approach might be to add those special cases as additional options (see ie In #116 the problem to be addressed isn't the 0-width/height element being checked for inview early but rather that that element will then change size and yet the inview event is not fired anymore (correct?). If that's the case I'd see (let's say) an For this PR instead, I still can't understand when an element which should have an offset parent could be checked for inview but not have an offset parent just yet. It seams like a but where we are not waiting for a dom ready event first (which should be addressed by angular). am I making sense? |
I agree the 0-height could be useful for some applications, that's why I didn't create the PR initially and only made an issue. However, in the fiddle I provided there is no change in height for the element that has the Incidentally @thenikso , you mentioned in #113 that you could fix the fiddle by changing the This PR looks like it would do the trick of detecting "display: none" and so returning "not-in-view" when that is the case. However Obviously I can't vouch for everyone using this plugin, but I tend to think that "in-view" should default to this behaviour (ie, returning false when it is not in the viewport or not visible). So instead adding an option like |
Nice work with the tests in deed. But does it really solve #111? If I run the following script in the browser console on Github:
...then it reports an offsetParent element even when using a different tab in Chromium. |
Fixes #111
requireOffsetParent
option, which when applied only reports an element as in view when it also has an offsetParent