Skip to content

Commit 9dbb639

Browse files
authored
fix(ui): align PR size thresholds with Prow/Kubernetes standard (#77)
* 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. * 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
1 parent bf6ae36 commit 9dbb639

7 files changed

Lines changed: 136 additions & 44 deletions

File tree

src/app/components/shared/SizeBadge.tsx

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ interface SizeBadgeProps {
66
additions: number;
77
deletions: number;
88
changedFiles: number;
9-
category?: "XS" | "S" | "M" | "L" | "XL";
9+
category?: "XS" | "S" | "M" | "L" | "XL" | "XXL";
1010
filesUrl?: string;
1111
}
1212

@@ -16,14 +16,16 @@ const SIZE_CONFIG = {
1616
M: "badge badge-warning badge-sm",
1717
L: "badge badge-error badge-sm",
1818
XL: "badge badge-error badge-sm",
19+
XXL: "badge badge-error badge-sm",
1920
} as const;
2021

21-
const SIZE_TOOLTIP: Record<"XS" | "S" | "M" | "L" | "XL", string> = {
22+
const SIZE_TOOLTIP: Record<"XS" | "S" | "M" | "L" | "XL" | "XXL", string> = {
2223
XS: "XS: <10 lines changed",
23-
S: "S: 10–99 lines changed",
24-
M: "M: 100–499 lines changed",
25-
L: "L: 500–999 lines changed",
26-
XL: "XL: 1000+ lines changed",
24+
S: "S: 10–29 lines changed",
25+
M: "M: 30–99 lines changed",
26+
L: "L: 100–499 lines changed",
27+
XL: "XL: 500–999 lines changed",
28+
XXL: "XXL: 1000+ lines changed",
2729
};
2830

2931
export default function SizeBadge(props: SizeBadgeProps) {

src/app/components/shared/filterTypes.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ export const prFilterGroups: FilterChipGroupDef[] = [
8484
{ value: "M", label: "M" },
8585
{ value: "L", label: "L" },
8686
{ value: "XL", label: "XL" },
87+
{ value: "XXL", label: "XXL" },
8788
],
8889
},
8990
];

src/app/stores/view.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ export const PullRequestFiltersSchema = z.object({
3232
reviewDecision: z.enum(["all", "APPROVED", "CHANGES_REQUESTED", "REVIEW_REQUIRED", "mergeable"]).default("all"),
3333
draft: z.enum(["all", "draft", "ready"]).default("all"),
3434
checkStatus: z.enum(["all", "success", "failure", "pending", "conflict", "blocked", "none"]).default("all"),
35-
sizeCategory: z.enum(["all", "XS", "S", "M", "L", "XL"]).default("all"),
35+
sizeCategory: z.enum(["all", "XS", "S", "M", "L", "XL", "XXL"]).default("all"),
3636
user: z.enum(["all"]).or(z.string()).default("all"),
3737
});
3838

src/shared/format.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -83,13 +83,14 @@ export function formatDuration(startedAt: string, completedAt: string | null): s
8383
/**
8484
* Categorizes a PR by size based on total lines changed.
8585
*/
86-
export function prSizeCategory(additions: number, deletions: number): "XS" | "S" | "M" | "L" | "XL" {
86+
export function prSizeCategory(additions: number, deletions: number): "XS" | "S" | "M" | "L" | "XL" | "XXL" {
8787
const total = (additions || 0) + (deletions || 0);
8888
if (total < 10) return "XS";
89-
if (total < 100) return "S";
90-
if (total < 500) return "M";
91-
if (total < 1000) return "L";
92-
return "XL";
89+
if (total < 30) return "S";
90+
if (total < 100) return "M";
91+
if (total < 500) return "L";
92+
if (total < 1000) return "XL";
93+
return "XXL";
9394
}
9495

9596
/**

tests/components/PullRequestsTab.test.tsx

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,9 @@ describe("PullRequestsTab", () => {
166166
const pr = makePullRequest({ id: 1, title: "Big PR", additions: 300, deletions: 100, repoFullName: "org/repo-a" });
167167
setAllExpanded("pullRequests", ["org/repo-a"], true);
168168
render(() => <PullRequestsTab pullRequests={[pr]} userLogin="" />);
169-
// prSizeCategory(300, 100) = 400 total -> M
170-
// "M" appears as a size badge
171-
const mEls = screen.getAllByText("M");
172-
const badgeEl = mEls.find((el) => el.tagName.toLowerCase() === "span");
169+
// prSizeCategory(300, 100) = 400 total -> L
170+
const lEls = screen.getAllByText("L");
171+
const badgeEl = lEls.find((el) => el.tagName.toLowerCase() === "span");
173172
expect(badgeEl).toBeDefined();
174173
});
175174

@@ -245,6 +244,18 @@ describe("PullRequestsTab", () => {
245244
expect(screen.queryByText("Large PR")).toBeNull();
246245
});
247246

247+
it("filters by sizeCategory 'XXL' tab filter", () => {
248+
const prs = [
249+
makePullRequest({ id: 1, title: "Huge PR", additions: 800, deletions: 500, repoFullName: "org/repo-a" }),
250+
makePullRequest({ id: 2, title: "Medium PR", additions: 30, deletions: 20, repoFullName: "org/repo-a" }),
251+
];
252+
viewStore.setTabFilter("pullRequests", "sizeCategory", "XXL");
253+
setAllExpanded("pullRequests", ["org/repo-a"], true);
254+
render(() => <PullRequestsTab pullRequests={prs} userLogin="" />);
255+
screen.getByText("Huge PR");
256+
expect(screen.queryByText("Medium PR")).toBeNull();
257+
});
258+
248259
it("groups PRs by repo with collapsible headers", () => {
249260
const prs = [
250261
makePullRequest({ id: 1, title: "PR in repo A", repoFullName: "org/repo-a" }),

tests/components/shared-badges.test.tsx

Lines changed: 65 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { describe, it, expect, vi } from "vitest";
1+
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
22
import { render, screen, fireEvent } from "@solidjs/testing-library";
33
import RoleBadge from "../../src/app/components/shared/RoleBadge";
44
import ReviewBadge from "../../src/app/components/shared/ReviewBadge";
@@ -67,9 +67,29 @@ describe("SizeBadge", () => {
6767
screen.getByText("1 file");
6868
});
6969

70-
it("renders XL badge for large changes", () => {
71-
render(() => <SizeBadge additions={800} deletions={500} changedFiles={42} />);
70+
it("renders S badge for small-medium changes", () => {
71+
render(() => <SizeBadge additions={15} deletions={14} changedFiles={2} />);
72+
screen.getByText("S");
73+
});
74+
75+
it("renders M badge for medium changes", () => {
76+
render(() => <SizeBadge additions={15} deletions={15} changedFiles={3} />);
77+
screen.getByText("M");
78+
});
79+
80+
it("renders L badge for large changes", () => {
81+
render(() => <SizeBadge additions={200} deletions={100} changedFiles={10} />);
82+
screen.getByText("L");
83+
});
84+
85+
it("renders XL badge for extra-large changes", () => {
86+
render(() => <SizeBadge additions={400} deletions={200} changedFiles={20} />);
7287
screen.getByText("XL");
88+
});
89+
90+
it("renders XXL badge for large changes", () => {
91+
render(() => <SizeBadge additions={800} deletions={500} changedFiles={42} />);
92+
screen.getByText("XXL");
7393
screen.getByText("+800");
7494
screen.getByText("-500");
7595
screen.getByText("42 files");
@@ -85,18 +105,47 @@ describe("SizeBadge", () => {
85105
screen.getByText("XS");
86106
});
87107

88-
it("shows tooltip with size description on hover", () => {
89-
vi.useFakeTimers();
90-
const { container } = render(() => (
91-
<SizeBadge additions={3} deletions={2} changedFiles={1} />
92-
));
93-
const trigger = container.querySelector("span.inline-flex");
94-
expect(trigger).not.toBeNull();
95-
fireEvent.pointerEnter(trigger!);
96-
vi.advanceTimersByTime(300);
97-
expect(document.body.textContent).toContain("XS: <10 lines changed");
98-
fireEvent.pointerLeave(trigger!);
99-
vi.advanceTimersByTime(500);
100-
vi.useRealTimers();
108+
describe("tooltips", () => {
109+
beforeEach(() => vi.useFakeTimers());
110+
afterEach(() => vi.useRealTimers());
111+
112+
it("shows tooltip with size description on hover", () => {
113+
const { container } = render(() => (
114+
<SizeBadge additions={3} deletions={2} changedFiles={1} />
115+
));
116+
const trigger = container.querySelector("span.inline-flex");
117+
expect(trigger).not.toBeNull();
118+
fireEvent.pointerEnter(trigger!);
119+
vi.advanceTimersByTime(300);
120+
expect(document.body.textContent).toContain("XS: <10 lines changed");
121+
fireEvent.pointerLeave(trigger!);
122+
vi.advanceTimersByTime(500);
123+
});
124+
125+
it("shows tooltip with S size description on hover", () => {
126+
const { container } = render(() => (
127+
<SizeBadge additions={10} deletions={5} changedFiles={2} />
128+
));
129+
const trigger = container.querySelector("span.inline-flex");
130+
expect(trigger).not.toBeNull();
131+
fireEvent.pointerEnter(trigger!);
132+
vi.advanceTimersByTime(300);
133+
expect(document.body.textContent).toContain("S: 10–29 lines changed");
134+
fireEvent.pointerLeave(trigger!);
135+
vi.advanceTimersByTime(500);
136+
});
137+
138+
it("shows tooltip with XXL size description on hover", () => {
139+
const { container } = render(() => (
140+
<SizeBadge additions={800} deletions={500} changedFiles={42} />
141+
));
142+
const trigger = container.querySelector("span.inline-flex");
143+
expect(trigger).not.toBeNull();
144+
fireEvent.pointerEnter(trigger!);
145+
vi.advanceTimersByTime(300);
146+
expect(document.body.textContent).toContain("XXL: 1000+ lines changed");
147+
fireEvent.pointerLeave(trigger!);
148+
vi.advanceTimersByTime(500);
149+
});
101150
});
102151
});

tests/lib/format.test.ts

Lines changed: 40 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -238,20 +238,24 @@ describe("prSizeCategory", () => {
238238
expect(prSizeCategory(3, 2)).toBe("XS");
239239
});
240240

241-
it("returns S for total 10-99", () => {
242-
expect(prSizeCategory(50, 30)).toBe("S");
241+
it("returns S for total 10-29", () => {
242+
expect(prSizeCategory(10, 5)).toBe("S");
243243
});
244244

245-
it("returns M for total 100-499", () => {
246-
expect(prSizeCategory(200, 100)).toBe("M");
245+
it("returns M for total 30-99", () => {
246+
expect(prSizeCategory(50, 30)).toBe("M");
247247
});
248248

249-
it("returns L for total 500-999", () => {
250-
expect(prSizeCategory(600, 200)).toBe("L");
249+
it("returns L for total 100-499", () => {
250+
expect(prSizeCategory(200, 100)).toBe("L");
251251
});
252252

253-
it("returns XL for total >= 1000", () => {
254-
expect(prSizeCategory(800, 500)).toBe("XL");
253+
it("returns XL for total 500-999", () => {
254+
expect(prSizeCategory(600, 200)).toBe("XL");
255+
});
256+
257+
it("returns XXL for total >= 1000", () => {
258+
expect(prSizeCategory(800, 500)).toBe("XXL");
255259
});
256260

257261
it("returns XS for (0, 0)", () => {
@@ -266,12 +270,36 @@ describe("prSizeCategory", () => {
266270
expect(prSizeCategory(5, 5)).toBe("S");
267271
});
268272

269-
it("returns L for total 999 (boundary below 1000)", () => {
270-
expect(prSizeCategory(500, 499)).toBe("L");
273+
it("returns S for total 29 (boundary below 30)", () => {
274+
expect(prSizeCategory(15, 14)).toBe("S");
275+
});
276+
277+
it("returns M for total 30 (boundary at 30)", () => {
278+
expect(prSizeCategory(15, 15)).toBe("M");
279+
});
280+
281+
it("returns M for total 99 (boundary below 100)", () => {
282+
expect(prSizeCategory(50, 49)).toBe("M");
283+
});
284+
285+
it("returns L for total 100 (boundary at 100)", () => {
286+
expect(prSizeCategory(50, 50)).toBe("L");
287+
});
288+
289+
it("returns L for total 499 (boundary below 500)", () => {
290+
expect(prSizeCategory(250, 249)).toBe("L");
291+
});
292+
293+
it("returns XL for total 500 (boundary at 500)", () => {
294+
expect(prSizeCategory(250, 250)).toBe("XL");
295+
});
296+
297+
it("returns XL for total 999 (boundary below 1000)", () => {
298+
expect(prSizeCategory(500, 499)).toBe("XL");
271299
});
272300

273-
it("returns XL for total 1000 (boundary at 1000)", () => {
274-
expect(prSizeCategory(500, 500)).toBe("XL");
301+
it("returns XXL for total 1000 (boundary at 1000)", () => {
302+
expect(prSizeCategory(500, 500)).toBe("XXL");
275303
});
276304

277305
it("handles NaN/undefined gracefully — defaults to XS", () => {

0 commit comments

Comments
 (0)