1
\$\begingroup\$

I'm still learning JavaScript and I'm building an Ajax website that loads in content for each page from external files (e.g. test1.php, test2.php). I spent several hours cobbling together some code that works, but it feels really clunky. Any recommendations on how to streamline it? Or anything I am doing that is stupid and should be done differently?

$(document).ready(function() {
						   
	var hash = window.location.hash.substr(1);
	
	var projectID = $('#topNav a').each(function(){
		var projectID = $(this).attr('id');
		if(hash==projectID){
			var toLoad = hash+'.php .content';
			$('.content').load(toLoad);
			$(this).siblings().removeClass('active');
			$(this).addClass('active');
		}							
	});

	$('#topNav a').click(function(){

        var toLoad = $(this).attr('href')+' .content',
			newId = $(this).attr('rel'),        
            oldHeight = $('#shell').css("height"),      
            viewportHeight = $(window).height()
		
		if (!$(this).hasClass("active")) {

			$(this).siblings().removeClass('active');
			$(this).addClass('active');
			
			$('<div/>', {
				id: newId,
				class: 'content'
			}).css({ top: viewportHeight + "px", display: "none" }).appendTo('#shell').load(toLoad);
			
			$('#' + newId).show(function() {
				$(this).animate({ top: "0px", queue: false}, 600);
				$('#shell > div:first').animate({ top: "-" + oldHeight, queue: false}, 600, function() {

					$('#shell > div:first').remove()
				});
			});
			
			var newHash = $(this).attr('id');
			window.history.pushState(null, "", "#" + newHash);
			
		}
		
		return false;
    });
	
	window.addEventListener("popstate", function(e) {
		
		var hash = window.location.hash.substr(1),
			oldHeight = $('#shell').css("height"),      
            viewportHeight = $(window).height()
	
		var projectID = $('#topNav a').each(function(){
			var projectID = $(this).attr('id');
			if(hash==projectID){
				var toLoad = hash+'.php .content'
				
				$(this).siblings().removeClass('active');
				$(this).addClass('active');
				
				$('<div/>', {
					id: hash,
					class: 'content'
				}).css({ top: viewportHeight + "px", display: "none" }).appendTo('#shell').load(toLoad, function() {
					
					$(this).show(function() {
						$(this).animate({ top: "0px", queue: false}, 600);
						$('#shell > div:first').animate({ top: "-" + oldHeight, queue: false}, 600, function() {
		
							$('#shell > div:first').remove()
						});
					});
					
				});
			
			}							
		});
		
	});
	
});
#topNav {
	position: fixed; overflow: auto; top: 0; left: 0;
	width: 100%; z-index: 100; background: #fff; height: 80px;
    }
	#topNav a { margin-right: 30px; color: #a9a9a9; }
	  #topNav a.active { color: #333; }

    #shell { margin: 0px; padding: 0px; width: 100%; height: 100%; }

    .content { 
      position: absolute; margin: 0px; padding-bottom: 0px;
      width: 100%; height: 100%;
    }
    #content1 { background: #000; color: #fff; }
    #content2 { background: red; }
    #content3 { background: blue; }
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>
<nav id="topNav">
    <a href="test1.php" id="test1" rel="content1" class="active">Test 1</a>
    <a href="test2.php" id="test2" rel="content2">Test 2</a>
    <a href="test3.php" id="test3"rel="content3">Test 3</a>
    </nav>

    <div id="shell">

    <div id="content1" class="content">
        
        <p>Here is the first page</p>
        
    </div>

    </div>

\$\endgroup\$
2
\$\begingroup\$

You should really combine code that is the same and put them into functions. Then call the functions.

I put your functionality into two categories.
1. Updating the tab links to show the active link. 2. Pulling the content from the specified pages.

So in order to do these I separated out the functionality into two separate functions and called them where appropriate. I took out the looping through of the tabs. If you had that in there as some sort of check to make sure the hash was valid then you may want to add that back in as another function.

After doing this one problem became obvious. You are losing the styles applied by id (#content1, #content2) when hitting the back button. This is because you are using the hash as the ID instead of the 'rel' attribute of the links as you are doing in other places.

Anyway here are my changes. These don't solve all problems (i.e. you now have global functions) but should lead you in the right direction.

    function loadContent(newId, toLoad) {

        var oldHeight = $('#shell').css("height"),
            viewportHeight = $(window).height();

        $('<div/>', {
            id: newId,
            class: 'content'
        }).css({ top: viewportHeight + "px", display: "none" }).appendTo('#shell').load(toLoad, function () {

            $(this).show(function () {
                $(this).animate({ top: "0px", queue: false }, 600);
                $('#shell > div:first').animate({ top: "-" + oldHeight, queue: false }, 600, function () {

                    $('#shell > div:first').remove()
                });
            });

        });

    }

    function setActiveTab(id) {
        $('#topNav a').removeClass('active');
        $('#' + id).addClass('active');
    }


    $(document).ready(function () {

        var hash = window.location.hash.substr(1);

        setActiveTab(hash);
        $('.content').load( hash + '.php .content');


        $('#topNav a').click(function () {

            var toLoad = $(this).attr('href') + ' .content',
                newId = $(this).attr('rel'),
                newHash = $(this).attr('id');

            if (!$(this).hasClass("active")) {

                setActiveTab(newHash);
                loadContent(newId, toLoad);

                window.history.pushState(null, "", "#" + newHash);

            }

            return false;
        });

        window.addEventListener("popstate", function (e) {

            var hash = window.location.hash.substr(1);
            setActive(hash);
            loadContent(hash, hash + '.php .content');

        });
    });
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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