Popup jQuery plugin

Question

I need a code review for my first jQuery plugin. It seems like a lot of code and I want to hear something about my plugin code structure and more.

;
(function($) {
  'use strict';

  $.fn.mpopup = function(options) {

    //Default settings
    var defaults = {
      overlayClass: 'mpopup-overlay',
      overlayColor: '#000',
      overlayOpacity: 0.7,
      overlayFadeSpeed: 500,
      popupFadeSpeed: 500,
      closePopup: '.close-popup',
      trigger: '.open-mpopup',
      effect: 'fadeUp'
    };

    //Settings
    var settings = $.extend({}, defaults, options);

    //Variables
    var popup = this,
      windowWidth = $(window).width(),
      windowHeight = $(window).height(),
      popupBlockHeight = popup.outerHeight(),
      popupBlockWidth = popup.outerWidth(),
      bodyElement = $('body'),
      trigger = $(settings.trigger),
      popupHorizontalPosition = (windowWidth - popupBlockWidth) / 2,
      closeFlag = false,
      transitionEffectVal = 100,
      indent = (settings.indent) ? settings.indent : windowHeight / 6, // windowHeight / 6 for better look on resize
      isShortPopup,
      popupVerticalPosition,
      onResize,
      newPopupVerticalPosition,
      documentHeight,
      overlay = $('<div />')
      .attr('class', settings.overlayClass)
      .css({
        'top': '0',
        'bottom': '0',
        'left': '0',
        'right': '0',
        'position': 'fixed',
        'z-index': 9998,
        'display': 'none'
      });

    //Hex to RGB with opactiy
    function hex2rgb(hex, opacity) {
      var h = hex.replace('#', '');
      h = h.match(new RegExp('(.{' + h.length / 3 + '})', 'g'));

      for (var i = 0; i < h.length; i++)
        h[i] = parseInt(h[i].length == 1 ? h[i] + h[i] : h[i], 16);

      if (typeof opacity != 'undefined') h.push(opacity);

      return 'rgba(' + h.join(',') + ')';
    }

    var rgbaValue = hex2rgb(settings.overlayColor, settings.overlayOpacity);

    // Custom event for callback functions
    $.each(settings, function(key, value) {
      if (typeof value === 'function') {
        popup.on(key + '.mpopup', function() {
          value(popup);
        })
      }
    });

    //Append popup into overlay block
    function appendPopup() {
      overlay.css('background', rgbaValue).appendTo(bodyElement).append(popup);
      popup.css({
        'position': 'relative',
        'z-index': 9999
      });
    }

    //Show popup
    function triggerCall() {
      if (popup.length > 0) {
        appendPopup();
        trigger.click(function(e) {
          e.preventDefault();
          onResize = false;
          closeFlag = false;
          popup.trigger('beforeCreated.mpopup');
          shortPopup();
          longPopup();
          doEffect();
          popup.trigger('created.mpopup');
        });
        resizePopup();
      } else {
        console.log('Pop-up is undefined');
      }
    }

    // Close popup
    function close() {
      var svgCloseElem = $(settings.closePopup).find('*');

      overlay.click(function(e) {
        closeFlag = true;
        if ($(e.target).is(settings.closePopup) ||
          $(e.target).is(svgCloseElem) || // for svg elements
          $(e.target).is(overlay)) {
          overlay.css('overflow-y', 'hidden');

          // Remove overflow-y and padding-right styles from body
          documentHeight = $('html, body').height();
          if (documentHeight > windowHeight) {
            bodyElement.css({
              'overflow-y': '',
              'padding-right': ''
            });
          }
          doEffect();
        }
        popup.trigger('afterClose.mpopup');
      });
      // Close popup when clicking the esc keyboard button
      $(document).keyup(function(e) {
        if (e.which == '27') {
          overlay.removeClass('show-popup');
          popup.removeClass('show-popup');
          popup.trigger('afterClose.mpopup');
        }
      });

    }

    // If popup block height LESS than window height
    function shortPopup() {
      if (popupBlockHeight < windowHeight) {
        isShortPopup = true;
        popupVerticalPosition = (windowHeight - popupBlockHeight) / 2;
        newPopupVerticalPosition = popupVerticalPosition + transitionEffectVal;
        popup.css({
          'left': popupHorizontalPosition,
          'margin': 0,
          'top': (settings.effect === 'fadeUp' && !onResize) ? newPopupVerticalPosition : popupVerticalPosition
        })
      }
    }

    // If popup block height BIGGER or EQUAL to window height
    function longPopup() {
      if (popupBlockHeight >= windowHeight) {
        isShortPopup = false;
        popupVerticalPosition = indent;

        // Remove body scroll and add right padding equal to Scrollbar size
        documentHeight = $('html, body').height();
        if (documentHeight > windowHeight) {

          var horizontalScrollWidth = window.innerWidth - document.documentElement.clientWidth;

          bodyElement.css({
            'overflow-y': 'hidden',
            'padding-right': horizontalScrollWidth
          });
        }
        overlay.css('overflow-y', 'scroll');

        newPopupVerticalPosition = indent + transitionEffectVal;
        popup.css({
          'left': popupHorizontalPosition,
          'top': 0,
          'margin-top': (settings.effect === 'fadeUp' && !onResize) ? newPopupVerticalPosition : popupVerticalPosition,
          'margin-bottom': popupVerticalPosition
        })
      }
    }

    //Show and close effects
    function doEffect() {
      switch (settings.effect) {
        case 'fade':
          if (!closeFlag)
            overlay.fadeIn(settings.overlayFadeSpeed).find(popup).fadeIn(settings.popupFadeSpeed);
          else
            overlay.fadeOut(settings.overlayFadeSpeed).find(popup).fadeOut(settings.popupFadeSpeed);
          break;
        case 'fadeUp':
          if (!closeFlag) {
            if (isShortPopup) {
              popup.animate({
                'top': '-=' + transitionEffectVal
              }, settings.popupFadeSpeed);
            } else {
              popup.animate({
                'margin-top': '-=' + transitionEffectVal
              }, settings.popupFadeSpeed);
            }
            overlay.fadeIn({
              queue: false,
              duration: settings.overlayFadeSpeed
            }).find(popup).fadeIn({
              queue: false,
              duration: settings.popupFadeSpeed
            });

          } else {
            if (isShortPopup) {
              popup.animate({
                'top': '+=' + transitionEffectVal
              }, settings.popupFadeSpeed);
            } else {
              popup.animate({
                'margin-top': '+=' + transitionEffectVal
              }, settings.popupFadeSpeed);
            }
            overlay.fadeOut({
              queue: false,
              duration: settings.overlayFadeSpeed
            }).find(popup).fadeOut({
              queue: false,
              duration: settings.popupFadeSpeed
            });
          }
          break;
        default:
          console.log('No effect');
      }
    }

    // Popup on mobile devices
    function resizePopup() {
      $(window).resize(function() {
        onResize = true;
        windowWidth = $(window).width();
        windowHeight = $(window).height();
        popupBlockWidth = popup.outerWidth();

        popupHorizontalPosition = (windowWidth - popupBlockWidth) / 2;
        if (popupBlockHeight < windowHeight) {
          popupVerticalPosition = (windowHeight - popupBlockHeight) / 2;
        } else {
          popupVerticalPosition = windowHeight / 6;
        }
        if (popup.css('display') === 'block') {
          shortPopup();
          longPopup();
        }
        popup.trigger('onResize.mpopup');
      });

    }

    function init() {
      triggerCall();
      close();
    }
    init();
    return this;
  }

}(jQuery));

