fix: ReactFlow controls not working in dashboard graph visualizations #429
Conversation
SafeDep Report Summary
Package Details
This report is generated by SafeDep Github App. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds ReactFlowProvider around the graph page. Introduces interactive mode toggling, fit-to-view, and refresh controls in the graph visualization. Updates components to respect interactivity (selection, drag, zoom/pan), adds Panel buttons (Fit View, Refresh), and wires state/handlers via React hooks and React Flow context. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant P as Graph Page
participant RFP as ReactFlowProvider
participant GTD as GraphTemplateDetail
participant GV as GraphVisualization
participant RF as React Flow (context)
participant DS as Data Source
U->>P: Navigate to Graph Page
P->>RFP: Provide React Flow context
RFP->>GTD: Render component tree
GTD->>GV: Render with isInteractive state
note over GV,RF: Initial render with interactivity props
U->>GV: Click "Fit View"
GV->>RF: useReactFlow.fitView({ padding, duration })
RF-->>GV: Viewport adjusted
U->>GV: Toggle Interactivity (Controls)
GV->>GTD: onInteractiveChange(newState)
GTD->>GV: Re-render with isInteractive=newState
GV->>RF: Apply updated interaction flags
U->>GV: Click "Refresh"
GV->>DS: loadGraphStructure()
DS-->>GV: Graph data
GV-->>U: Updated visualization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @pankajydv07, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical user experience issue in the Exosphere dashboard by fixing non-functional ReactFlow graph controls. The core problem stemmed from missing Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes the non-functional ReactFlow controls by wrapping the graph components with ReactFlowProvider. The implementation also enhances interactivity toggling and adds a custom control panel for better UX. My review focuses on a few points to improve consistency and avoid redundancy: ensuring all ReactFlow instances are covered by a provider, simplifying a handler, and removing duplicate UI controls. Overall, this is a great contribution that significantly improves the graph visualization feature.
| <ReactFlowProvider> | ||
| <GraphVisualization | ||
| namespace={namespace} | ||
| runId={runId} | ||
| onGraphTemplateRequest={handleOpenGraphTemplate} | ||
| /> | ||
| </ReactFlowProvider> |
There was a problem hiding this comment.
The ReactFlowProvider is correctly added for GraphVisualization, but the GraphTemplateDetail component, which also uses React Flow controls, is not wrapped by a provider. This will likely cause its controls to not function as expected.
A better approach would be to have a single ReactFlowProvider at a higher level, wrapping all components that need the React Flow context. I'd suggest moving the provider to wrap the top-level div in this component's return statement (at line 75). This would provide the context to both GraphVisualization and GraphTemplateDetail.
Example:
// at line 74
return (
<ReactFlowProvider>
<div className="min-h-screen bg-background">
{/* ... rest of the component ... */}
</div>
</ReactFlowProvider>
);With that change, you can remove the ReactFlowProvider wrapper from around GraphVisualization here.
| } | ||
|
|
||
| const [isInteractive, setIsInteractive] = useState(true); | ||
| const handleInteractiveChange = useCallback((val: boolean) => setIsInteractive(val), []); |
There was a problem hiding this comment.
You can simplify the code by removing this useCallback. The state setter function from useState (setIsInteractive) is guaranteed by React to be stable and doesn't need to be memoized. You can pass it directly to the onInteractiveChange prop in the <Controls> component at line 297:
onInteractiveChange={setIsInteractive}| <Controls /> | ||
| <Controls | ||
| showInteractive={true} | ||
| showFitView={true} |
There was a problem hiding this comment.
Since you've added a custom 'Fit View' button in the top-right panel which provides a better user experience with smooth animation, it's a good idea to hide the default 'Fit View' button from the built-in controls to avoid duplication and provide a cleaner UI.
| showFitView={true} | |
| showFitView={false} |
| <Panel position="top-right" className="flex gap-2 bg-card/90 backdrop-blur-sm rounded-lg p-2 border shadow-lg"> | ||
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={handleFitView} | ||
| className="text-xs h-8" | ||
| title="Fit graph to screen" | ||
| > | ||
| <Maximize2 className="w-3 h-3 mr-1" /> | ||
| Fit View | ||
| </Button> | ||
| <Button | ||
| size="sm" | ||
| variant="outline" | ||
| onClick={loadGraphStructure} | ||
| className="text-xs h-8" | ||
| title="Refresh graph data" | ||
| > | ||
| <RefreshCw className="w-3 h-3 mr-1" /> | ||
| Refresh | ||
| </Button> | ||
| </Panel> |
There was a problem hiding this comment.
This custom panel is a great UX improvement. However, the 'Refresh' button here duplicates an existing 'Refresh' button at the top of the GraphVisualization component (line 394). Both buttons call the same loadGraphStructure function. To avoid user confusion from redundant controls, it would be best to have only one. Since this new one inside the panel is contextually better placed, I'd recommend removing the older one from the component's header.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
dashboard/src/app/graph/[namespace]/[runId]/page.tsx(2 hunks)dashboard/src/components/GraphTemplateDetail.tsx(3 hunks)dashboard/src/components/GraphVisualization.tsx(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
dashboard/src/components/GraphVisualization.tsx (1)
dashboard/src/types/state-manager.ts (2)
GraphStructureResponse(197-205)NodeRunDetailsResponse(207-220)
dashboard/src/app/graph/[namespace]/[runId]/page.tsx (1)
dashboard/src/components/GraphVisualization.tsx (1)
GraphVisualization(125-599)
🪛 Biome (2.1.2)
dashboard/src/components/GraphTemplateDetail.tsx
[error] 260-260: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 261-261: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🔇 Additional comments (4)
dashboard/src/components/GraphTemplateDetail.tsx (1)
3-3: LGTM on imports.useState/useCallback import is required for the new interactive behavior.
dashboard/src/components/GraphVisualization.tsx (2)
130-146: Good use of useReactFlow for smooth fit view.fitView with duration/padding enhances UX. Ensure this component stays wrapped by ReactFlowProvider (it is, per Graph page).
556-577: Custom Panel actions look good.Fit View and Refresh are clear, accessible, and complement Controls well.
dashboard/src/app/graph/[namespace]/[runId]/page.tsx (1)
107-114: Correctly wrapped with ReactFlowProvider; no other GraphVisualization usages found.
| const [isInteractive, setIsInteractive] = useState(true); | ||
| const handleInteractiveChange = useCallback((val: boolean) => setIsInteractive(val), []); | ||
|
|
There was a problem hiding this comment.
Fix rules-of-hooks violation (hooks after early return).
useState/useCallback are declared after a conditional return (Line 240), which will crash with "Rendered fewer hooks than expected" when nodes length changes.
Apply this diff to remove the hooks here:
- const [isInteractive, setIsInteractive] = useState(true);
- const handleInteractiveChange = useCallback((val: boolean) => setIsInteractive(val), []);Then add them immediately after the useEdgesState declaration (before the early return), e.g.:
// place right after:
const [flowEdgesState, , onEdgesChange] = useEdgesState(flowEdges);
const [isInteractive, setIsInteractive] = useState(true);
const handleInteractiveChange = useCallback((val: boolean) => setIsInteractive(val), []);🧰 Tools
🪛 Biome (2.1.2)
[error] 260-260: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
[error] 261-261: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.
Hooks should not be called after an early return.
For React to preserve state between calls, hooks needs to be called unconditionally and always in the same order.
See https://reactjs.org/docs/hooks-rules.html#only-call-hooks-at-the-top-level
(lint/correctness/useHookAtTopLevel)
🤖 Prompt for AI Agents
In dashboard/src/components/GraphTemplateDetail.tsx around lines 240-262, the
useState/useCallback hooks for isInteractive are currently declared after an
early conditional return which violates the Rules of Hooks; remove the existing
declarations at lines ~260-262 and instead add them immediately after the
useEdgesState declaration (right after the line where flowEdgesState and
onEdgesChange are declared) so the hooks run unconditionally before any returns,
e.g., move the two hook declarations to just after useEdgesState.
| elementsSelectable={isInteractive} | ||
| nodesConnectable={false} | ||
| nodesDraggable={false} | ||
| nodesDraggable={isInteractive} | ||
| zoomOnScroll={isInteractive} | ||
| panOnScroll={isInteractive} | ||
| panOnDrag={isInteractive} | ||
| zoomOnPinch={isInteractive} | ||
| zoomOnDoubleClick={isInteractive} | ||
| className="bg-background" |
There was a problem hiding this comment.
nodesDraggable toggle is ineffective due to per-node draggable=false.
Each node is created with draggable: false (Lines 193–194), which overrides the global nodesDraggable={isInteractive} and prevents dragging even in interactive mode.
Fix: remove the per-node draggable flag from node creation so the global toggle works:
// In flowNodes.push(...) remove this line:
draggable: false,Optional: if you want the graph to reflect updated interactivity when props change, sync state like in GraphVisualization:
const [flowNodesState, setFlowNodesState, onNodesChange] = useNodesState(flowNodes);
const [flowEdgesState, setFlowEdgesState, onEdgesChange] = useEdgesState(flowEdges);
useEffect(() => {
setFlowNodesState(flowNodes);
setFlowEdgesState(flowEdges);
}, [flowNodes, flowEdges, setFlowNodesState, setFlowEdgesState]);🤖 Prompt for AI Agents
In dashboard/src/components/GraphTemplateDetail.tsx around lines 285 to 293,
nodes are still created with a per-node draggable: false which overrides the
global nodesDraggable={isInteractive} and prevents dragging; remove the
draggable: false property from each node in the flowNodes creation so the global
nodesDraggable prop takes effect, and optionally, if you need the nodes/edges to
reflect prop changes, initialize and sync state using
useNodesState/useEdgesState and a useEffect to set the flow state when
flowNodes/flowEdges change.
| <Controls | ||
| position="bottom-left" | ||
| onInteractiveChange={handleInteractiveChange} | ||
| /> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Explicitly show interactive/fit buttons for Controls (consistency).
Make Controls behavior explicit and consistent with GraphVisualization.
- <Controls
- position="bottom-left"
- onInteractiveChange={handleInteractiveChange}
- />
+ <Controls
+ position="bottom-left"
+ showInteractive={true}
+ showFitView={true}
+ onInteractiveChange={handleInteractiveChange}
+ />📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Controls | |
| position="bottom-left" | |
| onInteractiveChange={handleInteractiveChange} | |
| /> | |
| <Controls | |
| position="bottom-left" | |
| showInteractive={true} | |
| showFitView={true} | |
| onInteractiveChange={handleInteractiveChange} | |
| /> |
🤖 Prompt for AI Agents
In dashboard/src/components/GraphTemplateDetail.tsx around lines 295 to 298,
Controls is rendered without explicitly enabling the interactive/fit buttons
which makes its behavior inconsistent with GraphVisualization; update the
Controls invocation to pass the same explicit props used by GraphVisualization
(e.g., showInteractiveButton and showFitButton or the specific prop names used
in your Controls API) and set them to true so the interactive and fit controls
are always visible and consistent across components.
| elementsSelectable={isInteractive} | ||
| nodesConnectable={false} | ||
| nodesDraggable={false} | ||
| nodesDraggable={isInteractive} | ||
| zoomOnScroll={isInteractive} | ||
| panOnScroll={isInteractive} | ||
| panOnDrag={isInteractive} | ||
| zoomOnPinch={isInteractive} | ||
| zoomOnDoubleClick={isInteractive} | ||
| > |
There was a problem hiding this comment.
nodesDraggable toggle is overridden by node.draggable=false.
Nodes are constructed with draggable: false (Line 286), which disables dragging even when isInteractive is true.
Fix: remove the per-node flag so the global nodesDraggable={isInteractive} takes effect:
// In reactFlowNodes.push(...) remove:
draggable: false,🤖 Prompt for AI Agents
In dashboard/src/components/GraphVisualization.tsx around lines 540 to 548 and
node creation at ~line 286, the nodes are being created with draggable: false
which overrides the global nodesDraggable={isInteractive}; remove the per-node
draggable: false property from the object pushed into reactFlowNodes so the
global nodesDraggable prop can control draggability, then run the component to
verify nodes are draggable when isInteractive is true.
| <Controls | ||
| showInteractive={true} | ||
| showFitView={true} | ||
| position="bottom-left" | ||
| onInteractiveChange={setIsInteractive} | ||
| /> | ||
| <Panel position="top-right" className="flex gap-2 bg-card/90 backdrop-blur-sm rounded-lg p-2 border shadow-lg"> |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Avoid duplicate Fit View actions (optional).
You already provide a “Fit View” button in the Panel. Consider disabling the Control’s Fit View to reduce UI duplication.
- <Controls
- showInteractive={true}
- showFitView={true}
+ <Controls
+ showInteractive={true}
+ showFitView={false}
position="bottom-left"
onInteractiveChange={setIsInteractive}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Controls | |
| showInteractive={true} | |
| showFitView={true} | |
| position="bottom-left" | |
| onInteractiveChange={setIsInteractive} | |
| /> | |
| <Panel position="top-right" className="flex gap-2 bg-card/90 backdrop-blur-sm rounded-lg p-2 border shadow-lg"> | |
| <Controls | |
| showInteractive={true} | |
| showFitView={false} | |
| position="bottom-left" | |
| onInteractiveChange={setIsInteractive} | |
| /> | |
| <Panel position="top-right" className="flex gap-2 bg-card/90 backdrop-blur-sm rounded-lg p-2 border shadow-lg"> |
🤖 Prompt for AI Agents
In dashboard/src/components/GraphVisualization.tsx around lines 550 to 556, the
Controls component renders a duplicate "Fit View" action that is also provided
in the Panel; remove or disable the duplicate by changing the Controls props to
not show the Fit View (e.g., set showFitView={false} or remove the prop) so only
the Panel contains the Fit View button, keeping onInteractiveChange and other
props unchanged.
|
Hi @pankajydv07 Thank you for your PR, kindly check the comments made by AI reviews, feel free to resolve invalid comments. |
|
@nk-ag requesting your review for this PR. |
2 participants
Fix ReactFlow Controls Not Working in Dashboard
Fixes issue no. #396
🐛 Problem Description
The ReactFlow controls (Fit to Screen, Toggle Interactivity, Zoom buttons) were not functioning properly in the Exosphere dashboard's graph visualization components. Users could not:
🎬 Demo
screen-capture.1._2.mp4
🔍 Root Cause Analysis
ReactFlow v11+ Requirement: ReactFlow version 11 and above requires all ReactFlow components to be wrapped with
ReactFlowProviderto access the React Flow context. Without this provider:✅ Solution Implemented
1. Fixed ReactFlowProvider Wrapper Issues
Added proper
ReactFlowProviderwrapping in 3 locations:dashboard/src/app/graph/[namespace]/[runId]/page.tsx- Main graph visualization pagedashboard/src/components/GraphTemplateDetail.tsx- Template detail visualizationdashboard/src/app/test-reactflow/page.tsx- Test page (for validation)2. Enhanced Toggle Interactivity Functionality
GraphVisualization.tsx:
3. Custom Control Panel Enhancement
Added custom Fit View and Refresh buttons in the top-right corner:
🤔 Why Custom Controls When Built-in Exist?
I added custom Fit View and Refresh buttons in the top-right corner for these UX reasons:
However, I understand this might be redundant since ReactFlow already provides a fit view button in the bottom-left controls.
🧪 Testing & Validation
Test Cases Verified:
Main Graph Visualization (
/graph/[namespace]/[runId]):Template Detail Visualization (Visualize button on run dashboard):
Sample Data Testing:
Manual Testing Steps:
npm run dev(dashboard) +uvicorn app.main:app --reload(state-manager)http://localhost:3000/graph/default/e8f930a8-49b4-4489-807c-10a5792427a7📊 Impact
Before Fix:
After Fix:
🔧 Technical Details
Files Changed:
dashboard/src/app/graph/[namespace]/[runId]/page.tsx- Added ReactFlowProvider wrapperDependencies:
Breaking Changes:
🎯 HacktoBerFest 2025 Contribution
This contribution addresses a significant UX issue in the Exosphere dashboard that affects all users working with graph visualizations. The fix:
📝 Checklist
🚀 Ready for Review
This PR is ready for review and merge. The fix has been thoroughly tested and validated across all affected components.
Test URL:
http://localhost:3000/graph/default/e8f930a8-49b4-4489-807c-10a5792427a7💭 Feedback Welcome
Please let me know if you'd prefer me to remove the custom control panel and keep only the ReactFlow built-in controls. I'm flexible and want to maintain consistency with the project's design philosophy!