Add copy link and highlight to comments

Opening a comment link expands the revision the comment is in, even if
the link is opened while already on the page, and scrolls to it.

This also fixes scrolling to URL fragments that aren't valid CSS
selectors, such as comment ids starting with a digit.

src/components/Comment.svelte

@@ -1,12 +1,17 @@
 <script lang="ts">
   import type { BaseUrl, Comment } from "@http-client";
 
+  import debounce from "lodash/debounce";
+  import { onMount } from "svelte";
+
   import * as utils from "@app/lib/utils";
 
   type Side = "left" | "right";
   type SelectionAnchor = { side: Side; lineNumber: number };
   type SelectionRange = { start: SelectionAnchor; end?: SelectionAnchor };
 
+  import Icon from "@app/components/Icon.svelte";
+  import IconButton from "@app/components/IconButton.svelte";
   import Id from "@app/components/Id.svelte";
   import Markdown from "@app/components/Markdown.svelte";
   import NodeId from "@app/components/NodeId.svelte";
@@ -40,6 +45,26 @@
     }
   }
   $: selectionRange = rangeAnchorsFromCodeLocation(location);
+
+  let linkCopied = false;
+  let targeted = false;
+
+  const restoreLinkIcon = debounce(() => {
+    linkCopied = false;
+  }, 1000);
+
+  async function copyLink() {
+    const { origin, pathname, search } = window.location;
+    await utils.toClipboard(`${origin}${pathname}${search}#${id}`);
+    linkCopied = true;
+    restoreLinkIcon();
+  }
+
+  function updateTargeted() {
+    targeted = utils.fragmentId(window.location.hash) === id;
+  }
+
+  onMount(updateTargeted);
 </script>
 
 <style>
@@ -48,6 +73,23 @@
     flex-direction: column;
     padding: 0.5rem 0;
     gap: 0.5rem;
+    border-radius: var(--border-radius-sm);
+    scroll-margin-top: 5rem;
+  }
+  .card.targeted {
+    box-shadow: 0 0 0 2px var(--color-border-brand);
+  }
+  .copy-link {
+    margin-left: auto;
+  }
+  @media (hover: hover) {
+    .copy-link {
+      opacity: 0;
+    }
+    .card:hover .copy-link,
+    .copy-link:has(:global(:focus-visible)) {
+      opacity: 1;
+    }
   }
   .card-header {
     display: flex;
@@ -118,7 +160,14 @@
   }
 </style>
 
