Generate uniform <h1> headings for all modules

In #223, we began generating default `<h1>` headings for modules. However, these headings were only generated for modules that had a description. If a module had no comment to begin with, it would not get a generated heading. Furthermore, since #280, each module's full name has been prominently displayed at the top of its page. These names are visually redundant with generated headings, which also use the full name. To unify the design and ensure that all modules have an `<h1>`, this commit converts each module's full name into an `<h1>` heading inside an `<hgroup>`, and uses postprocessing to pull any comment-based `<h1>` into the `<hgroup>`. This commit also renames the "Included Modules" section to "Inherits From", and moves any listed base class from the top of the page to the top of that section. This change focuses the `<h1>`, both visually and from an SEO perspective.
rails · Aug 20, 2023 · cf5b68e · cf5b68e
1 parent 947abf9
commit cf5b68e
Show file tree

Hide file tree

Showing 10 changed files with 157 additions and 88 deletions.
diff --git a/lib/rdoc/generator/template/rails/_context.rhtml b/lib/rdoc/generator/template/rails/_context.rhtml
@@ -1,16 +1,13 @@
 <div id="context">
   <% unless (description = context.description).empty? %>
-    <% unless context.comment_title %>
-      <h1><%= context.title %></h1>
-    <% end %>
     <div class="description">
       <%= description %>
     </div>
   <% end %>
 
 
+  <%# File only: requires %>
   <% unless context.requires.empty? %>
-    <!-- File only: requires -->
     <div class="content__divider">Required Files</div>
     <ul>
       <% context.requires.each do |req| %>
@@ -20,6 +17,20 @@
   <% end %>
 
 
+  <%# Module only: ancestors %>
+  <% unless context.is_a?(RDoc::TopLevel) || (ancestors = module_ancestors(context)).empty? %>
+    <div class="content__divider">Inherits From</div>
+    <ul>
+      <% ancestors.each do |kind, ancestor| %>
+        <li>
+          <span class="kind"><%= kind %></span>
+          <%= ancestor.is_a?(String) ? full_name(ancestor) : link_to(ancestor) %>
+        </li>
+      <% end %>
+    </ul>
+  <% end %>
+
+
   <% sections = context.sections.select { |s| s.title }.sort_by{ |s| s.title.to_s } %>
   <% unless sections.empty? then %>
     <!-- Sections -->
@@ -51,22 +62,6 @@
     </dl>
   <% end %>
 
-  <% unless context.includes.empty? %>
-    <!-- Includes -->
-    <div class="content__divider">Included Modules</div>
-    <ul>
-      <% context.includes.each do |inc| %>
-        <li>
-          <% if inc.module.is_a?(String) %>
-            <%= full_name inc.name %>
-          <% else %>
-            <%= link_to inc.module %>
-          <% end %>
-        </li>
-      <% end %>
-    </ul>
-  <% end %>
-
 
 
   <% context.each_section do |section, constants, attributes| %>
@@ -185,7 +180,7 @@
     <ul>
       <% (context.modules.sort + context.classes.sort).each do |mod| %>
         <li>
-          <span class="type"><%= mod.type.upcase %></span>
+          <span class="kind"><%= mod.type %></span>
           <%= link_to mod %>
         </li>
       <% end %>

diff --git a/lib/rdoc/generator/template/rails/class.rhtml b/lib/rdoc/generator/template/rails/class.rhtml
@@ -19,14 +19,9 @@
     <%= include_template '_panel.rhtml' %>
 
     <main id="content">
-        <div class="content__full-name">
-          <span class="qualifier"><%= klass.type %></span>
-          <%= module_breadcrumbs klass %>
-          <% if !klass.module? && superclass = klass.superclass %>
-            <span class="qualifier">&lt;</span>
-            <%= superclass.is_a?(String) ? full_name(superclass) : link_to(superclass) %>
-          <% end %>
-        </div>
+        <hgroup class="content__title">
+          <h1><span class="kind"><%= klass.type %></span> <%= module_breadcrumbs klass %></h1>
+        </hgroup>
 
         <%= include_template '_context.rhtml', {:context => klass} %>
 

diff --git a/lib/rdoc/generator/template/rails/file.rhtml b/lib/rdoc/generator/template/rails/file.rhtml
@@ -16,10 +16,9 @@
     <%= include_template '_panel.rhtml' %>
 
     <main id="content">
