Extract component to render a list of links

This way it's easier to refactor it and/or change it.

Back in commit c156621a4 I wrote:

> Generally speaking, I'm not a big fan of helpers, but there are
> methods which IMHO qualify as helpers when (...) many Rails helpers,
> like `tag`, follow these principles.

It's time to modify these criteria a little bit. In some situations,
it's great to have a helper method so it can be easily used in view
(like `link_to`). However, from the maintenance point of view, helper
methods are usually messy because extracting methods requires making
sure there isn't another helper method with that name.

So we can use the best part of these worlds and provide a helper so it
can be easily called from the view, but internally make that helper
render a component and enjoy the advantages associated with using an
isolated Ruby class.

app/components/shared/link_list_component.rb

@@ -0,0 +1,26 @@
+class Shared::LinkListComponent < ApplicationComponent
+  attr_reader :links, :options
+
+  def initialize(*links, **options)
+    @links = links
+    @options = options
+  end
+
+  def render?
+    links.select(&:present?).any?
+  end
+
+  def call
+    tag.ul(options) do
+      safe_join(links.select(&:present?).map do |text, url, current = false, **link_options|
+        tag.li(({ "aria-current": true } if current)) do
+          if url
+            link_to text, url, link_options
+          else
+            text
+          end
+        end
+      end, "\n")
+    end
+  end
+end

app/helpers/link_list_helper.rb

@@ -1,17 +1,5 @@
 module LinkListHelper
   def link_list(*links, **options)
-    return "" if links.select(&:present?).empty?
+    render Shared::LinkListComponent.new(*links, **options)
-
-    tag.ul(options) do
-      safe_join(links.select(&:present?).map do |text, url, current = false, **link_options|
-        tag.li(({ "aria-current": true } if current)) do
-          if url
-            link_to text, url, link_options
-          else
-            text
-          end
-        end
-      end, "\n")
-    end
   end
 end

spec/components/shared/link_list_component_spec.rb

@@ -0,0 +1,93 @@
+require "rails_helper"
+
+describe Shared::LinkListComponent, type: :component do
+  it "renders nothing with an empty list" do
+    render_inline Shared::LinkListComponent.new
+
+    expect(page).not_to have_css "ul"
+  end
+
+  it "returns nothing with a list of nil elements" do
+    render_inline Shared::LinkListComponent.new(nil, nil)
+
+    expect(page).not_to have_css "ul"
+  end
+
+  it "generates a list of links" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/"], ["Info", "/info"], class: "menu"
+    )
+    list = page.find("body").native.inner_html
+
+    expect(list).to eq '<ul class="menu">' + "\n" +
+      '<li><a href="/">Home</a></li>' + "\n" +
+      '<li><a href="/info">Info</a></li>' + "\n</ul>"
+  end
+
+  it "accepts anchor tags" do
+    render_inline Shared::LinkListComponent.new(
+      '<a href="/">Home</a>'.html_safe, ["Info", "/info"], class: "menu"
+    )
+    list = page.find("body").native.inner_html
+
+    expect(list).to eq '<ul class="menu">' + "\n" +
+      '<li><a href="/">Home</a></li>' + "\n" +
+      '<li><a href="/info">Info</a></li>' + "\n</ul>"
+  end
+
+  it "accepts options for links" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/", class: "root"], ["Info", "/info", id: "info"]
+    )
+
+    expect(page).to have_css "a", count: 2
+    expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+    expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+  end
+
+  it "ignores nil entries" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/", class: "root"], nil, ["Info", "/info", id: "info"]
+    )
+
+    expect(page).to have_css "li", count: 2
+    expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+    expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+  end
+
+  it "ignores empty entries" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/", class: "root"], "", ["Info", "/info", id: "info"]
+    )
+
+    expect(page).to have_css "li", count: 2
+    expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+    expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+  end
+
+  it "accepts an optional condition to check the active element" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/", false],
+      ["Info", "/info", true],
+      ["Help", "/help"]
+    )
+
+    expect(page).to have_css "li", count: 3
+    expect(page).to have_css "li[aria-current='true']", count: 1, exact_text: "Info"
+  end
+
+  it "allows passing both the active condition and link options" do
+    render_inline Shared::LinkListComponent.new(
+      ["Home", "/", false, class: "root"],
+      ["Info", "/info", true, id: "info"],
+      ["Help", "/help", rel: "help"]
+    )
+
+    expect(page).to have_css "li", count: 3
+    expect(page).to have_css "li[aria-current='true']", count: 1, exact_text: "Info"
+
+    expect(page).to have_css "a.root", count: 1, exact_text: "Home"
+    expect(page).to have_css "a#info", count: 1, exact_text: "Info"
+    expect(page).to have_css "a[rel='help']", count: 1, exact_text: "Help"
+  end
+end

