0
\$\begingroup\$

This is quite a simple function, but I just wanted some opinions on it, like is this the correct way to run a function and check if an element exists or not?

const button = document.querySelector('.js-brand-video--thumbnail')
const iframe = document.querySelector('.js-brand-video--iframe')

function playVideo () {
  if (button) {
    button.classList.add('video-poster--hidden')
    iframe.src = iframe.getAttribute('data-src')
  }
}

if (button)
  button.addEventListener('click', playVideo, false)
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

querySelector can return null. So while you are checking button, you aren't checking for iframe. You will possibly get Assigning property src to undefined or something like that.

classList is one of the newer properties. If you see your code being used in older browsers, you should watch out. Check the compatibility table for support.

A practice used by jQuery and other libraries is to use sets. It's where you put results in an array and for each item, operate on it. If no items are on the set, no code executes.

// Use querySelectorAll to get elements. querySelectorAll returns a
// NodeList. To use array methods, we convert it to an array with
// slice.
const buttons = Array.prototype.slice.call(document.querySelectorAll('.js-brand-video--thumbnail'));
const iframes = Array.prototype.slice.call(document.querySelectorAll('.js-brand-video--iframe'));

// forEach runs the callback for each item in the array. If no items
// reside, no loop runs, therefore no code executes.
button.forEach(function(button){
  button.addEventListener('click', function(){
    button.classList.add('video-poster--hidden')
    iframe.forEach(function(iframe){
      iframe.getAttribute('data-src');
    }); 
  }, false)
});
\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.