bootstrap.native: Wrong behaviour of modal.dismiss in bootstrap.native.js

Let’s go!

First changes includes adding a special toggled subscription routines for DOM events. The main purpose of it to generate a toggled subscribe/unsubscribe methods. First call of getSubscriptionToggler(…) returns a toggler, which when called subscribes/unsubscribes from listeners in a chainable manner;

function subscribe(element, event, handler) {
    element.addEventListener(event, handler, false);
    return function() {
        return unsubscribe(element, event, handler);
    };
};
function unsubscribe(element, event, handler) {
    element.removeEventListener(element, event, handler);
    return function() {
        return subscribe(element, event, handler);
    };
};
function getSubscriptionToggler(element, event, handler) {
    return function() {
        return subscribe(element, event, handler);
    };
};

Next, I’ve removed handlers generation from keydown and dismiss methods, and place them in common space of instance. Right after:

var self = this, timer, ..., checkScrollbar = function () {...}

goes our new handlers:

keyHandler = function(e) {
    if (self.options.keyboard && e.which == 27) {
        self.close();
    }
},
dismissHandler = function (e) {
    if ( e.target.parentNode.getAttribute('data-dismiss') === 'modal' 
        || e.target.getAttribute('data-dismiss') === 'modal' 
        || e.target === self.modal ) {
            e.preventDefault(); self.close()
    }
},

And after that we declare a scoped variables keyListenerToggler and clickListenerToggler:

keyListenerToggler = getSubscriptionToggler(document, 'keydown', keyHandler),
clickListenerToggler = getSubscriptionToggler(this.modal, 'click', dismissHandler);

This will hold our togglers inside modal instance object for future use.

And we are almost finished. The last step is to refactor this.keydown and this.dismiss. It’s very simple, just use out listener togglers appropriate:

this.keydown = function() {
    keyListenerToggler = keyListenerToggler();
};
this.dismiss = function() {
    clickListenerToggler = clickListenerToggler();
};

A couple other code problems in Modal:

• if (!/\bin/.test(this.modal.className)) {

I think you meant to put the “word boundary” on both sides of in:

if (!/\bin\b/.test(this.modal.className)) {

• clearTimeout(currentOpen.getAttribute('data-timer')); in this.open looks for an attribute data-timer that is never set in the code. So this line of code does nothing useful.

And two problems with removeClass() itself in Utils:

  removeClass = function(el,c) {
    if (el.classList) { el.classList.remove(c); } else { el.className = el.className.replace(c,'').replace(/^\s+|\s+$/g,''); }
  },

• Specifically, el.className.replace(c,'') will remove the first occurrence, e.g., of ‘in’, which, if any other class included the substring ‘in’ would lead to unintended consequences. For example, a class named ‘raining’ would become ‘raing’. So you probably need a regexp there, too. Such as:

el.className.replace(new RegExp('\\b'+c+'\\b'),'')

• Also, replace(/^\s+|\s+$/g,'') only trims whitespace from the beginning or end of className, not the middle, where c could have been.