spec/helpers/link_list_helper_spec.rb

@@ -1,91 +0,0 @@
-require "rails_helper"
-
-describe LinkListHelper do
-  describe "#link_list" do
-    it "returns an empty string with an empty list" do
-      expect(helper.link_list).to eq ""
-    end
-
-    it "returns nothing with a list of nil elements" do
-      expect(helper.link_list(nil, nil)).to eq ""
-    end
-
-    it "generates a list of links" do
-      list = helper.link_list(["Home", "/"], ["Info", "/info"], class: "menu")
-
-      expect(list).to eq '<ul class="menu">' +
-        '<li><a href="/">Home</a></li>' + "\n" +
-        '<li><a href="/info">Info</a></li></ul>'
-      expect(list).to be_html_safe
-    end
-
-    it "accepts anchor tags" do
-      list = helper.link_list(link_to("Home", "/"), ["Info", "/info"], class: "menu")
-
-      expect(list).to eq '<ul class="menu">' +
-        '<li><a href="/">Home</a></li>' + "\n" +
-        '<li><a href="/info">Info</a></li></ul>'
-      expect(list).to be_html_safe
-    end
-
-    it "accepts options for links" do
-      render helper.link_list(["Home", "/", class: "root"], ["Info", "/info", id: "info"])
-
-      expect(page).to have_css "a", count: 2
-      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
-      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
-    end
-
-    it "ignores nil entries" do
-      render helper.link_list(["Home", "/", class: "root"], nil, ["Info", "/info", id: "info"])
-
-      expect(page).to have_css "li", count: 2
-      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
-      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
-    end
-
-    it "ignores empty entries" do
-      render helper.link_list(["Home", "/", class: "root"], "", ["Info", "/info", id: "info"])
-
-      expect(page).to have_css "li", count: 2
-      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
-      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
-    end
-
-    it "accepts an optional condition to check the active element" do
-      render helper.link_list(
-        ["Home", "/", false],
-        ["Info", "/info", true],
-        ["Help", "/help"]
-      )
-
-      expect(page).to have_css "li", count: 3
-      expect(page).to have_css "li[aria-current='true']", count: 1, exact_text: "Info"
-    end
-
-    it "allows passing both the active condition and link options" do
-      render helper.link_list(
-        ["Home", "/", false, class: "root"],
-        ["Info", "/info", true, id: "info"],
-        ["Help", "/help", rel: "help"]
-      )
-
-      expect(page).to have_css "li", count: 3
-      expect(page).to have_css "li[aria-current='true']", count: 1, exact_text: "Info"
-
-      expect(page).to have_css "a.root", count: 1, exact_text: "Home"
-      expect(page).to have_css "a#info", count: 1, exact_text: "Info"
-      expect(page).to have_css "a[rel='help']", count: 1, exact_text: "Help"
-    end
-  end
-
-  attr_reader :content
-
-  def render(content)
-    @content = content
-  end
-
-  def page
-    Capybara::Node::Simple.new(content)
-  end
-end