-        <h1 class="content__full-name">
-          <span class="qualifier">File</span>
-          <%= full_name file %>
-        </h1>
+        <hgroup class="content__title">
+          <h1><span class="kind">File</span> <%= full_name file %></h1>
+        </hgroup>
 
         <% if source_url = github_url(file.relative_name) %>
           <p><%= link_to_external "View on GitHub", source_url %></p>

diff --git a/lib/rdoc/generator/template/rails/resources/css/main.css b/lib/rdoc/generator/template/rails/resources/css/main.css
@@ -55,9 +55,6 @@ a {
 a:has(> code:only-child) {
   text-decoration: none;
 }
-code a {
-  text-decoration: none;
-}
 
 /* TODO: Remove this hack when Firefox supports `:has()` */
 a code {
@@ -74,6 +71,15 @@ a code {
   display: none;
 }
 
+.kind {
+  font-family: monospace;
+  font-weight: bold;
+}
+
+.kind::after {
+  content: " "; /* Ensure trailing space has width of 1 monospace char */
+}
+
 table {
   border-collapse: collapse;
 }
@@ -380,18 +386,22 @@ html {
   font-style: normal;
 }
 
-.content__full-name {
-  font-family: monospace;
-  font-size: 1.4em;
+.content__title :is(h1, p) {
+  font-size: 1.6em;
   line-height: 1.25;
-  padding: 0.125em 0;
+}
+
+.content__title h1 {
+  font-weight: normal;
 
   margin-left: 1em;
   text-indent: -1em;
 }
 
-.content__full-name .qualifier {
-  font-weight: bold;
+.content__title p {
+  font-style: italic;
+
+  margin-top: 0;
 }
 
 .content__section-title {
@@ -513,6 +523,10 @@ html {
  * Description of method or module
  */
 
+#context > .description {
+  margin-top: var(--space-lg);
+}
+
 .description :is(h1, h2, h3, h4, h5, h6) {
   line-height: 1.25;
   padding: 0.125em 0;

diff --git a/lib/sdoc/generator.rb b/lib/sdoc/generator.rb
@@ -27,16 +27,6 @@ class RDoc::Options
   attr_accessor :search_index
 end
 
-module RDoc::Generator::Markup
-  def comment_title
-    @comment_title ||= @comment.to_s.match(/\A[=#] (.*)$/) {|match| match[1] }
-  end
-
-  def title
-    comment_title || full_name
-  end
-end
-
 class RDoc::Generator::SDoc
   RDoc::RDoc.add_generator self
 

diff --git a/lib/sdoc/helpers.rb b/lib/sdoc/helpers.rb
@@ -127,6 +127,17 @@ def module_breadcrumbs(rdoc_module)
     "<code>#{crumbs.join("::<wbr>")}</code>"
   end
 
+  def module_ancestors(rdoc_module)
+    ancestors = rdoc_module.includes.map { |inc| ["module", inc.module] }
+
+    if !rdoc_module.module? && superclass = rdoc_module.superclass
+      superclass_name = superclass.is_a?(String) ? superclass : superclass.full_name
+      ancestors.unshift(["class", superclass]) unless superclass_name == "Object"
+    end
+
+    ancestors
+  end
+
   def method_signature(rdoc_method)
     if rdoc_method.call_seq
       rdoc_method.call_seq.split(/\n+/).map do |line|

diff --git a/lib/sdoc/postprocessor.rb b/lib/sdoc/postprocessor.rb
@@ -13,6 +13,7 @@ def process(rendered)
     version_rails_guides_urls!(document)
     unlink_unintentional_ref_links!(document)
     style_ref_links!(document)
+    unify_h1_headings!(document)
     highlight_code_blocks!(document)
 
     document.to_s
@@ -88,6 +89,15 @@ def style_ref_links!(document)
     end
   end
 
+  def unify_h1_headings!(document)
+    if h1 = document.at_css("#context > .description h1:first-child")
+      if hgroup = document.at_css("#content > hgroup")
+        h1.remove
+        hgroup.add_child(%(<p>#{h1.inner_html}</p>))
+      end
+    end
+  end
+
   def highlight_code_blocks!(document)
     document.css(".description pre > code, .method__source pre > code").each do |element|
       code = element.inner_text

diff --git a/spec/helpers_spec.rb b/spec/helpers_spec.rb
@@ -523,6 +523,44 @@ module Foo; module Bar; module Qux; end; end; end
     end
   end
 
+  describe "#module_ancestors" do
+    it "returns a list with the base class (if applicable) and included modules" do
+      # RDoc chokes on ";" when parsing includes, so replace with "\n".
+      top_level = rdoc_top_level_for <<~RUBY.gsub(";", "\n")
+        module M1; end
+        module M2; end
+        class C1; end
+
+        module Foo; include M1; include M2; end
+        class Bar < C1; include M2; include M1; end
+        class Qux < Cx; include Foo; include Mx; end
+      RUBY
+
+      m1, m2, c1, foo, bar, qux = %w[M1 M2 C1 Foo Bar Qux].map { |name| top_level.find_module_named(name) }
+
+      _(@helpers.module_ancestors(foo)).must_equal [["module", m1], ["module", m2]]
+      _(@helpers.module_ancestors(bar)).must_equal [["class", c1], ["module", m2], ["module", m1]]
+      _(@helpers.module_ancestors(qux)).must_equal [["class", "Cx"], ["module", foo], ["module", "Mx"]]
+    end
+
+    it "excludes the default base class (Object) from the result" do
+      # RDoc chokes on ";" when parsing includes, so replace with "\n".
+      top_level = rdoc_top_level_for <<~RUBY.gsub(";", "\n")
+        class Object; end
+        class Foo; include M1; end
+      RUBY
+
+      _(@helpers.module_ancestors(top_level.find_module_named("Object"))).must_equal [["class", "BasicObject"]]
+      _(@helpers.module_ancestors(top_level.find_module_named("Foo"))).must_equal [["module", "M1"]]
+
+      top_level = rdoc_top_level_for <<~RUBY.gsub(";", "\n")
+        class Foo; include M1; end
+      RUBY
+
+      _(@helpers.module_ancestors(top_level.find_module_named("Foo"))).must_equal [["module", "M1"]]
+    end
+  end
+
   describe "#method_signature" do
     it "returns the method signature wrapped in <code>" do
       method = rdoc_top_level_for(<<~RUBY).find_module_named("Foo").find_method("bar", false)

diff --git a/spec/postprocessor_spec.rb b/spec/postprocessor_spec.rb
@@ -124,6 +124,59 @@
       _(SDoc::Postprocessor.process(rendered)).must_include expected
     end
 
+    it "unifies <h1> headings for a context" do
+      rendered = <<~HTML
+        <div id="content">
+          <hgroup><h1>module Foo</h1></hgroup>
+
+          <div id="context">
+            <div class="description"><h1>The Foo</h1><p>Lorem ipsum.</p></div>
+          </div>
+        </div>
+      HTML
+
+      expected = <<~HTML
+        <div id="content">
+          <hgroup><h1>module Foo</h1><p>The Foo</p></hgroup>
+
+          <div id="context">
+            <div class="description"><p>Lorem ipsum.</p></div>
+          </div>
+        </div>
+      HTML
+
+      _(SDoc::Postprocessor.process(rendered)).must_include expected
+    end
+
+    it "does not relocate non-leading <h1> headings" do
+      rendered = <<~HTML
+        <div id="content">
+          <hgroup><h1>module Foo</h1></hgroup>
+
+          <div id="context">
+            <div class="description"><p>Lorem ipsum.</p><h1>Red Herring</h1></div>
+            <div class="method">
+              <div class="description"><h1>Red Herring</h1></div>
+            </div>
+          </div>
+        </div>
+      HTML
+
+      _(SDoc::Postprocessor.process(rendered)).must_include rendered
+    end
+
+    it "does not relocate <h1> headings when <hgroup> is not present" do
+      rendered = <<~HTML
+        <div id="content">
+          <div id="context">
+            <div class="description"><h1>Main Page</h1></div>
+          </div>
+        </div>
+      HTML
+
+      _(SDoc::Postprocessor.process(rendered)).must_include rendered
+    end
+
     it "highlights code blocks" do
       rendered = <<~HTML
         <div class="description">

diff --git a/spec/rdoc_generator_markup_spec.rb b/spec/rdoc_generator_markup_spec.rb