Merge pull request #252 from jonathanhefner/toggle-source-via-details

Replace source code toggler JavaScript with `<details>`

lib/rdoc/generator/template/rails/_context.rhtml

@@ -159,32 +159,25 @@
           <% end %>
 
           <% source_code, source_url = method_source_code_and_url(method) %>
-          <% if source_code || source_url %>
-            <div class="sourcecode">
-              <p class="source-link">
-                Source:
-
-                <% if source_code %>
-                  <a href="javascript:toggleSource('<%= method.aref %>_source')" id="l_<%= method.aref %>_source">show</a>
-                <% end %>
-
-                <% if source_code && source_url %> | <% end %>
-
+          <% if source_code %>
+            <details class="sourcecode">
+              <summary>
+                <span class="label">Source code</span>
                 <% if source_url %>
-                  <a href="<%= source_url %>" target="_blank" class="github_url">on GitHub</a>
+                  <%= link_to_external "GitHub", source_url %>
                 <% end %>
-              </p>
-
-              <% if source_code %>
-                <div id="<%= method.aref %>_source" class="dyn-source">
-                  <pre><code class="ruby"><%= source_code %></code></pre>
-                </div>
-              <% end %>
-            </div>
-            <% end %>
-          </div>
-        <% end %><%# methods.each %>
-      <% end %><%# visibilities.each %>
+              </summary>
+
+              <div class="dyn-source">
+                <pre><code class="ruby"><%= source_code %></code></pre>
+              </div>
+            </details>
+          <% elsif source_url %>
+            <p><%= link_to_external "GitHub", source_url %></p>
+          <% end %>
+        </div>
+      <% end %><%# methods.each %>
+    <% end %><%# visibilities.each %>
     <% end %><%# context.methods_by_type %>
   <% end %><%# context.each_section %>
 

lib/rdoc/generator/template/rails/resources/css/main.css

@@ -12,6 +12,10 @@ body {
   display: grid;
   grid-template-rows: min-content min-content auto min-content;
   grid-template-columns: 300px auto;
+
+  --link-color: #CC0000;
+  --link-hover-color: #990000;
+  --icon-color: #777777;
 }
 
 body {
@@ -61,12 +65,12 @@ body {
 }
 
 a:link, a:active, a:visited, a:hover {
-  color: #CC0000;
+  color: var(--link-color);
   text-decoration: none;
 }
 
 a:hover {
-  color: #990000;
+  color: var(--link-hover-color);
   text-decoration: underline;
 }
 
@@ -78,6 +82,13 @@ h1 a, h2 a, .banner a {
   color: #fff;
 }
 
+.external-link {
+  padding-left: 1.3em;
+  background-image: url('../i/external-link.svg');
+  background-repeat: no-repeat;
+  background-size: 1.1em;
+}
+
 p {
   margin-bottom: 1em;
 }
@@ -290,7 +301,6 @@ tt {
 }
 
 .dyn-source {
-  display: none;
   background: #fffde8;
   color: #000;
   border: #ffe0bb dotted 1px;
@@ -343,19 +353,40 @@ tt {
   top: 0;
 }
 
-.method .sourcecode p.source-link {
-  text-indent: 0em;
-  margin-top: 0.5em;
-}
-
 .method .aka {
   margin-left: 1.2em;
   font-style: italic;
 }
 
-.method .source-link
-{
-    font-size: 0.85em;
+.sourcecode {
+  width: fit-content; /* Reduce clickable area when collapsed */
+}
+
+.sourcecode[open] {
+  width: auto;
+}
+
+.sourcecode summary {
+  cursor: pointer;
+}
+
+.sourcecode summary::marker {
+  font-size: 1.25em;
+  color: var(--icon-color);
+}
+
+.sourcecode summary .label {
+  margin-left: -0.4em;
+  font-weight: bold;
+  color: var(--link-color);
+}
+
+.sourcecode summary .label:hover {
+  color: var(--link-hover-color);
+}
+
+.sourcecode summary .external-link {
+  margin-left: 1em;
 }
 
 @keyframes spotlight {

lib/rdoc/generator/template/rails/resources/js/main.js

@@ -1,9 +1,3 @@
-function toggleSource(id) {
-  var src = $('#' + id).toggle();
-  var isVisible = src.is(':visible');
-  $('#l_' + id).html(isVisible ? 'hide' : 'show');
-}
-
 window.spotlight = function(url) {
   var hash = url.match(/#([^#]+)$/);
   if (hash) {

lib/sdoc/helpers.rb

@@ -33,6 +33,14 @@ def link_to(text, url, html_attributes = {})
     %(<a href="#{h url}"#{attribute_string}>#{h text}</a>)
   end
 
+  def link_to_external(text, url, html_attributes = {})
+    html_attributes = html_attributes.transform_keys(&:to_s)
+    html_attributes = { "target" => "_blank", "class" => nil }.merge(html_attributes)
+    html_attributes["class"] = [*html_attributes["class"], "external-link"].join(" ")
+
+    link_to(text, url, html_attributes)
+  end
+
   def base_tag_for_context(context)
     if context == :index
       %(<base href="./" data-current-path=".">)

spec/helpers_spec.rb

@@ -135,6 +135,28 @@ module Foo; class Bar; def qux; end; end; end
     end
   end
 
+  describe "#link_to_external" do
+    it "sets class='external-link' and target='_blank' by default" do
+      _(@helpers.link_to_external("foo", "bar")).
+        must_equal %(<a href="bar" target="_blank" class="external-link">foo</a>)
+    end
+
+    it "supports additional classes" do
+      _(@helpers.link_to_external("foo", "bar", class: "qux")).
+        must_equal %(<a href="bar" target="_blank" class="qux external-link">foo</a>)
+    end
+
+    it "supports overriding target" do
+      _(@helpers.link_to_external("foo", "bar", target: "_self")).
+        must_equal %(<a href="bar" target="_self" class="external-link">foo</a>)
+    end
+
+    it "supports additional attributes" do
+      _(@helpers.link_to_external("foo", "bar", "data-hoge": "fuga")).
+        must_equal %(<a href="bar" target="_blank" class="external-link" data-hoge="fuga">foo</a>)
+    end
+  end
+
   describe "#base_tag_for_context" do
     it "returns an idempotent <base> tag for the :index context" do
       _(@helpers.base_tag_for_context(:index)).