jQuery plugin authoring: Vertical Center example

Question

I am fairly new at writing jQuery plugins. Moreover, I am a huge fan of best practice.

I have written a plugin which centers an element inside its parent. I would love some feedback on how I could have written it better.

(function( $ ){

  $.fn.verticalCenter = function(options) {

    var settings = $.extend({
      min: 0, // Minimum width of the viewport for the centering to take effect
      max: 10000 // Maximum width of the viewport
    }, options);


    var center_element = function(element, parent){
      var elementHeight = element.height();
      parent.css({
        position:'relative'
      });
      element.css({
        position: 'absolute',
        top: "50%",
        marginTop:-elementHeight/2
      });
    }

    var remove_style = function(element, parent){
      parent.css({ position:'' });
      element.css({ position: '', top: '', marginTop: '' });
    }

    var determine_action = function(element, parent){

      $(window).on('load resize orientationchange', function() {

        var windowWidth = window.innerWidth

        if (windowWidth > settings.min && windowWidth < settings.max) {
          center_element(element, parent);
        }
        else {
          remove_style(element, parent);
        }

      });

    }

    return this.each(function(){

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

      determine_action($elementToCenter, $parent);

    });

  };

})( jQuery );

The plugin is supposed to be responsive, and it allows the user to pass in viewport min and max values. These values determine if the centering should occur.

Answer 1

Interesting question, from a once over:

• Consider a 'use strict' after (function( $ ){
• You are mixing lowerCamelCase and snake_case, you should stick to lowerCamelCase
• I am not a fan of remove_style, it might cause all kinds of unintended consequences, it would be better to store the original values of element and parent and then restore them if we don't center.
• I am also not a fan of having 1 listener function per element ( however, if you were to keep the original css info in a closure, then that's fine )
• The code would probably be cleaner if you let center_element determine the parent.
• The newlines before closing curly braces make the code a bit too sparse towards the end

I would probably try something like this:

(function($){
  'use strict';
  $.fn.verticalCenter = function(options) {

    var settings = $.extend({
      min: 0,    // Minimum width of the viewport for centering to take effect
      max: 10000 // Maximum width of the viewport
    }, options);

    var centerElement = function(element,parent){

      parent.css({ position:'relative' });
      element.css({
        position: 'absolute',
        top: "50%",
        marginTop:-element.height() / 2
      });
    };

    var determine_action = function(element){

      var parent = element.parent(),
          originalParentStyle = parent.style(['position']),
          originalElementStyle = element.style(['position','top','marginTop']);

      $(window).on('load resize orientationchange', function() {

        var windowWidth = window.innerWidth;

        if (windowWidth > settings.min && windowWidth < settings.max) {
          centerElement(element, parent);
        }
        else {
          parent.css(originalParentStyle);
          element.css(originalElementStyle);
        }
      });
    };

    return this.each(function(){
      determine_action($(this));
    });
  };
})(jQuery);