-<div class="card" class:card-empty-body={!body} {id} class:reply={isReply}>
+<svelte:window on:hashchange={updateTargeted} />
+
+<div
+  class="card"
+  class:card-empty-body={!body}
+  class:targeted
+  {id}
+  class:reply={isReply}>
   <div style:position="relative">
     {#if isReply}
       <div class="reply-dot"></div>
@@ -163,6 +212,15 @@
           • edited
         </div>
       {/if}
+      <div class="copy-link">
+        <IconButton
+          title={linkCopied ? "Link copied" : "Copy link to comment"}
+          ariaLabel="Copy link to comment"
+          stylePadding="0.25rem"
+          on:click={copyLink}>
+          <Icon name={linkCopied ? "checkmark" : "link"} />
+        </IconButton>
+      </div>
     </div>
   </div>
 

src/lib/router.ts

@@ -219,8 +219,9 @@ async function navigate(
       window.scrollTo(0, restoreScrollY ?? 0);
       return;
     }
-    if (hash.length > 1) {
-      const el = document.querySelector(hash);
+    const fragment = utils.fragmentId(hash);
+    if (fragment !== undefined) {
+      const el = document.getElementById(fragment);
       if (el) {
         const top =
           el.getBoundingClientRect().top +

src/lib/utils.ts

@@ -246,6 +246,18 @@ export function scrollIntoView(id: string, options?: ScrollIntoViewOptions) {
   if (lineElement) lineElement.scrollIntoView(options);
 }
 
+export function fragmentId(hash: string): string | undefined {
+  const fragment = hash.startsWith("#") ? hash.slice(1) : hash;
+  if (fragment === "") {
+    return undefined;
+  }
+  try {
+    return decodeURIComponent(fragment);
+  } catch {
+    return undefined;
+  }
+}
+
 // Check whether the given path has a markdown file extension.
 export function isMarkdownPath(path: string): boolean {
   return /\.(md|mkd|markdown)$/i.test(path);

src/views/repos/Cob/Revision.svelte

@@ -13,7 +13,7 @@
   import * as utils from "@app/lib/utils";
   import { HttpdClient } from "@http-client";
   import { cachedGetDiff } from "@app/views/repos/router";
-  import { onMount } from "svelte";
+  import { onMount, tick } from "svelte";
 
   import CobCommitTeaser from "@app/views/repos/Cob/CobCommitTeaser.svelte";
   import CommentComponent from "@app/components/Comment.svelte";
@@ -55,7 +55,41 @@
   export let previousRevOid: string | undefined = undefined;
   export let first: boolean;
 
-  let expanded = initiallyExpanded;
+  function timelinesContain(commentId: string | undefined): boolean {
+    if (commentId === undefined) {
+      return false;
+    }
+    const inThread = (thread: { root: Comment; replies: Comment[] }) =>
+      thread.root.id === commentId ||
+      thread.replies.some(reply => reply.id === commentId);
+    return timelines.some(element => {
+      switch (element.type) {
+        case "thread":
+          return inThread(element.inner);
+        case "review": {
+          const review = element.inner[1];
+          return review.id === commentId || review.threads.some(inThread);
+        }
+        case "merge":
+          return false;
+      }
+    });
+  }
+
+  let expanded =
+    initiallyExpanded ||
+    timelinesContain(utils.fragmentId(window.location.hash));
+
+  async function expandToHash() {
+    const commentId = utils.fragmentId(window.location.hash);
+    if (expanded || commentId === undefined || !timelinesContain(commentId)) {
+      return;
+    }
+    expanded = true;
+    await tick();
+    utils.scrollIntoView(commentId);
+  }
+
   const api = new HttpdClient(baseUrl);
   const lastEdit = revisionEdits.at(-1);
 
@@ -273,6 +307,8 @@
   }
 </style>
 
+<svelte:window on:hashchange={expandToHash} />
+
 <div class="revision" style:margin-bottom={expanded ? "2rem" : "0.5rem"}>
   <div class="revision-box" class:expanded>
     <div class="revision-header">

tests/e2e/clipboard.spec.ts

@@ -87,6 +87,19 @@ test("copy to clipboard", async ({ page, browserName, context }) => {
     await expectClipboard("Inline `code` has `back-ticks around` it.\n");
   }
 
+  // Link to a patch comment.
+  {
+    await page.goto(`${cobUrl}/patches`);
+    await page
+      .getByRole("link", { name: "Taking another stab at the README" })
+      .click();
+    const review = page.locator(".card", { hasText: "rejected revision" });
+    const reviewId = await review.getAttribute("id");
+    await review.hover();
+    await review.getByRole("button", { name: "Copy link to comment" }).click();
+    await expectClipboard(`${page.url()}#${reviewId}`);
+  }
+
   // Clear the system clipboard contents so developers don't wonder why there's
   // random stuff in their clipboard after running tests.
   await page.evaluate<string>("navigator.clipboard.writeText('')");

tests/e2e/repo/patch.spec.ts

@@ -154,3 +154,43 @@ test("commit listing ordering keeping stable on browser navigation", async ({
     .waitFor();
   await expectCorrectCommitListing();
 });
+
+test("link to a comment in an older revision", async ({ page }) => {
+  await page.goto(`${cobUrl}/patches`);
+  await page
+    .getByRole("link", { name: "Taking another stab at the README" })
+    .click();
+  await page.getByLabel("expand").first().click();
+  const review = page.locator(".card", { hasText: "accepted revision" });
+  const reviewId = (await review.getAttribute("id")) ?? "";
+  expect(reviewId).toMatch(/^[a-f0-9]{40}$/);
+  const patchUrl = page.url();
+
+  await page.goto(`${patchUrl}#${reviewId}`);
+  await expect(review).toBeVisible();
+  await expect(review).toHaveClass(/targeted/);
+  await expect(review).toBeInViewport();
+
+  await page.goto(patchUrl);
+  await expect(review).toBeHidden();
+  await page.evaluate(id => (window.location.hash = id), reviewId);
+  await expect(review).toBeVisible();
+  await expect(review).toHaveClass(/targeted/);
+  await expect(review).toBeInViewport();
+});
+
+test("malformed fragment does not break the patch page", async ({ page }) => {
+  await page.goto(`${cobUrl}/patches`);
+  await page
+    .getByRole("link", { name: "Taking another stab at the README" })
+    .click();
+  await page
+    .getByRole("heading", { name: "Taking another stab at the README" })
+    .waitFor();
+
+  await page.goto(`${page.url()}#%`);
+  await expect(
+    page.getByRole("heading", { name: "Taking another stab at the README" }),
+  ).toBeVisible();
+  await expect(page.locator(".card.targeted")).toHaveCount(0);
+});

tests/unit/utils.test.ts

@@ -429,3 +429,22 @@ describe("untwemoji", () => {
     expect(node.innerHTML).toEqual("ab");
   });
 });
+
+describe("fragmentId", () => {
+  test.each([
+    { hash: "", expected: undefined },
+    { hash: "#", expected: undefined },
+    { hash: "#delegate", expected: "delegate" },
+    { hash: "delegate", expected: "delegate" },
+    {
+      hash: "#0a1b2c3d4e5f60718293a4b5c6d7e8f901234567",
+      expected: "0a1b2c3d4e5f60718293a4b5c6d7e8f901234567",
+    },
+    { hash: "#caf%C3%A9", expected: "café" },
+    { hash: "#%", expected: undefined },
+    { hash: "#%zz", expected: undefined },
+    { hash: "#%E0%A4%A", expected: undefined },
+  ])("fragmentId($hash) => $expected", ({ hash, expected }) => {
+    expect(utils.fragmentId(hash)).toEqual(expected);
+  });
+});