port-suggestion-menu

[mck]

User description

Adds a port search menu when dragging an edge out

---

PR Type

Enhancement, Tests

---

Description

• Introduced a new PortSuggestionMenu component to allow users to select compatible ports when dragging a port.
• Implemented a search field within the menu for filtering ports by name or node title.
• Added a PortRegistry class to manage and query compatible ports, improving the logic for port compatibility checks.
• Integrated the PortSuggestionMenu into the editor, enabling dynamic port selection and edge creation.
• Updated the version to 4.3.9 to reflect the new feature additions.
• Styled the PortSuggestionMenu component with a focus on usability and aesthetics.

---

Changes walkthrough 📝

	Relevant files
Enhancement	portSuggestionMenu.tsx Introduced `PortSuggestionMenu` component with search and grouping. packages/graph-editor/src/components/flow/portSuggestionMenu.tsx Added a new PortSuggestionMenu component for selecting compatible ports. Implemented a search functionality to filter ports by name or node title. Grouped ports by node type for better organization in the menu. Integrated the component with React's createPortal for rendering. +90/-0    graph.tsx Integrated port dragging and `PortSuggestionMenu` into editor. packages/graph-editor/src/editor/graph.tsx Added logic to handle port dragging and dropping with position tracking. Integrated PortSuggestionMenu into the editor for port selection. Implemented compatibility checks for ports using PortRegistry. Enhanced event handling for connecting ports and creating edges. +154/-36 PortRegistry.ts Added `PortRegistry` for managing and querying ports.        packages/graph-editor/src/services/PortRegistry.ts Created a PortRegistry class to manage and index ports. Implemented functionality to find compatible ports based on type. Added methods to build and query the port index. +82/-0    portSuggestionMenu.module.css Styled `PortSuggestionMenu` component with CSS.                    packages/graph-editor/src/components/flow/portSuggestionMenu.module.css Added styles for the PortSuggestionMenu component. Styled the search input, port list, and individual port items. Included hover effects and responsive design considerations. +62/-0
Configuration changes	version.ts Bumped version to 4.3.9.                                                                  packages/graph-editor/src/data/version.ts Updated the version from 4.3.8 to 4.3.9. +1/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3aa330c

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Possible Performance Concern The filtering and grouping logic for compatiblePorts in the PortSuggestionMenu component could become a performance bottleneck if the number of ports is large. Consider optimizing or memoizing this logic. const filteredPorts = compatiblePorts.filter(port => port.portName.toLowerCase().includes(searchTerm.toLowerCase()) || port.nodeTitle.toLowerCase().includes(searchTerm.toLowerCase()) ); // Group ports by node type const groupedPorts = filteredPorts.reduce<Record<string, PortInfo[]>>( (acc, port) => { if (!acc[port.nodeTitle]) { acc[port.nodeTitle] = []; } acc[port.nodeTitle].push(port); return acc; }, {} ); Error Handling The buildPortIndex method in the PortRegistry class catches errors but only logs them. Consider whether additional error handling or recovery mechanisms are needed. private buildPortIndex() { Object.entries(this.nodeTypes).forEach(([nodeType, NodeClass]) => { try { const tempNode = new NodeClass({ graph: this.graph }); const nodeTitle = tempNode.factory.title || nodeType; Object.entries(tempNode.inputs).forEach(([portName, port]) => { if (!port.variadic) { this.indexPort(portName, nodeType, nodeTitle, port); } }); } catch (error) { console.warn(`Failed to index node type: ${nodeType}`, error); } Debugging Code There are multiple console.log statements in the EditorApp component for debugging purposes. These should be removed or replaced with a proper logging mechanism before merging. console.log('Computing compatible ports for:', draggedPort); if (!draggedPort) return []; const ports = portRegistry.findCompatiblePorts(draggedPort.port); console.log('Found compatible ports:', ports.length, ports); return ports; }, [draggedPort, portRegistry]); const onPortSelect = useCallback( (portInfo: PortInfo) => { if (!draggedPort || !draggedPort.flowPosition) return; // Create the node at the flow position const result = handleSelectNewNodeType({ type: portInfo.nodeType, position: draggedPort.flowPosition, }); if (!result) return; // Connect the ports const sourceNode = graph.getNode(draggedPort.port.node.id); const targetNode = result.graphNode; if (!sourceNode || !targetNode) return; const sourcePort = sourceNode.outputs[draggedPort.port.name]; const targetPort = targetNode.inputs[portInfo.portName]; if (!sourcePort || !targetPort) return; // Create the connection in the graph engine const graphEdge = graph.connect(sourceNode, sourcePort, targetNode, targetPort); // Create the visual edge for ReactFlow const newEdge = { id: graphEdge.id, source: sourceNode.id, target: targetNode.id, sourceHandle: draggedPort.port.name, targetHandle: portInfo.portName, type: 'custom' }; // Add the edge to ReactFlow setEdges(eds => [...eds, newEdge]); // Clear the dragged port state setDraggedPort(null); }, [draggedPort, graph, handleSelectNewNodeType, setEdges] );

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
General	Add error handling for node instantiation to ensure the indexing process is robust against invalid node types Add error handling for the new NodeClass instantiation to prevent the entire indexing process from failing if one node type is invalid. packages/graph-editor/src/services/PortRegistry.ts [25-26] -const tempNode = new NodeClass({ graph: this.graph }); +let tempNode; +try { + tempNode = new NodeClass({ graph: this.graph }); +} catch (error) { + console.warn(`Failed to instantiate node type: ${nodeType}`, error); + return; +} const nodeTitle = tempNode.factory.title || nodeType; Suggestion importance[1-10]: 9 Why: The suggestion adds error handling for node instantiation, which is critical for ensuring that the indexing process does not fail entirely due to one invalid node type. This significantly improves the resilience of the code.	9
Possible issue	Add a safety check for reactFlowInstance to avoid runtime errors when it is undefined Add a null check for reactFlowInstance before calling screenToFlowPosition to prevent runtime errors when the instance is not initialized. packages/graph-editor/src/editor/graph.tsx [308-310] -const flowPosition = reactFlowInstance.screenToFlowPosition({ - x: clientX, - y: clientY, -}); +const flowPosition = reactFlowInstance + ? reactFlowInstance.screenToFlowPosition({ x: clientX, y: clientY }) + : null; Suggestion importance[1-10]: 8 Why: Adding a null check for reactFlowInstance is crucial to prevent runtime errors when the instance is not initialized. This enhances the robustness of the code.	8
	Validate draggedPort.position before rendering PortSuggestionMenu to prevent rendering issues Ensure that draggedPort.position is validated before rendering PortSuggestionMenu to avoid rendering issues when the position is undefined. packages/graph-editor/src/editor/graph.tsx [940-946] -{draggedPort && compatiblePorts.length > 0 && ( +{draggedPort && draggedPort.position && compatiblePorts.length > 0 && ( <PortSuggestionMenu sourcePort={draggedPort.port} compatiblePorts={compatiblePorts} position={draggedPort.position} onSelect={onPortSelect} /> )} Suggestion importance[1-10]: 8 Why: Validating draggedPort.position ensures that the PortSuggestionMenu is only rendered when the position is defined, preventing potential rendering issues and improving code reliability.	8
	Add a unique identifier to the key in the JSX map to prevent potential rendering issues Ensure that each key in the JSX map is unique across all iterations by including a unique identifier for the parent group in addition to portInfo.nodeType and portInfo.portName. packages/graph-editor/src/components/flow/portSuggestionMenu.tsx [72-82] -{ports.map((portInfo) => ( +{ports.map((portInfo, index) => ( <div - key={`${portInfo.nodeType}-${portInfo.portName}`} + key={`${nodeTitle}-${portInfo.nodeType}-${portInfo.portName}-${index}`} onClick={() => handleSelect(portInfo)} className={styles.portItem} > <span className={styles.portName}>{portInfo.portName}</span> <span className={styles.separator}>▸</span> <span className={styles.nodeTitle}>{portInfo.nodeTitle}</span> </div> ))} Suggestion importance[1-10]: 7 Why: The suggestion improves the uniqueness of the key attribute in the JSX map, which is important for React's reconciliation process. Including the nodeTitle and an index ensures no duplicate keys, reducing potential rendering issues.	7

[SorsOps]

Remove console logs when ready

[SorsOps]

We don't need this explicitly

[SorsOps]

Does this need to be in the Hotkeys group?

[SorsOps]

Does this need to be an observer based on its usage?