feat(ui): redesign the Exploitability card and the issues verdict filter (#13093)

ui/components/issues/issues-summary.tsx

@@ -1,59 +0,0 @@
-import { CircleHelp, Layers, ShieldCheck, TriangleAlert } from "lucide-react";
-
-import { CardVariant, ResourceStatsCard } from "@/components/shadcn";
-import { IssuesOverviewResponse } from "@/types/issues";
-
-/** The fold, as the tiles above the issue list. */
-export const IssuesSummary = ({
-  overview,
-}: {
-  overview: IssuesOverviewResponse["data"]["attributes"];
-}) => {
-  return (
-    <div className="mb-6 grid grid-cols-1 gap-4 md:grid-cols-2 xl:grid-cols-5">
-      <ResourceStatsCard
-        badge={{ icon: Layers, count: overview.findings }}
-        label="Failed findings folded"
-      />
-      <ResourceStatsCard
-        accent={CardVariant.info}
-        badge={{
-          icon: Layers,
-          count: overview.issues,
-          variant: CardVariant.info,
-        }}
-        label="Issues"
-      />
-      <ResourceStatsCard
-        badge={{
-          icon: TriangleAlert,
-          count: overview.actionable,
-          variant: CardVariant.fail,
-        }}
-        label="Exploitable or likely"
-      />
-      <ResourceStatsCard
-        badge={{
-          icon: ShieldCheck,
-          count: overview.not_exploitable,
-          variant: CardVariant.pass,
-        }}
-        label="Not exploitable"
-        stats={[
-          {
-            icon: Layers,
-            label: `${overview.not_exploitable_findings} findings`,
-          },
-        ]}
-      />
-      <ResourceStatsCard
-        badge={{
-          icon: CircleHelp,
-          count: overview.unknown,
-          variant: CardVariant.warning,
-        }}
-        label="Unknown"
-      />
-    </div>
-  );
-};

ui/components/issues/issues.integration.test.tsx

@@ -237,7 +237,6 @@ describe("Issues responsive layout", () => {
             issues={1}
             findings={3}
             actionable={1}
-            notExploitableFindings={0}
             scope={{}}
           />
           <TopIssues issues={[issue]} scope={{}} />

ui/components/issues/verdict-filter.test.tsx

@@ -25,41 +25,99 @@ const counts = {
   unknown: 0,
 };
 
-const hrefOf = (name: RegExp) =>
-  screen.getByRole("link", { name }).getAttribute("href");
+const lastPushedParams = () =>
+  new URL(navigationState.push.mock.calls.at(-1)![0], "http://localhost")
+    .searchParams;
 
 describe("VerdictFilter", () => {
-  it("drops the page when a verdict is set or cleared, keeping other params", () => {
+  it("renders one tab per verdict with its count and the fold", () => {
+    navigationState.searchParams = new URLSearchParams();
+    render(<VerdictFilter summary={{ counts, total: 2, findings: 9 }} />);
+
+    expect(screen.getByRole("tab", { name: "All, 2 issues" })).toHaveAttribute(
+      "aria-selected",
+      "true",
+    );
+    expect(
+      screen.getByRole("tab", { name: "Exploitable, 2 issues" }),
+    ).toHaveAttribute("aria-selected", "false");
+    expect(screen.getAllByRole("tab")).toHaveLength(6);
+    expect(
+      screen.getByText("9 failed findings folded into 2 issues"),
+    ).toBeInTheDocument();
+  });
+
+  it("selects the verdict tab and resets the page, keeping other params", async () => {
+    const user = userEvent.setup();
     navigationState.searchParams = new URLSearchParams(
       "page=3&pageSize=10&filter[verdict]=likely",
     );
-    render(<VerdictFilter summary={{ counts, total: 2 }} />);
+    render(<VerdictFilter summary={{ counts, total: 2, findings: 9 }} />);
 
-    const set = new URLSearchParams(hrefOf(/^exploitable 2$/i)!.split("?")[1]);
-    expect(set.get("page")).toBeNull();
-    expect(set.get("pageSize")).toBe("10");
-    expect(set.get("filter[verdict]")).toBe("exploitable");
+    expect(
+      screen.getByRole("tab", { name: "Likely, 0 issues" }),
+    ).toHaveAttribute("aria-selected", "true");
+    await user.click(
+      screen.getByRole("tab", { name: "Exploitable, 2 issues" }),
+    );
 
-    const cleared = new URLSearchParams(hrefOf(/likely/i)!.split("?")[1]);
-    expect(cleared.get("page")).toBeNull();
-    expect(cleared.get("filter[verdict]")).toBeNull();
+    const params = lastPushedParams();
+    expect(params.get("page")).toBe("1");
+    expect(params.get("pageSize")).toBe("10");
+    expect(params.get("filter[verdict]")).toBe("exploitable");
   });
 
-  it("offers an All chip that clears the verdict and is active without one", () => {
+  it("marks the tabs a combined filter covers and lets All replace it", async () => {
+    const user = userEvent.setup();
+    navigationState.searchParams = new URLSearchParams(
+      "filter[verdict__in]=exploitable,likely",
+    );
+    render(<VerdictFilter summary={{ counts, total: 2, findings: 9 }} />);
+
+    // No single tab owns the combined filter, so none is selected...
+    expect(
+      screen
+        .getAllByRole("tab")
+        .every((tab) => tab.getAttribute("aria-selected") === "false"),
+    ).toBe(true);
+    // ...but the covered tabs are marked, and there is no chip for it.
+    expect(
+      screen.getByRole("tab", { name: "Exploitable, 2 issues" }),
+    ).toHaveAttribute("data-highlighted", "true");
+    expect(
+      screen.getByRole("tab", { name: "Likely, 0 issues" }),
+    ).toHaveAttribute("data-highlighted", "true");
+    expect(
+      screen.getByRole("tab", { name: "Conditional, 0 issues" }),
+    ).not.toHaveAttribute("data-highlighted");
+    expect(screen.queryByRole("region", { name: "Active filters" })).toBeNull();
+    expect(screen.getByRole("tablist")).toHaveAccessibleDescription(
+      "Showing Exploitable and Likely issues",
+    );
+
+    await user.click(screen.getByRole("tab", { name: "All, 2 issues" }));
+
+    expect(navigationState.push).toHaveBeenLastCalledWith("/issues", {
+      scroll: false,
+    });
+  });
+
+  it("shows no chip strip for a verdict picked from the tabs", () => {
     navigationState.searchParams = new URLSearchParams(
       "filter[verdict]=likely",
     );
-    render(<VerdictFilter summary={{ counts, total: 2 }} />);
+    render(<VerdictFilter summary={{ counts, total: 2, findings: 9 }} />);
 
-    expect(hrefOf(/^all 2$/i)).toBe("/issues");
+    expect(screen.queryByRole("region", { name: "Active filters" })).toBeNull();
   });
+
   it("shows the pattern filter and lets the user remove it", async () => {
     // Given
     const user = userEvent.setup();
     navigationState.searchParams = new URLSearchParams(
       "page=3&filter[pattern]=aws_public_bucket&filter[provider_id__in]=provider-1",
     );
-    render(<VerdictFilter summary={{ counts, total: 2 }} />);
+    render(<VerdictFilter summary={{ counts, total: 2, findings: 9 }} />);
     // When
     await user.click(
       screen.getByRole("button", {
@@ -67,14 +125,9 @@ describe("VerdictFilter", () => {
       }),
     );
     // Then
-    const target = new URL(
-      navigationState.push.mock.calls.at(-1)![0],
-      "http://localhost",
-    );
-    expect(target.searchParams.has("filter[pattern]")).toBe(false);
-    expect(target.searchParams.get("filter[provider_id__in]")).toBe(
-      "provider-1",
-    );
-    expect(target.searchParams.get("page")).toBe("1");
+    const target = lastPushedParams();
+    expect(target.has("filter[pattern]")).toBe(false);
+    expect(target.get("filter[provider_id__in]")).toBe("provider-1");
+    expect(target.get("page")).toBe("1");
   });
 });

ui/components/issues/verdict-filter.tsx

@@ -1,104 +1,164 @@
 "use client";
 
-import Link from "next/link";
 import { useSearchParams } from "next/navigation";
+import { ComponentProps, useId } from "react";
 
 import { ClearFiltersButton } from "@/components/filters/clear-filters-button";
 import { FilterSummaryStrip } from "@/components/filters/filter-summary-strip";
-import { Button } from "@/components/shadcn/button/button";
+import { Badge, Tabs, TabsList, TabsTrigger } from "@/components/shadcn";
 import { useUrlFilters } from "@/hooks/use-url-filters";
-import { VERDICTS } from "@/lib/issues/verdicts";
+import { VERDICT_TONE, VERDICTS, VerdictTone } from "@/lib/issues/verdicts";
+import { cn } from "@/lib/utils";
 import type { Verdict } from "@/types/issues";
 
 interface VerdictSummary {
   counts: Record<Verdict, number>;
+  /** Issues in scope, whatever their verdict. */
   total: number;
+  /** Failed findings those issues fold. */
+  findings: number;
 }
 
+const ALL_TAB = "all";
+
+/** The active tab takes the verdict's colour instead of the default accent. */
+const TONE_UNDERLINE: Record<VerdictTone, string> = {
+  [VERDICT_TONE.fail]: "after:bg-bg-fail",
+  [VERDICT_TONE.warning]: "after:bg-bg-data-medium",
+  [VERDICT_TONE.pass]: "after:bg-bg-pass",
+  [VERDICT_TONE.neutral]: "after:bg-bg-data-muted",
+};
+
+const TONE_BADGE: Record<VerdictTone, ComponentProps<typeof Badge>["variant"]> =
+  {
+    [VERDICT_TONE.fail]: "error",
+    [VERDICT_TONE.warning]: "warning",
+    [VERDICT_TONE.pass]: "success",
+    [VERDICT_TONE.neutral]: "tag",
+  };
+
+/**
+ * A combined verdict filter (the overview's "exploitable or likely" link)
+ * selects no single tab, so the tabs it covers are marked by hand.
+ */
+const HIGHLIGHT = "text-slate-900 after:scale-x-100 dark:text-white";
+
+/** Filters the issue list understands but no control on this page sets. */
 const EXTRA_FILTER_LABELS = {
   "filter[pattern]": "Pattern",
   "filter[cloud]": "Cloud",
   "filter[severity]": "Severity",
   "filter[provider_type]": "Provider type",
   "filter[provider_alias]": "Account",
-  "filter[verdict__in]": "Verdicts",
 } as const;
 
 export function VerdictFilter({ summary }: { summary?: VerdictSummary }) {
   const searchParams = useSearchParams();
-  const { clearFilter } = useUrlFilters();
+  const { clearFilter, navigateWithParams } = useUrlFilters();
   const active = searchParams.get("filter[verdict]");
-  const multiple = searchParams.get("filter[verdict__in]");
+  const combined = searchParams.get("filter[verdict__in]")?.split(",") ?? [];
+  const value = combined.length ? "" : (active ?? ALL_TAB);
+  // No tab is selected under a combined filter, so the tab list says what
+  // the list is showing.
+  const combinedLabels = combined.map(
+    (id) => VERDICTS.find((verdict) => verdict.id === id)?.label ?? id,
+  );
+  const descriptionId = useId();
 
-  const hrefFor = (verdict: Verdict | null) => {
-    const params = new URLSearchParams(searchParams.toString());
-    params.delete("page");
-    params.delete("filter[verdict__in]");
-    if (verdict === null || active === verdict)
-      params.delete("filter[verdict]");
-    else params.set("filter[verdict]", verdict);
-    return params.size ? `/issues?${params}` : "/issues";
+  const handleChange = (next: string) => {
+    navigateWithParams((params) => {
+      params.delete("filter[verdict__in]");
+      if (next === ALL_TAB) params.delete("filter[verdict]");
+      else params.set("filter[verdict]", next);
+    });
   };
 
-  const options = [
-    { id: null, label: "All", count: summary?.total },
-    ...VERDICTS.map(({ id, label }) => ({
+  const tabs = [
+    {
+      id: ALL_TAB,
+      label: "All",
+      count: summary?.total,
+      underline: "",
+      badge: "tag" as const,
+      highlighted: false,
+    },
+    ...VERDICTS.map(({ id, label, tone }) => ({
       id,
       label,
       count: summary?.counts[id],
+      underline: TONE_UNDERLINE[tone],
+      badge: TONE_BADGE[tone],
+      highlighted: combined.includes(id),
     })),
   ];
   const chips = Object.entries(EXTRA_FILTER_LABELS).flatMap(([key, label]) => {
-    const value = searchParams.get(key);
-    if (!value) return [];
-    const displayValue =
-      key === "filter[verdict__in]"
-        ? value
-            .split(",")
-            .map(
-              (id) =>
-                VERDICTS.find((verdict) => verdict.id === id)?.label ?? id,
-            )
-            .join(", ")
-        : value;
-    return [{ key, label, value, displayValue }];
+    const chipValue = searchParams.get(key);
+    return chipValue ? [{ key, label, value: chipValue }] : [];
   });
 
   return (
     <div className="mb-6 flex flex-col gap-4">
-      <div
-        className="flex flex-wrap gap-2"
-        role="group"
-        aria-label="Filter issues by verdict"
-      >
-        {options.map((option) => {
-          const isActive =
-            !multiple &&
-            (active === option.id || (!active && option.id === null));
-          return (
-            <Button
-              key={option.id ?? "all"}
-              asChild
-              variant={isActive ? "default" : "outline"}
-              size="sm"
+      <Tabs value={value} onValueChange={handleChange}>
+        <div className="flex flex-wrap items-center justify-between gap-x-6 gap-y-2">
+          <div className="min-w-0 overflow-x-auto">
+            {combinedLabels.length ? (
+              <p id={descriptionId} className="sr-only">
+                Showing {combinedLabels.join(" and ")} issues
+              </p>
+            ) : null}
+            <TabsList
+              aria-label="Filter issues by verdict"
+              aria-describedby={
+                combinedLabels.length ? descriptionId : undefined
+              }
             >
-              <Link
-                href={hrefFor(option.id)}
-                aria-current={isActive ? "true" : undefined}
-                scroll={false}
-              >
-                {option.label}
-                {option.count === undefined ? "" : ` ${option.count}`}
-              </Link>
-            </Button>
-          );
-        })}
-      </div>
-      <FilterSummaryStrip
-        chips={chips}
-        onRemove={(key) => clearFilter(key)}
-        trailingContent={<ClearFiltersButton />}
-      />
+              {tabs.map((tab) => (
+                <TabsTrigger
+                  key={tab.id}
+                  value={tab.id}
+                  data-highlighted={tab.highlighted || undefined}
+                  className={cn(
+                    "shrink-0",
+                    tab.underline,
+                    tab.highlighted && HIGHLIGHT,
+                  )}
+                  aria-label={
+                    tab.count === undefined
+                      ? undefined
+                      : `${tab.label}, ${tab.count.toLocaleString()} ${tab.count === 1 ? "issue" : "issues"}`
+                  }
+                  adornment={
+                    tab.count === undefined ? undefined : (
+                      <Badge variant={tab.badge} size="sm">
+                        {tab.count.toLocaleString()}
+                      </Badge>
+                    )
+                  }
+                >
+                  {tab.label}
+                </TabsTrigger>
+              ))}
+            </TabsList>
+          </div>
+          {summary ? (
+            // The fold: how much noise the issue count removed.
+            <p className="text-text-neutral-tertiary text-xs">
+              {summary.findings.toLocaleString()} failed findings folded into{" "}
+              {summary.total.toLocaleString()}{" "}
+              {summary.total === 1 ? "issue" : "issues"}
+            </p>
+          ) : null}
+        </div>
+      </Tabs>
+      {/* Only filters that arrived by link get a chip; the tabs and the scope
+          selectors are their own controls, so they never need one. */}
+      {chips.length ? (
+        <FilterSummaryStrip
+          chips={chips}
+          onRemove={(key) => clearFilter(key)}
+          trailingContent={<ClearFiltersButton />}
+        />
+      ) : null}
     </div>
   );
 }