feat: Add PR badge to sidebar task list (#2422)

Co-authored-by: Charles Vien <charles.v@posthog.com>

Author: rafaeelaudibert

apps/code/src/renderer/components/ui/NestedButton.test.tsx

@@ -0,0 +1,34 @@
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { describe, expect, it, vi } from "vitest";
+import { NestedButton } from "./NestedButton";
+
+describe("NestedButton", () => {
+  it("renders an accessible button", () => {
+    render(<NestedButton onActivate={() => {}}>x</NestedButton>);
+    expect(screen.getByRole("button")).toBeTruthy();
+  });
+
+  it("calls onActivate on click without bubbling to the parent", async () => {
+    const onActivate = vi.fn();
+    const onParentClick = vi.fn();
+    render(
+      // biome-ignore lint/a11y/useKeyWithClickEvents: test-only wrapper
+      // biome-ignore lint/a11y/noStaticElementInteractions: test-only wrapper
+      <div onClick={onParentClick}>
+        <NestedButton onActivate={onActivate}>x</NestedButton>
+      </div>,
+    );
+    await userEvent.click(screen.getByRole("button"));
+    expect(onActivate).toHaveBeenCalledTimes(1);
+    expect(onParentClick).not.toHaveBeenCalled();
+  });
+
+  it.each(["{Enter}", " "])("activates with the %s key", async (key) => {
+    const onActivate = vi.fn();
+    render(<NestedButton onActivate={onActivate}>x</NestedButton>);
+    screen.getByRole("button").focus();
+    await userEvent.keyboard(key);
+    expect(onActivate).toHaveBeenCalledTimes(1);
+  });
+});

apps/code/src/renderer/components/ui/NestedButton.tsx

@@ -0,0 +1,50 @@
+import type React from "react";
+import { forwardRef } from "react";
+
+interface NestedButtonProps extends React.HTMLAttributes<HTMLSpanElement> {
+  onActivate: () => void;
+}
+
+/**
+ * A button nested inside another button. Rows like SidebarItem render as a real
+ * `<button>`, and HTML forbids nesting a `<button>` inside one, so this is a
+ * `<span role="button">` with full keyboard support instead. Click, double
+ * click and Enter/Space stop propagation so the parent button does not also
+ * fire. Forwards its ref and composes any injected handlers so it works as a
+ * Radix `asChild` trigger (e.g. wrapped in a Tooltip).
+ */
+export const NestedButton = forwardRef<HTMLSpanElement, NestedButtonProps>(
+  function NestedButton(
+    { onActivate, onClick, onDoubleClick, onKeyDown, children, ...rest },
+    ref,
+  ) {
+    return (
+      // biome-ignore lint/a11y/useSemanticElements: nested clickable inside a parent <button> (e.g. SidebarItem)
+      <span
+        {...rest}
+        ref={ref}
+        role="button"
+        tabIndex={0}
+        onClick={(e) => {
+          e.stopPropagation();
+          onClick?.(e);
+          onActivate();
+        }}
+        onDoubleClick={(e) => {
+          e.stopPropagation();
+          onDoubleClick?.(e);
+        }}
+        onKeyDown={(e) => {
+          onKeyDown?.(e);
+          if (e.key === "Enter" || e.key === " ") {
+            e.preventDefault();
+            e.stopPropagation();
+            onActivate();
+          }
+        }}
+      >
+        {children}
+      </span>
+    );
+  },
+);

apps/code/src/renderer/features/sidebar/components/TaskListView.tsx

@@ -107,6 +107,7 @@ function TaskRow({
       slackThreadUrl={task.slackThreadUrl}
       prState={prState}
       hasDiff={hasDiff}
+      prUrl={task.cloudPrUrl}
       timestamp={timestamp}
       onClick={onClick}
       onDoubleClick={onDoubleClick}

apps/code/src/renderer/features/sidebar/components/items/TaskIcon.tsx

@@ -1,4 +1,5 @@
 import { DotsCircleSpinner } from "@components/DotsCircleSpinner";
+import { NestedButton } from "@components/ui/NestedButton";
 import { Tooltip } from "@components/ui/Tooltip";
 import type { SidebarPrState } from "@features/sidebar/hooks/useTaskPrStatus";
 import type { WorkspaceMode } from "@main/services/workspace/schemas";
@@ -14,8 +15,8 @@ import {
   PushPin,
   SlackLogo,
 } from "@phosphor-icons/react";
-import { trpcClient } from "@renderer/trpc/client";
 import { isTerminalStatus, type TaskRunStatus } from "@shared/types";
+import { openUrlInBrowser } from "@utils/browser";
 
 export const ICON_SIZE = 12;
 
@@ -39,14 +40,10 @@ function getOriginProductMeta(
   return originProduct ? ORIGIN_PRODUCT_META[originProduct] : undefined;
 }
 
-// Renders the icon inside a span. When `link` is set the span becomes
-// clickable and opens the originating thread externally. SidebarItem renders
-// the row as a `<button>`, so a real `<a>` here would be invalid HTML — match
-// the inline role="button" pattern used by TaskHoverToolbar.
-//
-// Returned as a plain React element (not a component) so the span is the
-// direct child of Tooltip — Radix's `asChild` Slot needs a host element to
-// attach hover handlers to.
+// Renders the icon inside a span. When `link` is set the icon becomes a
+// clickable NestedButton that opens the originating thread externally.
+// SidebarItem renders the row as a `<button>`, so a real `<a>` or a nested
+// `<button>` here would be invalid HTML.
 function renderIconSpan({
   icon,
   link,
@@ -59,31 +56,16 @@ function renderIconSpan({
   if (!link) {
     return <span className="flex items-center justify-center">{icon}</span>;
   }
-  const open = () => {
-    void trpcClient.os.openExternal.mutate({ url: link });
-  };
   return (
-    // biome-ignore lint/a11y/useSemanticElements: nested clickable inside SidebarItem button
-    <span
-      role="button"
-      tabIndex={0}
+    <NestedButton
       aria-label={ariaLabel}
       className="flex cursor-pointer items-center justify-center rounded transition-opacity hover:opacity-70"
-      onClick={(e) => {
-        e.stopPropagation();
-        open();
-      }}
-      onDoubleClick={(e) => e.stopPropagation()}
-      onKeyDown={(e) => {
-        if (e.key === "Enter" || e.key === " ") {
-          e.preventDefault();
-          e.stopPropagation();
-          open();
-        }
+      onActivate={() => {
+        void openUrlInBrowser(link);
       }}
     >
       {icon}
-    </span>
+    </NestedButton>
   );
 }
 

apps/code/src/renderer/features/sidebar/components/items/TaskItem.tsx

@@ -1,13 +1,33 @@
+import { NestedButton } from "@components/ui/NestedButton";
 import { Tooltip } from "@components/ui/Tooltip";
 import type { SidebarPrState } from "@features/sidebar/hooks/useTaskPrStatus";
 import type { WorkspaceMode } from "@main/services/workspace/schemas";
-import { Archive, PushPin } from "@phosphor-icons/react";
+import { Archive, GitPullRequest, PushPin } from "@phosphor-icons/react";
+import { parseGithubUrl } from "@posthog/git/utils";
 import type { TaskRunStatus } from "@shared/types";
+import { openUrlInBrowser } from "@utils/browser";
 import { formatRelativeTimeShort } from "@utils/time";
-import { useCallback, useEffect, useRef, useState } from "react";
+import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import { SidebarItem } from "../SidebarItem";
 import { TaskIcon } from "./TaskIcon";
 
+function PrBadge({ url, number }: { url: string; number: number }) {
+  return (
+    <Tooltip content="Open pull request" side="top">
+      <NestedButton
+        aria-label={`Open pull request #${number}`}
+        className="flex h-4 shrink-0 cursor-pointer items-center gap-0.5 rounded bg-gray-3 px-1 text-[11px] text-gray-11 transition-colors hover:bg-gray-4 hover:text-gray-12"
+        onActivate={() => {
+          void openUrlInBrowser(url);
+        }}
+      >
+        <GitPullRequest size={10} weight="bold" />
+        {`#${number}`}
+      </NestedButton>
+    </Tooltip>
+  );
+}
+
 interface TaskItemProps {
   depth?: number;
   taskId: string;
@@ -27,6 +47,7 @@ interface TaskItemProps {
   slackThreadUrl?: string;
   prState?: SidebarPrState;
   hasDiff?: boolean;
+  prUrl?: string | null;
   timestamp?: number;
   isEditing?: boolean;
   onClick: (e: React.MouseEvent) => void;
@@ -53,50 +74,24 @@ function TaskHoverToolbar({
     <span className="hidden shrink-0 items-center gap-0.5 group-hover:flex">
       {onTogglePin && (
         <Tooltip content={isPinned ? "Unpin task" : "Pin task"} side="top">
-          {/* biome-ignore lint/a11y/useSemanticElements: Cannot use button inside parent button (SidebarItem) */}
-          <span
-            role="button"
-            tabIndex={0}
+          <NestedButton
+            aria-label={isPinned ? "Unpin task" : "Pin task"}
             className="flex h-5 w-5 cursor-pointer items-center justify-center rounded text-gray-10 transition-colors hover:bg-gray-4 hover:text-gray-12"
-            onClick={(e) => {
-              e.stopPropagation();
-              onTogglePin();
-            }}
-            onDoubleClick={(e) => e.stopPropagation()}
-            onKeyDown={(e) => {
-              if (e.key === "Enter" || e.key === " ") {
-                e.preventDefault();
-                e.stopPropagation();
-                onTogglePin();
-              }
-            }}
+            onActivate={onTogglePin}
           >
             <PushPin size={12} weight={isPinned ? "fill" : "regular"} />
-          </span>
+          </NestedButton>
         </Tooltip>
       )}
       {onArchive && (
         <Tooltip content="Archive task" side="top">
-          {/* biome-ignore lint/a11y/useSemanticElements: Cannot use button inside parent button (SidebarItem) */}
-          <span
-            role="button"
-            tabIndex={0}
+          <NestedButton
+            aria-label="Archive task"
             className="flex h-5 w-5 cursor-pointer items-center justify-center rounded text-gray-10 transition-colors hover:bg-gray-4 hover:text-gray-12"
-            onClick={(e) => {
-              e.stopPropagation();
-              onArchive();
-            }}
-            onDoubleClick={(e) => e.stopPropagation()}
-            onKeyDown={(e) => {
-              if (e.key === "Enter" || e.key === " ") {
-                e.preventDefault();
-                e.stopPropagation();
-                onArchive();
-              }
-            }}
+            onActivate={onArchive}
           >
             <Archive size={12} />
-          </span>
+          </NestedButton>
         </Tooltip>
       )}
     </span>
@@ -123,6 +118,7 @@ export function TaskItem({
   slackThreadUrl,
   prState,
   hasDiff,
+  prUrl,
   timestamp,
   isEditing = false,
   onClick,
@@ -149,11 +145,19 @@ export function TaskItem({
     />
   );
 
-  const timestampNode = timestamp ? (
-    <span className="shrink-0 text-[11px] text-gray-11 group-hover:hidden">
-      {formatRelativeTimeShort(timestamp)}
-    </span>
-  ) : null;
+  const prRef = useMemo(() => (prUrl ? parseGithubUrl(prUrl) : null), [prUrl]);
+  const prBadge =
+    prUrl && prRef?.kind === "pr" ? (
+      <PrBadge url={prUrl} number={prRef.number} />
+    ) : null;
+
+  // The PR badge takes the timestamp's slot, so hide the timestamp when shown.
+  const timestampNode =
+    timestamp && !prBadge ? (
+      <span className="shrink-0 text-[11px] text-gray-11 group-hover:hidden">
+        {formatRelativeTimeShort(timestamp)}
+      </span>
+    ) : null;
 
   const toolbar =
     !hideHoverActions && (onArchive || onTogglePin) ? (
@@ -165,8 +169,9 @@ export function TaskItem({
     ) : null;
 
   const endContent =
-    timestampNode || toolbar ? (
+    prBadge || timestampNode || toolbar ? (
       <>
+        {prBadge}
         {timestampNode}
         {toolbar}
       </>