refactor: inspection panel improvements#11863
Closed
viktoravelino wants to merge 14 commits intomainfrom
Closed
Conversation
Descriptions and edit button now render unconditionally, preventing visual changes on canvas when toggling the inspector panel.
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThe pull request refactors canvas controls layout and inspection panel visibility management, relocating controls to bottom-left and adding a right-side inspector toggle. Additionally, the inspection panel now filters out fields with existing incoming connections and removes its dependency from node rendering logic. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 5❌ Failed checks (1 error, 3 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
Contributor
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report❌ Patch coverage is ❌ Your project status has failed because the head coverage (41.93%) is below the target coverage (60.00%). You can increase the head coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #11863 +/- ##
==========================================
+ Coverage 35.38% 35.41% +0.03%
==========================================
Files 1527 1527
Lines 73484 73506 +22
Branches 11041 11048 +7
==========================================
+ Hits 25999 26033 +34
+ Misses 46073 46057 -16
- Partials 1412 1416 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
github-actions
bot
added
refactor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/frontend/src/CustomNodes/GenericNode/index.tsx (1)
449-465:⚠️ Potential issue | 🟡 Minor
hasBreakingChangeused inside the memo but missing from its dependency array.
isOutdated,isUserEdited, andhasBreakingChangeall come from the samecomponentUpdatedestructure. The first two are in the deps, buthasBreakingChange(used at line 406 ashasBreakingChange={hasBreakingChange}) is not. IfcomponentUpdatechanges such that onlyhasBreakingChangeflips — withoutisOutdatedorisUserEditedchanging —NodeToolbarComponentwill receive a stale value.🐛 Proposed fix
}, [ data, deleteNode, takeSnapshot, setNode, showNode, updateNodeCode, isOutdated, isUserEdited, + hasBreakingChange, selected, shortcuts, editNameDescription, hasChangedNodeDescription, toggleEditNameDescription, selectedNodesCount, rightClickedNodeId, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/CustomNodes/GenericNode/index.tsx` around lines 449 - 465, The memo/effect that builds props for NodeToolbarComponent is missing hasBreakingChange in its dependency array: add hasBreakingChange (from componentUpdate destructure) to the dependency list alongside isOutdated and isUserEdited so that NodeToolbarComponent receives updated hasBreakingChange values; update the dependency array referenced where NodeToolbarComponent is rendered (the hook that includes data, deleteNode, takeSnapshot, setNode, showNode, updateNodeCode, isOutdated, isUserEdited, selected, shortcuts, editNameDescription, hasChangedNodeDescription, toggleEditNameDescription, selectedNodesCount, rightClickedNodeId) to include hasBreakingChange.
🧹 Nitpick comments (2)
src/frontend/src/CustomNodes/GenericNode/index.tsx (1)
410-410:-z-10on a non-positioned<div>has no CSS effect.Tailwind's
z-*utilities only apply to elements withposition: relative/absolute/fixed/sticky. This wrapper<div>has none, soz-index: -10is silently ignored. The button inside correctly usesabsolute z-50, which is all that matters for stacking. The wrapper class can be removed to avoid confusion.♻️ Proposed cleanup
- <div className="-z-10"> <Button unstyled onClick={() => { ... }} ... > ... </Button> - </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/CustomNodes/GenericNode/index.tsx` at line 410, Remove the redundant "-z-10" Tailwind class from the non-positioned wrapper div in the GenericNode component (index.tsx) because z-index utilities require a positioned element; locate the wrapper <div> that currently has className="-z-10" and delete that class (or the wrapper if unused) while keeping the inner button's absolute z-50 intact to preserve stacking behavior.src/frontend/src/pages/FlowPage/components/InspectionPanel/components/InspectionPanelFields.tsx (1)
27-38:connectedFieldsmemo produces a newSetreference on every edge change anywhere in the flow.
useFlowStore((state) => state.edges)subscribes to the full edge array; its reference changes on every unrelated edge operation. SinceconnectedFieldsalways constructs a newSet,advancedFieldsrecomputes on every edge mutation in the entire flow — not just ones that touch this node.Consider scoping the selector to only the edges relevant to this node and using
useShallow(or a custom equality function) so the subscription only fires when edges targetingdata.idactually change:♻️ Proposed refactor
- const edges = useFlowStore((state) => state.edges); - - const connectedFields = useMemo(() => { - const fields = new Set<string>(); - for (const edge of edges) { - if (edge.target === data.id) { - const fieldName = edge.data?.targetHandle?.fieldName; - if (fieldName) fields.add(fieldName); - } - } - return fields; - }, [edges, data.id]); + // Only re-render when the set of field-names connected to THIS node changes. + const connectedFieldNames = useFlowStore( + useShallow((state) => + state.edges + .filter( + (e) => e.target === data.id && e.data?.targetHandle?.fieldName, + ) + .map((e) => e.data!.targetHandle!.fieldName as string), + ), + ); + + const connectedFields = useMemo( + () => new Set(connectedFieldNames), + [connectedFieldNames], + );
useShallowperforms a shallow array comparison, soconnectedFieldNamesonly produces a new reference (and triggers a re-render) when the list of connected field names actually changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/frontend/src/pages/FlowPage/components/InspectionPanel/components/InspectionPanelFields.tsx` around lines 27 - 38, The connectedFields memo is rebuilding on any edge array change because useFlowStore((state) => state.edges) subscribes to the whole edges list; change the selector to only subscribe to edges that target this node (filter by edge.target === data.id and map to edge.data?.targetHandle?.fieldName), return a stable array of field names (e.g. connectedFieldNames) and use useShallow (or a custom equality) when selecting from useFlowStore so the component only re-renders when the actual list of connected field names for this node changes; update any dependent code that references connectedFields/advancedFields to use the new connectedFieldNames selector.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/frontend/src/CustomNodes/GenericNode/index.tsx`:
- Around line 449-465: The memo/effect that builds props for
NodeToolbarComponent is missing hasBreakingChange in its dependency array: add
hasBreakingChange (from componentUpdate destructure) to the dependency list
alongside isOutdated and isUserEdited so that NodeToolbarComponent receives
updated hasBreakingChange values; update the dependency array referenced where
NodeToolbarComponent is rendered (the hook that includes data, deleteNode,
takeSnapshot, setNode, showNode, updateNodeCode, isOutdated, isUserEdited,
selected, shortcuts, editNameDescription, hasChangedNodeDescription,
toggleEditNameDescription, selectedNodesCount, rightClickedNodeId) to include
hasBreakingChange.
---
Nitpick comments:
In `@src/frontend/src/CustomNodes/GenericNode/index.tsx`:
- Line 410: Remove the redundant "-z-10" Tailwind class from the non-positioned
wrapper div in the GenericNode component (index.tsx) because z-index utilities
require a positioned element; locate the wrapper <div> that currently has
className="-z-10" and delete that class (or the wrapper if unused) while keeping
the inner button's absolute z-50 intact to preserve stacking behavior.
In
`@src/frontend/src/pages/FlowPage/components/InspectionPanel/components/InspectionPanelFields.tsx`:
- Around line 27-38: The connectedFields memo is rebuilding on any edge array
change because useFlowStore((state) => state.edges) subscribes to the whole
edges list; change the selector to only subscribe to edges that target this node
(filter by edge.target === data.id and map to
edge.data?.targetHandle?.fieldName), return a stable array of field names (e.g.
connectedFieldNames) and use useShallow (or a custom equality) when selecting
from useFlowStore so the component only re-renders when the actual list of
connected field names for this node changes; update any dependent code that
references connectedFields/advancedFields to use the new connectedFieldNames
selector.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/frontend/src/CustomNodes/GenericNode/index.tsxsrc/frontend/src/CustomNodes/helpers/parameter-filtering.tssrc/frontend/src/components/core/canvasControlsComponent/CanvasControls.tsxsrc/frontend/src/pages/FlowPage/components/InspectionPanel/components/InspectionPanelFields.tsx
github-actions
bot
added
refactor
github-actions
bot
added
refactor
github-actions
bot
added
refactor
github-actions
bot
added
refactor
…anel button visibility
viktoravelino
force-pushed
the
refactor/inspection-panel
branch
from
8611a7b to
9cfa969
Compare
github-actions
bot
added
refactor
keval718
approved these changes
Feb 23, 2026
olayinkaadelakun
approved these changes
Feb 23, 2026
github-actions
bot
added
refactor
The toggle button became disabled immediately after closing the panel because selectedNode was derived from inspectionPanelVisible. Now selectedNode is based solely on the current node selection, keeping the toggle enabled as long as a node is selected.
Replace PanelRight/PanelRightClose with SlidersHorizontal to avoid confusion with the Playground button which uses PanelRightOpen. Add bg-accent background highlight when the panel is active.
github-actions
bot
added
refactor
Update lucide-react to v0.575.0 and use list-indent-increase/decrease icons to visually indicate the inspection panel's open/closed state.
github-actions
bot
added
refactor
github-actions
bot
added
refactor
github-actions
bot
added
refactor
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ENABLE_INSPECTION_PANELfeature flaginspectionPanelVisiblecoupling fromGenericNode— node descriptions and the edit name/description button now render unconditionally regardless of inspector statetype === "other"), readonly fields, and fields that already have an incoming connectionTest plan
Screen.Recording.2026-02-23.at.7.23.59.AM.mov
Summary by CodeRabbit
New Features
Improvements