0

I'm trying to call a function based on screen size inside a loop'.

The function works fine when the page is loaded, however, the content doesn't change when the window is resized.

Here's an example: https://jsfiddle.net/celine305/BaNRq/27/

Any help would be much appreciated.

for (var i = 0; i < 2; i++) {
  function red() {
    $('#' + i + '').css('background', '#B60C0C')
      .text('Screen Size RED');
  }

  function orange() {
    $('#' + i + '').css('background', '#EBAE10')
      .text('Screen Size ORANGE');
  }

  function green() {
    $('#' + i + '').css('background', '#83ba2b')
      .text('Screen Size GREEN');
  }

  var widths = [0, 500, 850];

  function resizeFn() {
    if (window.innerWidth >= widths[0] && window.innerWidth < widths[1]) {
      red();
    } else if (window.innerWidth >= widths[1] && window.innerWidth < widths[2]) {
      orange();
    } else {
      green();
    }
  }
  resizeFn();
  window.onresize = resizeFn();
}
3
  • 2
    why is java tagged? Commented Jul 21, 2016 at 18:30
  • I don't see the point of the for loop, it's just wasted computation. Commented Jul 21, 2016 at 18:33
  • 1
    Try defining your functions outside the loop, and then calling them with parameters for the i'th iteration of the loop Commented Jul 21, 2016 at 18:33

3 Answers 3

1
  1. Move your functions outside of the for loop
  2. Merge 3 functions to one
  3. Use a jQuery listener instead of JavaScript since you're using jQuery already

// You could also assign classes and use $(".className") instead of loop
function changeColor(color) {
    for(var i=0; i<2; i++){
        $('#'+i+'').css('background',color).text('Screen Size' + color);
    }
}

var widths = [0, 500, 850];

function resizeFn() {
    if (window.innerWidth>=widths[0] &&window.innerWidth<widths[1]) {
        changeColor('#B60C0C');
    } else if (window.innerWidth>=widths[1] && window.innerWidth<widths[2]) {
        changeColor('#EBAE10');
    } else {
        changeColor('#83ba2b');
    }
}

resizeFn();
$(window).resize(function() {
    console.log("resize");
    resizeFn();
})

JSFiddle renders the content inside a div so your code was never detecting the resize. I added a container to detect the resize, but otherwise you should use $(window).

Fiddle: https://jsfiddle.net/BaNRq/29/

Sign up to request clarification or add additional context in comments.

Comments

0

The whole point of your loop seems to be selecting the elements. You can do this much simpler by selecting all your elements with one query:

$("#1, #2")

Because you're doing EVERYTHING in the loop, you are redefining functions and are assigning the resize-function multiple times.

Oh, and by the way, you are not really assigning a function to the window.onresize event handler, but the return value of a function. Try assigning it without the trailing braces.

2 Comments

@spencerWieczorek this is just a simple version of a much larger code (with around 50 elements).
In this case, give all elements a common class and select them by the class. If that is not possible for you, create a selector string in a loop and pass that string to jQuery.
0

If you want to use the current code, you will likely have to move the function declarations outside of the for loop. See this article.

As a side note though, I would recommend moving away from javascript for handeling these style changes. CSS offers a good way to handle style changes for predetermined sizes in the form of media queries.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.