Skip to content

Commit 41a8153

Browse files
wgordon17wgordon17
committed
refactor(sort): makes sort preference global instead of per-tab
1 parent 4667cce commit 41a8153

File tree

7 files changed

+69
-86
lines changed

7 files changed

+69
-86
lines changed

‎src/app/components/dashboard/DashboardPage.tsx‎

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,7 @@ import PullRequestsTab from "./PullRequestsTab";
99
import PersonalSummaryStrip from "./PersonalSummaryStrip";
1010
import { config, setConfig, type TrackedUser } from "../../stores/config";
1111
import { viewState, updateViewState, setSortPreference } from "../../stores/view";
12-
import { sortOptions as issueSortOptions } from "./IssuesTab";
13-
import { sortOptions as prSortOptions } from "./PullRequestsTab";
12+
import type { SortOption } from "../shared/SortDropdown";
1413
import type { Issue, PullRequest, WorkflowRun } from "../../services/api";
1514
import { fetchOrgs } from "../../services/api";
1615
import {
@@ -373,20 +372,17 @@ export default function DashboardPage() {
373372
];
374373
});
375374

376-
const activeSortOptions = createMemo(() => {
377-
switch (activeTab()) {
378-
case "issues": return issueSortOptions;
379-
case "pullRequests": return prSortOptions;
380-
default: return undefined;
381-
}
382-
});
383-
384-
const activeSortPref = createMemo(() => {
385-
const tab = activeTab();
386-
if (tab === "actions") return undefined;
387-
const pref = viewState.sortPreferences[tab];
388-
return pref ?? { field: "updatedAt", direction: "desc" as const };
389-
});
375+
const globalSortOptions: SortOption[] = [
376+
{ label: "Repo", field: "repo", type: "text" },
377+
{ label: "Title", field: "title", type: "text" },
378+
{ label: "Author", field: "author", type: "text" },
379+
{ label: "Comments", field: "comments", type: "number" },
380+
{ label: "Checks", field: "checkStatus", type: "text" },
381+
{ label: "Review", field: "reviewDecision", type: "text" },
382+
{ label: "Size", field: "size", type: "number" },
383+
{ label: "Created", field: "createdAt", type: "date" },
384+
{ label: "Updated", field: "updatedAt", type: "date" },
385+
];
390386

