Compacting jQuery Code for Framebox Plugin

Question

I have written my first jQuery plugin for a website. It is, essentially, a Fancybox style popup that uses AJAX to load content from other pages. It's kind of a frankenstein build from multiple tutorials and best guess uses of the jQuery API. It works, but I am looking to refine it further.

As this is my first major step into writing jQuery or js, I am not sure how I can compact the code. Things seem a bit bloated, and I am sure bits could be trimmed down. The rules of reusing variables or functions are where I get tied up.

If anyone has any advice, input, criticism, etc., it would be great to hear and learn from.

Code:

$(document).ready(function(){

  var $ = jQuery,
  $bodyCall = $('body'),
  homePage = window.location.pathname;

  // Initialize framebox
  //-------------------------------------------------------------------/                                                                 

  $('.framebox-trigger').click(function(e){

    // Prevent default linking
    e.preventDefault();

    var $toLoad = $(this).attr('href')+' #single-project-wrapper',
    thisURL = $(this).attr('href'),
    singleContent = '<div id="framebox-overlay">' +
                    '<div id="framebox-close-panel-left"></div>' +
                    '<div id="framebox-close-panel-left"></div>' +
                    '<div id="framebox-wrapper">' +
                    '<div id="framebox-nav-wrapper">' +
                    '<p><a href="#prev" id="framebox-prev" class="framebox-nav"><i class="fa fa-long-arrow-left"></i>Previous</a>' +
                    '||' +
                    '<a href="#" id="framebox-next" class="framebox-nav">Next<i class="fa fa-long-arrow-right"></i></a></p>' +
                    '</div>' +
                    '<section id="framebox-content">' +
                    //AJAX to insert #single-project-wrapper content here
                    '</section>' +
                    '<div id="framebox-close">' +
                    '<p>Click to close</p>' +
                    '</div>' +
                    '<ul class="ajax-list"></ul>' +
                    '</div>' +
                    '</div>';

    // Fix body & Append Content
    $bodyCall.removeClass('framebox-freeze');
    $bodyCall.addClass('framebox-freeze').append(singleContent).fadeIn('fast', loadContent);

    // Add loading beacon
    $('.framebox-freeze').append('<span id="framebox-loading">+ Loading +</span>');
    $('#framebox-loading, #framebox-overlay').fadeIn('fast');

    // Load AJAX content
    function loadContent(){
      $('#framebox-content').load($toLoad , showNewContent);
    }

    // Show loaded AJAX content and hide loading bar
    function showNewContent(){
      $('#single-project-wrapper').show('normal', hideLoader);
    }

    // Hide loading bar
    function hideLoader(){
      $('#framebox-loading').fadeOut('fast');
    }

    // Match height to viewport height
    $('#framebox-content').css('min-height', $(window).height());

    // Add class of Current
    $(this).parent('li').addClass('current-link');

    // Push URL
    history.pushState(null, null, thisURL);
  });

  // Next
  //-------------------------------------------------------------------/

  $bodyCall.on('click', 'a#framebox-next', function(e) {
    e.preventDefault();

    var next = $('#portfolio-wrapper li.current-link').next('li'),
    first = $('#portfolio-wrapper li:first-child');

    // Remove current class and conditionally move it to the next
    if ($('.current-link').is(':last-child')) {
      $('#portfolio-wrapper li.current-link').removeClass('current-link');
      $(first).addClass('current-link');
    } 
    else {
      $('#portfolio-wrapper li.current-link').removeClass('current-link');
      $(next).addClass('current-link');
    }

    // Load next content 
    var $nexthref = $('#portfolio-wrapper li.current-link > a').attr('href')+' #single-project-wrapper',
    nextURL = $('#portfolio-wrapper li.current-link > a').attr('href');

    // First remove existing content
    $('#single-project-wrapper').fadeOut('fast', function(){
      $(this).remove();
    });

    // Add loading beacon
    $('.framebox-freeze').append('<span id="framebox-loading">+ Loading +</span>');
    $('#framebox-loading').fadeIn('fast', loadNextContent);    

    function loadNextContent(){
      $('#framebox-content').load($nexthref , showNextContent);
    };

    // Show loaded AJAX content and hide loading bar
    function showNextContent(){
      $('#single-project-wrapper').show('normal', hideLoader);
    };

    // Hide loading bar
    function hideLoader(){
      $('#framebox-loading').fadeOut('fast')
    };

    // Push URL
    history.pushState(null, null, nextURL);

    return false;

  });

  // Prev
  //-------------------------------------------------------------------/

  $bodyCall.on('click', 'a#framebox-prev', function(e) {
    e.preventDefault();     

    //remove and add selected class
    var prev = $('#portfolio-wrapper li.current-link').prev('li');
    last = $('#portfolio-wrapper li:last-child');

    // Remove current class and conditionally move it to the prev
    if ($('.current-link').is(':first-child')) {
      $('#portfolio-wrapper li.current-link').removeClass('current-link');
      $(last).addClass('current-link');
    } else {
      $('#portfolio-wrapper li.current-link').removeClass('current-link');
      $(prev).addClass('current-link');
    }

    // Load prev content 
    var $prevhref = $('#portfolio-wrapper li.current-link > a').attr('href')+' #single-project-wrapper',
    prevURL = $('#portfolio-wrapper li.current-link > a').attr('href');

    // First remove existing content
    $('#single-project-wrapper').fadeOut('fast', function(){
      $(this).remove();
    });

    // Add loading beacon
    $('.framebox-freeze').append('<span id="framebox-loading">+ Loading +</span>');
    $('#framebox-loading').fadeIn('fast', loadNextContent);    

    function loadNextContent(){
     $('#framebox-content').load($prevhref , showNextContent)
   };

    // Show loaded AJAX content and hide loading bar
    function showNextContent(){
      $('#single-project-wrapper').show('normal', hideLoader);
    };

    // Hide loading bar
    function hideLoader(){
      $('#framebox-loading').fadeOut('fast')
    };

    // Push URL
    history.pushState(null, null, nextURL);
    return false;
  });

  // Set min-height to Window Height
  //-------------------------------------------------------------------/

  function setHeight(){
    $('#framebox-content').css('min-height', $(window).height());
  };

  $(window).resize(function(){
    setHeight;
  });

  // Close out Framebox

  var findme;

  jQuery(document).on('click', '#framebox-close, #framebox-close-panel-left, #framebox-close-panel-right', function(){
    $bodyCall.removeClass('framebox-freeze');
    $('#framebox-overlay').fadeOut();
    $('#framebox-overlay, #framebox-loading').remove();
    $('.project-tile').removeClass('current-link');

    history.pushState(null, null, homePage);
  });
});

