Improve js written for guides

* Stop using unnecessary global variable * Use function defined by jQuery for simplicity * Fix event listner for "more info button" to handle event on resize browser
rails · Jun 24, 2013 · 501e663 · 501e663
1 parent f6cdc22
commit 501e663
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 36 deletions.
diff --git a/guides/assets/javascripts/guides.js b/guides/assets/javascripts/guides.js
@@ -1,57 +1,53 @@
-function guideMenu(){
-  if (document.getElementById('guides').style.display == "none") {
-    document.getElementById('guides').style.display = "block";
-  } else {
-    document.getElementById('guides').style.display = "none";
-  }
-}
-
-$.fn.selectGuide = function(guide){
+$.fn.selectGuide = function(guide) {
   $("select", this).val(guide);
-}
+};
 
-guidesIndex = {
-  bind: function(){
+var guidesIndex = {
+  bind: function() {
     var currentGuidePath = window.location.pathname;
     var currentGuide = currentGuidePath.substring(currentGuidePath.lastIndexOf("/")+1);
     $(".guides-index-small").
       on("change", "select", guidesIndex.navigate).
       selectGuide(currentGuide);
-    $(".more-info-button:visible").click(function(e){
+    $(document).on("click", ".more-info-button", function(e){
       e.stopPropagation();
-      if($(".more-info-links").is(":visible")){
+      if ($(".more-info-links").is(":visible")) {
         $(".more-info-links").addClass("s-hidden").unwrap();
       } else {
         $(".more-info-links").wrap("<div class='more-info-container'></div>").removeClass("s-hidden");
       }
-      $(document).on("click", function(e){
-        var $button = $(".more-info-button");
-        var element;
+    });
+    $("#guidesMenu").on("click", function(e) {
+      $("#guides").toggle();
+      return false;
+    });
+    $(document).on("click", function(e){
+      e.stopPropagation();
+      var $button = $(".more-info-button");
+      var element;
 
-        // Cross browser find the element that had the event
-        if (e.target) element = e.target;
-        else if (e.srcElement) element = e.srcElement;
+      // Cross browser find the element that had the event
+      if (e.target) element = e.target;
+      else if (e.srcElement) element = e.srcElement;
 
-        // Defeat the older Safari bug:
-        // http://www.quirksmode.org/js/events_properties.html
-        if (element.nodeType == 3) element = element.parentNode;
+      // Defeat the older Safari bug:
+      // http://www.quirksmode.org/js/events_properties.html
+      if (element.nodeType === 3) element = element.parentNode;
 
-        var $element = $(element);
+      var $element = $(element);
 
-        var $container = $element.parents(".more-info-container");
+      var $container = $element.parents(".more-info-container");
 
-        // We've captured a click outside the popup
-        if($container.length == 0){
-          $container = $button.next(".more-info-container");
-          $container.find(".more-info-links").addClass("s-hidden").unwrap();
-          $(document).off("click");
-        }
-      });
+      // We've captured a click outside the popup
+      if($container.length === 0){
+        $container = $button.next(".more-info-container");
+        $container.find(".more-info-links").addClass("s-hidden").unwrap();
+      }
     });
   },
   navigate: function(e){
     var $list = $(e.target);
-    url = $list.val();
+    var url = $list.val();
     window.location = url;
   }
-}
+};
diff --git a/guides/source/layout.html.erb b/guides/source/layout.html.erb
@@ -48,7 +48,7 @@
       <ul class="nav">
         <li><a class="nav-item" href="index.html">Home</a></li>
         <li class="guides-index guides-index-large">
-          <a href="index.html" onclick="guideMenu(); return false;" id="guidesMenu" class="guides-index-item nav-item">Guides Index</a>
+          <a href="index.html" id="guidesMenu" class="guides-index-item nav-item">Guides Index</a>
           <div id="guides" class="clearfix" style="display: none;">
             <hr />
             <% ['L', 'R'].each do |position| %>
@@ -141,7 +141,7 @@
   <script type="text/javascript" src="javascripts/syntaxhighlighter/shBrushSql.js"></script>
   <script type="text/javascript" src="javascripts/syntaxhighlighter/shBrushPlain.js"></script>
   <script type="text/javascript">
-    SyntaxHighlighter.all()
+    SyntaxHighlighter.all();
     $(guidesIndex.bind);
   </script>
 </body>