fix(canvas): correct comment Build links and metadata

hub/app/javascript/features/prototype-canvas-hub-panel/comment-visibility.test.tsx

@@ -23,6 +23,48 @@ afterEach(() => {
 })
 
 describe("HubPanel()", () => {
+  it("opens an earlier comment Build on the selected Hub rather than the loopback canvas", async () => {
+    vi.stubGlobal(
+      "fetch",
+      vi.fn<typeof fetch>().mockImplementation((input) => {
+        if (input === "/api/hub/events") return Promise.reject(new Error("Offline"))
+        return Promise.resolve(
+          Response.json({ comments: [octocatRootComment], hasMore: false, nextCursor: null }),
+        )
+      }),
+    )
+    render(
+      <ThemeProvider>
+        <HubPanel
+          config={{ name: "Welcome", prototypeUrl: "http://localhost:4321/", token: "local" }}
+          frameRef={createRef<HTMLIFrameElement>()}
+          status={{
+            ...publicationHub,
+            state: "connected",
+            prototype: {
+              ...importedPrototype,
+              build: { ...importedPrototype.build!, number: 2 },
+            },
+          }}
+          onStatusChange={async () => {}}
+          onSelectHub={async () => {}}
+          onConnect={() => {}}
+          open={false}
+          onClose={() => {}}
+        />
+      </ThemeProvider>,
+    )
+    fireEvent.click(await screen.findByRole("button", { name: /^Comments, 1/ }))
+
+    const buildLink = screen.getByRole("link", { name: "build 1" })
+    expect(buildLink).toHaveAttribute(
+      "href",
+      `https://hub.example.com${octocatRootComment.buildPath}`,
+    )
+    expect(buildLink).toHaveAttribute("target", "_blank")
+    expect(buildLink).toHaveAttribute("rel", "noopener noreferrer")
+  })
+
   it("keeps throttled comments and the draft until the Hub changes", async () => {
     const message = "Too many requests. No changes were made. Wait 60 seconds, then try again."
     const fetchMock = vi.fn<typeof fetch>().mockImplementation((input, options) => {

hub/app/javascript/features/prototype-canvas-hub-panel/index.tsx

@@ -1357,6 +1357,7 @@ export function HubPanel({
           }}
           threads={visibleThreads}
           currentBuildNumber={currentBuild?.number || 0}
+          buildLinkOrigin={status?.hubOrigin}
           totalCount={visibleThreads.length}
           hasThreads={threads.length > 0}
           filters={filters}

hub/app/javascript/features/review-comment-composer/index.test.tsx

@@ -35,7 +35,6 @@ const COPY: ReviewCommentsCopy = {
   emptyLabel: "No comments have been added.",
   noMatchesLabel: "No comments match these filters.",
   buildLabel: "build %{number}",
-  checkingAnchorLabel: "Checking",
   movedAnchorLabel: "Moved",
   changedAnchorLabel: "Context changed",
   missingAnchorLabel: "Not found",

hub/app/javascript/features/review-comment-row/index.test.tsx

@@ -1,4 +1,4 @@
-import { render, screen } from "@testing-library/react"
+import { fireEvent, render, screen } from "@testing-library/react"
 import { describe, expect, it, vi } from "vitest"
 
 import type { ReviewThread } from "@utils/review-comment/types"
@@ -64,7 +64,6 @@ const THREAD: ReviewThread = {
 const COPY = {
   buildLabel: "build %{number}",
   changedAnchorLabel: "Context changed",
-  checkingAnchorLabel: "Checking",
   deletedCommentLabel: "Comment deleted",
   missingAnchorLabel: "Not found",
   movedAnchorLabel: "Moved",
@@ -77,6 +76,25 @@ const COPY = {
 }
 
 describe("ReviewCommentRow()", () => {
+  it("shows no target status until the preview returns a result", () => {
+    const props = {
+      thread: THREAD,
+      currentBuildNumber: 2,
+      selected: false,
+      selectionBlocked: false,
+      copy: COPY,
+      onSelect: vi.fn(),
+    }
+    const { rerender } = render(<ReviewCommentRow {...props} />)
+
+    expect(screen.queryByText("Checking")).not.toBeInTheDocument()
+    expect(screen.queryByText("Not found")).not.toBeInTheDocument()
+    expect(screen.getByRole("button", { name: /Please align this control/ })).toBeEnabled()
+
+    rerender(<ReviewCommentRow {...props} resolution={{ commentId: 1, status: "missing" }} />)
+    expect(screen.getByText("Not found")).toBeInTheDocument()
+  })
+
   it("uses a circular Brand avatar for both known and missing images", () => {
     const props = {
       currentBuildNumber: 2,
@@ -181,6 +199,7 @@ describe("ReviewCommentRow()", () => {
   })
 
   it("opens an overridden Build link without replacing the canvas", () => {
+    const onSelect = vi.fn()
     render(
       <ReviewCommentRow
         thread={THREAD}
@@ -189,7 +208,7 @@ describe("ReviewCommentRow()", () => {
         selected={false}
         selectionBlocked={false}
         copy={COPY}
-        onSelect={vi.fn()}
+        onSelect={onSelect}
       />,
     )
 
@@ -198,5 +217,17 @@ describe("ReviewCommentRow()", () => {
       "rel",
       "noopener noreferrer",
     )
+    const buildLink = screen.getByRole("link", { name: "build 2" })
+    expect(buildLink).toHaveAttribute(
+      "href",
+      "https://hub.example.com/repositories/2001/prototypes/1?build=2",
+    )
+    expect(buildLink.parentElement).toHaveTextContent("(build 2)")
+    expect(screen.getByText("The Octocat").parentElement).toContainElement(buildLink)
+    expect(buildLink.closest("button")).toBeNull()
+    fireEvent.click(buildLink)
+    expect(onSelect).not.toHaveBeenCalled()
+    fireEvent.click(screen.getByRole("button", { name: /Please align this control/ }))
+    expect(onSelect).toHaveBeenCalledWith(THREAD)
   })
 })

hub/app/javascript/features/review-comment-row/index.tsx

@@ -1,4 +1,5 @@
 import { Label } from "@primer/react-brand"
+import { useId } from "react"
 
 import { HubUserAvatar } from "@components/hub-user-avatar"
 import { ReviewRelativeTime } from "@features/review-relative-time"
@@ -7,7 +8,6 @@ import type { ReviewCommentRowProps } from "./types"
 
 import "./styles.css"
 
-/** Uses a native button so long comments do not overlap the author or status. */
 export function ReviewCommentRow({
   thread,
   currentBuildNumber,
@@ -19,6 +19,7 @@ export function ReviewCommentRow({
   onSelect,
   buildHref,
 }: ReviewCommentRowProps) {
+  const id = useId()
   const { root } = thread
   const author = root.author.name || `@${root.author.login}`
   const buildLabel = copy.buildLabel.replace("%{number}", root.buildNumber.toString())
@@ -33,7 +34,7 @@ export function ReviewCommentRow({
   } else if (anchorStatusLabel !== undefined) {
     statusLabel = anchorStatusLabel
   } else if (!resolution) {
-    statusLabel = copy.checkingAnchorLabel
+    statusLabel = null
   } else if (resolution.status === "resolved") {
     statusLabel = null
   } else if (resolution.status === "moved") {
@@ -46,73 +47,78 @@ export function ReviewCommentRow({
 
   return (
     <article className={`review-comment-row${selected ? " review-comment-row--selected" : ""}`}>
-      <button
-        type="button"
-        className="review-comment-row__button"
-        aria-pressed={selected}
-        disabled={selectionBlocked && !selected}
-        title={selectionBlocked && !selected ? copy.selectionBlockedLabel : undefined}
-        onClick={() => onSelect(thread)}
-      >
-        {thread.unread ? (
-          <span
-            role="img"
-            aria-label={copy.unreadLabel}
-            title={copy.unreadLabel}
-            className="review-comment-row__unread"
-          />
-        ) : null}
+      {thread.unread ? (
+        <span
+          role="img"
+          aria-label={copy.unreadLabel}
+          title={copy.unreadLabel}
+          className="review-comment-row__unread"
+        />
+      ) : null}
 
-        <span className="review-comment-row__layout">
-          <HubUserAvatar
-            src={root.author.avatarUrl}
-            size={28}
-            className="review-comment-row__avatar"
-          />
+      <div className="review-comment-row__layout">
+        <HubUserAvatar
+          src={root.author.avatarUrl}
+          size={28}
+          className="review-comment-row__avatar"
+        />
 
-          <span className="review-comment-row__content">
-            <span className="review-comment-row__copy">
-              <span className="review-comment-row__metadata">
-                <strong className="review-comment-row__author">{author}</strong>
+        <div className="review-comment-row__content">
+          <div className="review-comment-row__copy">
+            <div className="review-comment-row__metadata">
+              <span className="review-comment-row__author-and-build">
+                <strong id={`${id}-author`} className="review-comment-row__author">
+                  {author}
+                </strong>
 
-                <ReviewRelativeTime value={thread.lastActivityAt} />
+                {showBuildLink ? (
+                  <span className="review-comment-row__build">
+                    (
+                    <a
+                      href={buildHref || root.buildPath}
+                      target={buildHref ? "_blank" : undefined}
+                      rel={buildHref ? "noopener noreferrer" : undefined}
+                    >
+                      {buildLabel}
+                    </a>
+                    )
+                  </span>
+                ) : null}
               </span>
 
-              <span className="review-comment-row__body">
+              <ReviewRelativeTime value={thread.lastActivityAt} />
+            </div>
+
+            <button
+              type="button"
+              className="review-comment-row__button"
+              aria-labelledby={`${id}-author ${id}-body`}
+              aria-pressed={selected}
+              disabled={selectionBlocked && !selected}
+              title={selectionBlocked && !selected ? copy.selectionBlockedLabel : undefined}
+              onClick={() => onSelect(thread)}
+            >
+              <span id={`${id}-body`} className="review-comment-row__body">
                 {root.body ?? copy.deletedCommentLabel}
               </span>
-            </span>
-          </span>
-        </span>
-      </button>
+            </button>
+          </div>
 
-      {showBuildLink || statusLabel || thread.replies.length > 0 ? (
-        <span className="review-comment-row__context">
-          {showBuildLink ? (
-            <span className="review-comment-row__build">
-              <a
-                href={buildHref || root.buildPath}
-                target={buildHref ? "_blank" : undefined}
-                rel={buildHref ? "noopener noreferrer" : undefined}
-              >
-                {buildLabel}
-              </a>
-            </span>
-          ) : (
-            <span className="review-comment-row__build" />
-          )}
+          {statusLabel || thread.replies.length > 0 ? (
+            <div className="review-comment-row__context">
+              {thread.replies.length > 0 ? (
+                <span className="review-comment-row__replies">{replyCountLabel}</span>
+              ) : null}
 
-          {thread.replies.length > 0 ? (
-            <span className="review-comment-row__replies">{replyCountLabel}</span>
+              {statusLabel ? (
+                <Label size="small" className="review-comment-row__status">
+                  {statusLabel}
+                </Label>
+              ) : null}
+            </div>
           ) : null}
-
-          {statusLabel ? (
-            <Label size="small" className="review-comment-row__status">
-              {statusLabel}
-            </Label>
-          ) : null}
-        </span>
-      ) : null}
+        </div>
+      </div>
     </article>
   )
 }

hub/app/javascript/features/review-comment-row/stories.tsx

@@ -37,6 +37,21 @@ export const Single: Story = {}
 export const MultipleMessages: Story = {
   args: {
     thread: repliedReviewThread,
+    currentBuildNumber: 2,
+    buildHref: `https://hub.example.com${repliedReviewThread.root.buildPath}`,
+  },
+}
+
+export const LongAuthor: Story = {
+  args: {
+    thread: {
+      ...repliedReviewThread,
+      root: {
+        ...repliedReviewThread.root,
+        author: { ...repliedReviewThread.root.author, name: "The Octocat with a long name" },
+      },
+    },
+    currentBuildNumber: 2,
   },
 }