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

Three layout Gallery

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

Problem

I have made a gallery function, this function handles three different types of gallery, one with horizontal nav, other vertical and last one vertical with no nav.

```
function vid() {
var vid = $(".videoBg"),
btn = $(".wVid"),
wrap = $(".home");
$(wrap).css({
"margin-top": "220px"
});
$(vid).addClass('active');
$(btn).on("click", function() {
var vidH = $(vid).height();
if ($(vid).hasClass('active')) {
$(wrap).css({
"margin-top": vidH - 25 + "px"
});
$(this).css({
"top": vidH - 100 + "px",
"background-position": "top center"
});
$(this).text("Close");
$(vid).removeClass('active');
} else {
$(wrap).css({
"margin-top": "220px"
});
$(this).css({
"top": "100px",
"background-position": "bottom center"
});
$(this).text("Watch Video");
$(vid).addClass('active');
}
return false;
});
}

function showHide() {
var btn = $(".postComment button");
var popUp = $(".postComment .comForm.pForm");
var popUpT = $(".postComment .comForm.thanks");
var close = $(".postComment .comForm .cls");
var submt = $(".postComment .comForm button");
$(popUp).hide();
$(popUpT).hide();
$(btn).on("click", function() {
$(popUp).show();
$(close).on("click", function() {
$(this).parent().hide();
return false;
});
$(submt).on("click", function() {
$(popUpT).show();
});
});
}

function gallery(galObj, showNav, galOrient) {
$(galObj).each(function() {
var cGalObj = $(this);

//declaring gallery elements
var bigImgWrap = $(cGalObj).find(".gal .story");
var mask = $(cGalObj).find(".thumbnails");
var thumbsWrap = $(cGalObj).find(".thu

Solution

var per variable

It's personal preference, but it's a practical preference. If you had something like this in the middle of the code:

var vid = $(".videoBg"),
btn = $(".wVid"),
wrap = $(".home");


You would readily think "Oh, new variable vid and an assigment to existing variables btn and warp". But then you missed the commas. Also, it's easier to move around the variables if they have their own var.

Consider creating a jQuery plugin

gallery(".photoGalD");
gallery(".photoGalUpdate");
gallery(".vidgal");
gallery(".eventgal");


Not really that bad, but what if you could have done it like:

$('.class-common-to-gallery-elements').gallery()


Consider making it a jQuery plugin, no need to publish it online. jQuery plugins are, in leman's terms, self-contained, reusable units of code. Invest time learning how to make one.

Separate the config

I see you are using owlCarousel, but what if you needed more than just 3? Would you copy-paste the code? or just add a configuration, and all is set? I'd go for the latter, in this manner:

// Place configs here
var owlCarouselConfigs = {
  '.carousel-demo' : {
    items: 1,
    navigation: true,
    slideSpeed: 300,
    pagination: false,
    singleItem: true
  },
  '.carousel-demo.artCar' : {
    items: 1,
    navigation: true,
    slideSpeed: 300,
    pagination: false,
    singleItem: true
  },
  ...
}

// Execute
$.each(owlCarouselConfigs,function(selector,config){
  $(selector).owlCarousel(config);
});


Same could be applied to validate

Break apart code

Usually, in a jQuery plugin, I do this (see jQuery Boilerplate for the structure):

;(function ($, window, document, undefined) {

  // Plugin defaults
  var pluginName = "gallery",
    defaults = {
      propertyName: "value"
    };

  // Plugin constructor and prototype
  function Plugin(element, options) {
    // Configure instance properties and configurations
    this.element = element;
    this.settings = $.extend({}, defaults, options);
    this._defaults = defaults;
    this._name = pluginName;
    // Initialize the plugin
    this.init()
  }
  Plugin.prototype = {
    init: function () {
      // Here, I call out some stuff related to setups in UI, AJAX etc.
      this.setups();
      this.someMoreSetups();
    },
    // Functionality is broken off into their own functions
    setups: function () {
      // calling other functions in a function
      this.anAjaxFunction();
    },
    someMoreSetups: function () {...
    },
    anAjaxFunction: function () {...
    },
  };

  // Standard plugin stuff. Don't touch.
  $.fn[pluginName] = function (options) {
    this.each(function () {
      if (!$.data(this, "plugin_" + pluginName)) {
        $.data(this, "plugin_" + pluginName, new Plugin(this, options))
      }
    });
    return this
  }
})(jQuery, window, document);

$('.myElement').gallery();

Code Snippets

var vid = $(".videoBg"),
btn = $(".wVid"),
wrap = $(".home");
gallery(".photoGalD");
gallery(".photoGalUpdate");
gallery(".vidgal");
gallery(".eventgal");
$('.class-common-to-gallery-elements').gallery()
// Place configs here
var owlCarouselConfigs = {
  '.carousel-demo' : {
    items: 1,
    navigation: true,
    slideSpeed: 300,
    pagination: false,
    singleItem: true
  },
  '.carousel-demo.artCar' : {
    items: 1,
    navigation: true,
    slideSpeed: 300,
    pagination: false,
    singleItem: true
  },
  ...
}

// Execute
$.each(owlCarouselConfigs,function(selector,config){
  $(selector).owlCarousel(config);
});
;(function ($, window, document, undefined) {

  // Plugin defaults
  var pluginName = "gallery",
    defaults = {
      propertyName: "value"
    };

  // Plugin constructor and prototype
  function Plugin(element, options) {
    // Configure instance properties and configurations
    this.element = element;
    this.settings = $.extend({}, defaults, options);
    this._defaults = defaults;
    this._name = pluginName;
    // Initialize the plugin
    this.init()
  }
  Plugin.prototype = {
    init: function () {
      // Here, I call out some stuff related to setups in UI, AJAX etc.
      this.setups();
      this.someMoreSetups();
    },
    // Functionality is broken off into their own functions
    setups: function () {
      // calling other functions in a function
      this.anAjaxFunction();
    },
    someMoreSetups: function () {...
    },
    anAjaxFunction: function () {...
    },
  };

  // Standard plugin stuff. Don't touch.
  $.fn[pluginName] = function (options) {
    this.each(function () {
      if (!$.data(this, "plugin_" + pluginName)) {
        $.data(this, "plugin_" + pluginName, new Plugin(this, options))
      }
    });
    return this
  }
})(jQuery, window, document);

$('.myElement').gallery();

Context

StackExchange Code Review Q#47223, answer score: 5

Revisions (0)

No revisions yet.