6
\$\begingroup\$

I don't have formal training in coding, so I would like some input in my code. It is working really well, without any issues. I just wanted to see if I can somehow improve my code. I divided the major things into separate function.

Here is a working demo.

(function($) {
    // Use _ to template the resultsTemplating details
    function resultsTemplating(data, inputValue) {
        var results = data,
            resultsTemplate = _.template($("#results-template").html()),
            resultingHtml = resultsTemplate({
                results : results,
                searchVal : inputValue,
                amount: results.length
            });

        // place the generated data into the html
        $("#results-container").html(resultingHtml);
    }
    // Use _ to template the overlay details
    function overlayTemplating(data, id) {
         // loop through JSON and match clicked element, then template   
         for(var i = 0; i < data.length; i++) {
            if(data[i].created == id) {
                var overlayTemplate = _.template($("#overlay-template").html()),
                    overlayHtml = overlayTemplate({
                        name : data[i].name,
                        username : data[i].username,
                        language: data[i].language,
                        url: data[i].url,
                        description: data[i].description
                    }); 


            }
        }
        // place the generated data into the html
        $("#overlay-container").html(overlayHtml);
    }
    // Grab Deatils of clicked node, and template it
    function repoDetails(data, id) {
        var container = $('#overlay-container');
        container.fadeIn('fast');

        overlayTemplating(data, id);

        // Closes the overlay
        container.find('.close').on('click', function() {
            container.fadeOut('fast');
            return false;
        });


    }
    // Scroll Back to the top of the page
    function backToTop() {
        $('html, body').animate({
            scrollTop: 0
        }, 'fast');
    }
    function searchGit() {
        //grab value of search field
        var search = $('#search').val();

        // Validates entered value
        if (search.length) {
            $(this).find('.error').hide();
            backToTop();

            //pull json data from url
            $.ajax({
                url: 'https://api.github.com/legacy/repos/search/' + search,
                dataType: 'json',
                cache: true,
                success: function(data) {

                    var results = data.repositories;

                    $('body').addClass('post');
                    $('#results-container').show();

                    // use the results to template the results html using _
                    resultsTemplating(results, search);

                    $('.viewDeatails').on('click', function(e) {
                        var id = $(this).attr('href');

                        // use the results to template the repo details html using _
                        repoDetails(results, id);

                        e.preventDefault();
                    });
                }
            });

            // Back to Home
            $('.logo').on('click', function() {
               $('body').removeClass('post'); 
               $('#results-container').hide();
            });
        } else {
            // Show error if search field is empty
            $(this).find('.error').fadeIn();
        }

        return false;
    }

   $(function() {
    if($('body').hasClass('js')) { 

        // ANIMATIONS
        $('#search').focus();

        $('.logo object').addClass('scaleInOut').one('webkitAnimationEnd mozAnimationEnd MSAnimationEnd oanimationend animationend', function() {
            $(this).animate({
                left: 0,
                marginLeft: 0},
                400, function() {
                 $(this).next().addClass('fadeInDown').one('webkitAnimationEnd mozAnimationEnd MSAnimationEnd oanimationend animationend', function() {
                    $(this).css({
                        opacity : 1
                    });
                      $('form').addClass('fadeInUp').css({opacity : 1});
                      setTimeout(function() { $('.footer').fadeIn(); }, 400);
                 });
            });
        });  

        // Search Event
        $('#searchForm').on('submit', searchGit);
    }
   });
})(jQuery);
\$\endgroup\$
  • \$\begingroup\$ I expected the listed results to be links... it is boring to copy-paste the url in the browser tab \$\endgroup\$ – Caridorc May 4 '15 at 19:38
3
\$\begingroup\$

A small review, but your naming seems a bit repetitive.

This:

// Use _ to template the resultsTemplating details
function resultsTemplating(data, inputValue) {
    var results = data,
        resultsTemplate = _.template($("#results-template").html()),
        resultingHtml = resultsTemplate({
            results : results,
            searchVal : inputValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(resultingHtml);
}

has results and resulting all over the place, I would go for something like this:

// Use _ to template the resultsTemplating details
function templateResults(results, searchValue) {
    var template = _.template($("#results-template").html()),
        html = resultsTemplate({
            results : results,
            searchVal : searchValue,
            amount: results.length
        });

    // place the generated data into the html
    $("#results-container").html(html);
}

I also removed the move from data to results to save a line and renamed inputValue to searchValue. I noticed that you have inputValue and searchVal, in my mind you want to be consistent and have inputValue and searchValue or inputVal and searchVal. I much prefer the fully spelled out name.

This bit is funny:

                overlayHtml = overlayTemplate({
                    name : data[i].name,
                    username : data[i].username,
                    language: data[i].language,
                    url: data[i].url,
                    description: data[i].description
                }); 

You are creating an object with the exact same properties which already exist in data[i]. Unless you have unwanted fields in data[i] that you really dont want in your template, you can simply

                overlayHtml = overlayTemplate(data[i]);

All in all, this is really great code for someone without formal training. I like the size of your functions, the level of commenting and how easy the code is to follow.

\$\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.