Answer 1

Mutually exclusive functions

if (popup.css('display') === 'block') {
  shortPopup();
  longPopup();
}

I don't like that you have two mutually exclusive functions. If I read the code above, I presume you open both a short and long popup. You should pull the logic that makes them mutually exclusive out of the functions, and only call one of them based on the logic:

if (popup.css('display') === 'block') {
  if (popupBlockHeight < windowHeight) {
    shortPopup();
  } else {
    longPopup();
  }
}

triggerCall() and close() and resizePopup()

The triggerCall() and close() functions do not name what they actually do: Attaching a handler to open the popup and attaching a handler to close the popup. From the name alone, I would assume resizePopup() would resize an existing popup, but instead it attaches a handler to the resize handler of the window.

I don't see a situation where you would want to be able to open a popup, but not be able to close it again. Calling any of these functions multiple times will result in wrong behaviour. I would suggest at least renaming the functions to better name what they do, but probably just remove those functions and move the attaching of handlers to the init function. It is the only time you want to call them anyway.

Misc

You have at least two calls to popup.css({ ... }), but you forgot to close the statement with a ;.

I recommend against declaring variables using the var var1, var2, var3 ... syntax. The reason for this is that it is easy to accidentally drop a variable in the global namespace by forgetting a comma. The following snippet reads weird to me, even though it is correct. If you had to define more variables, you would have continued with a comma at the end, making it even more unreadable. Consider either indenting it more, so it does not read at a glance as some more variables being declared, or move the assignment down after you declare your variables.

var popup = this,
  windowWidth = $(window).width(),
  //some more declarations
  documentHeight,
  overlay = $('<div />')
  .attr('class', settings.overlayClass)
  .css({
    'top': '0',
    'bottom': '0',
    'left': '0',
    'right': '0',
    'position': 'fixed',
    'z-index': 9998,
    'display': 'none'
  });