Skip to content

Commit 7ab7419

Browse files
authored
fix(ui): restore a green Frontend Quality job (#98)
* fix(ui): satisfy the react-hooks lint rules that break CI The `Frontend Quality` job fails on main because `npm run lint` reports two errors from the react-hooks rules that ship with eslint-config-next 16.3.x: * `react-hooks/immutability` in `file-upload.tsx` - `formatSize` was declared after the `handleFileSelect` callback that reads it. * `react-hooks/set-state-in-effect` in the errors page - the effect called a fetch helper that synchronously flipped the loading state. `formatSize` was duplicated in `file-upload.tsx` and `file-browser.tsx`, so it moves to `lib/utils.ts` as `formatBytes` (module scope, covered by tests) and both components use it. The errors page moves to React Query, matching every other data-loading page in the app, which removes the effect entirely. * fix(ui): set explicit display names on the primitive wrappers Recent Radix UI releases no longer assign `displayName` to their primitives, so every wrapper that copied it (`Label.displayName = LabelPrimitive.Root.displayName` and friends) ended up with `undefined`. That is invisible in the browser but breaks eight assertions in the component suites and leaves React DevTools showing anonymous `ForwardRef` nodes. Each wrapper now declares its own name, which is what the rest of the ui components already did. The scroll bar keeps the name of the exported component, `ScrollBar`, instead of the Radix-internal `ScrollAreaScrollbar`. * test(ui): simulate a full click when dismissing the popover Radix now passes `deferPointerDownOutside: true` for popovers, so an outside pointerdown no longer dismisses on its own - the layer waits for the click that follows, which stops a drag that ends outside the popover from closing it. The test fired only `pointerDown` and therefore never saw `onOpenChange`.
1 parent 2294805 commit 7ab7419

17 files changed

Lines changed: 148 additions & 87 deletions

ui/src/app/[locale]/errors/page.tsx

Lines changed: 55 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,17 @@
11
"use client";
22

3-
import { useCallback, useEffect, useState } from "react";
3+
import { useState } from "react";
4+
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
45
import { CheckCircle, RefreshCw, Copy } from "lucide-react";
56
import { format } from "date-fns";
67

8+
/** Base path of the error-tracking endpoints. */
9+
const ERRORS_ENDPOINT = "/api/v1/errors";
10+
/** React Query cache key for the error list. */
11+
const ERRORS_QUERY_KEY = "errors";
12+
/** Filter value that disables source filtering. */
13+
const ALL_SOURCES = "all";
14+
715
type ErrorEvent = {
816
id: string;
917
source_component: string;
@@ -15,43 +23,53 @@ type ErrorEvent = {
1523
created_at: string;
1624
};
1725

26+
async function fetchErrorEvents(source: string): Promise<ErrorEvent[]> {
27+
const url = source !== ALL_SOURCES
28+
? `${ERRORS_ENDPOINT}?source=${encodeURIComponent(source)}`
29+
: ERRORS_ENDPOINT;
30+
31+
const res = await fetch(url);
32+
if (!res.ok) {
33+
throw new Error(`Failed to fetch errors: ${res.status}`);
34+
}
35+
const data = await res.json();
36+
return data ?? [];
37+
}
38+
39+
async function resolveErrorEvent(id: string): Promise<void> {
40+
const res = await fetch(`${ERRORS_ENDPOINT}/${encodeURIComponent(id)}/resolve`, {
41+
method: "PATCH",
42+
});
43+
if (!res.ok) {
44+
throw new Error(`Failed to resolve error: ${res.status}`);
45+
}
46+
}
47+
1848
export default function ErrorsPage() {
19-
const [errors, setErrors] = useState<ErrorEvent[]>([]);
20-
const [loading, setLoading] = useState(true);
21-
const [filterSource, setFilterSource] = useState("all");
22-
23-
const fetchErrors = useCallback(async () => {
24-
setLoading(true);
25-
try {
26-
const url = filterSource !== "all"
27-
? `/api/v1/errors?source=${filterSource}`
28-
: `/api/v1/errors`;
29-
30-
const res = await fetch(url);
31-
if (res.ok) {
32-
const data = await res.json();
33-
setErrors(data || []);
34-
}
35-
} catch (e) {
36-
console.error("Failed to fetch errors", e);
37-
} finally {
38-
setLoading(false);
39-
}
40-
}, [filterSource]);
41-
42-
useEffect(() => {
43-
void fetchErrors();
44-
}, [fetchErrors]);
45-
46-
const handleResolve = async (id: string) => {
47-
try {
48-
const res = await fetch(`/api/v1/errors/${id}/resolve`, { method: "PATCH" });
49-
if (res.ok) {
50-
await fetchErrors();
51-
}
52-
} catch (e) {
49+
const [filterSource, setFilterSource] = useState(ALL_SOURCES);
50+
const queryClient = useQueryClient();
51+
52+
const {
53+
data: errors = [],
54+
isFetching: loading,
55+
refetch,
56+
} = useQuery({
57+
queryKey: [ERRORS_QUERY_KEY, filterSource],
58+
queryFn: () => fetchErrorEvents(filterSource),
59+
});
60+
61+
const resolveMutation = useMutation({
62+
mutationFn: resolveErrorEvent,
63+
onSuccess: () => {
64+
void queryClient.invalidateQueries({ queryKey: [ERRORS_QUERY_KEY] });
65+
},
66+
onError: (e) => {
5367
console.error("Failed to resolve error", e);
54-
}
68+
},
69+
});
70+
71+
const handleResolve = (id: string) => {
72+
resolveMutation.mutate(id);
5573
};
5674

5775
const getSeverityColor = (severity: string) => {
@@ -89,7 +107,7 @@ export default function ErrorsPage() {
89107
<option value="worker">Worker</option>
90108
</select>
91109
<button
92-
onClick={() => void fetchErrors()}
110+
onClick={() => void refetch()}
93111
className="p-2 hover:bg-white/10 rounded-md transition-colors"
94112
>
95113
<RefreshCw className={`w-5 h-5 ${loading ? "animate-spin" : ""}`} />

ui/src/components/file-browser.tsx

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ import {
2525
HardDrive,
2626
Check,
2727
} from "lucide-react";
28+
import { formatBytes } from "@/lib/utils";
29+
30+
/** Placeholder rendered where a size or date is unknown. */
31+
const EMPTY_VALUE_LABEL = "—";
2832

2933
interface FileBrowserProps {
3034
onSelect: (path: string) => void;
@@ -82,15 +86,8 @@ export function FileBrowser({ onSelect, selectedPath, mediaOnly = true }: FileBr
8286
setSearchQuery("");
8387
};
8488

85-
const formatSize = (bytes: number): string => {
86-
if (bytes === 0) return "—";
87-
const units = ["B", "KB", "MB", "GB", "TB"];
88-
const i = Math.floor(Math.log(bytes) / Math.log(1024));
89-
return `${(bytes / Math.pow(1024, i)).toFixed(1)} ${units[i]}`;
90-
};
91-
9289
const formatDate = (timestamp: number): string => {
93-
if (!timestamp) return "—";
90+
if (!timestamp) return EMPTY_VALUE_LABEL;
9491
return new Date(timestamp * 1000).toLocaleDateString();
9592
};
9693

@@ -250,7 +247,7 @@ export function FileBrowser({ onSelect, selectedPath, mediaOnly = true }: FileBr
250247
</div>
251248
{!entry.is_directory && (
252249
<div className="text-xs text-muted-foreground flex items-center gap-2">
253-
<span>{formatSize(entry.size)}</span>
250+
<span>{formatBytes(entry.size, EMPTY_VALUE_LABEL)}</span>
254251
<span></span>
255252
<span>{formatDate(entry.mod_time)}</span>
256253
{entry.extension && (

ui/src/components/file-upload.tsx

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import { useState, useCallback, useRef } from "react";
44
import { useMutation } from "@tanstack/react-query";
55
import { uploadFile, reportError, UploadProgress, UploadResponse } from "@/lib/api";
6+
import { formatBytes } from "@/lib/utils";
67
import { Progress } from "@/components/ui/progress";
78
import { Button } from "@/components/ui/button";
89
import {
@@ -68,7 +69,7 @@ export function FileUpload({
6869
(file: File) => {
6970
// Validate file size
7071
if (file.size > maxSize) {
71-
setError(`File size exceeds maximum of ${formatSize(maxSize)}`);
72+
setError(`File size exceeds maximum of ${formatBytes(maxSize)}`);
7273
setState("error");
7374
return;
7475
}
@@ -162,13 +163,6 @@ export function FileUpload({
162163
return <File className="h-8 w-8 text-muted-foreground" />;
163164
};
164165

165-
const formatSize = (bytes: number): string => {
166-
if (bytes === 0) return "0 B";
167-
const units = ["B", "KB", "MB", "GB", "TB"];
168-
const i = Math.floor(Math.log(bytes) / Math.log(1024));
169-
return `${(bytes / Math.pow(1024, i)).toFixed(1)} ${units[i]}`;
170-
};
171-
172166
return (
173167
<div className="space-y-4">
174168
<input
@@ -212,7 +206,7 @@ export function FileUpload({
212206
{isDragging ? "Drop your file here" : "Drag and drop your video file"}
213207
</p>
214208
<p className="text-sm text-muted-foreground mt-1">
215-
or click to browse • Max {formatSize(maxSize)}
209+
or click to browse • Max {formatBytes(maxSize)}
216210
</p>
217211
</div>
218212
<div className="flex gap-2 flex-wrap justify-center text-xs text-muted-foreground">
@@ -238,7 +232,7 @@ export function FileUpload({
238232
<div className="flex-1 min-w-0">
239233
<p className="font-medium truncate">{selectedFile.name}</p>
240234
<div className="flex items-center gap-3 text-sm text-muted-foreground mt-1">
241-
<span>{formatSize(selectedFile.size)}</span>
235+
<span>{formatBytes(selectedFile.size)}</span>
242236
<span></span>
243237
<span>{selectedFile.type || "Unknown type"}</span>
244238
</div>
@@ -275,7 +269,7 @@ export function FileUpload({
275269
<p className="font-medium truncate">{selectedFile.name}</p>
276270
<div className="flex items-center gap-3 text-sm text-muted-foreground mt-1">
277271
<span>
278-
{formatSize(progress.loaded)} / {formatSize(progress.total)}
272+
{formatBytes(progress.loaded)} / {formatBytes(progress.total)}
279273
</span>
280274
<span></span>
281275
<span className="text-violet-400 font-medium">{progress.percentage}%</span>

ui/src/components/ui/checkbox.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,6 @@ const Checkbox = React.forwardRef<
2525
</CheckboxPrimitive.Indicator>
2626
</CheckboxPrimitive.Root>
2727
))
28-
Checkbox.displayName = CheckboxPrimitive.Root.displayName
28+
Checkbox.displayName = "Checkbox"
2929

3030
export { Checkbox }

ui/src/components/ui/dialog.tsx

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const DialogOverlay = React.forwardRef<
2727
{...props}
2828
/>
2929
))
30-
DialogOverlay.displayName = DialogPrimitive.Overlay.displayName
30+
DialogOverlay.displayName = "DialogOverlay"
3131

3232
const DialogContent = React.forwardRef<
3333
React.ElementRef<typeof DialogPrimitive.Content>,
@@ -51,7 +51,7 @@ const DialogContent = React.forwardRef<
5151
</DialogPrimitive.Content>
5252
</DialogPortal>
5353
))
54-
DialogContent.displayName = DialogPrimitive.Content.displayName
54+
DialogContent.displayName = "DialogContent"
5555

5656
const DialogHeader = ({
5757
className,
@@ -94,7 +94,7 @@ const DialogTitle = React.forwardRef<
9494
{...props}
9595
/>
9696
))
97-
DialogTitle.displayName = DialogPrimitive.Title.displayName
97+
DialogTitle.displayName = "DialogTitle"
9898

9999
const DialogDescription = React.forwardRef<
100100
React.ElementRef<typeof DialogPrimitive.Description>,
@@ -106,7 +106,7 @@ const DialogDescription = React.forwardRef<
106106
{...props}
107107
/>
108108
))
109-
DialogDescription.displayName = DialogPrimitive.Description.displayName
109+
DialogDescription.displayName = "DialogDescription"
110110

111111
export {
112112
Dialog,

ui/src/components/ui/label.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,6 @@ const Label = React.forwardRef<
2121
{...props}
2222
/>
2323
))
24-
Label.displayName = LabelPrimitive.Root.displayName
24+
Label.displayName = "Label"
2525

2626
export { Label }

ui/src/components/ui/popover.test.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -222,8 +222,15 @@ describe('Popover', () => {
222222
expect(screen.getByText('Content')).toBeInTheDocument()
223223
})
224224

225-
fireEvent.pointerDown(screen.getByTestId('outside'))
226-
expect(handleOpenChange).toHaveBeenCalledWith(false)
225+
// Radix defers the popover's dismissal to the click that follows the
226+
// outside pointerdown, so the full interaction has to be simulated.
227+
const outside = screen.getByTestId('outside')
228+
fireEvent.pointerDown(outside)
229+
fireEvent.click(outside)
230+
231+
await waitFor(() => {
232+
expect(handleOpenChange).toHaveBeenCalledWith(false)
233+
})
227234
})
228235
})
229236

ui/src/components/ui/popover.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,6 @@ const PopoverContent = React.forwardRef<
2828
/>
2929
</PopoverPrimitive.Portal>
3030
))
31-
PopoverContent.displayName = PopoverPrimitive.Content.displayName
31+
PopoverContent.displayName = "PopoverContent"
3232

3333
export { Popover, PopoverTrigger, PopoverContent, PopoverAnchor }

ui/src/components/ui/progress.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,6 @@ const Progress = React.forwardRef<
2323
/>
2424
</ProgressPrimitive.Root>
2525
))
26-
Progress.displayName = ProgressPrimitive.Root.displayName
26+
Progress.displayName = "Progress"
2727

2828
export { Progress }

ui/src/components/ui/scroll-area.test.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ describe('ScrollArea', () => {
179179
describe('ScrollBar', () => {
180180
describe('rendering', () => {
181181
it('should have correct displayName', () => {
182-
expect(ScrollBar.displayName).toBe('ScrollAreaScrollbar')
182+
expect(ScrollBar.displayName).toBe('ScrollBar')
183183
})
184184
})
185185

0 commit comments

Comments
 (0)