Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Prevent event propagation on close #1

Closed
wants to merge 3 commits into from

4 participants

@knicklabs

Handy script, thanks a lot for making this. I put this in use on a site today, but had one issue. When the close method was triggered by clicking on the close selector, the click event on the close selector was propagated, resulting in the page scrolling to the top.

I modified the script to pass the click event to the close method so that the event propagation could be stopped. I did not minify the modified script as I was unsure what you were using for minification. If you accept this patch, the script will need to be minified.

@crcastle

FWIW, I can confirm both that I had the same issue and this pull request fixes it.

EDIT: While this fixes the even propagation problem it seems to create or bring to the surface another bug. Other elements with click events are no longer working. Gotta close up for the day, but I'll try to put together a fix soon.

@knicklabs

I checked this out and confirmed that the 'fix' prevents event propagation on other anchor elements. This was indeed a side effect of the code I injected. Thanks for spotting that.

knicklabs added some commits
@knicklabs knicklabs Fixed issue where preventing event propagation on close prevented oth…
…er click events on page from firing. It is now confined to the close button in the pop box.
0c434cf
@knicklabs knicklabs Update popbox.js cd34f55
@tristanoneil

Thanks guys. I think we'll pull this one in soon. /cc @seanbehan

@seanbehan
Owner

Thanks! I think I'm going to prevent event propagation for close on bind.

/// something like... 
$(settings['open'], this).parent().find(settings['close']).bind('click', function(event){
   event.preventDefault();
   methods.close();
});
@seanbehan seanbehan closed this in 4e6aa4b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on May 9, 2012
  1. Prevent event propagation on close action

    Nickolas Kenyeres authored
Commits on May 16, 2012
  1. @knicklabs

    Fixed issue where preventing event propagation on close prevented oth…

    knicklabs authored
    …er click events on page from firing. It is now confined to the close button in the pop box.
  2. @knicklabs

    Update popbox.js

    knicklabs authored
This page is out of date. Refresh to see the latest.
Showing with 6 additions and 3 deletions.
  1. +6 −3 popbox.js
View
9 popbox.js
@@ -27,20 +27,23 @@
}
},
- close: function(){
+ close: function(event){
+ if ($(event.target).closest(settings['selector']).length && $(event.target).hasClass(settings['close'].slice(1))) {
+ event.preventDefault();
+ }
$(settings['box']).fadeOut("fast");
}
};
$(document).bind('keyup', function(event){
if(event.keyCode == 27){
- methods.close();
+ methods.close(event);
}
});
$(document).bind('click', function(event){
if(!$(event.target).closest(settings['selector']).length){
- methods.close();
+ methods.close(event);
}
});
Something went wrong with that request. Please try again.