1

I was tasked with converting a jQuery function into plain JavaScript. The function is used to check if an element is within the viewport. If it is within the viewport take the data-bglazy attribute and add a background image style to that element using the value of that attribute. The function that needs converted is:

$.fn.isInViewport = function() {
    var elementTop = $(this).offset().top;
    var elementBottom = elementTop + $(this).outerHeight();

    var viewportTop = $(window).scrollTop();
    var viewportBottom = viewportTop + $(window).height();

    return elementBottom > viewportTop && elementTop < viewportBottom;
  };

 $(window).on('resize scroll', function() {
    $('.bgLazy').each(function() {
      if ($(this).isInViewport()) {
        var lazyImg = $(this).attr('data-bglazy');
        $(this).css('background-image', 'url(' + lazyImg + ')');
      }
    });
  });


Currently what I have when trying to convert the above function to JavaScript:


function isInViewport(el){
    var elementTop = el.offsetTop;
    var elementBottom = elementTop + el.offsetHeight;

    var viewportTop = window.scrollTop;
    var viewportBottom = viewportTop + window.offsetHeight;

    return elementBottom > viewportTop && elementTop < viewportBottom;

  };

    var bgElements = document.querySelectorAll('.bgLazy');

    bgElements.forEach(bgElementLoop);

    function bgElementLoop(item, index) {
      if(item.isInViewport()){
        var lazyImg = item.getAttribute('data-bglazy');
        item.style.backgroundImage = 'url(' + lazyImg + ')';
      }
    }


    window.addEventListener("resize, scroll", bgElementLoop);

I am trying to figure out which part I screwed up on when attempting to convert the jQuery function to JavaScript

EDIT:

I made a view changes after reading some of the comments. the isInViewport function is not changed, but what I did change is the following:

    var bgElements = Array.prototype.slice.call(document.querySelectorAll('.bgLazy'));

    bgElements.forEach(bgElementLoop);

    function bgElementLoop(item, index) {
      if(item.isInViewport(item)){
        var lazyImg = item.getAttribute('data-bglazy');
        item.style.backgroundImage = 'url(' + lazyImg + ')';
      }
    }

    window.addEventListener("resize", bgElementLoop);
    window.addEventListener("scroll", bgElementLoop);

So what I did here is changed the bgElements variable from

var bgElements = document.querySelectorAll('.bgLazy');

to

var bgElements = Array.prototype.slice.call(document.querySelectorAll('.bgLazy'));

I then separated the resize and scroll event listeners into:

window.addEventListener("resize", bgElementLoop);
window.addEventListener("scroll", bgElementLoop);
10
  • 3
    querySelectorAll returns a NodeList, not an array. Commented Jun 13, 2019 at 19:14
  • window.addEventListener("resize, scroll", bgElementLoop); you also shouldn't have the comma in the event string Commented Jun 13, 2019 at 19:15
  • 2
    for one if(item.isInViewport()) should be if(isInViewport(item)) Commented Jun 13, 2019 at 19:16
  • 2
    @DanielA.White - nodelist has a forEach method as well. Commented Jun 13, 2019 at 19:17
  • 2
    can everyone stop trying to convert nodelists to arrays. it's fine as is. Commented Jun 13, 2019 at 19:17

1 Answer 1

1

Here's a workign example with all my suggestions from the comments.

function isInViewport(el) {
  var b = el.getBoundingClientRect();
  return b.top >= 0 &&
    b.left >= 0 &&
    b.right <= (window.innerWidth || document.documentElement.clientWidth) &&
    b.bottom <= (window.innerHeight || document.documentElement.clientHeight);
};

var bgElements = document.querySelectorAll('.bgLazy');

function onScrollResize() {
  bgElements.forEach((item, index) => {
    if (isInViewport(item)) {
      var lazyImg = item.getAttribute('data-bglazy');
      setTimeout(() => item.innerHTML = 'loaded', 1000);
      item.style.backgroundImage = 'url("' + lazyImg + '")';
    }
  });
}

document.addEventListener("DOMContentLoaded", onScrollResize);
window.addEventListener("resize", onScrollResize);
window.addEventListener("scroll", onScrollResize);
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>
<div class='bgLazy' data-bglazy="http://i.imgur.com/rw0Jwpm.jpg">stuff</div><br>

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

5 Comments

And again, from the comments, this will not work in IE as NodeList does not implement forEach. IE would need a polyfill.
I implemented this and it worked perfectly! Going with the above content for IE support I set up the bgElements variable using: Array.prototype.slice.call(document.querySelectorAll('.bgLazy'));
ie needs to die, and it won't die if we keep supporting it. only 2% of users use the very latest version of ie anyway.
Those kinds of generic statistics are meaningless. What matters is what your users are using. If you're building a tool for enterprises, hospitals, the government, retirees or a stubborn CEO then IE is definitely NOT dead and probably won't be dead any time soon.
it doesn't matter. it isn't even a real statistic. it's hyperbole meant to illustrate that the number of IE users in the wild is minuscule and generally not worth consideration. comments aren't for extended discussion. feel free to send me a chat invite.

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.