HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

Using jQuery 1.7 .on() and event.stopPropagation to close boxes when clicking in document body

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
closeboxesclickingjqueryusingdocumentwhenandbodystoppropagation

Problem

I have the following code which works but seems like it could be improved. I'm not sure if I'm using event.stopPropagation() correctly or if it is the best solution.

The elements are loaded via AJAX and so need to have .on() (previously .live(), which is now deprecated in jQuery 1.7).

I've used the example given here as a basis.

$('html').on('click', '.poster:not(.active) .image-effect', function (event) {

        var obj = $(this).parent();

        // If there are no .close spans
        if (obj.find('.close').length === 0) {

            // Add the close element by javascript to remain semantic
            obj.find('.quick-view').append('×');
        }

        // Close any open Quick Views
        closeQuickView();

        // Add the active class (controls opacity)
        obj.addClass('active');

        // Fade in the Quick View
        obj.find('.quick-view').fadeIn(200, 'easeInOutQuint');

        event.preventDefault();
        event.stopPropagation();

    });

    $('html').on('click', '.active', function () {
        return false;
    });

    $('html').on('click', '.close', function () {
        closeQuickView();
    });

    $('html').on('click', '.quick-view', function (event) {
        event.stopPropagation();
    });

    // Close the QuickView with a button
    $('html').on('click', function () {
        closeQuickView();
    });

    function closeQuickView() {
        $('.poster').removeClass('active');
        $('.quick-view').fadeOut(200, 'easeInOutQuint');
    }


For some reason clicking the document isn't working in Chrome (the closeQuickView is not being called) which may be an indicator that the code is not functioning 100%.

My markup is as follows:


    
        
        
    
    

        Content

    


Is this the right way to approach the scenario? Are there any conflicting functions?

Solution

It did work in my Chrome. However, I felt your code was confusing with all those handlers. Let me suggest the following:

$('html').on('click', '.poster', function(e) {

  var target = $(e.target);
  var poster = $(this);      

  if (!poster.hasClass('active')) {
    closeQuickView();
    poster.addClass('active').find('.quick-view').fadeIn(200);
  }
  if (target.hasClass('quick-view') || target.hasClass('image-effect')) e.stopPropagation(); // prevent bubbling and thus closeClickView call below
});

$('html').on('click', closeQuickView);


This way, it's more concise and hopefully clearer what's handling what.

PS: I threw out the conditional '.close' appendage here, nothing wrong with having it in the vanilla HTML I'd say.

Code Snippets

$('html').on('click', '.poster', function(e) {

  var target = $(e.target);
  var poster = $(this);      

  if (!poster.hasClass('active')) {
    closeQuickView();
    poster.addClass('active').find('.quick-view').fadeIn(200);
  }
  if (target.hasClass('quick-view') || target.hasClass('image-effect')) e.stopPropagation(); // prevent bubbling and thus closeClickView call below
});

$('html').on('click', closeQuickView);

Context

StackExchange Code Review Q#5821, answer score: 2

Revisions (0)

No revisions yet.