refactor: make status indicator presentation reusable

src/components/servers/ServerStatusIndicator.test.tsx

@@ -142,11 +142,4 @@ describe("ServerStatusIndicator", () => {
       screen.getByRole("button", { name: "github-notify status: Authorization. Show details" }),
     ).toBeInTheDocument();
   });
-
-  it("renders as plain text where a button cannot nest", () => {
-    renderWithProviders(<ServerStatusIndicator server={server} interactive={false} />);
-
-    expect(screen.queryByRole("button")).not.toBeInTheDocument();
-    expect(screen.getByText("Active")).toBeInTheDocument();
-  });
 });

src/components/servers/ServerStatusIndicator.tsx

@@ -1,7 +1,11 @@
 import { useState } from "react";
 import { useIntl } from "react-intl";
 
-import { StatusIndicator } from "@/components/ui/status-indicator";
+import {
+  StatusIndicator,
+  StatusIndicatorIcon,
+  STATUS_INDICATOR_TRIGGER_CLASS,
+} from "@/components/ui/status-indicator";
 import { cn } from "@/lib/utils";
 import {
   getAvailabilityPresentation,
@@ -19,8 +23,6 @@ interface ServerStatusIndicatorProps {
   oauthTokenStatus?: OAuthTokenStatus;
   /** Use the short label, for narrow columns. Screen readers still get the full one. */
   compact?: boolean;
-  /** Render as plain text rather than a button. Required inside another button. */
-  interactive?: boolean;
   /**
    * Starts the OAuth authorization flow. Given only where the caller can run
    * it; without it the `auth` state explains itself like every other state.
@@ -43,7 +45,6 @@ export function ServerStatusIndicator({
   server,
   oauthTokenStatus,
   compact = false,
-  interactive = true,
   onAuthorize,
   className,
 }: ServerStatusIndicatorProps) {
@@ -61,18 +62,7 @@ export function ServerStatusIndicator({
   const fullLabel = intl.formatMessage({ id: presentation.labelId });
   const isAbbreviated = compact && presentation.shortLabelId !== presentation.labelId;
 
-  if (interactive && authorize) {
-    const layout = "inline-flex items-center gap-1.5 text-xs";
-    const trigger =
-      "rounded hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring";
-    const icon = (
-      <StatusIcon
-        className={cn("h-3.5 w-3.5 shrink-0", presentation.iconClassName)}
-        aria-hidden="true"
-        focusable="false"
-      />
-    );
-
+  if (authorize) {
     const runAuthorize = async () => {
       setIsAuthorizing(true);
       try {
@@ -91,9 +81,9 @@ export function ServerStatusIndicator({
           { id: "mcpServer.status.authorizeTrigger" },
           { name: server.name },
         )}
-        className={cn(layout, trigger, "disabled:opacity-70", className)}
+        className={cn(STATUS_INDICATOR_TRIGGER_CLASS, "disabled:opacity-70", className)}
       >
-        {icon}
+        <StatusIndicatorIcon Icon={StatusIcon} className={presentation.iconClassName} />
         <span className="grid justify-items-start text-muted-foreground">
           <span className="col-start-1 row-start-1">{label}</span>
           <span className="invisible col-start-1 row-start-1" aria-hidden="true">
@@ -118,7 +108,6 @@ export function ServerStatusIndicator({
         { id: "mcpServer.status.detail.label" },
         { name: server.name },
       )}
-      interactive={interactive}
       className={className}
     >
       <ServerStatusDetail

src/components/ui/status-indicator.test.tsx

@@ -42,6 +42,53 @@ describe("StatusIndicator", () => {
     expect(await screen.findByRole("dialog", { name: "Acme add failure details" })).toBeVisible();
   });
 
+  it("names the popover from the label where the caller gives no name", async () => {
+    const user = userEvent.setup();
+    renderWithProviders(
+      <StatusIndicator {...props} fullLabel="Error adding server" triggerAriaLabel="Show details">
+        <p>Unable to add this server.</p>
+      </StatusIndicator>,
+    );
+
+    await user.click(screen.getByRole("button", { name: "Show details" }));
+
+    expect(await screen.findByRole("dialog", { name: "Error adding server" })).toBeVisible();
+  });
+
+  it("falls through a blank name rather than leaving the popover unnamed", async () => {
+    const user = userEvent.setup();
+    renderWithProviders(
+      <StatusIndicator {...props} contentAriaLabel="" triggerAriaLabel="Show details">
+        <p>Unable to add this server.</p>
+      </StatusIndicator>,
+    );
+
+    await user.click(screen.getByRole("button", { name: "Show details" }));
+
+    expect(await screen.findByRole("dialog", { name: "Error" })).toBeVisible();
+  });
+
+  // A button is named by its content, so the trigger needs no aria-label fallback.
+  it("names the trigger from the visible label where the caller gives no name", () => {
+    renderWithProviders(
+      <StatusIndicator {...props}>
+        <p>Unable to add this server.</p>
+      </StatusIndicator>,
+    );
+
+    expect(screen.getByRole("button", { name: "Error" })).toBeInTheDocument();
+  });
+
+  it("names the trigger from the full label where the visible one is abbreviated", () => {
+    renderWithProviders(
+      <StatusIndicator {...props} fullLabel="Error adding server">
+        <p>Unable to add this server.</p>
+      </StatusIndicator>,
+    );
+
+    expect(screen.getByRole("button", { name: "Error adding server" })).toBeInTheDocument();
+  });
+
   it("announces the full label where the visible one is abbreviated", () => {
     renderWithProviders(
       <StatusIndicator {...props} fullLabel="Error adding server" triggerAriaLabel="Show details">
@@ -73,17 +120,6 @@ describe("StatusIndicator", () => {
     expect(screen.getByText("Error")).toHaveClass("text-foreground");
   });
 
-  it("renders as plain text where a button cannot nest", () => {
-    renderWithProviders(
-      <StatusIndicator {...props} interactive={false}>
-        <p>Detail</p>
-      </StatusIndicator>,
-    );
-
-    expect(screen.queryByRole("button")).not.toBeInTheDocument();
-    expect(screen.getByText("Error")).toBeInTheDocument();
-  });
-
   it("renders as plain text with nothing to explain", () => {
     renderWithProviders(<StatusIndicator {...props} />);
 

src/components/ui/status-indicator.tsx

@@ -14,24 +14,38 @@ interface StatusIndicatorProps {
   label: string;
   /** The unabbreviated label, announced in place of `label` when the two differ. */
   fullLabel?: string;
-  /** Accessible name for the trigger. Required wherever `children` are given. */
+  /**
+   * Overrides the name the visible label gives the trigger. Pass it where the
+   * label alone does not say what the status is about.
+   */
   triggerAriaLabel?: string;
-  /** Accessible name for the popover, which Radix leaves unnamed. */
+  /** Accessible name for the popover, which Radix leaves unnamed. Falls back to the label. */
   contentAriaLabel?: string;
   size?: "xs" | "sm";
-  /** Render as plain text rather than a button. Required inside another button. */
-  interactive?: boolean;
   /** Popover contents. Without them the indicator has nothing to open and stays plain text. */
   children?: ReactNode;
   /** Fires on open and on close, for a caller that treats reading the popover as an action. */
   onOpenChange?: (open: boolean) => void;
   className?: string;
 }
 
+const LAYOUT_CLASS = "inline-flex items-center gap-1.5";
 const SIZE_CLASS = {
   xs: "text-xs",
   sm: "text-sm",
 } as const;
+const TRIGGER_CLASS =
+  "rounded hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring";
+
+/** The interactive shell, for a caller whose trigger holds something this cannot render. */
+export const STATUS_INDICATOR_TRIGGER_CLASS = cn(LAYOUT_CLASS, SIZE_CLASS.xs, TRIGGER_CLASS);
+
+/** The status icon, shared with a caller that builds its own trigger. */
+export function StatusIndicatorIcon({ Icon, className }: { Icon: LucideIcon; className?: string }) {
+  return (
+    <Icon className={cn("h-3.5 w-3.5 shrink-0", className)} aria-hidden="true" focusable="false" />
+  );
+}
 
 /**
  * Status icon and label, optionally opening a popover that explains the state.
@@ -49,29 +63,24 @@ export function StatusIndicator({
   triggerAriaLabel,
   contentAriaLabel,
   size = "xs",
-  interactive = true,
   children,
   onOpenChange,
   className,
 }: StatusIndicatorProps) {
   const isAbbreviated = Boolean(fullLabel && fullLabel !== label);
-  const layout = cn("inline-flex items-center gap-1.5", SIZE_CLASS[size]);
+  const layout = cn(LAYOUT_CLASS, SIZE_CLASS[size]);
 
   const content = (
     <>
-      <Icon
-        className={cn("h-3.5 w-3.5 shrink-0", iconClassName)}
-        aria-hidden="true"
-        focusable="false"
-      />
+      <StatusIndicatorIcon Icon={Icon} className={iconClassName} />
       <span className={labelClassName} aria-hidden={isAbbreviated || undefined}>
         {label}
       </span>
       {isAbbreviated && <span className="sr-only">{fullLabel}</span>}
     </>
   );
 
-  if (!interactive || !children) {
+  if (!children) {
     return <span className={cn(layout, className)}>{content}</span>;
   }
 
@@ -80,15 +89,16 @@ export function StatusIndicator({
       <PopoverTrigger
         type="button"
         aria-label={triggerAriaLabel}
-        className={cn(
-          layout,
-          "rounded hover:text-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring",
-          className,
-        )}
+        className={cn(layout, TRIGGER_CLASS, className)}
       >
         {content}
       </PopoverTrigger>
-      <PopoverContent align="end" aria-label={contentAriaLabel} className="w-auto max-w-xs p-3">
+      <PopoverContent
+        align="end"
+        // `||` not `??`: a blank label must fall through rather than leave the dialog unnamed.
+        aria-label={contentAriaLabel || fullLabel || label}
+        className="w-auto max-w-xs p-3"
+      >
         {children}
       </PopoverContent>
     </Popover>