Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 120 additions & 0 deletions src/features/pr-review-status.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,126 @@ describe("injectPRReviewStatus", () => {
expect(badge8.title).toBe("2/5 review thread(s) resolved");
});

it("builds a thread-detail popover on the unresolved badge", async () => {
twoPRRows();
vi.mocked(fetchPRReviewStatuses).mockResolvedValue([
{
number: 7,
totalThreads: 2,
resolvedThreads: 0,
unresolved: [
{ path: "src/lib/github-api.ts", line: 42, isOutdated: false, author: "alice", snippet: "Guard the null case before dereferencing this.", url: "https://github.com/owner/repo/pull/7#discussion_r1" },
{ path: "README.md", line: null, isOutdated: true, author: "bob", snippet: "Stale wording here.", url: "https://github.com/owner/repo/pull/7#discussion_r2" },
],
},
]);

await injectPRReviewStatus();

const badge = document.querySelector("#issue_7 .better-github-review-status") as HTMLElement;
expect(badge.classList.contains("better-github-review-has-popover")).toBe(true);

const items = badge.querySelectorAll(".better-github-review-popover-item");
expect(items).toHaveLength(2);

// First item: anchor to the thread, basename-only location, author-prefixed snippet.
const first = items[0] as HTMLAnchorElement;
expect(first.tagName).toBe("A");
expect(first.getAttribute("href")).toBe("https://github.com/owner/repo/pull/7#discussion_r1");
const loc = first.querySelector(".better-github-review-popover-loc-text") as HTMLElement;
expect(loc.textContent).toBe("github-api.ts:42");
expect(loc.title).toBe("src/lib/github-api.ts:42"); // full path preserved in tooltip
expect(first.querySelector(".better-github-review-popover-body")?.textContent).toBe(
"@alice: Guard the null case before dereferencing this.",
);
expect(first.querySelector(".better-github-review-popover-outdated")).toBeNull();

// Second item: no line number, flagged outdated.
const second = items[1] as HTMLElement;
expect(second.querySelector(".better-github-review-popover-loc-text")?.textContent).toBe("README.md");
expect(second.querySelector(".better-github-review-popover-outdated")?.textContent).toBe("outdated");
});

