Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I am working a project which uses a function to show a modal dialog. The dialog can be hidden by calling the hideModal() function. This function is triggered by:

  • Pressing the ESC key
  • Clicking on the modal background
  • Clicking on the close button

My current code is:

 $("#modal").click(function() {
    hideModal();
});

$("#modal-object-container > a").click(function(e) {
    hideModal();
    e.preventDefault();
});

$(document).keydown(function (e) {

    if (e.keyCode == '27') {
        hideModal();
    }

});

I have the feeling there should be a faster way to bind all these events at once.

share|improve this question
2  
Since your event sources (elements, document etc) are different and there are custom actions you have to implement each time, there is no better way of doing this, unfortunately. – Arthur P Nov 21 '11 at 21:18
I thought the same thing but was hoping for a clever trick :) – osi Nov 22 '11 at 9:08
for click events you can use event delegation , but that's about it – tereško Nov 24 '11 at 1:21

1 Answer

up vote 0 down vote accepted

Your code isn't all that bad. However because you are binding key events on the document, you need to allow room, for other key events to be bound, for other tasks, so your code doesn't interfere with other ESC functions.

The concept is that you bind on click, and unbind upon hiding your modal. This way the key events are only applicable when your code is run.

// save your doc once, it's faster
var doc = $(document);
    // bind a function, which you can unbind later, so esc can be used in other cases as well, without triggering your function
    doc.bind('keydown', _processKey);

// on click binding for as many elements as you like. 
// The preventDefault shouldn't influence other elements in this case.
$("#modal, #modal-object-container > a").click(function(ev) {
    ev.preventDefault();
    hideModal();
});

// when you hide your modal, you unbind the event, so the document is free for other events and functions
function hideModal() {
    doc.unbind('keydown', _processKey);
}

// have a 'private' function to do what you want. This way you can unbind it anytime you like
function _processKey(ev) {
    if (ev.keyCode == '27') {
        hideModal();
    }
}
share|improve this answer
Really good arguments, I will change this in my code. Instead of bind() I will use on() and off() as described in andismith.com/blog/2011/11/on-and-off Thanks! – osi Nov 22 '11 at 12:17

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.