Answer 1

From a once over review

• Code is well commented
• You have 2 variables that you did not declare with a var : last and nextURL
• Your resize listener will not work, use setHeight(); instead of setHeight;
• setHeight is a one liner, which is called once, you should inline it into your resize listener
• You declare some functions twice ( with the same code! ) like hideLoader or showNextContent, take those functions out of the listeners, right before Initialize framebox, this should cut down the size of your code.
• The HTML string looks terrible, since it is a plug in, you cannot have the markup in the HTML so I would least indent the HTML strings so that it is grokkable. Something like this:

singleContent = '<div id="framebox-overlay">' +
                  '<div id="framebox-close-panel-left"></div>' +
                  '<div id="framebox-close-panel-left"></div>' +
                  '<div id="framebox-wrapper">' +
                    '<div id="framebox-nav-wrapper">' +
                      '<p>' +
                        '<a href="#prev" id="framebox-prev" class="framebox-nav">' +
                          '<i class="fa fa-long-arrow-left"></i>' +
                          'Previous' + 
                        '</a>' +
                        '||' +
                        '<a href="#" id="framebox-next" class="framebox-nav">' +
                          'Next' + 
                          '<i class="fa fa-long-arrow-right"></i>' + 
                        '</a>' + 
                      '</p>' +
                    '</div>' +
                    '<section id="framebox-content">' +
                      //AJAX to insert #single-project-wrapper content here
                    '</section>' +
                    '<div id="framebox-close">' +
                      '<p>Click to close</p>' +
                    '</div>' +
                    '<ul class="ajax-list"></ul>' +
                  '</div>' +
                '</div>';

• I now see that $bodyCall.on('click', 'a#framebox-prev' and $bodyCall.on('click', 'a#framebox-next' are pretty much the same. You must identify the parts that are the same, extract that code in to a function ( which you place prior to Initialize framebox ).

If you are truly unable to merge those 2 listeners, I would suggest you gain some rep and put a bounty out.