Skip to content

Commit 1e63a2e

Browse files
authored
fix(ui): resolve Recharts dimension warnings and prevent Websocket close warning (#8)
1 parent fb0f56a commit 1e63a2e

File tree

3 files changed

+173
-110
lines changed

3 files changed

+173
-110
lines changed

ui/src/components/charts/HistoryChart.tsx

Lines changed: 103 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,6 @@
1-
import { useState } from 'react';
1+
import { useEffect, useRef, useState } from 'react';
22
import { useQuery } from '@tanstack/react-query';
3-
import {
4-
BarChart,
5-
Bar,
6-
LineChart,
7-
Line,
8-
XAxis,
9-
YAxis,
10-
Tooltip,
11-
ResponsiveContainer,
12-
} from 'recharts';
3+
import { BarChart, Bar, LineChart, Line, XAxis, YAxis, Tooltip } from 'recharts';
134
import { api } from '../../api/client';
145
import type { HistoryStatsTimeRange } from '../../types';
156

@@ -34,6 +25,8 @@ const COLORS = {
3425
cancelled: '#71717a', // zinc-500
3526
};
3627

28+
const CHART_HEIGHT = 192; // h-48 = 12rem = 192px
29+
3730
function formatTimestamp(timestamp: string, range: HistoryStatsTimeRange): string {
3831
const date = new Date(timestamp);
3932

@@ -82,9 +75,33 @@ function CustomTooltip({ active, payload, label }: CustomTooltipProps) {
8275
}
8376

8477
export function HistoryChart({ groupId }: HistoryChartProps) {
78+
const containerRef = useRef<HTMLDivElement>(null);
79+
const [containerWidth, setContainerWidth] = useState(0);
8580
const [timeRange, setTimeRange] = useState<HistoryStatsTimeRange>('auto');
8681
const [chartType, setChartType] = useState<ChartType>('bar');
8782

83+
// Track container width with ResizeObserver.
84+
useEffect(() => {
85+
const container = containerRef.current;
86+
if (!container) return;
87+
88+
const updateWidth = () => {
89+
const { width } = container.getBoundingClientRect();
90+
if (width > 0) {
91+
setContainerWidth(width);
92+
}
93+
};
94+
95+
// Check immediately.
96+
updateWidth();
97+
98+
// Observe for size changes.
99+
const observer = new ResizeObserver(updateWidth);
100+
observer.observe(container);
101+
102+
return () => observer.disconnect();
103+
}, []);
104+
88105
const { data, isLoading, error } = useQuery({
89106
queryKey: ['historyStats', groupId, timeRange],
90107
queryFn: () => api.getHistoryStats(groupId, timeRange),
@@ -152,7 +169,7 @@ export function HistoryChart({ groupId }: HistoryChartProps) {
152169
</div>
153170

154171
{/* Chart */}
155-
<div className="h-48">
172+
<div ref={containerRef} className="h-48">
156173
{isLoading ? (
157174
<div className="flex h-full items-center justify-center">
158175
<div className="text-sm text-zinc-500">Loading...</div>
@@ -165,72 +182,80 @@ export function HistoryChart({ groupId }: HistoryChartProps) {
165182
<div className="flex h-full items-center justify-center">
166183
<div className="text-sm text-zinc-500">No historical data available</div>
167184
</div>
168-
) : (
169-
<ResponsiveContainer width="100%" height="100%">
170-
{chartType === 'bar' ? (
171-
<BarChart data={chartData} margin={{ top: 5, right: 5, left: -20, bottom: 5 }}>
172-
<XAxis
173-
dataKey="timestamp"
174-
tick={{ fontSize: 10, fill: '#71717a' }}
175-
tickLine={false}
176-
axisLine={{ stroke: '#3f3f46' }}
177-
interval="preserveStartEnd"
178-
/>
179-
<YAxis
180-
tick={{ fontSize: 10, fill: '#71717a' }}
181-
tickLine={false}
182-
axisLine={false}
183-
allowDecimals={false}
184-
/>
185-
<Tooltip content={<CustomTooltip />} />
186-
<Bar dataKey="completed" stackId="a" fill={COLORS.completed} name="Completed" />
187-
<Bar dataKey="failed" stackId="a" fill={COLORS.failed} name="Failed" />
188-
<Bar dataKey="cancelled" stackId="a" fill={COLORS.cancelled} name="Cancelled" />
189-
</BarChart>
190-
) : (
191-
<LineChart data={chartData} margin={{ top: 5, right: 5, left: -20, bottom: 5 }}>
192-
<XAxis
193-
dataKey="timestamp"
194-
tick={{ fontSize: 10, fill: '#71717a' }}
195-
tickLine={false}
196-
axisLine={{ stroke: '#3f3f46' }}
197-
interval="preserveStartEnd"
198-
/>
199-
<YAxis
200-
tick={{ fontSize: 10, fill: '#71717a' }}
201-
tickLine={false}
202-
axisLine={false}
203-
allowDecimals={false}
204-
/>
205-
<Tooltip content={<CustomTooltip />} />
206-
<Line
207-
type="monotone"
208-
dataKey="completed"
209-
stroke={COLORS.completed}
210-
strokeWidth={2}
211-
dot={false}
212-
name="Completed"
213-
/>
214-
<Line
215-
type="monotone"
216-
dataKey="failed"
217-
stroke={COLORS.failed}
218-
strokeWidth={2}
219-
dot={false}
220-
name="Failed"
221-
/>
222-
<Line
223-
type="monotone"
224-
dataKey="cancelled"
225-
stroke={COLORS.cancelled}
226-
strokeWidth={2}
227-
dot={false}
228-
name="Cancelled"
229-
/>
230-
</LineChart>
231-
)}
232-
</ResponsiveContainer>
233-
)}
185+
) : containerWidth > 0 ? (
186+
chartType === 'bar' ? (
187+
<BarChart
188+
width={containerWidth}
189+
height={CHART_HEIGHT}
190+
data={chartData}
191+
margin={{ top: 5, right: 5, left: -20, bottom: 5 }}
192+
>
193+
<XAxis
194+
dataKey="timestamp"
195+
tick={{ fontSize: 10, fill: '#71717a' }}
196+
tickLine={false}
197+
axisLine={{ stroke: '#3f3f46' }}
198+
interval="preserveStartEnd"
199+
/>
200+
<YAxis
201+
tick={{ fontSize: 10, fill: '#71717a' }}
202+
tickLine={false}
203+
axisLine={false}
204+
allowDecimals={false}
205+
/>
206+
<Tooltip content={<CustomTooltip />} />
207+
<Bar dataKey="completed" stackId="a" fill={COLORS.completed} name="Completed" />
208+
<Bar dataKey="failed" stackId="a" fill={COLORS.failed} name="Failed" />
209+
<Bar dataKey="cancelled" stackId="a" fill={COLORS.cancelled} name="Cancelled" />
210+
</BarChart>
211+
) : (
212+
<LineChart
213+
width={containerWidth}
214+
height={CHART_HEIGHT}
215+
data={chartData}
216+
margin={{ top: 5, right: 5, left: -20, bottom: 5 }}
217+
>
218+
<XAxis
219+
dataKey="timestamp"
220+
tick={{ fontSize: 10, fill: '#71717a' }}
221+
tickLine={false}
222+
axisLine={{ stroke: '#3f3f46' }}
223+
interval="preserveStartEnd"
224+
/>
225+
<YAxis
226+
tick={{ fontSize: 10, fill: '#71717a' }}
227+
tickLine={false}
228+
axisLine={false}
229+
allowDecimals={false}
230+
/>
231+
<Tooltip content={<CustomTooltip />} />
232+
<Line
233+
type="monotone"
234+
dataKey="completed"
235+
stroke={COLORS.completed}
236+
strokeWidth={2}
237+
dot={false}
238+
name="Completed"
239+
/>
240+
<Line
241+
type="monotone"
242+
dataKey="failed"
243+
stroke={COLORS.failed}
244+
strokeWidth={2}
245+
dot={false}
246+
name="Failed"
247+
/>
248+
<Line
249+
type="monotone"
250+
dataKey="cancelled"
251+
stroke={COLORS.cancelled}
252+
strokeWidth={2}
253+
dot={false}
254+
name="Cancelled"
255+
/>
256+
</LineChart>
257+
)
258+
) : null}
234259
</div>
235260

236261
{/* Legend with totals */}

ui/src/components/charts/MiniHistoryChart.tsx

Lines changed: 46 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
1+
import { useEffect, useRef, useState } from 'react';
12
import { useQuery } from '@tanstack/react-query';
2-
import {
3-
BarChart,
4-
Bar,
5-
XAxis,
6-
YAxis,
7-
Tooltip,
8-
ResponsiveContainer,
9-
} from 'recharts';
3+
import { BarChart, Bar, XAxis, YAxis, Tooltip } from 'recharts';
104
import { api } from '../../api/client';
115

126
interface MiniHistoryChartProps {
@@ -51,13 +45,40 @@ function CustomTooltip({ active, payload, label }: CustomTooltipProps) {
5145
);
5246
}
5347

48+
const CHART_HEIGHT = 80; // h-20 = 5rem = 80px
49+
5450
export function MiniHistoryChart({ groupId }: MiniHistoryChartProps) {
51+
const containerRef = useRef<HTMLDivElement>(null);
52+
const [containerWidth, setContainerWidth] = useState(0);
53+
5554
const { data, isLoading } = useQuery({
5655
queryKey: ['historyStats', groupId, '24h'],
5756
queryFn: () => api.getHistoryStats(groupId, '24h'),
5857
refetchInterval: 60000,
5958
});
6059

60+
// Track container width with ResizeObserver.
61+
useEffect(() => {
62+
const container = containerRef.current;
63+
if (!container) return;
64+
65+
const updateWidth = () => {
66+
const { width } = container.getBoundingClientRect();
67+
if (width > 0) {
68+
setContainerWidth(width);
69+
}
70+
};
71+
72+
// Check immediately.
73+
updateWidth();
74+
75+
// Observe for size changes.
76+
const observer = new ResizeObserver(updateWidth);
77+
observer.observe(container);
78+
79+
return () => observer.disconnect();
80+
}, []);
81+
6182
const chartData = data?.buckets.map((bucket) => ({
6283
timestamp: formatTimestamp(bucket.timestamp),
6384
completed: bucket.completed,
@@ -67,26 +88,23 @@ export function MiniHistoryChart({ groupId }: MiniHistoryChartProps) {
6788

6889
const hasData = data && data.totals.completed + data.totals.failed + data.totals.cancelled > 0;
6990

70-
if (isLoading) {
71-
return (
72-
<div className="h-20 flex items-center justify-center">
73-
<div className="text-xs text-zinc-600">Loading...</div>
74-
</div>
75-
);
76-
}
77-
78-
if (!hasData) {
79-
return (
80-
<div className="h-20 flex items-center justify-center">
81-
<div className="text-xs text-zinc-600">No history data</div>
82-
</div>
83-
);
84-
}
85-
8691
return (
87-
<div className="h-20">
88-
<ResponsiveContainer width="100%" height="100%">
89-
<BarChart data={chartData} margin={{ top: 5, right: 5, left: -30, bottom: 0 }}>
92+
<div ref={containerRef} className="h-20">
93+
{isLoading ? (
94+
<div className="flex h-full items-center justify-center">
95+
<div className="text-xs text-zinc-600">Loading...</div>
96+
</div>
97+
) : !hasData ? (
98+
<div className="flex h-full items-center justify-center">
99+
<div className="text-xs text-zinc-600">No history data</div>
100+
</div>
101+
) : containerWidth > 0 ? (
102+
<BarChart
103+
width={containerWidth}
104+
height={CHART_HEIGHT}
105+
data={chartData}
106+
margin={{ top: 5, right: 5, left: -30, bottom: 0 }}
107+
>
90108
<XAxis
91109
dataKey="timestamp"
92110
tick={{ fontSize: 8, fill: '#52525b' }}
@@ -106,7 +124,7 @@ export function MiniHistoryChart({ groupId }: MiniHistoryChartProps) {
106124
<Bar dataKey="failed" stackId="a" fill={COLORS.failed} />
107125
<Bar dataKey="cancelled" stackId="a" fill={COLORS.cancelled} />
108126
</BarChart>
109-
</ResponsiveContainer>
127+
) : null}
110128
</div>
111129
);
112130
}

ui/src/hooks/useWebSocket.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
3333
const subscribedGroupsRef = useRef<Set<string>>(new Set());
3434
const queryClient = useQueryClient();
3535
const [isConnected, setIsConnected] = useState(false);
36+
// Flag to prevent reconnect and close pending connections when intentionally disconnecting
37+
const isDisconnectingRef = useRef(false);
3638

3739
// Store options and queryClient in refs to avoid stale closures
3840
const optionsRef = useRef(options);
@@ -47,6 +49,9 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
4749
}, [options, queryClient]);
4850

4951
const connect = useCallback(() => {
52+
// Reset the disconnecting flag when connecting
53+
isDisconnectingRef.current = false;
54+
5055
const token = api.getToken();
5156
if (!token) {
5257
return;
@@ -109,6 +114,11 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
109114
wsRef.current = ws;
110115

111116
ws.onopen = () => {
117+
// If disconnect was called while we were connecting, close immediately
118+
if (isDisconnectingRef.current) {
119+
ws.close();
120+
return;
121+
}
112122
setIsConnected(true);
113123
// Re-subscribe to all groups
114124
subscribedGroupsRef.current.forEach((groupId) => {
@@ -119,8 +129,10 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
119129
ws.onclose = () => {
120130
setIsConnected(false);
121131
wsRef.current = null;
122-
// Attempt to reconnect after 3 seconds using ref
123-
reconnectTimeoutRef.current = setTimeout(() => connectRef.current(), 3000);
132+
// Only attempt to reconnect if we're not intentionally disconnecting
133+
if (!isDisconnectingRef.current) {
134+
reconnectTimeoutRef.current = setTimeout(() => connectRef.current(), 3000);
135+
}
124136
};
125137

126138
ws.onerror = () => {
@@ -137,7 +149,9 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
137149
};
138150
} catch {
139151
// Connection failed, will retry using ref
140-
reconnectTimeoutRef.current = setTimeout(() => connectRef.current(), 3000);
152+
if (!isDisconnectingRef.current) {
153+
reconnectTimeoutRef.current = setTimeout(() => connectRef.current(), 3000);
154+
}
141155
}
142156
}, []);
143157

@@ -161,11 +175,17 @@ export function useWebSocket(options: UseWebSocketOptions = {}) {
161175
}, []);
162176

163177
const disconnect = useCallback(() => {
178+
isDisconnectingRef.current = true;
164179
if (reconnectTimeoutRef.current) {
165180
clearTimeout(reconnectTimeoutRef.current);
181+
reconnectTimeoutRef.current = undefined;
166182
}
167183
if (wsRef.current) {
168-
wsRef.current.close();
184+
// Only close if already open - if still connecting, the onopen handler
185+
// will check isDisconnectingRef and close it then
186+
if (wsRef.current.readyState === WebSocket.OPEN) {
187+
wsRef.current.close();
188+
}
169189
wsRef.current = null;
170190
}
171191
setIsConnected(false);

0 commit comments

Comments
 (0)