391387
return (
392388
<div class="min-h-screen bg-base-200">
@@ -413,9 +409,9 @@ export default function DashboardPage() {
413409
isRefreshing={_coordinator()?.isRefreshing() ?? dashboardData.loading}
414410
lastRefreshedAt={_coordinator()?.lastRefreshAt() ?? dashboardData.lastRefreshedAt}
415411
onRefresh={() => _coordinator()?.manualRefresh()}
416-
sortOptions={activeSortOptions()}
417-
sortValue={activeSortPref()?.field}
418-
sortDirection={activeSortPref()?.direction}
412+
sortOptions={globalSortOptions}
413+
sortValue={viewState.globalSort.field}
414+
sortDirection={viewState.globalSort.direction}
419415
onSortChange={(field, dir) => setSortPreference(activeTab(), field, dir)}
420416
/>
421417
</div>

‎src/app/components/dashboard/IssuesTab.tsx‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@ import type { Issue, RepoRef } from "../../services/api";
55
import ItemRow from "./ItemRow";
66
import UserAvatarBadge, { buildSurfacedByUsers } from "../shared/UserAvatarBadge";
77
import IgnoreBadge from "./IgnoreBadge";
8-
import type { SortOption } from "../shared/SortDropdown";
98
import PaginationControls from "../shared/PaginationControls";
109
import { scopeFilterGroup, type FilterChipGroupDef } from "../shared/filterTypes";
1110
import FilterToolbar from "../shared/FilterToolbar";
@@ -50,14 +49,6 @@ const issueFilterGroups: FilterChipGroupDef[] = [
5049
},
5150
];
5251

53-
export const sortOptions: SortOption[] = [
54-
{ label: "Repo", field: "repo", type: "text" },
55-
{ label: "Title", field: "title", type: "text" },
56-
{ label: "Author", field: "author", type: "text" },
57-
{ label: "Comments", field: "comments", type: "number" },
58-
{ label: "Created", field: "createdAt", type: "date" },
59-
{ label: "Updated", field: "updatedAt", type: "date" },
60-
];
6152

6253
export default function IssuesTab(props: IssuesTabProps) {
6354
const [page, setPage] = createSignal(0);
@@ -122,10 +113,7 @@ export default function IssuesTab(props: IssuesTabProps) {
122113
const isInvolvedItem = (item: Issue) =>
123114
isUserInvolved(item, userLoginLower(), monitoredRepoNameSet());
124115

125-
const sortPref = createMemo(() => {
126-
const pref = viewState.sortPreferences["issues"];
127-
return pref ?? { field: "updatedAt", direction: "desc" as const };
128-
});
116+
const sortPref = createMemo(() => viewState.globalSort);
129117

130118
const filteredSortedWithMeta = createMemo(() => {
131119
const filter = viewState.globalFilter;

‎src/app/components/dashboard/PullRequestsTab.tsx‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import ItemRow from "./ItemRow";
99
import UserAvatarBadge, { buildSurfacedByUsers } from "../shared/UserAvatarBadge";
1010
import StatusDot from "../shared/StatusDot";
1111
import IgnoreBadge from "./IgnoreBadge";
12-
import type { SortOption } from "../shared/SortDropdown";
1312
import PaginationControls from "../shared/PaginationControls";
1413
import { scopeFilterGroup, type FilterChipGroupDef } from "../shared/filterTypes";
1514
import FilterToolbar from "../shared/FilterToolbar";
@@ -118,16 +117,6 @@ const prFilterGroups: FilterChipGroupDef[] = [
118117
},
119118
];
120119

121-
export const sortOptions: SortOption[] = [
122-
{ label: "Repo", field: "repo", type: "text" },
123-
{ label: "Title", field: "title", type: "text" },
124-
{ label: "Author", field: "author", type: "text" },
125-
{ label: "Checks", field: "checkStatus", type: "text" },
126-
{ label: "Review", field: "reviewDecision", type: "text" },
127-
{ label: "Size", field: "size", type: "number" },
128-
{ label: "Created", field: "createdAt", type: "date" },
129-
{ label: "Updated", field: "updatedAt", type: "date" },
130-
];
131120

132121
export default function PullRequestsTab(props: PullRequestsTabProps) {
133122
const [page, setPage] = createSignal(0);
@@ -193,10 +182,7 @@ export default function PullRequestsTab(props: PullRequestsTabProps) {
193182
isUserInvolved(item, userLoginLower(), monitoredRepoNameSet(),
194183
item.enriched !== false ? item.reviewerLogins : undefined);
195184

196-
const sortPref = createMemo(() => {
197-
const pref = viewState.sortPreferences["pullRequests"];
198-
return pref ?? { field: "updatedAt", direction: "desc" as const };
199-
});
185+
const sortPref = createMemo(() => viewState.globalSort);
200186

201187
const filteredSortedWithMeta = createMemo(() => {
202188
const filter = viewState.globalFilter;

‎src/app/components/layout/FilterBar.tsx‎

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@ interface FilterBarProps {
88
isRefreshing?: boolean;
99
lastRefreshedAt?: Date | null;
1010
onRefresh?: () => void;
11-
sortOptions?: SortOption[];
12-
sortValue?: string;
13-
sortDirection?: "asc" | "desc";
14-
onSortChange?: (field: string, direction: "asc" | "desc") => void;
11+
sortOptions: SortOption[];
12+
sortValue: string;
13+
sortDirection: "asc" | "desc";
14+
onSortChange: (field: string, direction: "asc" | "desc") => void;
1515
}
1616

1717
export default function FilterBar(props: FilterBarProps) {
@@ -111,14 +111,12 @@ export default function FilterBar(props: FilterBarProps) {
111111
</Select.Portal>
112112
</Select>
113113

114-
<Show when={props.sortOptions}>
115-
<SortDropdown
116-
options={props.sortOptions!}
117-
value={props.sortValue!}
118-
direction={props.sortDirection!}
119-
onChange={props.onSortChange!}
120-
/>
121-
</Show>
114+
<SortDropdown
115+
options={props.sortOptions}
116+
value={props.sortValue}
117+
direction={props.sortDirection}
118+
onChange={props.onSortChange}
119+
/>
122120

123121
<div class="flex-1" />
124122

‎src/app/stores/view.ts‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,10 @@ export const ViewStateSchema = z.object({
4848
})
4949
)
5050
.default({}),
51+
globalSort: z.object({
52+
field: z.string(),
53+
direction: z.enum(["asc", "desc"]),
54+
}).default({ field: "updatedAt", direction: "desc" }),
5155
ignoredItems: z
5256
.array(
5357
z.object({
@@ -180,13 +184,13 @@ export function pruneStaleIgnoredItems(): void {
180184
}
181185

182186
export function setSortPreference(
183-
tabId: string,
187+
_tabId: string,
184188
field: string,
185189
direction: "asc" | "desc"
186190
): void {
187191
setViewState(
188192
produce((draft) => {
189-
draft.sortPreferences[tabId] = { field, direction };
193+
draft.globalSort = { field, direction };
190194
})
191195
);
192196
}

‎tests/components/layout/FilterBar.test.tsx‎

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -39,69 +39,79 @@ afterEach(() => {
3939
vi.useRealTimers();
4040
});
4141

42+
const defaultSortProps = {
43+
sortOptions: [
44+
{ label: "Updated", field: "updatedAt", type: "date" as const },
45+
{ label: "Created", field: "createdAt", type: "date" as const },
46+
],
47+
sortValue: "updatedAt",
48+
sortDirection: "desc" as const,
49+
onSortChange: vi.fn(),
50+
};
51+
4252
describe("FilterBar", () => {
4353
it("renders org and repo filter dropdowns", () => {
44-
render(() => <FilterBar />);
54+
render(() => <FilterBar {...defaultSortProps} />);
4555
screen.getByLabelText("Filter by organization");
4656
screen.getByLabelText("Filter by repository");
4757
});
4858

4959
it("renders refresh button", () => {
50-
render(() => <FilterBar />);
60+
render(() => <FilterBar {...defaultSortProps} />);
5161
screen.getByLabelText("Refresh data");
5262
});
5363

5464
it("refresh button is enabled by default", () => {
55-
render(() => <FilterBar />);
65+
render(() => <FilterBar {...defaultSortProps} />);
5666
const refreshBtn = screen.getByLabelText("Refresh data") as HTMLButtonElement;
5767
expect(refreshBtn.disabled).toBe(false);
5868
});
5969

6070
it("refresh button is disabled when isRefreshing=true", () => {
61-
render(() => <FilterBar isRefreshing={true} />);
71+
render(() => <FilterBar {...defaultSortProps} isRefreshing={true} />);
6272
const refreshBtn = screen.getByLabelText("Refresh data") as HTMLButtonElement;
6373
expect(refreshBtn.disabled).toBe(true);
6474
});
6575

6676
it("shows 'Refreshing...' when isRefreshing=true", () => {
67-
render(() => <FilterBar isRefreshing={true} />);
77+
render(() => <FilterBar {...defaultSortProps} isRefreshing={true} />);
6878
screen.getByText("Refreshing...");
6979
});
7080

7181
it("shows last refreshed time when lastRefreshedAt provided", () => {
7282
const now = new Date();
7383
const tenSecondsAgo = new Date(now.getTime() - 10_000);
74-
render(() => <FilterBar lastRefreshedAt={tenSecondsAgo} />);
84+
render(() => <FilterBar {...defaultSortProps} lastRefreshedAt={tenSecondsAgo} />);
7585
screen.getByText("Updated 10s ago");
7686
});
7787

7888
it("shows minutes when lastRefreshedAt is more than 60s ago", () => {
7989
const twoMinutesAgo = new Date(Date.now() - 2 * 60 * 1000);
80-
render(() => <FilterBar lastRefreshedAt={twoMinutesAgo} />);
90+
render(() => <FilterBar {...defaultSortProps} lastRefreshedAt={twoMinutesAgo} />);
8191
screen.getByText("Updated 2m ago");
8292
});
8393

8494
it("does not show updated label when lastRefreshedAt is null", () => {
85-
render(() => <FilterBar lastRefreshedAt={null} />);
86-
expect(screen.queryByText(/Updated/)).toBeNull();
95+
render(() => <FilterBar {...defaultSortProps} lastRefreshedAt={null} />);
96+
expect(screen.queryByText(/Updated \d+[sm] ago/)).toBeNull();
8797
});
8898

8999
it("calls onRefresh when refresh button clicked", async () => {
90100
const user = userEvent.setup({ delay: null });
91101
const onRefresh = vi.fn();
92-
render(() => <FilterBar onRefresh={onRefresh} />);
102+
render(() => <FilterBar {...defaultSortProps} onRefresh={onRefresh} />);
93103
await user.click(screen.getByLabelText("Refresh data"));
94104
expect(onRefresh).toHaveBeenCalledOnce();
95105
});
96106

97107
it("org trigger shows 'All orgs' by default", () => {
98-
render(() => <FilterBar />);
108+
render(() => <FilterBar {...defaultSortProps} />);
99109
const orgTrigger = screen.getByLabelText("Filter by organization");
100110
expect(orgTrigger.textContent).toContain("All orgs");
101111
});
102112

103113
it("repo trigger shows 'All repos' by default", () => {
104-
render(() => <FilterBar />);
114+
render(() => <FilterBar {...defaultSortProps} />);
105115
const repoTrigger = screen.getByLabelText("Filter by repository");
106116
expect(repoTrigger.textContent).toContain("All repos");
107117
});
@@ -111,14 +121,14 @@ describe("FilterBar", () => {
111121
org: "myorg",
112122
repo: null,
113123
};
114-
render(() => <FilterBar />);
124+
render(() => <FilterBar {...defaultSortProps} />);
115125
const orgTrigger = screen.getByLabelText("Filter by organization");
116126
expect(orgTrigger.textContent).toContain("myorg");
117127
});
118128

119129
it("clicking org trigger opens listbox with org options", async () => {
120130
const user = userEvent.setup({ delay: null });
121-
render(() => <FilterBar />);
131+
render(() => <FilterBar {...defaultSortProps} />);
122132
const orgTrigger = screen.getByLabelText("Filter by organization");
123133
await user.click(orgTrigger);
124134
// Options should now be visible in the listbox
@@ -128,7 +138,7 @@ describe("FilterBar", () => {
128138

129139
it("clicking org trigger opens listbox with repo options", async () => {
130140
const user = userEvent.setup({ delay: null });
131-
render(() => <FilterBar />);
141+
render(() => <FilterBar {...defaultSortProps} />);
132142
const repoTrigger = screen.getByLabelText("Filter by repository");
133143
await user.click(repoTrigger);
134144
expect(screen.getByRole("option", { name: "myorg/repo-a" })).toBeDefined();
@@ -138,7 +148,7 @@ describe("FilterBar", () => {
138148

139149
it("selecting an org option calls setGlobalFilter and resets repo", async () => {
140150
const user = userEvent.setup({ delay: null });
141-
render(() => <FilterBar />);
151+
render(() => <FilterBar {...defaultSortProps} />);
142152
const orgTrigger = screen.getByLabelText("Filter by organization");
143153
await user.click(orgTrigger);
144154
const myorgOption = screen.getByRole("option", { name: "myorg" });
@@ -148,7 +158,7 @@ describe("FilterBar", () => {
148158

149159
it("selecting a repo option calls setGlobalFilter with current org and new repo", async () => {
150160
const user = userEvent.setup({ delay: null });
151-
render(() => <FilterBar />);
161+
render(() => <FilterBar {...defaultSortProps} />);
152162
const repoTrigger = screen.getByLabelText("Filter by repository");
153163
await user.click(repoTrigger);
154164
const repoOption = screen.getByRole("option", { name: "myorg/repo-a" });
@@ -158,7 +168,7 @@ describe("FilterBar", () => {
158168

159169
it("selecting 'All orgs' option calls setGlobalFilter with null org", async () => {
160170
const user = userEvent.setup({ delay: null });
161-
render(() => <FilterBar />);
171+
render(() => <FilterBar {...defaultSortProps} />);
162172
const orgTrigger = screen.getByLabelText("Filter by organization");
163173
await user.click(orgTrigger);
164174
const allOrgsOption = screen.getByRole("option", { name: "All orgs" });

‎tests/stores/view.test.ts‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -92,22 +92,23 @@ describe("setGlobalFilter", () => {
9292
});
9393

9494
describe("setSortPreference", () => {
95-
it("sets sort field and direction for a tab", () => {
95+
it("sets global sort field and direction", () => {
9696
setSortPreference("issues", "updatedAt", "desc");
97-
expect(viewState.sortPreferences["issues"]).toEqual({ field: "updatedAt", direction: "desc" });
97+
expect(viewState.globalSort).toEqual({ field: "updatedAt", direction: "desc" });
9898
});
9999

100-
it("updates existing sort preference for a tab", () => {
100+
it("updates existing global sort preference", () => {
101101
setSortPreference("issues", "updatedAt", "desc");
102102
setSortPreference("issues", "title", "asc");
103-
expect(viewState.sortPreferences["issues"]).toEqual({ field: "title", direction: "asc" });
103+
expect(viewState.globalSort).toEqual({ field: "title", direction: "asc" });
104104
});
105105

106-
it("sets preferences for multiple tabs independently", () => {
106+
it("global sort is shared across tabs", () => {
107107
setSortPreference("issues", "updatedAt", "desc");
108108
setSortPreference("pullRequests", "createdAt", "asc");
109-
expect(viewState.sortPreferences["issues"].field).toBe("updatedAt");
110-
expect(viewState.sortPreferences["pullRequests"].field).toBe("createdAt");
109+
// Global sort reflects the last call regardless of tab
110+
expect(viewState.globalSort.field).toBe("createdAt");
111+
expect(viewState.globalSort.direction).toBe("asc");
111112
});
112113
});
113114

0 commit comments

Comments
 (0)