it("toggles the popover on click and dismisses it on an outside click / Escape", async () => {
twoPRRows();
vi.mocked(fetchPRReviewStatuses).mockResolvedValue([
{
number: 7,
totalThreads: 1,
resolvedThreads: 0,
unresolved: [
{ path: "a.ts", line: 1, isOutdated: false, author: "alice", snippet: "fix", url: "https://github.com/owner/repo/pull/7#discussion_r1" },
],
},
]);

await injectPRReviewStatus();

const badge = document.querySelector("#issue_7 .better-github-review-status") as HTMLElement;
expect(badge.getAttribute("role")).toBe("button");
expect(badge.getAttribute("aria-expanded")).toBe("false");

// Click the badge → opens.
badge.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(true);
expect(badge.getAttribute("aria-expanded")).toBe("true");

// Click the badge again → closes (toggle).
badge.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(false);

// Open again, then an outside click dismisses it.
badge.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(true);
document.body.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(false);

// Open again, then Escape dismisses it.
badge.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(true);
document.dispatchEvent(new KeyboardEvent("keydown", { key: "Escape", bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(false);
});

it("does not toggle the popover when a thread row inside it is clicked", async () => {
twoPRRows();
vi.mocked(fetchPRReviewStatuses).mockResolvedValue([
{
number: 7,
totalThreads: 1,
resolvedThreads: 0,
unresolved: [
{ path: "a.ts", line: 1, isOutdated: false, author: "alice", snippet: "fix", url: "https://github.com/owner/repo/pull/7#discussion_r1" },
],
},
]);

await injectPRReviewStatus();

const badge = document.querySelector("#issue_7 .better-github-review-status") as HTMLElement;
badge.dispatchEvent(new MouseEvent("click", { bubbles: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(true);

// Clicking the link row must fall through to navigation, leaving the popover open.
const item = badge.querySelector(".better-github-review-popover-item") as HTMLElement;
item.dispatchEvent(new MouseEvent("click", { bubbles: true, cancelable: true }));
expect(badge.classList.contains("better-github-review-popover-open")).toBe(true);
});

it("omits the popover when the API returned no unresolved details", async () => {
twoPRRows();
vi.mocked(fetchPRReviewStatuses).mockResolvedValue([
{ number: 7, totalThreads: 3, resolvedThreads: 1 }, // legacy payload, no `unresolved`
]);

await injectPRReviewStatus();

const badge = document.querySelector("#issue_7 .better-github-review-status") as HTMLElement;
expect(badge.textContent).toBe("2 unresolved");
expect(badge.classList.contains("better-github-review-has-popover")).toBe(false);
expect(badge.querySelector(".better-github-review-popover")).toBeNull();
});

it("renders no badge for a PR with zero review threads", async () => {
twoPRRows();
vi.mocked(fetchPRReviewStatuses).mockResolvedValue([
Expand Down
157 changes: 157 additions & 0 deletions src/features/pr-review-status.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,158 @@
import { isPRListPage, getRepoInfo } from "../lib/page-detect";
import { fetchPRReviewStatuses } from "../lib/github-api";
import type { ReviewThreadDetail } from "../lib/messages";
import { getOrCreateInfoRow } from "../lib/info-row";

const STATUS_CLASS = "better-github-review-status";
const POPOVER_CLASS = "better-github-review-popover";
const OPEN_CLASS = "better-github-review-popover-open";

/** Cap the list so a PR with dozens of threads can't grow an unwieldy popover. */
const MAX_POPOVER_ITEMS = 8;

/** Collapse every open popover (used by the outside-click / Escape handlers). */
function closeAllPopovers(): void {
for (const open of document.querySelectorAll(`.${OPEN_CLASS}`)) {
open.classList.remove(OPEN_CLASS);
open.setAttribute("aria-expanded", "false");
}
}

// A single pair of document-level listeners dismisses popovers on an outside
// click or Escape — attached once, no matter how often the feature re-injects.
let globalListenersAttached = false;
function ensureGlobalListeners(): void {
if (globalListenersAttached) return;
globalListenersAttached = true;
// Badge clicks call stopPropagation, so this only fires for clicks elsewhere.
document.addEventListener("click", closeAllPopovers);
document.addEventListener("keydown", (e) => {
if (e.key === "Escape") closeAllPopovers();
});
}

/** Wire a badge to open/close its popover on click (and keyboard). */
function makeBadgeInteractive(badge: HTMLElement): void {
badge.setAttribute("role", "button");
badge.setAttribute("tabindex", "0");
// "dialog" rather than "true"/"menu": the popover is a list of links, not a menu.
badge.setAttribute("aria-haspopup", "dialog");
badge.setAttribute("aria-expanded", "false");

const toggle = () => {
const willOpen = !badge.classList.contains(OPEN_CLASS);
closeAllPopovers();
badge.classList.toggle(OPEN_CLASS, willOpen);
badge.setAttribute("aria-expanded", String(willOpen));
};

// Only react to events on the badge itself; events bubbling up from inside the
// popover (a thread row click, Enter on a focused link) are left to navigate.
badge.addEventListener("click", (e) => {
if (e.target !== badge) return;
e.stopPropagation(); // don't let the document handler immediately re-close it
toggle();
});
badge.addEventListener("keydown", (e) => {
if (e.target !== badge) return;
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
e.stopPropagation();
toggle();
}
});

ensureGlobalListeners();
}

/** Octicon `comment` (16px) — GitHub's own glyph for review conversations. */
const COMMENT_ICON =
'<svg viewBox="0 0 16 16" width="14" height="14" aria-hidden="true">' +
'<path d="M1 2.75C1 1.784 1.784 1 2.75 1h10.5c.966 0 1.75.784 1.75 1.75v7.5A1.75 1.75 0 0 1 13.25 12H9.06l-2.573 2.573A1.458 1.458 0 0 1 4 13.543V12H2.75A1.75 1.75 0 0 1 1 10.25Zm1.75-.25a.25.25 0 0 0-.25.25v7.5c0 .138.112.25.25.25h2a.75.75 0 0 1 .75.75v2.19l2.72-2.72a.749.749 0 0 1 .53-.22h4.5a.25.25 0 0 0 .25-.25v-7.5a.25.25 0 0 0-.25-.25Z"></path>' +
"</svg>";

/** Last path segment, so a long `a/b/c/file.ts` shows as just `file.ts`. */
function basename(path: string): string {
if (!path) return "";
const parts = path.split("/");
return parts[parts.length - 1] || path;
}

/**
* Build the click-to-open popover listing each unresolved thread. Styled after
* GitHub's own hovercard / checks dropdown: a caret, a bordered header, and tidy
* rows. Long text is tamed entirely in CSS (path → basename + ellipsis, body →
* 2-line clamp), so the popover keeps a fixed width no matter how verbose a
* comment is. Each row links to its first comment's `#discussion_r…` anchor.
*/
function buildPopover(threads: ReviewThreadDetail[]): HTMLElement {
const popover = document.createElement("div");
popover.className = POPOVER_CLASS;

// Keep clicks inside the popover from reaching the document dismiss handler, so
// it stays open while a thread link navigates (and selecting text won't close it).
popover.addEventListener("click", (e) => e.stopPropagation());

// Upward caret pointing back at the badge.
const caret = document.createElement("span");
caret.className = "better-github-review-popover-caret";
popover.appendChild(caret);

// Header row, like "3 unresolved threads" with a comment glyph.
const header = document.createElement("div");
header.className = "better-github-review-popover-header";
const icon = document.createElement("span");
icon.className = "better-github-review-popover-header-icon";
icon.innerHTML = COMMENT_ICON;
const headerLabel = document.createElement("span");
headerLabel.textContent = `${threads.length} unresolved thread${threads.length === 1 ? "" : "s"}`;
header.append(icon, headerLabel);
popover.appendChild(header);

for (const t of threads.slice(0, MAX_POPOVER_ITEMS)) {
// An <a> so clicking jumps straight to the thread; falls back to a div when
// the API gave us no permalink.
const item = document.createElement(t.url ? "a" : "div");
item.className = "better-github-review-popover-item";
if (t.url && item instanceof HTMLAnchorElement) {
item.href = t.url;
}

const head = document.createElement("div");
head.className = "better-github-review-popover-loc";
const file = t.path ? basename(t.path) : "general comment";
const locText = document.createElement("span");
locText.className = "better-github-review-popover-loc-text";
locText.textContent = t.line != null ? `${file}:${t.line}` : file;
if (t.path) locText.title = t.line != null ? `${t.path}:${t.line}` : t.path;
head.appendChild(locText);
if (t.isOutdated) {
const tag = document.createElement("span");
tag.className = "better-github-review-popover-outdated";
tag.textContent = "outdated";
head.appendChild(tag);
}

const body = document.createElement("div");
body.className = "better-github-review-popover-body";
const author = t.author ? `@${t.author}` : "";
const snippet = t.snippet.trim();
body.textContent = author && snippet ? `${author}: ${snippet}` : author || snippet || "(no comment)";

item.appendChild(head);
item.appendChild(body);
popover.appendChild(item);
}

if (threads.length > MAX_POPOVER_ITEMS) {
const more = document.createElement("div");
more.className = "better-github-review-popover-more";
more.textContent = `+${threads.length - MAX_POPOVER_ITEMS} more`;
popover.appendChild(more);
}

return popover;
}

export async function injectPRReviewStatus(): Promise<void> {
if (!isPRListPage()) return;
Expand Down Expand Up @@ -57,6 +207,13 @@ export async function injectPRReviewStatus(): Promise<void> {
badge.classList.add("better-github-review-unresolved");
badge.textContent = `${unresolved} unresolved`;
badge.title = `${status.resolvedThreads}/${status.totalThreads} review thread(s) resolved`;

const details = status.unresolved ?? [];
if (details.length > 0) {
badge.classList.add("better-github-review-has-popover");
badge.appendChild(buildPopover(details));
makeBadgeInteractive(badge);
}
}

infoRow.appendChild(badge);
Expand Down
17 changes: 17 additions & 0 deletions src/lib/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,27 @@ export interface PRBranchInfo {
headRef: string;
}

export interface ReviewThreadDetail {
/** File path the thread is anchored to (empty for file-level / outdated threads). */
path: string;
/** Line number in the diff, or null when the thread is outdated / not line-anchored. */
line: number | null;
/** Whether the diff the thread was left on has since changed. */
isOutdated: boolean;
/** Login of the author of the first comment in the thread. */
author: string;
/** Plain-text body of the first comment (already stripped of Markdown by the API). */
snippet: string;
/** Permalink to the first comment, for jumping straight to the thread. */
url: string;
}

export interface PRReviewStatus {
number: number;
totalThreads: number;
resolvedThreads: number;
/** Details for the unresolved threads only (resolved ones are omitted to keep the payload small). */
unresolved?: ReviewThreadDetail[];
}

export interface PRDiffStats {
Expand Down
Loading