dinbror / bpopup

bPopup is a lightweight jQuery modal popup plugin (only 1.34KB gzipped). It doesn't create or style your popup but provides you with all the logic like centering, modal overlay, events and more. It gives you a lot of opportunities to customize so it will fit your needs.
http://dinbror.dk/bPopup
427 stars 260 forks source link

Calling bPopup().close() doesn't fire onClose callback #51

Closed fkenji closed 9 years ago

fkenji commented 9 years ago

If we open a new modal with a onClose callback, say as in:

        $("#modal").bPopup({
          onClose: function() {
            console.log('Closed');
          }
        });

When we try to close it using $('#modal').bPopup().close() the onClose callback is not fired. This happens because when $('#modal').bPopup() is called, it overrides the variable that holds the options with those of the default values of $.fn.bPopup.defaults. This means that what we had set before as onClose always gets set back to false.

This PR changes it so that we also check this.data('bPopup') for previous set options for the modal. If we redefine them in another call, say:

        $("#modal").bPopup({
          onClose: function() {
            console.log('Closed again');
          }
        });

It should still work since we're still extending the var o with options.

dinbror commented 9 years ago

Hey,

It's because you're doing it wrong ;) Try this instead:

var bPopup = $(‘element_to_pop_up’).bPopup(); bPopup.close();

fkenji commented 9 years ago

Hey @dinbror , I am calling the code like that. In the example I have used $('#modal').bPopup().close() . But calling $('#modal').bPopup(), which we have to do to get the reference to the modal, also makes it lose its settings.

This happens because the settings themselves are all set in the var o in the following code: https://github.com/dinbror/bpopup/blob/master/jquery.bpopup.js#L20 . $.extend({}, $.fn.bPopup.defaults, options) is being used, which tries to merge the plugin's defaults, $.fn.bPopup.defaults , with was passed as options.

Since we're just calling $('#modal').bPopup(), options is null so the var o ends up getting all the default values instead. This means that onClose's gets its default value, false.

The PR tries to correct this my making it also merge in any previous data that was set when the popup was first called, by also passing in this.data('bPopup') in the $.extend call.

dinbror commented 9 years ago

You're still doing it wrong ;)

I made a quick example, please take a look: http://jsbin.com/meniyekaga/1/

fkenji commented 9 years ago

Ah, you're right, didn't see the variable :P . Thanks, and sorry about the confusion!