Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 3.8k
fix(subflows): subflow-child selection issues, subflow error logs#3656
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
ebd570ab110966972d0a567657b9e6b4f78cd4adf669853869f22e6aFile filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -221,3 +221,68 @@ export function resolveParentChildSelectionConflicts( | ||
| return hasConflict ? resolved : nodes | ||
| } | ||
| export function getNodeSelectionContextId( | ||
| node: Pick<Node, 'id' | 'parentId'>, | ||
| blocks: Record<string, { data?: { parentId?: string } }> | ||
| ): string | null { | ||
| return node.parentId || blocks[node.id]?.data?.parentId || null | ||
| } | ||
| export function getEdgeSelectionContextId( | ||
| edge: Pick<Edge, 'source' | 'target'>, | ||
| nodes: Array<Pick<Node, 'id' | 'parentId'>>, | ||
| blocks: Record<string, { data?: { parentId?: string } }> | ||
| ): string | null { | ||
| const sourceNode = nodes.find((node) => node.id === edge.source) | ||
| const targetNode = nodes.find((node) => node.id === edge.target) | ||
| const sourceContextId = sourceNode ? getNodeSelectionContextId(sourceNode, blocks) : null | ||
| const targetContextId = targetNode ? getNodeSelectionContextId(targetNode, blocks) : null | ||
| if (sourceContextId) return sourceContextId | ||
| if (targetContextId) return targetContextId | ||
| return null | ||
| } | ||
| export function resolveSelectionContextConflicts( | ||
| nodes: Node[], | ||
| blocks: Record<string, { data?: { parentId?: string } }>, | ||
| preferredContextId?: string | null | ||
| ): Node[] { | ||
| const selectedNodes = nodes.filter((node) => node.selected) | ||
| if (selectedNodes.length <= 1) return nodes | ||
| const allowedContextId = | ||
| preferredContextId !== undefined | ||
| ? preferredContextId | ||
| : getNodeSelectionContextId(selectedNodes[0], blocks) | ||
| let hasConflict = false | ||
| const resolved = nodes.map((node) => { | ||
| if (!node.selected) return node | ||
| const contextId = getNodeSelectionContextId(node, blocks) | ||
| if (contextId !== allowedContextId) { | ||
| hasConflict = true | ||
| return { ...node, selected: false } | ||
| } | ||
| return node | ||
| }) | ||
| return hasConflict ? resolved : nodes | ||
| } | ||
| export function resolveSelectionConflicts( | ||
| nodes: Node[], | ||
| blocks: Record<string, { data?: { parentId?: string } }>, | ||
| preferredNodeId?: string | ||
| ): Node[] { | ||
| const afterParentChild = resolveParentChildSelectionConflicts(nodes, blocks) | ||
| const preferredContextId = | ||
| preferredNodeId !== undefined | ||
| ? afterParentChild.find((n) => n.id === preferredNodeId && n.selected) | ||
| ? getNodeSelectionContextId(afterParentChild.find((n) => n.id === preferredNodeId)!, blocks) | ||
| : undefined | ||
| : undefined | ||
| return resolveSelectionContextConflicts(afterParentChild, blocks, preferredContextId) | ||
| } | ||
icecrasher321 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -59,11 +59,13 @@ import { | ||
| filterProtectedBlocks, | ||
| getClampedPositionForNode, | ||
| getDescendantBlockIds, | ||
| getEdgeSelectionContextId, | ||
| getNodeSelectionContextId, | ||
| getWorkflowLockToggleIds, | ||
| isBlockProtected, | ||
| isEdgeProtected, | ||
| isInEditableElement, | ||
| resolveParentChildSelectionConflicts, | ||
| resolveSelectionConflicts, | ||
| validateTriggerPaste, | ||
| } from '@/app/workspace/[workspaceId]/w/[workflowId]/utils' | ||
| import { useSocket } from '@/app/workspace/providers/socket-provider' | ||
| @@ -168,16 +170,17 @@ function mapEdgesByNode(edges: Edge[], nodeIds: Set<string>): Map<string, Edge[] | ||
| /** | ||
| * Syncs the panel editor with the current selection state. | ||
| * Shows block details when exactly one block is selected, clears otherwise. | ||
| * Shows the last selected block in the panel. Clears when nothing is selected. | ||
| */ | ||
| function syncPanelWithSelection(selectedIds: string[]) { | ||
| const { currentBlockId, clearCurrentBlock, setCurrentBlockId } = usePanelEditorStore.getState() | ||
| if (selectedIds.length === 1 && selectedIds[0] !== currentBlockId) { | ||
| setCurrentBlockId(selectedIds[0]) | ||
| } else if (selectedIds.length === 0 && currentBlockId) { | ||
| clearCurrentBlock() | ||
| } else if (selectedIds.length > 1 && currentBlockId) { | ||
| clearCurrentBlock() | ||
| if (selectedIds.length === 0) { | ||
| if (currentBlockId) clearCurrentBlock() | ||
| } else { | ||
| const lastSelectedId = selectedIds[selectedIds.length - 1] | ||
| if (lastSelectedId !== currentBlockId) { | ||
| setCurrentBlockId(lastSelectedId) | ||
| } | ||
| } | ||
| } | ||
| @@ -246,7 +249,6 @@ const WorkflowContent = React.memo( | ||
| const [selectedEdges, setSelectedEdges] = useState<SelectedEdgesMap>(new Map()) | ||
| const [isErrorConnectionDrag, setIsErrorConnectionDrag] = useState(false) | ||
| const canvasContainerRef = useRef<HTMLDivElement>(null) | ||
| const selectedIdsRef = useRef<string[] | null>(null) | ||
| const embeddedFitFrameRef = useRef<number | null>(null) | ||
| const hasCompletedInitialEmbeddedFitRef = useRef(false) | ||
| const canvasMode = useCanvasModeStore((state) => state.mode) | ||
| @@ -2477,6 +2479,16 @@ const WorkflowContent = React.memo( | ||
| // Local state for nodes - allows smooth drag without store updates on every frame | ||
| const [displayNodes, setDisplayNodes] = useState<Node[]>([]) | ||
| const selectedNodeIds = useMemo( | ||
| () => displayNodes.filter((node) => node.selected).map((node) => node.id), | ||
| [displayNodes] | ||
| ) | ||
| const selectedNodeIdsKey = selectedNodeIds.join(',') | ||
| useEffect(() => { | ||
| syncPanelWithSelection(selectedNodeIds) | ||
| }, [selectedNodeIdsKey]) | ||
| useEffect(() => { | ||
| // Check for pending selection (from paste/duplicate), otherwise preserve existing selection | ||
| if (pendingSelection && pendingSelection.length > 0) { | ||
| @@ -2488,10 +2500,8 @@ const WorkflowContent = React.memo( | ||
| ...node, | ||
| selected: pendingSet.has(node.id), | ||
| })) | ||
| const resolved = resolveParentChildSelectionConflicts(withSelection, blocks) | ||
| const resolved = resolveSelectionConflicts(withSelection, blocks) | ||
| setDisplayNodes(resolved) | ||
| const selectedIds = resolved.filter((node) => node.selected).map((node) => node.id) | ||
| syncPanelWithSelection(selectedIds) | ||
| return | ||
| } | ||
| @@ -2709,19 +2719,20 @@ const WorkflowContent = React.memo( | ||
| /** Handles node changes - applies changes and resolves parent-child selection conflicts. */ | ||
| const onNodesChange = useCallback( | ||
| (changes: NodeChange[]) => { | ||
| selectedIdsRef.current = null | ||
| setDisplayNodes((nds) => { | ||
| const updated = applyNodeChanges(changes, nds) | ||
| const hasSelectionChange = changes.some((c) => c.type === 'select') | ||
| const hasSelectionChange = changes.some((c) => c.type === 'select') | ||
| setDisplayNodes((currentNodes) => { | ||
| const updated = applyNodeChanges(changes, currentNodes) | ||
| if (!hasSelectionChange) return updated | ||
| const resolved = resolveParentChildSelectionConflicts(updated, blocks) | ||
| selectedIdsRef.current = resolved.filter((node) => node.selected).map((node) => node.id) | ||
| return resolved | ||
| const preferredNodeId = [...changes] | ||
| .reverse() | ||
| .find( | ||
| (change): change is NodeChange & { id: string; selected: boolean } => | ||
| change.type === 'select' && 'selected' in change && change.selected === true | ||
| )?.id | ||
| return resolveSelectionConflicts(updated, blocks, preferredNodeId) | ||
| }) | ||
| const selectedIds = selectedIdsRef.current as string[] | null | ||
| if (selectedIds !== null) { | ||
| syncPanelWithSelection(selectedIds) | ||
| } | ||
| // Handle position changes (e.g., from keyboard arrow key movement) | ||
| // Update container dimensions when child nodes are moved and persist to backend | ||
| @@ -3160,7 +3171,10 @@ const WorkflowContent = React.memo( | ||
| parentId: currentParentId, | ||
| }) | ||
| // Capture all selected nodes' positions for multi-node undo/redo | ||
| // Capture all selected nodes' positions for multi-node undo/redo. | ||
| // Also include the dragged node itself — during shift+click+drag, ReactFlow | ||
| // may have toggled (deselected) the node before drag starts, so it might not | ||
| // appear in the selected set yet. | ||
| const allNodes = getNodes() | ||
| const selectedNodes = allNodes.filter((n) => n.selected) | ||
| multiNodeDragStartRef.current.clear() | ||
| @@ -3174,6 +3188,33 @@ const WorkflowContent = React.memo( | ||
| }) | ||
| } | ||
| }) | ||
| if (!multiNodeDragStartRef.current.has(node.id)) { | ||
| multiNodeDragStartRef.current.set(node.id, { | ||
| x: node.position.x, | ||
| y: node.position.y, | ||
| parentId: currentParentId ?? undefined, | ||
| }) | ||
| } | ||
| // When shift+clicking an already-selected node, ReactFlow toggles (deselects) | ||
| // it via onNodesChange before drag starts. Re-select the dragged node so all | ||
| // previously selected nodes move together as a group — but only if the | ||
| // deselection wasn't from a parent-child conflict (e.g. dragging a child | ||
| // when its parent subflow is selected). | ||
| const draggedNodeInSelected = allNodes.find((n) => n.id === node.id) | ||
| if (draggedNodeInSelected && !draggedNodeInSelected.selected && selectedNodes.length > 0) { | ||
| const draggedParentId = blocks[node.id]?.data?.parentId | ||
| const parentIsSelected = | ||
| draggedParentId && selectedNodes.some((n) => n.id === draggedParentId) | ||
| const contextMismatch = | ||
| getNodeSelectionContextId(draggedNodeInSelected, blocks) !== | ||
| getNodeSelectionContextId(selectedNodes[0], blocks) | ||
| if (!parentIsSelected && !contextMismatch) { | ||
| setDisplayNodes((currentNodes) => | ||
| currentNodes.map((n) => (n.id === node.id ? { ...n, selected: true } : n)) | ||
| ) | ||
| } | ||
| } | ||
icecrasher321 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| }, | ||
| [blocks, setDragStartPosition, getNodes, setPotentialParentId] | ||
| ) | ||
| @@ -3453,7 +3494,7 @@ const WorkflowContent = React.memo( | ||
| }) | ||
| // Apply visual deselection of children | ||
| setDisplayNodes((allNodes) => resolveParentChildSelectionConflicts(allNodes, blocks)) | ||
| setDisplayNodes((allNodes) => resolveSelectionConflicts(allNodes, blocks)) | ||
| }, | ||
| [blocks] | ||
| ) | ||
| @@ -3604,36 +3645,36 @@ const WorkflowContent = React.memo( | ||
| /** | ||
| * Handles node click to select the node in ReactFlow. | ||
| * Parent-child conflict resolution happens automatically in onNodesChange. | ||
| * Uses the controlled display node state so parent-child conflicts are resolved | ||
| * consistently for click, shift-click, and marquee selection. | ||
| */ | ||
| const handleNodeClick = useCallback( | ||
| (event: React.MouseEvent, node: Node) => { | ||
| const isMultiSelect = event.shiftKey || event.metaKey || event.ctrlKey | ||
| setNodes((nodes) => | ||
| nodes.map((n) => ({ | ||
| ...n, | ||
| selected: isMultiSelect ? (n.id === node.id ? true : n.selected) : n.id === node.id, | ||
| setDisplayNodes((currentNodes) => { | ||
| const updated = currentNodes.map((currentNode) => ({ | ||
| ...currentNode, | ||
| selected: isMultiSelect | ||
| ? currentNode.id === node.id | ||
| ? true | ||
| : currentNode.selected | ||
| : currentNode.id === node.id, | ||
| })) | ||
| ) | ||
| return resolveSelectionConflicts(updated, blocks, isMultiSelect ? node.id : undefined) | ||
| }) | ||
| }, | ||
| [setNodes] | ||
| [blocks] | ||
| ) | ||
| /** Handles edge selection with container context tracking and Shift-click multi-selection. */ | ||
| const onEdgeClick = useCallback( | ||
| (event: React.MouseEvent, edge: any) => { | ||
| event.stopPropagation() // Prevent bubbling | ||
| // Determine if edge is inside a loop by checking its source/target nodes | ||
| const sourceNode = getNodes().find((n) => n.id === edge.source) | ||
| const targetNode = getNodes().find((n) => n.id === edge.target) | ||
| // An edge is inside a loop if either source or target has a parent | ||
| // If source and target have different parents, prioritize source's parent | ||
| const parentLoopId = sourceNode?.parentId || targetNode?.parentId | ||
| // Create a unique identifier that combines edge ID and parent context | ||
| const contextId = `${edge.id}${parentLoopId ? `-${parentLoopId}` : ''}` | ||
| const contextId = `${edge.id}${(() => { | ||
| const selectionContextId = getEdgeSelectionContextId(edge, getNodes(), blocks) | ||
| return selectionContextId ? `-${selectionContextId}` : '' | ||
| })()}` | ||
| if (event.shiftKey) { | ||
| // Shift-click: toggle edge in selection | ||
| @@ -3651,7 +3692,7 @@ const WorkflowContent = React.memo( | ||
| setSelectedEdges(new Map([[contextId, edge.id]])) | ||
| } | ||
| }, | ||
| [getNodes] | ||
| [blocks, getNodes] | ||
| ) | ||
| /** Stable delete handler to avoid creating new function references per edge. */ | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.