Search code examples
javascriptjqueryhtmlcssexternal

JavaScript won't work when in the same file as other JavaScript code, and only when 'defer' is included. Why?


I have a piece of JQuery code that animates an inline link to scroll smoothly to a <section> with an assigned ID on the same page (below).

/*Smooth Scrolling effect*/
$('a[href*="#"]:not([href="#"])').click(function() {
	if (location.pathname.replace(/^\//, '') == this.pathname.replace(/^\//, '') && location.hostname == this.hostname) {
		var target = $(this.hash);
		target = target.length ? target : $('[name=' + this.hash.slice(1) + ']');
		if (target.length) {
			$('html, body').animate({
				scrollTop: target.offset().top
			}, 1000);
			return false;
		}
	}
});

For some reason, this will only work when it is placed externally of the rest of my JavaScript code

//*Side Navigation Menu*//
/* Open Side Nav - Set the width of the side navigation to 250px when burger menu icon is clicked. This perhaps needs rehashing a whole bunch more to make it more my own*/
function openNav() {
	document.getElementById("mySidenav").style.width = "300px";
}

/*Close Side Nav - Set the width of the side navigation to 0 when close button is clicked*/
function closeNav() {
	document.getElementById("mySidenav").style.width = "0";
}




//*Email Popup Form - Currently resets user's view to the top of the screen. This needs to be fixed.*//
$ = function(id) {
	return document.getElementById("popup");
}
var show = function(id) {
	$(id).style.display = 'block';
}
var hide = function(id) {
		$(id).style.display = 'none';
	}



	//*On hover over images on homescreen, display a black opacity box - Needs transferring to a seperate 'homepage' specific JavaScript file*//
$(function() {
	$('#img0').hover(function() {
		$('#fadeBox0').fadeIn(500);
	}, function() {
		$('#fadeBox0').fadeOut();
	});
});

$(function() {
	$('#img1').hover(function() {
		$('#fadeBox1').fadeIn(500);
	}, function() {
		$('#fadeBox1').fadeOut();
	});
});

$(function() {
	$('#img2').hover(function() {
		$('#fadeBox2').fadeIn(500);
	}, function() {
		$('#fadeBox2').fadeOut();
	});
});

$(function() {
	$('#img3').hover(function() {
		$('#fadeBox3').fadeIn(500);
	}, function() {
		$('#fadeBox3').fadeOut();
	});
});

I think the comments adequately (to my knowledge, I'm a beginner) explain what the JavaScript is supposed to do, but for some reason, some of this has stopped working as well. I don't know what I could have possibly changed, or where, as the rest of the website relies purely on HTML and CSS. (Note:After just testing something out, it appears that ALL of the above JavaScript has stopped working except for the small section labelled 'Side Navigation Menu'). Any help as to why this is happening would be much appreciated.

Thank you in advance!

Edit: I neglected to mention, the Smooth Scrolling Effect works when in an external JavaScript file, but only when Defer is used in the script tag. I've yet to try this with my other segments of JavaScript, but I don't want my code fragmented into individual JavaScript files for each individual function.


Solution

  • OK, more than "what is broken" let's try to wrap your head around the code.

    This says: (what happens in processing)

    • Get all elements that do not have an href attribute equal to "#" (ALL elements, really?)
    • THEN get all the a elements that have an href attribute with "#" in them in that set

      $('a[href*="#"]:not([href="#"])').click(function() {

    This says: (what happens in processing)

    • get all the "a" elements that have an href with # in them
    • THEN exclude those that do not have an href attribute equal to "#"

      $('a[href*="#"]').not('[href="#"]').on('click', function(){

    Thus that second form is more efficient:

    $('a[href*="#"]').not('[href="#"]').on('click', function() {
      if (location.pathname.replace(/^\//, '') == this.pathname.replace(/^\//, '') && location.hostname == this.hostname) {
        var target = $(this.hash);
        target = target.length ? target : $('[name=' + this.hash.slice(1) + ']');
        if (target.length) {
          $('html, body').animate({
            scrollTop: target.offset().top
          }, 1000);
          return false;
        }
      }
    });
    

    that $('html, body') - would $('body') work there? Why animate those/both?

    $(someselector).click(function(){ is shortcut for $(someselector).on('click',function()( so just use the second form.

    //Email Popup Form - Currently resets user's view to the top of the screen. This needs to be fixed.//

    In isolation this does nothing (DID overwrite jQuery alias $ before

    // do NOT replace the alias:
    var popme = function(id) {
      return document.getElementById("popup");
    };
    

    These are broken:

    var show = function(id) {
      $(id).style.display = 'block';
    };
    var hide = function(id) {
      $(id).style.display = 'none';
    };
    

    Fixed versions:

    var show = function(id) {
      $(id)[0].style.display = 'block';
    };
    var hide = function(id) {
      $(id)[0].style.display = 'none';
    };
    
    show("#myid");
    hide("#myid");
    

    Why this and not just use jQuery since you have it already?

    $("#myid").show();
    $("#myid").hide();
    

    //*On hover over images on homescreen, display a black opacity box - Needs transferring

    ONE document ready event handler:

    $(function() {
      $('#img0').hover(function() {
        $('#fadeBox0').fadeIn(500);
      }, function() {
        $('#fadeBox0').fadeOut();
      });
    
      $('#img1').hover(function() {
        $('#fadeBox1').fadeIn(500);
      }, function() {
        $('#fadeBox1').fadeOut();
      });
    
      $('#img2').hover(function() {
        $('#fadeBox2').fadeIn(500);
      }, function() {
        $('#fadeBox2').fadeOut();
      });
    
      $('#img3').hover(function() {
        $('#fadeBox3').fadeIn(500);
      }, function() {
        $('#fadeBox3').fadeOut();
      });
    });
    

    Alternate with classes (assumes the fadeBox class in on a child element)...

     $('#img0,#img1,#img2').hover(function() {
        $(this).find('.fadeBox').fadeIn(500);
      }, function() {
        $('.fadeBox').fadeOut();
      });
    

    Alternate 2, use classes on whatever those point to:

     $('.myImages').hover(function() {
        $(this).find('.fadeBox').fadeIn(500);
      }, function() {
        $('.fadeBox').fadeOut();
      });
    

    Note hover like this is short for mouseenter and mouseout event handlers. ref: https://api.jquery.com/hover/