From 530168136f56a52a61a61ae4bce269e25a9bd806 Mon Sep 17 00:00:00 2001 From: testvalue Date: Tue, 21 Apr 2026 21:36:13 -0400 Subject: [PATCH 1/2] fix(ui): align PR size thresholds with Prow/Kubernetes standard Adopts the industry-standard Prow size schema (XS <10, S 10-29, M 30-99, L 100-499, XL 500-999, XXL 1000+) replacing the previous broader thresholds. Adds XXL category for mega-PRs. --- src/app/components/shared/SizeBadge.tsx | 14 +++--- src/app/components/shared/filterTypes.ts | 1 + src/app/stores/view.ts | 2 +- src/shared/format.ts | 11 ++--- tests/components/PullRequestsTab.test.tsx | 7 ++- tests/components/shared-badges.test.tsx | 4 +- tests/lib/format.test.ts | 52 +++++++++++++++++------ 7 files changed, 61 insertions(+), 30 deletions(-) diff --git a/src/app/components/shared/SizeBadge.tsx b/src/app/components/shared/SizeBadge.tsx index b3ddf999..afb3b656 100644 --- a/src/app/components/shared/SizeBadge.tsx +++ b/src/app/components/shared/SizeBadge.tsx @@ -6,7 +6,7 @@ interface SizeBadgeProps { additions: number; deletions: number; changedFiles: number; - category?: "XS" | "S" | "M" | "L" | "XL"; + category?: "XS" | "S" | "M" | "L" | "XL" | "XXL"; filesUrl?: string; } @@ -16,14 +16,16 @@ const SIZE_CONFIG = { M: "badge badge-warning badge-sm", L: "badge badge-error badge-sm", XL: "badge badge-error badge-sm", + XXL: "badge badge-error badge-sm", } as const; -const SIZE_TOOLTIP: Record<"XS" | "S" | "M" | "L" | "XL", string> = { +const SIZE_TOOLTIP: Record<"XS" | "S" | "M" | "L" | "XL" | "XXL", string> = { XS: "XS: <10 lines changed", - S: "S: 10–99 lines changed", - M: "M: 100–499 lines changed", - L: "L: 500–999 lines changed", - XL: "XL: 1000+ lines changed", + S: "S: 10–29 lines changed", + M: "M: 30–99 lines changed", + L: "L: 100–499 lines changed", + XL: "XL: 500–999 lines changed", + XXL: "XXL: 1000+ lines changed", }; export default function SizeBadge(props: SizeBadgeProps) { diff --git a/src/app/components/shared/filterTypes.ts b/src/app/components/shared/filterTypes.ts index b733431b..40fd7ccf 100644 --- a/src/app/components/shared/filterTypes.ts +++ b/src/app/components/shared/filterTypes.ts @@ -84,6 +84,7 @@ export const prFilterGroups: FilterChipGroupDef[] = [ { value: "M", label: "M" }, { value: "L", label: "L" }, { value: "XL", label: "XL" }, + { value: "XXL", label: "XXL" }, ], }, ]; diff --git a/src/app/stores/view.ts b/src/app/stores/view.ts index 36582dce..af298b26 100644 --- a/src/app/stores/view.ts +++ b/src/app/stores/view.ts @@ -32,7 +32,7 @@ export const PullRequestFiltersSchema = z.object({ reviewDecision: z.enum(["all", "APPROVED", "CHANGES_REQUESTED", "REVIEW_REQUIRED", "mergeable"]).default("all"), draft: z.enum(["all", "draft", "ready"]).default("all"), checkStatus: z.enum(["all", "success", "failure", "pending", "conflict", "blocked", "none"]).default("all"), - sizeCategory: z.enum(["all", "XS", "S", "M", "L", "XL"]).default("all"), + sizeCategory: z.enum(["all", "XS", "S", "M", "L", "XL", "XXL"]).default("all"), user: z.enum(["all"]).or(z.string()).default("all"), }); diff --git a/src/shared/format.ts b/src/shared/format.ts index 4a73871e..1dd8a7f4 100644 --- a/src/shared/format.ts +++ b/src/shared/format.ts @@ -83,13 +83,14 @@ export function formatDuration(startedAt: string, completedAt: string | null): s /** * Categorizes a PR by size based on total lines changed. */ -export function prSizeCategory(additions: number, deletions: number): "XS" | "S" | "M" | "L" | "XL" { +export function prSizeCategory(additions: number, deletions: number): "XS" | "S" | "M" | "L" | "XL" | "XXL" { const total = (additions || 0) + (deletions || 0); if (total < 10) return "XS"; - if (total < 100) return "S"; - if (total < 500) return "M"; - if (total < 1000) return "L"; - return "XL"; + if (total < 30) return "S"; + if (total < 100) return "M"; + if (total < 500) return "L"; + if (total < 1000) return "XL"; + return "XXL"; } /** diff --git a/tests/components/PullRequestsTab.test.tsx b/tests/components/PullRequestsTab.test.tsx index dc1dc28d..5a41c5ba 100644 --- a/tests/components/PullRequestsTab.test.tsx +++ b/tests/components/PullRequestsTab.test.tsx @@ -166,10 +166,9 @@ describe("PullRequestsTab", () => { const pr = makePullRequest({ id: 1, title: "Big PR", additions: 300, deletions: 100, repoFullName: "org/repo-a" }); setAllExpanded("pullRequests", ["org/repo-a"], true); render(() => ); - // prSizeCategory(300, 100) = 400 total -> M - // "M" appears as a size badge - const mEls = screen.getAllByText("M"); - const badgeEl = mEls.find((el) => el.tagName.toLowerCase() === "span"); + // prSizeCategory(300, 100) = 400 total -> L + const lEls = screen.getAllByText("L"); + const badgeEl = lEls.find((el) => el.tagName.toLowerCase() === "span"); expect(badgeEl).toBeDefined(); }); diff --git a/tests/components/shared-badges.test.tsx b/tests/components/shared-badges.test.tsx index 82a78018..dc33de09 100644 --- a/tests/components/shared-badges.test.tsx +++ b/tests/components/shared-badges.test.tsx @@ -67,9 +67,9 @@ describe("SizeBadge", () => { screen.getByText("1 file"); }); - it("renders XL badge for large changes", () => { + it("renders XXL badge for large changes", () => { render(() => ); - screen.getByText("XL"); + screen.getByText("XXL"); screen.getByText("+800"); screen.getByText("-500"); screen.getByText("42 files"); diff --git a/tests/lib/format.test.ts b/tests/lib/format.test.ts index 9234cfaa..07ea7893 100644 --- a/tests/lib/format.test.ts +++ b/tests/lib/format.test.ts @@ -238,20 +238,24 @@ describe("prSizeCategory", () => { expect(prSizeCategory(3, 2)).toBe("XS"); }); - it("returns S for total 10-99", () => { - expect(prSizeCategory(50, 30)).toBe("S"); + it("returns S for total 10-29", () => { + expect(prSizeCategory(10, 5)).toBe("S"); }); - it("returns M for total 100-499", () => { - expect(prSizeCategory(200, 100)).toBe("M"); + it("returns M for total 30-99", () => { + expect(prSizeCategory(50, 30)).toBe("M"); }); - it("returns L for total 500-999", () => { - expect(prSizeCategory(600, 200)).toBe("L"); + it("returns L for total 100-499", () => { + expect(prSizeCategory(200, 100)).toBe("L"); }); - it("returns XL for total >= 1000", () => { - expect(prSizeCategory(800, 500)).toBe("XL"); + it("returns XL for total 500-999", () => { + expect(prSizeCategory(600, 200)).toBe("XL"); + }); + + it("returns XXL for total >= 1000", () => { + expect(prSizeCategory(800, 500)).toBe("XXL"); }); it("returns XS for (0, 0)", () => { @@ -266,12 +270,36 @@ describe("prSizeCategory", () => { expect(prSizeCategory(5, 5)).toBe("S"); }); - it("returns L for total 999 (boundary below 1000)", () => { - expect(prSizeCategory(500, 499)).toBe("L"); + it("returns S for total 29 (boundary below 30)", () => { + expect(prSizeCategory(15, 14)).toBe("S"); + }); + + it("returns M for total 30 (boundary at 30)", () => { + expect(prSizeCategory(15, 15)).toBe("M"); + }); + + it("returns M for total 99 (boundary below 100)", () => { + expect(prSizeCategory(50, 49)).toBe("M"); + }); + + it("returns L for total 100 (boundary at 100)", () => { + expect(prSizeCategory(50, 50)).toBe("L"); + }); + + it("returns L for total 499 (boundary below 500)", () => { + expect(prSizeCategory(250, 249)).toBe("L"); + }); + + it("returns XL for total 500 (boundary at 500)", () => { + expect(prSizeCategory(250, 250)).toBe("XL"); + }); + + it("returns XL for total 999 (boundary below 1000)", () => { + expect(prSizeCategory(500, 499)).toBe("XL"); }); - it("returns XL for total 1000 (boundary at 1000)", () => { - expect(prSizeCategory(500, 500)).toBe("XL"); + it("returns XXL for total 1000 (boundary at 1000)", () => { + expect(prSizeCategory(500, 500)).toBe("XXL"); }); it("handles NaN/undefined gracefully — defaults to XS", () => { From 1cfa6ddebd72ccabdc09e205daa4bf5691869940 Mon Sep 17 00:00:00 2001 From: testvalue Date: Thu, 23 Apr 2026 08:22:55 -0400 Subject: [PATCH 2/2] test(ui): add component-level coverage for size categories and tooltips - Add XXL sizeCategory filter integration test - Add S, M, L, XL badge rendering tests for SizeBadge - Add S and XXL tooltip text verification tests - Extract tooltip tests into nested describe with beforeEach/afterEach for robust timer lifecycle management --- tests/components/PullRequestsTab.test.tsx | 12 ++++ tests/components/shared-badges.test.tsx | 77 ++++++++++++++++++----- 2 files changed, 75 insertions(+), 14 deletions(-) diff --git a/tests/components/PullRequestsTab.test.tsx b/tests/components/PullRequestsTab.test.tsx index 5a41c5ba..b92c2415 100644 --- a/tests/components/PullRequestsTab.test.tsx +++ b/tests/components/PullRequestsTab.test.tsx @@ -244,6 +244,18 @@ describe("PullRequestsTab", () => { expect(screen.queryByText("Large PR")).toBeNull(); }); + it("filters by sizeCategory 'XXL' tab filter", () => { + const prs = [ + makePullRequest({ id: 1, title: "Huge PR", additions: 800, deletions: 500, repoFullName: "org/repo-a" }), + makePullRequest({ id: 2, title: "Medium PR", additions: 30, deletions: 20, repoFullName: "org/repo-a" }), + ]; + viewStore.setTabFilter("pullRequests", "sizeCategory", "XXL"); + setAllExpanded("pullRequests", ["org/repo-a"], true); + render(() => ); + screen.getByText("Huge PR"); + expect(screen.queryByText("Medium PR")).toBeNull(); + }); + it("groups PRs by repo with collapsible headers", () => { const prs = [ makePullRequest({ id: 1, title: "PR in repo A", repoFullName: "org/repo-a" }), diff --git a/tests/components/shared-badges.test.tsx b/tests/components/shared-badges.test.tsx index dc33de09..fcc0a286 100644 --- a/tests/components/shared-badges.test.tsx +++ b/tests/components/shared-badges.test.tsx @@ -1,4 +1,4 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { render, screen, fireEvent } from "@solidjs/testing-library"; import RoleBadge from "../../src/app/components/shared/RoleBadge"; import ReviewBadge from "../../src/app/components/shared/ReviewBadge"; @@ -67,6 +67,26 @@ describe("SizeBadge", () => { screen.getByText("1 file"); }); + it("renders S badge for small-medium changes", () => { + render(() => ); + screen.getByText("S"); + }); + + it("renders M badge for medium changes", () => { + render(() => ); + screen.getByText("M"); + }); + + it("renders L badge for large changes", () => { + render(() => ); + screen.getByText("L"); + }); + + it("renders XL badge for extra-large changes", () => { + render(() => ); + screen.getByText("XL"); + }); + it("renders XXL badge for large changes", () => { render(() => ); screen.getByText("XXL"); @@ -85,18 +105,47 @@ describe("SizeBadge", () => { screen.getByText("XS"); }); - it("shows tooltip with size description on hover", () => { - vi.useFakeTimers(); - const { container } = render(() => ( - - )); - const trigger = container.querySelector("span.inline-flex"); - expect(trigger).not.toBeNull(); - fireEvent.pointerEnter(trigger!); - vi.advanceTimersByTime(300); - expect(document.body.textContent).toContain("XS: <10 lines changed"); - fireEvent.pointerLeave(trigger!); - vi.advanceTimersByTime(500); - vi.useRealTimers(); + describe("tooltips", () => { + beforeEach(() => vi.useFakeTimers()); + afterEach(() => vi.useRealTimers()); + + it("shows tooltip with size description on hover", () => { + const { container } = render(() => ( + + )); + const trigger = container.querySelector("span.inline-flex"); + expect(trigger).not.toBeNull(); + fireEvent.pointerEnter(trigger!); + vi.advanceTimersByTime(300); + expect(document.body.textContent).toContain("XS: <10 lines changed"); + fireEvent.pointerLeave(trigger!); + vi.advanceTimersByTime(500); + }); + + it("shows tooltip with S size description on hover", () => { + const { container } = render(() => ( + + )); + const trigger = container.querySelector("span.inline-flex"); + expect(trigger).not.toBeNull(); + fireEvent.pointerEnter(trigger!); + vi.advanceTimersByTime(300); + expect(document.body.textContent).toContain("S: 10–29 lines changed"); + fireEvent.pointerLeave(trigger!); + vi.advanceTimersByTime(500); + }); + + it("shows tooltip with XXL size description on hover", () => { + const { container } = render(() => ( + + )); + const trigger = container.querySelector("span.inline-flex"); + expect(trigger).not.toBeNull(); + fireEvent.pointerEnter(trigger!); + vi.advanceTimersByTime(300); + expect(document.body.textContent).toContain("XXL: 1000+ lines changed"); + fireEvent.pointerLeave(trigger!); + vi.advanceTimersByTime(500); + }); }); });