1

On my homepage, I have a video module and the following JS below animated in those cards.

All my JS minifies, so, when going on to a page that doesn't have this module, I get Cannot read property 'top' of undefined errors.

To prevent that, I'm trying to check if the variable exists before executing the script, but it doesn't work. Why doesn't the following work:

var scrolled = false;

$(window).on('scroll', function() {
    scrolled = true;
});



function loadCards() {
    if (scrolled) {
      var wScroll = $(this).scrollTop();

      if(wScroll > $('.vidCard').offset().top - ($(window).height() / 1.5)) {

       $('.vidCard').each(function(i){

         setTimeout(function(){
           $('.vidCard' ).eq(i).addClass('is-showing');
         }, 150 * (i+1));
       });

      }
    }

}


$(window).scroll(function() {
  loadCards();
});

$(function() {
  loadCards();
});
Freddy
  • 683
  • 4
  • 35
  • 114
  • I don't see any obvious place where you check for non-null. – Pointy Oct 23 '19 at 18:44
  • what is `$('.vidCard')` selector returning? – Anthony Oct 23 '19 at 18:45
  • Also, the message means that `$('.vidCard').offset()` returns `undefined`, not any particular variable. Thus minification will have no effect on this. Most likely that selector doesn't match anything. – VLAZ Oct 23 '19 at 18:46
  • why do have two $(window).on('scroll', function() { scrolled = true; }); and $(window).scroll(function() { loadCards(); }); instead you can have one $(window).on('scroll', function() { scrolled = true; loadCards(); }); – mahadev kalyan srikanth Oct 23 '19 at 18:47

2 Answers2

1

I think the this keyword you use in var wScroll = $(this).scrollTop(); is cause the problem. If you want the scrollTop of any element, write the selector of element (I think in here it is window). Also if you want to make it work for the first time, then you should write another algorithm that scrolled variable is not false there for very first time.

But I look at snippet below:

A brief description: We have a function isVisible that detect if an element is visible in just window section (not above it and neither below). Also each scroll, limit the search area with selecting vidCards that are not have is-showing class (by using this approach, detection will be faster and the animation will be smooth and not take a long time to show!). I assumed that is-showing have opacity effect.

I hope it helps :)

function isVisible(el) {
  var $el = $(el),
    above = $el.offset().top - $(window).scrollTop() + $el.outerHeight() < 0,
    below = $el.offset().top > $(window).outerHeight() + $(window).scrollTop();
  return !above && !below;
}

function loadCards() {
  $('.vidCard:not(.is-showing)').each(function(i) {
    var card = $(this);

    if (isVisible(card) && !card.hasClass('is-showing')) {
      setTimeout(function() {
        card.addClass('is-showing');
      }, 150 * (i + 1));
    }
  });
}

$(window).on('scroll', function() {
  loadCards();
});

loadCards();
.vidCard {
  width: 100px;
  height: 100px;
  background-color: #eee;
  margin-bottom: 20px;
  opacity: 0;
}

.is-showing {
  opacity: 1;
  -webkit-transition: all ease 0.6s;
  -moz-transition: all ease 0.6s;
  -ms-transition: all ease 0.6s;
  -o-transition: all ease 0.6s;
  transition: all ease 0.6s;
}
<script src="https://cdnjs.cloudflare.com/ajax/libs/jquery/3.3.1/jquery.min.js"></script>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
<div class="vidCard"></div>
MMDM
  • 465
  • 3
  • 11
0

Use offsetTop instead of .offset().top

This answer can help you: offsetTop vs. jQuery.offset().top

pncsoares
  • 63
  • 10