Fix broken JS line-number handlers and add DiffComponent tests (#113)

Replace standalone line-number event listeners that ran once at page
load (before diffs existed in the DOM) with a DiffList LiveView hook
using event delegation. Add test coverage for DiffComponent rendering.

Author: ericmj

assets/js/app.js

@@ -60,30 +60,42 @@ window.Hooks.InfiniteScroll = {
   }
 }
 
-let liveSocket = new LiveSocket("/live", Socket, { hooks: window.Hooks })
-liveSocket.connect()
-
-/*
-Make it possible to click line numbers to update the address bar to a
-link directly to that line.
-*/
-if (location.hash) {
-  document.getElementById(location.hash.replace('#', '')).classList.add('selected')
-}
-
-const lines = document.querySelectorAll('.ghd-line-number')
-lines.forEach(line => {
-  line.addEventListener('click', e => {
-    const parent = line.parentNode
+window.Hooks.DiffList = {
+  mounted() {
+    this.el.addEventListener('click', e => {
+      const lineNumber = e.target.closest('.ghd-line-number')
+      if (!lineNumber) return
+
+      const parent = lineNumber.parentNode
+      if (parent && parent.id) {
+        this.el.querySelectorAll('.ghd-line.selected').forEach(el => {
+          el.classList.remove('selected')
+        })
+        parent.classList.add('selected')
+        history.replaceState(null, null, '#' + parent.id)
+      }
+    })
 
-    if (parent && parent.id) {
-      document.querySelectorAll('.ghd-line.selected').forEach(line => {
-        line.classList.remove('selected')
-      })
+    this.selectHash()
+  },
 
-      parent.classList.add('selected')
+  updated() {
+    this.selectHash()
+  },
 
-      history.replaceState(null, null, '#' + parent.id)
+  selectHash() {
+    if (location.hash) {
+      const el = document.getElementById(location.hash.replace('#', ''))
+      if (el) {
+        this.el.querySelectorAll('.ghd-line.selected').forEach(el => {
+          el.classList.remove('selected')
+        })
+        el.classList.add('selected')
+        el.scrollIntoView({ block: 'center' })
+      }
     }
-  })
-})
+  }
+}
+
+let liveSocket = new LiveSocket("/live", Socket, { hooks: window.Hooks })
+liveSocket.connect()

lib/diff_web/templates/live/diff.html.leex

@@ -28,7 +28,7 @@
       </div>
     <% end %>
 
-    <div class="ghd-container" id="diff-list">
+    <div class="ghd-container" id="diff-list" phx-hook="DiffList">
       <%= if @generating do %>
         <div class="loading-spinner">
           Generating diffs...

test/diff_web/live/components/diff_component_test.exs

@@ -0,0 +1,121 @@
+defmodule DiffWeb.DiffComponentTest do
+  use ExUnit.Case, async: true
+
+  alias DiffWeb.DiffComponent
+
+  defp render_component(diff, diff_id) do
+    assigns = %{diff: diff, diff_id: diff_id}
+
+    assigns
+    |> DiffComponent.render()
+    |> Phoenix.HTML.Safe.to_iodata()
+    |> IO.iodata_to_binary()
+  end
+
+  defp line(opts \\ []) do
+    %GitDiff.Line{
+      type: Keyword.get(opts, :type, :context),
+      from_line_number: Keyword.get(opts, :from, 1),
+      to_line_number: Keyword.get(opts, :to, 1),
+      text: Keyword.get(opts, :text, " context line")
+    }
+  end
+
+  defp chunk(lines) do
+    %GitDiff.Chunk{
+      header: "@@ -1,3 +1,3 @@",
+      lines: lines
+    }
+  end
+
+  describe "render/1" do
+    test "changed file" do
+      diff = %GitDiff.Patch{
+        from: "lib/app.ex",
+        to: "lib/app.ex",
+        chunks: [chunk([line()])]
+      }
+
+      html = render_component(diff, "diff-0")
+
+      assert html =~ "changed"
+      assert html =~ "lib/app.ex"
+      assert html =~ "diff-0-body"
+    end
+
+    test "added file" do
+      diff = %GitDiff.Patch{
+        from: nil,
+        to: "lib/new.ex",
+        chunks: [chunk([line(type: :add, from: "", to: 1, text: "+new line")])]
+      }
+
+      html = render_component(diff, "diff-1")
+
+      assert html =~ "added"
+      assert html =~ "lib/new.ex"
+      assert html =~ "diff-1-body"
+    end
+
+    test "removed file" do
+      diff = %GitDiff.Patch{
+        from: "lib/old.ex",
+        to: nil,
+        chunks: [chunk([line(type: :remove, from: 1, to: "", text: "-old line")])]
+      }
+
+      html = render_component(diff, "diff-2")
+
+      assert html =~ "removed"
+      assert html =~ "lib/old.ex"
+      assert html =~ "diff-2-body"
+    end
+
+    test "renamed file" do
+      diff = %GitDiff.Patch{
+        from: "lib/old_name.ex",
+        to: "lib/new_name.ex",
+        chunks: [chunk([line()])]
+      }
+
+      html = render_component(diff, "diff-3")
+
+      assert html =~ "renamed"
+      assert html =~ "lib/old_name.ex -> lib/new_name.ex"
+      assert html =~ "diff-3-body"
+    end
+
+    test "includes toggle icon and phx-click" do
+      diff = %GitDiff.Patch{
+        from: "lib/app.ex",
+        to: "lib/app.ex",
+        chunks: [chunk([line()])]
+      }
+
+      html = render_component(diff, "diff-0")
+
+      assert html =~ "<svg"
+      assert html =~ "phx-click"
+    end
+
+    test "renders multiple chunks and lines" do
+      lines = [
+        line(type: :context, from: 1, to: 1, text: " unchanged"),
+        line(type: :remove, from: 2, to: "", text: "-removed"),
+        line(type: :add, from: "", to: 2, text: "+added")
+      ]
+
+      diff = %GitDiff.Patch{
+        from: "lib/app.ex",
+        to: "lib/app.ex",
+        chunks: [chunk(lines), chunk([line(from: 10, to: 10)])]
+      }
+
+      html = render_component(diff, "diff-0")
+
+      assert html =~ "unchanged"
+      assert html =~ "removed"
+      assert html =~ "added"
+    end
+  end
+end