dimsemenov / Magnific-Popup

Light and responsive lightbox script with focus on performance.
http://dimsemenov.com/plugins/magnific-popup/
MIT License
11.39k stars 3.48k forks source link

Can mfp* events be fired with trigger instead of triggerHandler? #518

Open pjackson28 opened 10 years ago

pjackson28 commented 10 years ago

If trigger was used instead then it would be a lot easier to create a global handler for the events where I could just listen at the document node rather than having to attach handlers to each triggering element. Also makes it much easier to handle dynamically added triggers as then I don't have to attach new handlers as well (since sniffing at the document node will catch all dynamically added triggering elements as well).

Currently I have to use:

  1. No element: $( document ).on( "mfpClose", function( event ) {
  2. Element: $( elmSelector ).on( "mfpClose", function( event ) {

or a messy ($( document ).add( elmSelector ).on( "mfpClose", function( event ) {

In both cases I would have to add new handlers for each dynamically added triggering element.

If trigger was used instead of triggerHandler, then I could do the following instead:

$( document ).on( "mfpClose", function( event ) {

In this case I can have this once and catch all "mfpClose" events whether or not there is an element and whether or not trigger elements are added dynamically.

Would it be possible to make this change? Would make the events much more useful and a much more viable alternative to the callbacks.

Ambient-Impact commented 10 years ago

I would also love to see this make it in, as I sometimes have to build a site where MFP is used in multiple places, and it would be awesome to just bind once for all of them.

dimsemenov commented 10 years ago

You may add your own global events:

$.magnificPopup.instance.close = function() {

    $(document).trigger('mfp-global-close');

    $.magnificPopup.proto.close.call();
};

Or you may just use callbacks instead of events. Create callbacks object once, and pass it to each magnific popup initialization.

pjackson28 commented 10 years ago

We could add global events but that adds a lot of complexity for our downstream users. There are the events in the Magnific Popup documentation then there would also be custom global events in our documentation (not to mention the code bloat and extra execution from having to fire a second time for each event to make it global). Everything would be much simpler if the events were allowed to bubble up to the document node (which doesn't happen with triggerHandler).

As for callbacks, we are using them to add enhancements (since events currently aren't a viable option) but that creates a challenge for downstream users. If they try to specify their own callbacks then they could wipe out ours unless they know how to extend a callback properly (which many users don't, even when you explain it to them). This could easily be addressed with events that bubble up where everyone can add their own handlers (which is easy to document as well).

Would it be possible to switch to trigger for events or at least to add a global option for enabling that behaviour?

Dylan-Chapman commented 9 years ago

Was this ever considered any further? The solution @pjackson28 proposed makes a lot more sense than creating our own global events.

laminekalinko2 commented 6 years ago

thanjk

You may add your own global events:

$.magnificPopup.instance.close = function() {

    $(document).trigger('mfp-global-close');

    $.magnificPopup.proto.close.call();
};

Or you may just use callbacks instead of events. Create callbacks object once, and pass it to each magnific popup initialization.

this really help me. was trying too catch the close event. but if I open the Popup inside js callback it won't fire the close event