patternjavascriptMinor
Using EnquireJS to trigger swapping image sources per media query
Viewed 0 times
triggerimageperqueryenquirejsswappingusingsourcesmedia
Problem
For responsive images I have come up with a first draft of how I would like to swap image sources depending on media query. This works fine and with the added debouncing function from Paul Irish the window resize event is also taken care of.
Now what I am wondering is if I could write this even more concise and most of all if there are major flaws in my thinking that could lead to images not loading or the like.
I am also wondering whether to write
or
as I am not entirely sure what the difference between the two is.
With the first case I am making an object that contains all the image elements and I assume with the second case I am generating a variable that holds all the objects that are the image elements. Is this correct?
Here is the fiddle and following is the jQuery code.
```
// on DOM ready
$(document).ready(function() {
function responsiveImages () {
// store all responsive image elements in jQuery object
var $responsiveImage = $('.responsiveImage');
// go over each responsive image element
$responsiveImage.each(function () {
// check how many responsiveImg objects there are
console.log( $(this).length );
// declare variable for each of the images found
var image = $(this);
// declare variables for small, medium and large images and
// store information about the image with .data() method
var smallImage = image.data('smallImage');
var mediumImage = image.data('mediumImage');
var largeImage = image.data('largeImage');
// function to change the image source
// there are two arguments for this function, the current image and the replacement image
function replaceImage (currentImage, replacementImage) {
// get the current image '
Now what I am wondering is if I could write this even more concise and most of all if there are major flaws in my thinking that could lead to images not loading or the like.
I am also wondering whether to write
var $responsiveImage = $('.responsiveImage');or
var responsiveImage = $('.responsiveImage');as I am not entirely sure what the difference between the two is.
With the first case I am making an object that contains all the image elements and I assume with the second case I am generating a variable that holds all the objects that are the image elements. Is this correct?
Here is the fiddle and following is the jQuery code.
```
// on DOM ready
$(document).ready(function() {
function responsiveImages () {
// store all responsive image elements in jQuery object
var $responsiveImage = $('.responsiveImage');
// go over each responsive image element
$responsiveImage.each(function () {
// check how many responsiveImg objects there are
console.log( $(this).length );
// declare variable for each of the images found
var image = $(this);
// declare variables for small, medium and large images and
// store information about the image with .data() method
var smallImage = image.data('smallImage');
var mediumImage = image.data('mediumImage');
var largeImage = image.data('largeImage');
// function to change the image source
// there are two arguments for this function, the current image and the replacement image
function replaceImage (currentImage, replacementImage) {
// get the current image '
Solution
$sigils
I personally like to use
Noise
Frankly, I find your comments to be obnoxious.
This comment and debug logging statement could be eliminated — the answer is always 1 because that what
Some comments are just wrong:
This one is the worst:
I think that all of the comments can go away if you write less code and clearer code. Instead of redundantly narrating the code, write just one good comment that summarizes the big picture.
The
Flexibility
You hard-code "small", "medium", and "large", as well as their thresholds. Not only does that make your code inflexible, the lack of generality also leads to a lot of copy-and-pasted code.
Instead, take a cue from the design of many jQuery extensions that accept two parameters: a jQuery object with the affected elements, and some kind of optional
Triggering
You register three media queries for each image. So, anytime the browser is resized, enquire.js checks the applicability of 30 media queries. Instead, you should register only 3 queries.
The efficiency improvement from reducing the number of registered queries should be sufficient to eliminate the need for debouncing the window resize handler.
In fact, the
Suggested solution
I personally like to use
$sigils to indicate that a JavaScript variable holds a jQuery object. In my opinion, it's a useful convention that makes the code more readable, especially in code where you need to distinguish jQuery objects from DOM objects. If you use a sigil for $responsiveImage, though, you should also use them consistently and write var $image = $(this);.Noise
Frankly, I find your comments to be obnoxious.
This comment and debug logging statement could be eliminated — the answer is always 1 because that what
jQuery.each() means:// check how many responsiveImg objects there are
console.log( $(this).length );Some comments are just wrong:
setup : function() {
// load content via AJAX
},This one is the worst:
// call function responsiveImages
responsiveImages();I think that all of the comments can go away if you write less code and clearer code. Instead of redundantly narrating the code, write just one good comment that summarizes the big picture.
The
replaceImage() helper function is too trivial. You would be better off without it. Note that it never uses its currentImage parameter anyway.Flexibility
You hard-code "small", "medium", and "large", as well as their thresholds. Not only does that make your code inflexible, the lack of generality also leads to a lot of copy-and-pasted code.
Instead, take a cue from the design of many jQuery extensions that accept two parameters: a jQuery object with the affected elements, and some kind of optional
config parameter. The function should be renamed from responsiveImages() to a verb such as makeResponsive(…).Triggering
You register three media queries for each image. So, anytime the browser is resized, enquire.js checks the applicability of 30 media queries. Instead, you should register only 3 queries.
The efficiency improvement from reducing the number of registered queries should be sufficient to eliminate the need for debouncing the window resize handler.
In fact, the
smartresize handler actually makes things worse. When the window is being resized, you register another set of 30 media queries every 50 milliseconds.Suggested solution
$(function() {
/**
* Makes elements in the $responsiveImages jQuery object
* switch their image src based on media queries.
*
* config is an optional hash whose keys are CSS media
* queries and whose values are the name of the data
* attribute on the element that holds the URL to the
* image that should be activated when the media query is
* matched.
*/
function makeResponsive($responsiveImages, config) {
if (!config) config = {
'screen and (max-width:400px)': 'small-image',
'screen and (min-width:401px) and (max-width:700px)': 'medium-image',
'screen and (min-width:701px)' : 'large-image',
};
for (var mediaQuery in config) {
(function(mediaQuery) {
enquire.register(mediaQuery, function() {
console.log("Activated " + mediaQuery);
$responsiveImages.each(function() {
var $image = $(this);
$image.attr('src', $image.data(config[mediaQuery]));
});
});
})(mediaQuery);
}
};
makeResponsive($('.responsiveImage'), {
// Override width thresholds for this demo
'screen and (max-width:600px)': 'small-image',
'screen and (min-width:601px) and (max-width:800px)': 'medium-image',
'screen and (min-width:801px)' : 'large-image',
});
});
Code Snippets
// check how many responsiveImg objects there are
console.log( $(this).length );setup : function() {
// load content via AJAX
},// call function responsiveImages
responsiveImages();Context
StackExchange Code Review Q#126016, answer score: 3
Revisions (0)
No revisions yet.