Polish board approval card styling

Co-Authored-By: Paperclip <noreply@paperclip.ing>
2026-04-06 11:36:20 -05:00
parent 758219d53f
commit dc842ff7ea
3 changed files with 209 additions and 81 deletions
@@ -1,8 +1,15 @@
 import { CheckCircle2, XCircle, Clock } from "lucide-react";
 import { Link } from "@/lib/router";
+import { Badge } from "@/components/ui/badge";
 import { Button, buttonVariants } from "@/components/ui/button";
 import { Identity } from "./Identity";
-import { approvalLabel, typeIcon, defaultTypeIcon, ApprovalPayloadRenderer } from "./ApprovalPayload";
+import {
+  approvalSubject,
+  typeIcon,
+  defaultTypeIcon,
+  ApprovalPayloadRenderer,
+  typeLabel,
+} from "./ApprovalPayload";
 import { timeAgo } from "../lib/timeAgo";
 import type { Approval, Agent } from "@paperclipai/shared";
 import { cn } from "@/lib/utils";
@@ -34,80 +41,110 @@ export function ApprovalCard({
  isPending?: boolean;
  pendingAction?: "approve" | "reject" | null;
 }) {
+  const payload = approval.payload as Record<string, unknown> | null;
  const Icon = typeIcon[approval.type] ?? defaultTypeIcon;
-  const label = approvalLabel(approval.type, approval.payload as Record<string, unknown> | null);
+  const kindLabel = typeLabel[approval.type] ?? approval.type;
+  const subject = approvalSubject(payload);
  const showResolutionButtons =
    Boolean(onApprove && onReject) &&
    approval.type !== "budget_override_required" &&
    (approval.status === "pending" || approval.status === "revision_requested");
+  const hasFooter = showResolutionButtons || Boolean(detailLink || onOpen);

  return (
-    <div className="border border-border rounded-lg p-4 space-y-0">
-      {/* Header */}
-      <div className="flex items-start justify-between gap-3">
-        <div className="flex items-center gap-2">
-          <Icon className="h-4 w-4 text-muted-foreground shrink-0" />
-          <div className="flex items-center gap-2">
-            <span className="font-medium text-sm">{label}</span>
-            {requesterAgent && (
-              <span className="text-xs text-muted-foreground">
-                requested by <Identity name={requesterAgent.name} size="sm" className="inline-flex" />
-              </span>
-            )}
+    <div className="rounded-xl border border-border/70 bg-card p-4 shadow-sm">
+      <div className="flex items-start justify-between gap-4">
+        <div className="min-w-0 flex-1">
+          <div className="flex items-start gap-3">
+            <div className="flex h-9 w-9 shrink-0 items-center justify-center rounded-full border border-border/70 bg-background/80">
+              <Icon className="h-4 w-4 text-muted-foreground" />
+            </div>
+            <div className="min-w-0 flex-1 space-y-2">
+              <div className="flex flex-wrap items-center gap-2">
+                <Badge
+                  variant="outline"
+                  className="border-border/70 bg-background/70 px-2 py-0.5 text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground"
+                >
+                  {kindLabel}
+                </Badge>
+                {requesterAgent && (
+                  <div className="inline-flex min-w-0 items-center gap-1.5 text-xs text-muted-foreground">
+                    <span>Requested by</span>
+                    <Identity name={requesterAgent.name} size="sm" className="inline-flex" />
+                  </div>
+                )}
+              </div>
+              <div className="space-y-1">
+                <h3 className="text-base font-semibold leading-6 text-foreground">
+                  {subject ?? kindLabel}
+                </h3>
+                <p className="text-xs leading-5 text-muted-foreground">
+                  Approval request created {timeAgo(approval.createdAt)}
+                </p>
+              </div>
+            </div>
          </div>
        </div>
-        <div className="flex items-center gap-1.5 shrink-0">
-          {statusIcon(approval.status)}
-          <span className="text-xs text-muted-foreground capitalize">{approval.status}</span>
-          <span className="text-xs text-muted-foreground">· {timeAgo(approval.createdAt)}</span>
+        <div className="shrink-0">
+          <div className="inline-flex items-center gap-1.5 rounded-full border border-border/70 bg-background/80 px-2.5 py-1 text-xs text-muted-foreground">
+            {statusIcon(approval.status)}
+            <span className="capitalize">{approval.status.replace(/_/g, " ")}</span>
+          </div>
        </div>
      </div>

-      {/* Payload */}
-      <ApprovalPayloadRenderer type={approval.type} payload={approval.payload} />
+      <div className="mt-4 border-t border-border/60 pt-4">
+        <ApprovalPayloadRenderer
+          type={approval.type}
+          payload={approval.payload}
+          hidePrimaryTitle={Boolean(subject)}
+        />
+      </div>

-      {/* Decision note */}
      {approval.decisionNote && (
-        <div className="mt-3 text-xs text-muted-foreground italic border-t border-border pt-2">
-          Note: {approval.decisionNote}
+        <div className="mt-4 rounded-lg border border-border/60 bg-muted/30 px-3.5 py-3 text-xs leading-5 text-muted-foreground">
+          <span className="font-medium text-foreground">Decision note.</span> {approval.decisionNote}
        </div>
      )}

-      {/* Actions */}
-      {showResolutionButtons && (
-        <div className="flex gap-2 mt-4 pt-3 border-t border-border">
-          <Button
-            size="sm"
-            className="bg-green-700 hover:bg-green-600 text-white"
-            onClick={onApprove}
-            disabled={isPending}
-          >
-            {pendingAction === "approve" ? "Approving..." : "Approve"}
-          </Button>
-          <Button
-            variant="destructive"
-            size="sm"
-            onClick={onReject}
-            disabled={isPending}
-          >
-            {pendingAction === "reject" ? "Rejecting..." : "Reject"}
-          </Button>
-        </div>
-      )}
-      {(detailLink || onOpen) ? (
-        <div className="mt-3">
-          {detailLink ? (
-            <Link
-              to={detailLink}
-              className={cn(buttonVariants({ variant: "ghost", size: "sm" }), "px-0 text-xs")}
-            >
-              View details
-            </Link>
-          ) : (
-            <Button variant="ghost" size="sm" className="text-xs px-0" onClick={onOpen}>
-              View details
-            </Button>
-          )}
+      {hasFooter ? (
+        <div className="mt-4 flex flex-wrap items-center justify-between gap-3 border-t border-border/60 pt-4">
+          <div className="flex flex-wrap items-center gap-2">
+            {showResolutionButtons && (
+              <>
+                <Button
+                  size="sm"
+                  className="bg-green-700 hover:bg-green-600 text-white"
+                  onClick={onApprove}
+                  disabled={isPending}
+                >
+                  {pendingAction === "approve" ? "Approving..." : "Approve"}
+                </Button>
+                <Button
+                  variant="destructive"
+                  size="sm"
+                  onClick={onReject}
+                  disabled={isPending}
+                >
+                  {pendingAction === "reject" ? "Rejecting..." : "Reject"}
+                </Button>
+              </>
+            )}
+          </div>
+          {(detailLink || onOpen) ? (
+            detailLink ? (
+              <Link
+                to={detailLink}
+                className={cn(buttonVariants({ variant: "ghost", size: "sm" }), "h-auto px-2 text-xs text-muted-foreground")}
+              >
+                View details
+              </Link>
+            ) : (
+              <Button variant="ghost" size="sm" className="h-auto px-2 text-xs text-muted-foreground" onClick={onOpen}>
+                View details
+              </Button>
+            )
+          ) : null}
        </div>
      ) : null}
    </div>
@@ -42,6 +42,7 @@ describe("ApprovalPayloadRenderer", () => {
            summary: "Board asked for approval before posting the frog.",
            recommendedAction: "Approve the frog reply.",
            nextActionOnApproval: "Post the frog comment on the issue.",
+            risks: ["The frog might be too powerful."],
            proposedComment: "(o)<",
          }}
        />,
@@ -52,6 +53,7 @@ describe("ApprovalPayloadRenderer", () => {
    expect(container.textContent).toContain("Board asked for approval before posting the frog.");
    expect(container.textContent).toContain("Approve the frog reply.");
    expect(container.textContent).toContain("Post the frog comment on the issue.");
+    expect(container.textContent).toContain("The frog might be too powerful.");
    expect(container.textContent).toContain("(o)<");
    expect(container.textContent).not.toContain("\"recommendedAction\"");

@@ -59,4 +61,28 @@ describe("ApprovalPayloadRenderer", () => {
      root.unmount();
    });
  });
+
+  it("can hide the repeated title when the card header already shows it", () => {
+    const root = createRoot(container);
+
+    act(() => {
+      root.render(
+        <ApprovalPayloadRenderer
+          type="request_board_approval"
+          hidePrimaryTitle
+          payload={{
+            title: "Reply with an ASCII frog",
+            summary: "Board asked for approval before posting the frog.",
+          }}
+        />,
+      );
+    });
+
+    expect(container.textContent).toContain("Board asked for approval before posting the frog.");
+    expect(container.textContent).not.toContain("TitleReply with an ASCII frog");
+
+    act(() => {
+      root.unmount();
+    });
+  });
 });
@@ -17,15 +17,19 @@ function firstNonEmptyString(...values: unknown[]): string | null {
  return null;
 }

-/** Build a contextual label for an approval, e.g. "Hire Agent: Designer" */
-export function approvalLabel(type: string, payload?: Record<string, unknown> | null): string {
-  const base = typeLabel[type] ?? type;
-  const subject = firstNonEmptyString(
+export function approvalSubject(payload?: Record<string, unknown> | null): string | null {
+  return firstNonEmptyString(
    payload?.title,
    payload?.name,
    payload?.summary,
    payload?.recommendedAction,
  );
+}
+
+/** Build a contextual label for an approval, e.g. "Hire Agent: Designer" */
+export function approvalLabel(type: string, payload?: Record<string, unknown> | null): string {
+  const base = typeLabel[type] ?? type;
+  const subject = approvalSubject(payload);
  if (subject) {
    return `${base}: ${subject}`;
  }
@@ -144,39 +148,100 @@ export function BudgetOverridePayload({ payload }: { payload: Record<string, unk
  );
 }

-export function BoardApprovalPayload({ payload }: { payload: Record<string, unknown> }) {
+export function BoardApprovalPayload({
+  payload,
+  hideTitle = false,
+}: {
+  payload: Record<string, unknown>;
+  hideTitle?: boolean;
+}) {
+  const nextPayload = hideTitle ? { ...payload, title: undefined } : payload;
  return (
-    <div className="mt-3 space-y-2 text-sm">
-      <PayloadField label="Title" value={payload.title} />
-      {!!payload.summary && (
-        <div className="flex items-start gap-2">
-          <span className="text-muted-foreground w-20 sm:w-24 shrink-0 text-xs pt-0.5">Summary</span>
-          <span className="text-muted-foreground">{String(payload.summary)}</span>
+    <BoardApprovalPayloadContent payload={nextPayload} />
+  );
+}
+
+function BoardApprovalPayloadContent({ payload }: { payload: Record<string, unknown> }) {
+  const risks = Array.isArray(payload.risks)
+    ? payload.risks
+        .filter((value): value is string => typeof value === "string")
+        .map((value) => value.trim())
+        .filter(Boolean)
+    : [];
+  const title = firstNonEmptyString(payload.title);
+  const summary = firstNonEmptyString(payload.summary);
+  const recommendedAction = firstNonEmptyString(payload.recommendedAction);
+  const nextActionOnApproval = firstNonEmptyString(payload.nextActionOnApproval);
+  const proposedComment = firstNonEmptyString(payload.proposedComment);
+
+  return (
+    <div className="mt-4 space-y-3.5 text-sm">
+      {title && (
+        <div className="space-y-1">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground">Title</p>
+          <p className="font-medium leading-6 text-foreground">{title}</p>
        </div>
      )}
-      {!!payload.recommendedAction && (
-        <div className="rounded-md bg-muted/40 px-3 py-2 text-xs text-muted-foreground">
-          Recommended: {String(payload.recommendedAction)}
+      {summary && (
+        <div className="space-y-1">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground">Summary</p>
+          <p className="leading-6 text-foreground/90">{summary}</p>
        </div>
      )}
-      {!!payload.nextActionOnApproval && (
-        <div className="flex items-start gap-2">
-          <span className="text-muted-foreground w-20 sm:w-24 shrink-0 text-xs pt-0.5">On approval</span>
-          <span>{String(payload.nextActionOnApproval)}</span>
+      {recommendedAction && (
+        <div className="rounded-lg border border-amber-500/20 bg-amber-500/10 px-3.5 py-3">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-amber-700 dark:text-amber-300">
+            Recommended action
+          </p>
+          <p className="mt-1 leading-6 text-foreground">{recommendedAction}</p>
        </div>
      )}
-      {!!payload.proposedComment && (
-        <div className="rounded-md bg-muted/40 px-3 py-2 text-xs text-muted-foreground whitespace-pre-wrap font-mono max-h-48 overflow-y-auto">
-          {String(payload.proposedComment)}
+      {nextActionOnApproval && (
+        <div className="rounded-lg border border-border/60 bg-background/60 px-3.5 py-3">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground">On approval</p>
+          <p className="mt-1 leading-6 text-foreground">{nextActionOnApproval}</p>
+        </div>
+      )}
+      {risks.length > 0 && (
+        <div className="space-y-1.5">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground">Risks</p>
+          <ul className="space-y-1 text-sm text-muted-foreground">
+            {risks.map((risk) => (
+              <li key={risk} className="flex items-start gap-2">
+                <span className="mt-2 h-1.5 w-1.5 rounded-full bg-muted-foreground/60" />
+                <span className="leading-6">{risk}</span>
+              </li>
+            ))}
+          </ul>
+        </div>
+      )}
+      {proposedComment && (
+        <div className="space-y-1.5">
+          <p className="text-[11px] font-medium uppercase tracking-[0.08em] text-muted-foreground">
+            Proposed comment
+          </p>
+          <pre className="max-h-48 overflow-auto rounded-lg border border-border/60 bg-muted/50 px-3.5 py-3 font-mono text-xs leading-5 text-muted-foreground whitespace-pre-wrap">
+            {proposedComment}
+          </pre>
        </div>
      )}
    </div>
  );
 }

-export function ApprovalPayloadRenderer({ type, payload }: { type: string; payload: Record<string, unknown> }) {
+export function ApprovalPayloadRenderer({
+  type,
+  payload,
+  hidePrimaryTitle = false,
+}: {
+  type: string;
+  payload: Record<string, unknown>;
+  hidePrimaryTitle?: boolean;
+}) {
  if (type === "hire_agent") return <HireAgentPayload payload={payload} />;
  if (type === "budget_override_required") return <BudgetOverridePayload payload={payload} />;
-  if (type === "request_board_approval") return <BoardApprovalPayload payload={payload} />;
+  if (type === "request_board_approval") {
+    return <BoardApprovalPayload payload={payload} hideTitle={hidePrimaryTitle} />;
+  }
  return <CeoStrategyPayload payload={payload} />;
 }