Skip to content

Commit 6a3ca35

Browse files
committed
fix: address CodeRabbit review — hover perf and a11y labels
Use memoized graphEdges instead of mutable edges state in hover effect to avoid stale closure. Add equality checks in setNodes/setEdges updaters to skip unnecessary re-renders. Add aria-label to filter selects.
1 parent 89ce6cd commit 6a3ca35

1 file changed

Lines changed: 35 additions & 34 deletions

File tree

frontend/src/features/cookbook/components/composition-graph.tsx

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -315,44 +315,43 @@ export function CompositionGraph({ patterns, className }: CompositionGraphProps)
315315
return () => { cancelled = true }
316316
}, [filteredPatterns, graphEdges, setNodes, setEdges])
317317

318-
// Hover highlighting
318+
// Hover highlighting — use graphEdges (memoized) not edges (mutable state)
319319
useEffect(() => {
320-
if (!hoveredNode) {
321-
setNodes((nds) =>
322-
nds.map((n) => ({
323-
...n,
324-
data: { ...n.data, highlighted: false, dimmed: false },
325-
})),
326-
)
327-
setEdges((eds) => eds.map((e) => ({ ...e, animated: false })))
328-
return
329-
}
330-
331-
const connectedNodes = new Set<string>([hoveredNode])
332-
for (const e of edges) {
333-
if (e.source === hoveredNode || e.target === hoveredNode) {
334-
connectedNodes.add(e.source)
335-
connectedNodes.add(e.target)
320+
const connectedNodes = new Set<string>()
321+
if (hoveredNode) {
322+
connectedNodes.add(hoveredNode)
323+
for (const e of graphEdges) {
324+
if (e.source === hoveredNode || e.target === hoveredNode) {
325+
connectedNodes.add(e.source)
326+
connectedNodes.add(e.target)
327+
}
336328
}
337329
}
338330

339-
setNodes((nds) =>
340-
nds.map((n) => ({
341-
...n,
342-
data: {
343-
...n.data,
344-
highlighted: n.id === hoveredNode,
345-
dimmed: !connectedNodes.has(n.id),
346-
},
347-
})),
348-
)
349-
setEdges((eds) =>
350-
eds.map((e) => ({
351-
...e,
352-
animated: e.source === hoveredNode || e.target === hoveredNode,
353-
})),
354-
)
355-
}, [hoveredNode, edges, setNodes, setEdges])
331+
setNodes((nds) => {
332+
let changed = false
333+
const next = nds.map((n) => {
334+
const highlighted = hoveredNode ? n.id === hoveredNode : false
335+
const dimmed = hoveredNode ? !connectedNodes.has(n.id) : false
336+
const current = n.data as PatternNodeData
337+
if (current.highlighted === highlighted && current.dimmed === dimmed) return n
338+
changed = true
339+
return { ...n, data: { ...n.data, highlighted, dimmed } }
340+
})
341+
return changed ? next : nds
342+
})
343+
344+
setEdges((eds) => {
345+
let changed = false
346+
const next = eds.map((e) => {
347+
const animated = hoveredNode ? e.source === hoveredNode || e.target === hoveredNode : false
348+
if (e.animated === animated) return e
349+
changed = true
350+
return { ...e, animated }
351+
})
352+
return changed ? next : eds
353+
})
354+
}, [hoveredNode, graphEdges, setNodes, setEdges])
356355

357356
const onNodeClick: NodeMouseHandler = useCallback(
358357
(_event, node) => {
@@ -395,6 +394,7 @@ export function CompositionGraph({ patterns, className }: CompositionGraphProps)
395394
<div className="absolute top-3 left-3 z-10 flex flex-col gap-2 rounded-lg border bg-background/95 p-3 backdrop-blur-sm shadow-sm">
396395
<span className="text-xs font-semibold text-muted-foreground uppercase tracking-wider">Filters</span>
397396
<select
397+
aria-label="Filter by category"
398398
value={categoryFilter}
399399
onChange={(e) => setCategoryFilter(e.target.value)}
400400
className="rounded border bg-background px-2 py-1 text-xs"
@@ -405,6 +405,7 @@ export function CompositionGraph({ patterns, className }: CompositionGraphProps)
405405
))}
406406
</select>
407407
<select
408+
aria-label="Filter by industry"
408409
value={industryFilter}
409410
onChange={(e) => setIndustryFilter(e.target.value)}
410411
className="rounded border bg-background px-2 py-1 text-xs"

0 commit comments

Comments
 (0)