Skip to main content
added 10 characters in body
Source Link
VLS
  • 163
  • 2
  • 8

I second user1777136's suggestion of not appending to the DOM in a loop.

Additional suggestions:

  • Get rid of your setTimeout function. Instead, create a $(document).ready handler and prepend your instructions to the body.

  • Change your keypress handler to $('#search').change, which would also go inside $(document).ready. Perform input validation inside the handler to simplify your gitSearch function:

      // Get the value from the search field
      $('#search').change(function(e) {
          if (!$(this).val()) {
              alert("Please enter a term to search for");
          } else {
              gitSearch($(this).val());
          }
      });
    
  • Instead of having individual click listeners defined inside the each loop, store the data for each element and retrieve it with a delegated listener in $(document).ready:

      $('#results').on('click', 'li', function(event) {
           alert("Repo Language is : " + $(this).data('language') + "\n" +
               "There are " + $(this).data('followers') + " followers of this repo.\n" +
               "The URL of the repo is : " + $(this).data('url') + "\n" +
               "The description of the repo : " + $(this).data('description'));
       });
    
  • Keep one string containing all your CSS, including the styles for your li's; you don't need inline CSS for these and this will remove the need for hover listeners. So in addition to your body styles you'd have

      styles += "#results li { cursor: pointer; list-style-type: none; color: #fff; }";
      styles += "#results li:hover { color: #000; }";
    

I second user1777136's suggestion of not appending to the DOM in a loop.

Additional suggestions:

  • Get rid of your setTimeout function. Instead, create a $(document).ready handler and prepend your instructions to the body.

  • Change your keypress handler to $('#search').change, which would also go inside $(document).ready. Perform input validation inside the handler to simplify your gitSearch function:

      // Get the value from the search field
      $('#search').change(function(e) {
          if (!$(this).val()) {
              alert("Please enter a term to search for");
          } else {
              gitSearch($(this).val());
          }
      });
    
  • Instead of having individual click listeners defined inside the each loop, store the data for each element and retrieve it with a delegated listener in $(document).ready:

      $('#results').on('click', 'li', function(event) {
           alert("Repo Language is : " + this.data('language') + "\n" +
               "There are " + this.data('followers') + " followers of this repo.\n" +
               "The URL of the repo is : " + this.data('url') + "\n" +
               "The description of the repo : " + this.data('description'));
       });
    
  • Keep one string containing all your CSS, including the styles for your li's; you don't need inline CSS for these and this will remove the need for hover listeners. So in addition to your body styles you'd have

      styles += "#results li { cursor: pointer; list-style-type: none; color: #fff; }";
      styles += "#results li:hover { color: #000; }";
    

I second user1777136's suggestion of not appending to the DOM in a loop.

Additional suggestions:

  • Get rid of your setTimeout function. Instead, create a $(document).ready handler and prepend your instructions to the body.

  • Change your keypress handler to $('#search').change, which would also go inside $(document).ready. Perform input validation inside the handler to simplify your gitSearch function:

      // Get the value from the search field
      $('#search').change(function(e) {
          if (!$(this).val()) {
              alert("Please enter a term to search for");
          } else {
              gitSearch($(this).val());
          }
      });
    
  • Instead of having individual click listeners defined inside the each loop, store the data for each element and retrieve it with a delegated listener in $(document).ready:

      $('#results').on('click', 'li', function(event) {
           alert("Repo Language is : " + $(this).data('language') + "\n" +
               "There are " + $(this).data('followers') + " followers of this repo.\n" +
               "The URL of the repo is : " + $(this).data('url') + "\n" +
               "The description of the repo : " + $(this).data('description'));
       });
    
  • Keep one string containing all your CSS, including the styles for your li's; you don't need inline CSS for these and this will remove the need for hover listeners. So in addition to your body styles you'd have

      styles += "#results li { cursor: pointer; list-style-type: none; color: #fff; }";
      styles += "#results li:hover { color: #000; }";
    
Source Link
VLS
  • 163
  • 2
  • 8

I second user1777136's suggestion of not appending to the DOM in a loop.

Additional suggestions:

  • Get rid of your setTimeout function. Instead, create a $(document).ready handler and prepend your instructions to the body.

  • Change your keypress handler to $('#search').change, which would also go inside $(document).ready. Perform input validation inside the handler to simplify your gitSearch function:

      // Get the value from the search field
      $('#search').change(function(e) {
          if (!$(this).val()) {
              alert("Please enter a term to search for");
          } else {
              gitSearch($(this).val());
          }
      });
    
  • Instead of having individual click listeners defined inside the each loop, store the data for each element and retrieve it with a delegated listener in $(document).ready:

      $('#results').on('click', 'li', function(event) {
           alert("Repo Language is : " + this.data('language') + "\n" +
               "There are " + this.data('followers') + " followers of this repo.\n" +
               "The URL of the repo is : " + this.data('url') + "\n" +
               "The description of the repo : " + this.data('description'));
       });
    
  • Keep one string containing all your CSS, including the styles for your li's; you don't need inline CSS for these and this will remove the need for hover listeners. So in addition to your body styles you'd have

      styles += "#results li { cursor: pointer; list-style-type: none; color: #fff; }";
      styles += "#results li:hover { color: #000; }";