fix: prevent dependency graph from going blank when clicking nodes#238
Conversation
- Added safety check: only apply selection highlighting if selected node is visible - Fixed handlePanelFileClick to auto-enable 'show all' when clicking non-visible dependents - Applied fix to both clustered and non-clustered graph modes - Prevents edge dimming when selected node isn't in visible set
|
@DevanshuNEU is attempting to deploy a commit to the Dev's projects Team on Vercel. A member of the Team first needs to authorize it. |
Root cause: dimmed state had border-zinc-800 (same as background) + opacity-40 making nodes essentially invisible when selecting a file. Changes: - GraphNode: dark:border-zinc-600 dark:bg-zinc-800/80 opacity-50 - DirectoryNode: same fix for cluster mode Now dimmed nodes are visible but clearly de-emphasized.
📝 WalkthroughWalkthroughUpdated dimmed-state styles for DirectoryNode and GraphNode; reworked DependencyGraph layout/visibility/selection flows to only highlight visible nodes/edges, added guards for missing layout data, a renderKey remount trigger, tab-visibility listener, and deferred selection when clicking non-visible files; ImpactPanel no longer resets collapsible open state on file change. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Panel as Panel (file list)
participant Graph as DependencyGraph (ReactFlow)
participant Layout as Dagre/Layout
participant Browser as Browser (visibility)
User->>Panel: click file
Panel->>Graph: handlePanelFileClick(fileId)
alt file not visible
Graph->>Graph: set showAll = true
Graph->>Browser: add visibility listener
Browser-->>Graph: visibilitychange (visible)
Graph->>Graph: update renderKey (force remount)
end
Graph->>Layout: getLayoutedElements(nodes, edges)
Layout-->>Graph: positions (or fallback)
Graph->>Graph: compute effectiveSelectedId / effectiveSelectedIdCluster
Graph->>Graph: highlight selected nodes & connected edges
Graph->>Graph: fitView (with adjusted delay)
Graph-->>User: rendered focused view
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Quick fix for demo - instead of dimming non-selected nodes (which was causing them to become invisible), we now just highlight the selected node and its dependents while keeping others at default visibility. Changes: - Removed 'dimmed' state assignment in both normal and cluster modes - Removed edge dimming - Added guards to dagre layout function for edge cases - Kept selected/direct/transitive highlighting working This is a simpler UX that works reliably for the demo.
Multiple issues addressed: 1. Tab visibility - graph now re-renders when tab regains focus - Added visibilitychange event listener - Uses renderKey to force ReactFlow refresh - Calls fitView after tab becomes visible 2. Cluster mode timing - fixed race condition - Added check for clusteredData.dirNodes.length > 0 - Falls back to non-clustered mode if data not ready 3. FitView on panel open/close - Graph now refits when ImpactPanel opens/closes - Added selectedNodeId to fitView dependencies 4. Dagre layout guards - Added empty nodes guard - Added fallback positions for failed layouts - Only adds edges where both endpoints exist
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/components/DependencyGraph/index.tsx`:
- Around line 412-432: handlePanelFileClick can select a file that’s not
actually rendered because its parent directory is collapsed; update the handler
to detect and expand the file’s directory before selecting/zooming: inside
handlePanelFileClick (which currently references visibleNodeIds, showAll,
setShowAll, setSelectedNodeId, nodes, fitView) check the file’s parent directory
collapse state (e.g. collapsedDirs or isDirCollapsed(parentId)) and if
collapsed, call the directory-expand action (e.g. setCollapsed(parentId, false)
or onToggleDirectory(parentId)) — if showAll is false also setShowAll(true) —
then after a short delay (like the existing setTimeout) call
setSelectedNodeId(fileId) and fitView for the node; otherwise proceed as before.
Ensure you reference the actual state/updater names used for directory collapse
and parent lookup when implementing.
🧹 Nitpick comments (2)
frontend/src/components/DependencyGraph/index.tsx (2)
141-156: Clear the visibility-change timeout to avoid stray fitView calls.
setTimeoutisn’t cleared if the component unmounts or visibility toggles rapidly. A small cleanup prevents queued fitView calls after teardown.🧹 Proposed cleanup
useEffect(() => { const handleVisibilityChange = () => { if (document.visibilityState === 'visible') { // Force re-render by updating key setRenderKey(k => k + 1) // Also trigger fitView after a short delay - setTimeout(() => { + const timeoutId = window.setTimeout(() => { fitView({ padding: 0.2, duration: 200 }) - }, 100) + }, 100) + // Clear any pending fitView if visibility changes again quickly + return () => window.clearTimeout(timeoutId) } } document.addEventListener('visibilitychange', handleVisibilityChange) return () => document.removeEventListener('visibilitychange', handleVisibilityChange) }, [fitView])
384-391: Avoid stacked fitView timeouts on rapid state changes.This effect can queue multiple timeouts if dependencies update quickly (panel toggles, filtering), which can cause jitter. Consider cancelling the prior timeout.
🧹 Proposed cleanup
useEffect(() => { - if (nodes.length > 0) { - const minZoom = nodes.length > 20 ? 0.5 : 0.3 - // Delay to allow container resize when panel opens/closes - setTimeout(() => fitView({ padding: 0.2, duration: 300, minZoom }), 150) - } + if (nodes.length === 0) return + const minZoom = nodes.length > 20 ? 0.5 : 0.3 + // Delay to allow container resize when panel opens/closes + const timeoutId = window.setTimeout(() => { + fitView({ padding: 0.2, duration: 300, minZoom }) + }, 150) + return () => window.clearTimeout(timeoutId) }, [nodes.length, selectedNodeId, showAll, showTests, clusterByDir, expandedDirs, fitView])
The useEffect was resetting isOpen to defaultOpen whenever files changed, which caused the section to immediately close after clicking to open it.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Problem
When clicking on certain nodes in the dependency graph (especially when clicking on dependent files in the ImpactPanel that aren't currently visible), the entire graph would go blank.
Root Cause
When a user clicks on a file in the ImpactPanel that's not in the
visibleNodeIdsset (e.g., when only top 15 files are shown), theselectedNodeIdgets set to a non-visible file. This caused all other visible nodes to getstate = 'dimmed'even though the selected node wasn't displayed, breaking the visual state.Fix
effectiveSelectedIdsafety check - Only applies selection highlighting if the selected node is actually visible in the current viewhandlePanelFileClick- When clicking on a non-visible dependent, auto-enables "Show All" mode to make the file visible before selecting itTesting
Summary by CodeRabbit
Style
Improvements