Skip to content
This repository has been archived by the owner on Aug 9, 2022. It is now read-only.

Enter event firing for all elements matching selector even if outside of container #76

Open
gavin310 opened this issue Nov 11, 2020 · 6 comments
Assignees
Labels

Comments

@gavin310
Copy link

gavin310 commented Nov 11, 2020

I'm not sure if this is expected functionality or a bug. I initialize my OnScreen object using a container. In this case my container is a jQuery object named wrap. As soon as the enter event is set, all elements in my HTML with the class .reveal get immediately triggered, even if they are not inside of my container element. Is this correct? I assumed that only elements inside the container would be handled.

var os = new OnScreen( {
	tolerance : 100,
	debounce : false,
	container : wrap.get( 0 ),
} );
os.on( 'enter', '.reveal', function( elem, evt ) {
	$( elem ).addClass( 'active' );
} );
@gavin310
Copy link
Author

Btw for my own purposes I need only elements in the container to be selected, so (I think) I fixed it by changing the on function.

I changed the line:

for (var i = 0, elems = document.querySelectorAll(selector); i < elems.length; i++) {

to:

for (var i = 0, elems = this.options.container.querySelectorAll(selector); i < elems.length; i++) {

@silvestreh silvestreh self-assigned this Nov 11, 2020
@silvestreh silvestreh added the bug label Nov 11, 2020
@silvestreh
Copy link
Owner

You're right… if a container element has been passed then this should only affect children of that container. Good catch! I'll push a fix soon 😄

@gavin310
Copy link
Author

Nice, glad I could help this awesome project!

@gavin310
Copy link
Author

@silvestreh Hey I got one quick question if you don't mind. OnScreen doesn't seem to support scrolling inside my container, since my container element has overflow:scroll. When you scroll to the top of my container it triggers all the elements regardless of their vertical position in my container. Should it support this? I haven't dug into it yet to see if it's something I'm doing wrong but I thought I'd ask you first since you're around. Thanks!

@gavin310
Copy link
Author

Ok well it seems like it was the case that there was some issues when container had scrolling. I think I fixed it though. It wasn't easy :) Here's what I came up with. This replaces the last part of the inContainer function:

        var containerRect = options.container.getBoundingClientRect(),
            elRect = el.getBoundingClientRect();
        
        return (
            // // Check bottom boundary
            options.container.scrollTop + elRect.top + elRect.height - options.tolerance > containerRect.top + options.container.scrollTop &&

            // Check right boundary
            options.container.scrollLeft + elRect.left + elRect.width - options.tolerance > containerRect.left + options.container.scrollLeft &&

            // Check left boundary
            options.container.scrollLeft + elRect.left + options.tolerance < containerRect.left + containerRect.width + options.container.scrollLeft &&

            // // Check top boundary
            options.container.scrollTop + elRect.top + options.tolerance < containerRect.top + containerRect.height + options.container.scrollTop
        );

Also with the changes to the querySelectorAll in my previous comments I had to add something like this in the on event since window does not have a querySelectorAll method.

if (this.options.container === window) {
	selected = document.querySelectorAll(selector);
}
else {
	selected = this.options.container.querySelectorAll(selector);
}

I'm sure I haven't done things the best way, but hopefully it helps.

@gavin310
Copy link
Author

Sorry for a bunch of messages. Just want to add that there's some issues with my updates when the container is not a scrolling element. I'll post the fix when I get to it.

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

No branches or pull requests

2 participants