fit(loop-ui): fixed some UI issues, added tests for parallel and loop nodes (#421)

* basic change

* fixed drag

* testing and UI fixes

* ran bun

* added greptile comments

---------

Co-authored-by: Adam Gough <adamgough@Adams-MacBook-Pro.local>
This commit is contained in:
Adam Gough
2025-05-26 12:05:23 -07:00
committed by GitHub
parent 6afb453fc0
commit ad4060aa92
8 changed files with 1198 additions and 173 deletions

View File

@@ -0,0 +1,463 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
import { LoopNodeComponent } from './loop-node'
// Mock dependencies that don't need DOM
vi.mock('@/stores/workflows/workflow/store', () => ({
useWorkflowStore: vi.fn(),
}))
vi.mock('@/lib/logs/console-logger', () => ({
createLogger: vi.fn(() => ({
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
})),
}))
// Mock ReactFlow components and hooks
vi.mock('reactflow', () => ({
Handle: ({ id, type, position }: any) => ({ id, type, position }),
Position: {
Top: 'top',
Bottom: 'bottom',
Left: 'left',
Right: 'right',
},
useReactFlow: () => ({
getNodes: vi.fn(() => []),
}),
NodeResizer: ({ isVisible }: any) => ({ isVisible }),
memo: (component: any) => component,
}))
// Mock React hooks
vi.mock('react', async () => {
const actual = await vi.importActual('react')
return {
...actual,
memo: (component: any) => component,
useMemo: (fn: any) => fn(),
useRef: () => ({ current: null }),
}
})
// Mock UI components
vi.mock('@/components/ui/button', () => ({
Button: ({ children, onClick, ...props }: any) => ({ children, onClick, ...props }),
}))
vi.mock('@/components/ui/card', () => ({
Card: ({ children, ...props }: any) => ({ children, ...props }),
}))
vi.mock('@/components/icons', () => ({
StartIcon: ({ className }: any) => ({ className }),
}))
vi.mock('@/lib/utils', () => ({
cn: (...classes: any[]) => classes.filter(Boolean).join(' '),
}))
// Mock the LoopBadges component
vi.mock('./components/loop-badges', () => ({
LoopBadges: ({ loopId }: any) => ({ loopId }),
}))
describe('LoopNodeComponent', () => {
const mockRemoveBlock = vi.fn()
const mockGetNodes = vi.fn()
const defaultProps = {
id: 'loop-1',
type: 'loopNode',
data: {
width: 500,
height: 300,
state: 'valid',
},
selected: false,
zIndex: 1,
isConnectable: true,
xPos: 0,
yPos: 0,
dragging: false,
}
beforeEach(() => {
vi.clearAllMocks()
// Mock useWorkflowStore
;(useWorkflowStore as any).mockImplementation((selector: any) => {
const state = {
removeBlock: mockRemoveBlock,
}
return selector(state)
})
// Mock getNodes
mockGetNodes.mockReturnValue([])
})
describe('Component Definition and Structure', () => {
it('should be defined as a function component', () => {
expect(LoopNodeComponent).toBeDefined()
expect(typeof LoopNodeComponent).toBe('function')
})
it('should have correct display name', () => {
expect(LoopNodeComponent.displayName).toBe('LoopNodeComponent')
})
it('should be a memoized component', () => {
// Since we mocked memo to return the component as-is, we can verify it exists
expect(LoopNodeComponent).toBeDefined()
})
})
describe('Props Validation and Type Safety', () => {
it('should accept NodeProps interface', () => {
// Test that the component accepts the correct prop types
const validProps = {
id: 'test-id',
type: 'loopNode' as const,
data: {
width: 400,
height: 300,
state: 'valid' as const,
},
selected: false,
zIndex: 1,
isConnectable: true,
xPos: 0,
yPos: 0,
dragging: false,
}
// This tests that TypeScript compilation succeeds with these props
expect(() => {
// We're not calling the component, just verifying the types
const _component: typeof LoopNodeComponent = LoopNodeComponent
expect(_component).toBeDefined()
}).not.toThrow()
})
it('should handle different data configurations', () => {
const configurations = [
{ width: 500, height: 300, state: 'valid' },
{ width: 800, height: 600, state: 'invalid' },
{ width: 0, height: 0, state: 'pending' },
{},
]
configurations.forEach((data) => {
const props = { ...defaultProps, data }
expect(() => {
const _component: typeof LoopNodeComponent = LoopNodeComponent
expect(_component).toBeDefined()
}).not.toThrow()
})
})
})
describe('Store Integration', () => {
it('should integrate with workflow store', () => {
// Test that the component uses the store correctly
expect(useWorkflowStore).toBeDefined()
// Verify the store selector function works
const mockState = { removeBlock: mockRemoveBlock }
const selector = vi.fn((state) => state.removeBlock)
expect(() => {
selector(mockState)
}).not.toThrow()
expect(selector(mockState)).toBe(mockRemoveBlock)
})
it('should handle removeBlock function', () => {
expect(mockRemoveBlock).toBeDefined()
expect(typeof mockRemoveBlock).toBe('function')
// Test calling removeBlock
mockRemoveBlock('test-id')
expect(mockRemoveBlock).toHaveBeenCalledWith('test-id')
})
})
describe('Component Logic Tests', () => {
it('should handle nesting level calculation logic', () => {
// Test the nesting level calculation logic
const testCases = [
{ nodes: [], parentId: undefined, expectedLevel: 0 },
{ nodes: [{ id: 'parent', data: {} }], parentId: 'parent', expectedLevel: 1 },
{
nodes: [
{ id: 'parent', data: { parentId: 'grandparent' } },
{ id: 'grandparent', data: {} },
],
parentId: 'parent',
expectedLevel: 2,
},
]
testCases.forEach(({ nodes, parentId, expectedLevel }) => {
mockGetNodes.mockReturnValue(nodes)
// Simulate the nesting level calculation logic
let level = 0
let currentParentId = parentId
while (currentParentId) {
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
expect(level).toBe(expectedLevel)
})
})
it('should handle nested styles generation', () => {
// Test the nested styles logic
const testCases = [
{ nestingLevel: 0, state: 'valid', expectedBg: 'rgba(34,197,94,0.05)' },
{ nestingLevel: 0, state: 'invalid', expectedBg: 'transparent' },
{ nestingLevel: 1, state: 'valid', expectedBg: '#e2e8f030' },
{ nestingLevel: 2, state: 'valid', expectedBg: '#cbd5e130' },
]
testCases.forEach(({ nestingLevel, state, expectedBg }) => {
// Simulate the getNestedStyles logic
const styles: Record<string, string> = {
backgroundColor: state === 'valid' ? 'rgba(34,197,94,0.05)' : 'transparent',
}
if (nestingLevel > 0) {
const colors = ['#e2e8f0', '#cbd5e1', '#94a3b8', '#64748b', '#475569']
const colorIndex = (nestingLevel - 1) % colors.length
styles.backgroundColor = `${colors[colorIndex]}30`
}
expect(styles.backgroundColor).toBe(expectedBg)
})
})
})
describe('Component Configuration', () => {
it('should handle different dimensions', () => {
const dimensionTests = [
{ width: 500, height: 300 },
{ width: 800, height: 600 },
{ width: 0, height: 0 },
{ width: 10000, height: 10000 },
]
dimensionTests.forEach(({ width, height }) => {
const data = { width, height, state: 'valid' }
expect(data.width).toBe(width)
expect(data.height).toBe(height)
})
})
it('should handle different states', () => {
const stateTests = ['valid', 'invalid', 'pending', 'executing']
stateTests.forEach((state) => {
const data = { width: 500, height: 300, state }
expect(data.state).toBe(state)
})
})
})
describe('Event Handling Logic', () => {
it('should handle delete button click logic', () => {
const mockEvent = {
stopPropagation: vi.fn(),
}
// Simulate the delete button click handler
const handleDelete = (e: any, nodeId: string) => {
e.stopPropagation()
mockRemoveBlock(nodeId)
}
handleDelete(mockEvent, 'test-id')
expect(mockEvent.stopPropagation).toHaveBeenCalled()
expect(mockRemoveBlock).toHaveBeenCalledWith('test-id')
})
it('should handle event propagation prevention', () => {
const mockEvent = {
stopPropagation: vi.fn(),
}
// Test that stopPropagation is called
mockEvent.stopPropagation()
expect(mockEvent.stopPropagation).toHaveBeenCalled()
})
})
describe('Component Data Handling', () => {
it('should handle missing data properties gracefully', () => {
const testCases = [
undefined,
{},
{ width: 500 },
{ height: 300 },
{ state: 'valid' },
{ width: 500, height: 300 },
]
testCases.forEach((data) => {
const props = { ...defaultProps, data }
// Test default values logic
const width = Math.max(0, data?.width || 500)
const height = Math.max(0, data?.height || 300)
expect(width).toBeGreaterThanOrEqual(0)
expect(height).toBeGreaterThanOrEqual(0)
})
})
it('should handle parent ID relationships', () => {
const testCases = [
{ parentId: undefined, hasParent: false },
{ parentId: 'parent-1', hasParent: true },
{ parentId: '', hasParent: false },
]
testCases.forEach(({ parentId, hasParent }) => {
const data = { ...defaultProps.data, parentId }
expect(Boolean(data.parentId)).toBe(hasParent)
})
})
})
describe('Edge Cases and Error Handling', () => {
it('should handle circular parent references', () => {
// Test circular reference prevention
const nodes = [
{ id: 'node1', data: { parentId: 'node2' } },
{ id: 'node2', data: { parentId: 'node1' } },
]
mockGetNodes.mockReturnValue(nodes)
// Test the actual component's nesting level calculation logic
// This simulates the real useMemo logic from the component
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
// This is the actual logic pattern used in the component
while (currentParentId) {
// If we've seen this parent before, we have a cycle - break immediately
if (visited.has(currentParentId)) {
break
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// With proper circular reference detection, we should stop at level 2
// (node1 -> node2, then detect cycle when trying to go back to node1)
expect(level).toBe(2)
expect(visited.has('node1')).toBe(true)
expect(visited.has('node2')).toBe(true)
})
it('should handle complex circular reference chains', () => {
// Test more complex circular reference scenarios
const nodes = [
{ id: 'node1', data: { parentId: 'node2' } },
{ id: 'node2', data: { parentId: 'node3' } },
{ id: 'node3', data: { parentId: 'node1' } }, // Creates a 3-node cycle
]
mockGetNodes.mockReturnValue(nodes)
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
while (currentParentId) {
if (visited.has(currentParentId)) {
break // Cycle detected
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// Should traverse node1 -> node2 -> node3, then detect cycle
expect(level).toBe(3)
expect(visited.size).toBe(3)
})
it('should handle self-referencing nodes', () => {
// Test node that references itself
const nodes = [
{ id: 'node1', data: { parentId: 'node1' } }, // Self-reference
]
mockGetNodes.mockReturnValue(nodes)
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
while (currentParentId) {
if (visited.has(currentParentId)) {
break // Cycle detected immediately
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// Should detect self-reference immediately after first iteration
expect(level).toBe(1)
expect(visited.has('node1')).toBe(true)
})
it('should handle extreme values', () => {
const extremeValues = [
{ width: Number.MAX_SAFE_INTEGER, height: Number.MAX_SAFE_INTEGER },
{ width: -1, height: -1 },
{ width: 0, height: 0 },
{ width: null, height: null },
]
extremeValues.forEach((data) => {
expect(() => {
const width = data.width || 500
const height = data.height || 300
expect(typeof width).toBe('number')
expect(typeof height).toBe('number')
}).not.toThrow()
})
})
})
})

View File

@@ -26,12 +26,6 @@ const LoopNodeStyles: React.FC = () => {
box-shadow: 0 0 0 8px rgba(47, 179, 255, 0.1);
}
/* Make resizer handles more visible */
.react-flow__resize-control {
z-index: 10;
pointer-events: all !important;
}
/* Ensure parent borders are visible when hovering over resize controls */
.react-flow__node-group:hover,
.hover-highlight {
@@ -43,12 +37,7 @@ const LoopNodeStyles: React.FC = () => {
opacity: 1 !important;
visibility: visible !important;
}
/* React Flow position transitions within loops */
.react-flow__node[data-parent-node-id] {
transition: transform 0.05s ease;
pointer-events: all;
}
/* Prevent jumpy drag behavior */
.loop-drop-container .react-flow__node {

View File

@@ -0,0 +1,616 @@
import { beforeEach, describe, expect, it, vi } from 'vitest'
import { useWorkflowStore } from '@/stores/workflows/workflow/store'
import { ParallelNodeComponent } from './parallel-node'
// Mock dependencies that don't need DOM
vi.mock('@/stores/workflows/workflow/store', () => ({
useWorkflowStore: vi.fn(),
}))
vi.mock('@/lib/logs/console-logger', () => ({
createLogger: vi.fn(() => ({
debug: vi.fn(),
info: vi.fn(),
warn: vi.fn(),
error: vi.fn(),
})),
}))
// Mock ReactFlow components and hooks
vi.mock('reactflow', () => ({
Handle: ({ id, type, position }: any) => ({ id, type, position }),
Position: {
Top: 'top',
Bottom: 'bottom',
Left: 'left',
Right: 'right',
},
useReactFlow: () => ({
getNodes: vi.fn(() => []),
}),
NodeResizer: ({ isVisible }: any) => ({ isVisible }),
memo: (component: any) => component,
}))
// Mock React hooks
vi.mock('react', async () => {
const actual = await vi.importActual('react')
return {
...actual,
memo: (component: any) => component,
useMemo: (fn: any) => fn(),
useRef: () => ({ current: null }),
}
})
// Mock UI components
vi.mock('@/components/ui/button', () => ({
Button: ({ children, onClick, ...props }: any) => ({ children, onClick, ...props }),
}))
vi.mock('@/components/ui/card', () => ({
Card: ({ children, ...props }: any) => ({ children, ...props }),
}))
vi.mock('@/components/icons', () => ({
StartIcon: ({ className }: any) => ({ className }),
}))
vi.mock('@/lib/utils', () => ({
cn: (...classes: any[]) => classes.filter(Boolean).join(' '),
}))
// Mock the ParallelBadges component
vi.mock('./components/parallel-badges', () => ({
ParallelBadges: ({ parallelId }: any) => ({ parallelId }),
}))
describe('ParallelNodeComponent', () => {
const mockRemoveBlock = vi.fn()
const mockGetNodes = vi.fn()
const defaultProps = {
id: 'parallel-1',
type: 'parallelNode',
data: {
width: 500,
height: 300,
state: 'valid',
},
selected: false,
zIndex: 1,
isConnectable: true,
xPos: 0,
yPos: 0,
dragging: false,
}
beforeEach(() => {
vi.clearAllMocks()
// Mock useWorkflowStore
;(useWorkflowStore as any).mockImplementation((selector: any) => {
const state = {
removeBlock: mockRemoveBlock,
}
return selector(state)
})
// Mock getNodes
mockGetNodes.mockReturnValue([])
})
describe('Component Definition and Structure', () => {
it('should be defined as a function component', () => {
expect(ParallelNodeComponent).toBeDefined()
expect(typeof ParallelNodeComponent).toBe('function')
})
it('should have correct display name', () => {
expect(ParallelNodeComponent.displayName).toBe('ParallelNodeComponent')
})
it('should be a memoized component', () => {
// Since we mocked memo to return the component as-is, we can verify it exists
expect(ParallelNodeComponent).toBeDefined()
})
})
describe('Props Validation and Type Safety', () => {
it('should accept NodeProps interface', () => {
// Test that the component accepts the correct prop types
const validProps = {
id: 'test-id',
type: 'parallelNode' as const,
data: {
width: 400,
height: 300,
state: 'valid' as const,
},
selected: false,
zIndex: 1,
isConnectable: true,
xPos: 0,
yPos: 0,
dragging: false,
}
// This tests that TypeScript compilation succeeds with these props
expect(() => {
// We're not calling the component, just verifying the types
const _component: typeof ParallelNodeComponent = ParallelNodeComponent
expect(_component).toBeDefined()
}).not.toThrow()
})
it('should handle different data configurations', () => {
const configurations = [
{ width: 500, height: 300, state: 'valid' },
{ width: 800, height: 600, state: 'invalid' },
{ width: 0, height: 0, state: 'pending' },
{},
]
configurations.forEach((data) => {
const props = { ...defaultProps, data }
expect(() => {
const _component: typeof ParallelNodeComponent = ParallelNodeComponent
expect(_component).toBeDefined()
}).not.toThrow()
})
})
})
describe('Store Integration', () => {
it('should integrate with workflow store', () => {
// Test that the component uses the store correctly
expect(useWorkflowStore).toBeDefined()
// Verify the store selector function works
const mockState = { removeBlock: mockRemoveBlock }
const selector = vi.fn((state) => state.removeBlock)
expect(() => {
selector(mockState)
}).not.toThrow()
expect(selector(mockState)).toBe(mockRemoveBlock)
})
it('should handle removeBlock function', () => {
expect(mockRemoveBlock).toBeDefined()
expect(typeof mockRemoveBlock).toBe('function')
// Test calling removeBlock
mockRemoveBlock('test-id')
expect(mockRemoveBlock).toHaveBeenCalledWith('test-id')
})
})
describe('Component Logic Tests', () => {
it('should handle nesting level calculation logic', () => {
// Test the nesting level calculation logic (same as loop node)
const testCases = [
{ nodes: [], parentId: undefined, expectedLevel: 0 },
{ nodes: [{ id: 'parent', data: {} }], parentId: 'parent', expectedLevel: 1 },
{
nodes: [
{ id: 'parent', data: { parentId: 'grandparent' } },
{ id: 'grandparent', data: {} },
],
parentId: 'parent',
expectedLevel: 2,
},
]
testCases.forEach(({ nodes, parentId, expectedLevel }) => {
mockGetNodes.mockReturnValue(nodes)
// Simulate the nesting level calculation logic
let level = 0
let currentParentId = parentId
while (currentParentId) {
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
expect(level).toBe(expectedLevel)
})
})
it('should handle nested styles generation for parallel nodes', () => {
// Test the nested styles logic with parallel-specific colors
const testCases = [
{ nestingLevel: 0, state: 'valid', expectedBg: 'rgba(254,225,43,0.05)' },
{ nestingLevel: 0, state: 'invalid', expectedBg: 'transparent' },
{ nestingLevel: 1, state: 'valid', expectedBg: '#e2e8f030' },
{ nestingLevel: 2, state: 'valid', expectedBg: '#cbd5e130' },
]
testCases.forEach(({ nestingLevel, state, expectedBg }) => {
// Simulate the getNestedStyles logic for parallel nodes
const styles: Record<string, string> = {
backgroundColor: state === 'valid' ? 'rgba(254,225,43,0.05)' : 'transparent',
}
if (nestingLevel > 0) {
const colors = ['#e2e8f0', '#cbd5e1', '#94a3b8', '#64748b', '#475569']
const colorIndex = (nestingLevel - 1) % colors.length
styles.backgroundColor = `${colors[colorIndex]}30`
}
expect(styles.backgroundColor).toBe(expectedBg)
})
})
})
describe('Parallel-Specific Features', () => {
it('should handle parallel execution states', () => {
const parallelStates = ['valid', 'invalid', 'executing', 'completed', 'pending']
parallelStates.forEach((state) => {
const data = { width: 500, height: 300, state }
expect(data.state).toBe(state)
// Test parallel-specific state handling
const isExecuting = state === 'executing'
const isCompleted = state === 'completed'
expect(typeof isExecuting).toBe('boolean')
expect(typeof isCompleted).toBe('boolean')
})
})
it('should handle parallel node color scheme', () => {
// Test that parallel nodes use yellow color scheme
const parallelColors = {
background: 'rgba(254,225,43,0.05)',
ring: '#FEE12B',
startIcon: '#FEE12B',
}
expect(parallelColors.background).toContain('254,225,43')
expect(parallelColors.ring).toBe('#FEE12B')
expect(parallelColors.startIcon).toBe('#FEE12B')
})
it('should differentiate from loop node styling', () => {
// Ensure parallel nodes have different styling than loop nodes
const loopColors = {
background: 'rgba(34,197,94,0.05)',
ring: '#2FB3FF',
startIcon: '#2FB3FF',
}
const parallelColors = {
background: 'rgba(254,225,43,0.05)',
ring: '#FEE12B',
startIcon: '#FEE12B',
}
expect(parallelColors.background).not.toBe(loopColors.background)
expect(parallelColors.ring).not.toBe(loopColors.ring)
expect(parallelColors.startIcon).not.toBe(loopColors.startIcon)
})
})
describe('Component Configuration', () => {
it('should handle different dimensions', () => {
const dimensionTests = [
{ width: 500, height: 300 },
{ width: 800, height: 600 },
{ width: 0, height: 0 },
{ width: 10000, height: 10000 },
]
dimensionTests.forEach(({ width, height }) => {
const data = { width, height, state: 'valid' }
expect(data.width).toBe(width)
expect(data.height).toBe(height)
})
})
it('should handle different states', () => {
const stateTests = ['valid', 'invalid', 'pending', 'executing', 'completed']
stateTests.forEach((state) => {
const data = { width: 500, height: 300, state }
expect(data.state).toBe(state)
})
})
})
describe('Event Handling Logic', () => {
it('should handle delete button click logic', () => {
const mockEvent = {
stopPropagation: vi.fn(),
}
// Simulate the delete button click handler
const handleDelete = (e: any, nodeId: string) => {
e.stopPropagation()
mockRemoveBlock(nodeId)
}
handleDelete(mockEvent, 'test-id')
expect(mockEvent.stopPropagation).toHaveBeenCalled()
expect(mockRemoveBlock).toHaveBeenCalledWith('test-id')
})
it('should handle event propagation prevention', () => {
const mockEvent = {
stopPropagation: vi.fn(),
}
// Test that stopPropagation is called
mockEvent.stopPropagation()
expect(mockEvent.stopPropagation).toHaveBeenCalled()
})
})
describe('Component Data Handling', () => {
it('should handle missing data properties gracefully', () => {
const testCases = [
undefined,
{},
{ width: 500 },
{ height: 300 },
{ state: 'valid' },
{ width: 500, height: 300 },
]
testCases.forEach((data) => {
const props = { ...defaultProps, data }
// Test default values logic
const width = data?.width || 500
const height = data?.height || 300
expect(width).toBeGreaterThanOrEqual(0)
expect(height).toBeGreaterThanOrEqual(0)
})
})
it('should handle parent ID relationships', () => {
const testCases = [
{ parentId: undefined, hasParent: false },
{ parentId: 'parent-1', hasParent: true },
{ parentId: '', hasParent: false },
]
testCases.forEach(({ parentId, hasParent }) => {
const data = { ...defaultProps.data, parentId }
expect(Boolean(data.parentId)).toBe(hasParent)
})
})
})
describe('Handle Configuration', () => {
it('should have correct handle IDs for parallel nodes', () => {
const handleIds = {
startSource: 'parallel-start-source',
endSource: 'parallel-end-source',
}
expect(handleIds.startSource).toContain('parallel')
expect(handleIds.endSource).toContain('parallel')
expect(handleIds.startSource).not.toContain('loop')
expect(handleIds.endSource).not.toContain('loop')
})
it('should handle different handle positions', () => {
const positions = {
left: 'left',
right: 'right',
top: 'top',
bottom: 'bottom',
}
Object.values(positions).forEach((position) => {
expect(typeof position).toBe('string')
expect(position.length).toBeGreaterThan(0)
})
})
})
describe('Edge Cases and Error Handling', () => {
it('should handle circular parent references', () => {
// Test circular reference prevention
const nodes = [
{ id: 'node1', data: { parentId: 'node2' } },
{ id: 'node2', data: { parentId: 'node1' } },
]
mockGetNodes.mockReturnValue(nodes)
// Test the actual component's nesting level calculation logic
// This simulates the real useMemo logic from the component
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
// This is the actual logic pattern used in the component
while (currentParentId) {
// If we've seen this parent before, we have a cycle - break immediately
if (visited.has(currentParentId)) {
break
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// With proper circular reference detection, we should stop at level 2
// (node1 -> node2, then detect cycle when trying to go back to node1)
expect(level).toBe(2)
expect(visited.has('node1')).toBe(true)
expect(visited.has('node2')).toBe(true)
})
it('should handle complex circular reference chains', () => {
// Test more complex circular reference scenarios
const nodes = [
{ id: 'node1', data: { parentId: 'node2' } },
{ id: 'node2', data: { parentId: 'node3' } },
{ id: 'node3', data: { parentId: 'node1' } }, // Creates a 3-node cycle
]
mockGetNodes.mockReturnValue(nodes)
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
while (currentParentId) {
if (visited.has(currentParentId)) {
break // Cycle detected
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// Should traverse node1 -> node2 -> node3, then detect cycle
expect(level).toBe(3)
expect(visited.size).toBe(3)
})
it('should handle self-referencing nodes', () => {
// Test node that references itself
const nodes = [
{ id: 'node1', data: { parentId: 'node1' } }, // Self-reference
]
mockGetNodes.mockReturnValue(nodes)
let level = 0
let currentParentId = 'node1'
const visited = new Set<string>()
while (currentParentId) {
if (visited.has(currentParentId)) {
break // Cycle detected immediately
}
visited.add(currentParentId)
level++
const parentNode = nodes.find((n) => n.id === currentParentId)
if (!parentNode) break
currentParentId = parentNode.data?.parentId
}
// Should detect self-reference immediately after first iteration
expect(level).toBe(1)
expect(visited.has('node1')).toBe(true)
})
it('should handle extreme values', () => {
const extremeValues = [
{ width: Number.MAX_SAFE_INTEGER, height: Number.MAX_SAFE_INTEGER },
{ width: -1, height: -1 },
{ width: 0, height: 0 },
{ width: null, height: null },
]
extremeValues.forEach((data) => {
expect(() => {
const width = data.width || 500
const height = data.height || 300
expect(typeof width).toBe('number')
expect(typeof height).toBe('number')
}).not.toThrow()
})
})
it('should handle negative position values', () => {
const positions = [
{ xPos: -100, yPos: -200 },
{ xPos: 0, yPos: 0 },
{ xPos: 1000, yPos: 2000 },
]
positions.forEach(({ xPos, yPos }) => {
const props = { ...defaultProps, xPos, yPos }
expect(props.xPos).toBe(xPos)
expect(props.yPos).toBe(yPos)
expect(typeof props.xPos).toBe('number')
expect(typeof props.yPos).toBe('number')
})
})
})
describe('Component Comparison with Loop Node', () => {
it('should have similar structure to loop node but different type', () => {
expect(defaultProps.type).toBe('parallelNode')
expect(defaultProps.id).toContain('parallel')
// Should not be a loop node
expect(defaultProps.type).not.toBe('loopNode')
expect(defaultProps.id).not.toContain('loop')
})
it('should handle the same prop structure as loop node', () => {
// Test that parallel node accepts the same prop structure as loop node
const sharedPropStructure = {
id: 'test-parallel',
type: 'parallelNode' as const,
data: {
width: 400,
height: 300,
state: 'valid' as const,
},
selected: false,
zIndex: 1,
isConnectable: true,
xPos: 0,
yPos: 0,
dragging: false,
}
expect(() => {
const _component: typeof ParallelNodeComponent = ParallelNodeComponent
expect(_component).toBeDefined()
}).not.toThrow()
// Verify the structure
expect(sharedPropStructure.type).toBe('parallelNode')
expect(sharedPropStructure.data.width).toBe(400)
expect(sharedPropStructure.data.height).toBe(300)
})
it('should maintain consistency with loop node interface', () => {
// Both components should accept the same base props
const baseProps = [
'id',
'type',
'data',
'selected',
'zIndex',
'isConnectable',
'xPos',
'yPos',
'dragging',
]
baseProps.forEach((prop) => {
expect(defaultProps).toHaveProperty(prop)
})
})
})
})

View File

@@ -25,12 +25,6 @@ const ParallelNodeStyles: React.FC = () => {
box-shadow: 0 0 0 8px rgba(254, 225, 43, 0.1);
}
/* Make resizer handles more visible */
.react-flow__resize-control {
z-index: 10;
pointer-events: all !important;
}
/* Ensure parent borders are visible when hovering over resize controls */
.react-flow__node-group:hover,
.hover-highlight {
@@ -43,11 +37,6 @@ const ParallelNodeStyles: React.FC = () => {
visibility: visible !important;
}
/* React Flow position transitions within parallel blocks */
.react-flow__node[data-parent-node-id] {
transition: transform 0.05s ease;
pointer-events: all;
}
/* Prevent jumpy drag behavior */
.parallel-drop-container .react-flow__node {

View File

@@ -105,6 +105,7 @@ export function LongInput({
// Handle resize functionality
const startResize = (e: React.MouseEvent) => {
e.preventDefault()
e.stopPropagation()
isResizing.current = true
const startY = e.clientY
@@ -290,6 +291,9 @@ export function LongInput({
<div
className='absolute right-1 bottom-1 flex h-4 w-4 cursor-s-resize items-center justify-center rounded-sm bg-background'
onMouseDown={startResize}
onDragStart={(e) => {
e.preventDefault()
}}
>
<ChevronsUpDown className='h-3 w-3 text-muted-foreground/70' />
</div>

View File

@@ -426,7 +426,12 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
)}
{/* Block Header */}
<div className='workflow-drag-handle flex cursor-grab items-center justify-between border-b p-3 [&:active]:cursor-grabbing'>
<div
className='workflow-drag-handle flex cursor-grab items-center justify-between border-b p-3 [&:active]:cursor-grabbing'
onMouseDown={(e) => {
e.stopPropagation()
}}
>
<div className='flex items-center gap-3'>
<div
className='flex h-7 w-7 items-center justify-center rounded'
@@ -662,7 +667,13 @@ export function WorkflowBlock({ id, data }: NodeProps<WorkflowBlockProps>) {
</div>
{/* Block Content */}
<div ref={contentRef} className='cursor-pointer space-y-4 px-4 pt-3 pb-4'>
<div
ref={contentRef}
className='cursor-pointer space-y-4 px-4 pt-3 pb-4'
onMouseDown={(e) => {
e.stopPropagation()
}}
>
{subBlockRows.length > 0
? subBlockRows.map((row, rowIndex) => (
<div key={`row-${rowIndex}`} className='flex gap-4'>

View File

@@ -151,12 +151,6 @@ export const updateNodeParent = (
// Update both position and parent
updateBlockPosition(nodeId, relativePosition)
updateParentId(nodeId, newParentId, 'parent')
logger.info('Updated node parent', {
nodeId,
newParentId,
relativePosition,
})
} else if (currentParentId) {
// Removing parent - convert to absolute position
const absolutePosition = getNodeAbsolutePosition(nodeId, getNodes)
@@ -164,12 +158,6 @@ export const updateNodeParent = (
// Update position to absolute coordinates and remove parent
updateBlockPosition(nodeId, absolutePosition)
// Note: updateParentId function signature needs to handle null case
logger.info('Removed node parent', {
nodeId,
previousParentId: currentParentId,
absolutePosition,
})
}
// Resize affected loops
@@ -262,45 +250,51 @@ export const calculateLoopDimensions = (
let nodeHeight
if (node.type === 'loopNode' || node.type === 'parallelNode') {
// For nested containers, don't add excessive padding to the parent
// Use actual dimensions without additional padding to prevent cascading expansion
// For nested containers, use their actual dimensions
nodeWidth = node.data?.width || DEFAULT_CONTAINER_WIDTH
nodeHeight = node.data?.height || DEFAULT_CONTAINER_HEIGHT
} else if (node.type === 'workflowBlock') {
// Handle all workflowBlock types appropriately
const blockType = node.data?.type
// Use actual dynamic dimensions from the node data if available
// Fall back to block type defaults only if no dynamic dimensions exist
nodeWidth = node.data?.width || node.width
nodeHeight = node.data?.height || node.height
switch (blockType) {
case 'agent':
case 'api':
// Tall blocks
nodeWidth = 350
nodeHeight = 650
break
case 'condition':
case 'function':
nodeWidth = 250
nodeHeight = 200
break
case 'router':
nodeWidth = 250
nodeHeight = 350
break
default:
// Default dimensions for other block types
nodeWidth = 200
nodeHeight = 200
// If still no dimensions, use block type defaults as last resort
if (!nodeWidth || !nodeHeight) {
const blockType = node.data?.type
switch (blockType) {
case 'agent':
case 'api':
// Tall blocks
nodeWidth = nodeWidth || 350
nodeHeight = nodeHeight || 650
break
case 'condition':
case 'function':
nodeWidth = nodeWidth || 250
nodeHeight = nodeHeight || 200
break
case 'router':
nodeWidth = nodeWidth || 250
nodeHeight = nodeHeight || 350
break
default:
// Default dimensions for other block types
nodeWidth = nodeWidth || 200
nodeHeight = nodeHeight || 200
}
}
} else {
// Default dimensions for any other node types
nodeWidth = 200
nodeHeight = 200
// For any other node types, try to get actual dimensions first
nodeWidth = node.data?.width || node.width || 200
nodeHeight = node.data?.height || node.height || 200
}
minX = Math.min(minX, node.position.x)
minY = Math.min(minY, node.position.y)
minX = Math.min(minX, node.position.x + nodeWidth)
minY = Math.min(minY, node.position.y + nodeHeight)
maxX = Math.max(maxX, node.position.x + nodeWidth)
maxY = Math.max(maxY, node.position.y + nodeHeight)
maxY = Math.max(maxY, node.position.y + nodeHeight + 50)
})
// Add buffer padding to all sides (20px buffer before edges)
@@ -313,9 +307,12 @@ export const calculateLoopDimensions = (
// Reduce the excessive padding that was causing parent containers to be too large
const sidePadding = hasNestedContainers ? 150 : 120 // Reduced padding for containers containing other containers
// Add extra padding to the right side to prevent sidebar from covering the right handle
const extraPadding = 50
// Ensure the width and height are never less than the minimums
// Apply padding to all sides (left/right and top/bottom)
const width = Math.max(minWidth, maxX + sidePadding)
// Apply padding to all sides (left/right and top/bottom) with extra right padding
const width = Math.max(minWidth, maxX + sidePadding + extraPadding)
const height = Math.max(minHeight, maxY + sidePadding)
return { width, height }

View File

@@ -223,6 +223,12 @@ function WorkflowContent() {
}
}
}
// For loop and parallel nodes, use their end source handle
else if (block.type === 'loop') {
sourceHandle = 'loop-end-source'
} else if (block.type === 'parallel') {
sourceHandle = 'parallel-end-source'
}
return sourceHandle
}, [])
@@ -376,12 +382,6 @@ function WorkflowContent() {
extent: 'parent',
})
logger.info(`Added nested ${data.type} inside parent container`, {
nodeId: id,
parentId: containerInfo.loopId,
relativePosition,
})
// Auto-connect the nested container to nodes inside the parent container
const isAutoConnectEnabled = useGeneralStore.getState().isAutoConnectEnabled
if (isAutoConnectEnabled) {
@@ -485,13 +485,6 @@ function WorkflowContent() {
extent: 'parent',
})
logger.info('Added block inside container', {
blockId: id,
blockType: data.type,
containerId: containerInfo.loopId,
relativePosition,
})
// Resize the container node to fit the new block
// Immediate resize without delay
debouncedResizeLoopNodes()
@@ -499,42 +492,61 @@ function WorkflowContent() {
// Auto-connect logic for blocks inside containers
const isAutoConnectEnabled = useGeneralStore.getState().isAutoConnectEnabled
if (isAutoConnectEnabled && data.type !== 'starter') {
// Try to find other nodes in the container to connect to
const containerNodes = getNodes().filter((n) => n.parentId === containerInfo.loopId)
// First priority: Connect to the container's start node
const containerNode = getNodes().find((n) => n.id === containerInfo.loopId)
const containerType = containerNode?.type
if (containerNodes.length > 0) {
// Connect to the closest node in the container
const closestNode = containerNodes
.map((n) => ({
id: n.id,
distance: Math.sqrt(
(n.position.x - relativePosition.x) ** 2 +
(n.position.y - relativePosition.y) ** 2
),
}))
.sort((a, b) => a.distance - b.distance)[0]
if (containerType === 'loopNode' || containerType === 'parallelNode') {
// Connect from the container's start node to the new block
const startSourceHandle =
containerType === 'loopNode' ? 'loop-start-source' : 'parallel-start-source'
if (closestNode) {
// Get appropriate source handle
const sourceNode = getNodes().find((n) => n.id === closestNode.id)
const sourceType = sourceNode?.data?.type
addEdge({
id: crypto.randomUUID(),
source: containerInfo.loopId,
target: id,
sourceHandle: startSourceHandle,
targetHandle: 'target',
type: 'workflowEdge',
})
} else {
// Fallback: Try to find other nodes in the container to connect to
const containerNodes = getNodes().filter((n) => n.parentId === containerInfo.loopId)
// Default source handle
let sourceHandle = 'source'
if (containerNodes.length > 0) {
// Connect to the closest node in the container
const closestNode = containerNodes
.map((n) => ({
id: n.id,
distance: Math.sqrt(
(n.position.x - relativePosition.x) ** 2 +
(n.position.y - relativePosition.y) ** 2
),
}))
.sort((a, b) => a.distance - b.distance)[0]
// For condition blocks, use the condition-true handle
if (sourceType === 'condition') {
sourceHandle = 'condition-true'
if (closestNode) {
// Get appropriate source handle
const sourceNode = getNodes().find((n) => n.id === closestNode.id)
const sourceType = sourceNode?.data?.type
// Default source handle
let sourceHandle = 'source'
// For condition blocks, use the condition-true handle
if (sourceType === 'condition') {
sourceHandle = 'condition-true'
}
addEdge({
id: crypto.randomUUID(),
source: closestNode.id,
target: id,
sourceHandle,
targetHandle: 'target',
type: 'workflowEdge',
})
}
addEdge({
id: crypto.randomUUID(),
source: closestNode.id,
target: id,
sourceHandle,
targetHandle: 'target',
type: 'workflowEdge',
})
}
}
}
@@ -744,6 +756,7 @@ function WorkflowContent() {
type: 'workflowBlock',
position,
parentId: block.data?.parentId,
dragHandle: '.workflow-drag-handle',
extent: block.data?.extent || undefined,
data: {
type: block.type,
@@ -752,6 +765,9 @@ function WorkflowContent() {
isActive,
isPending,
},
// Include dynamic dimensions for container resizing calculations
width: block.isWide ? 450 : 350, // Standard width based on isWide state
height: Math.max(block.height || 100, 100), // Use actual height with minimum
})
})
@@ -815,9 +831,6 @@ function WorkflowContent() {
(changes: any) => {
changes.forEach((change: any) => {
if (change.type === 'remove') {
logger.info('Edge removal requested via ReactFlow:', {
edgeId: change.id,
})
removeEdge(change.id)
}
})
@@ -831,9 +844,6 @@ function WorkflowContent() {
if (connection.source && connection.target) {
// Prevent self-connections
if (connection.source === connection.target) {
logger.info('Rejected self-connection:', {
nodeId: connection.source,
})
return
}
@@ -862,12 +872,6 @@ function WorkflowContent() {
targetNode.parentId === sourceNode.id
) {
// This is a connection from container start to a node inside the container - always allow
logger.info('Creating container start connection:', {
edgeId,
sourceId: connection.source,
targetId: connection.target,
parentId: sourceNode.id,
})
addEdge({
...connection,
@@ -888,12 +892,6 @@ function WorkflowContent() {
(!sourceParentId && targetParentId) ||
(sourceParentId && targetParentId && sourceParentId !== targetParentId)
) {
logger.info('Rejected cross-boundary connection:', {
sourceId: connection.source,
targetId: connection.target,
sourceParentId,
targetParentId,
})
return
}
@@ -901,14 +899,6 @@ function WorkflowContent() {
const isInsideContainer = Boolean(sourceParentId) || Boolean(targetParentId)
const parentId = sourceParentId || targetParentId
logger.info('Creating connection:', {
edgeId,
sourceId: connection.source,
targetId: connection.target,
isInsideContainer,
parentId,
})
// Add appropriate metadata for container context
addEdge({
...connection,
@@ -1087,12 +1077,6 @@ function WorkflowContent() {
// Store the original parent ID when starting to drag
const currentParentId = node.parentId || blocks[node.id]?.data?.parentId || null
setDragStartParentId(currentParentId)
logger.info('Node drag started', {
nodeId: node.id,
startParentId: currentParentId,
nodeType: node.type,
})
},
[blocks]
)
@@ -1109,13 +1093,6 @@ function WorkflowContent() {
// Don't process if the node hasn't actually changed parent or is being moved within same parent
if (potentialParentId === dragStartParentId) return
logger.info('Node drag stopped', {
nodeId: node.id,
dragStartParentId,
potentialParentId,
nodeType: node.type,
})
// Check if this is a starter block - starter blocks should never be in containers
const isStarterBlock = node.data?.type === 'starter'
if (isStarterBlock) {
@@ -1180,16 +1157,6 @@ function WorkflowContent() {
// Create a unique identifier that combines edge ID and parent context
const contextId = `${edge.id}${parentLoopId ? `-${parentLoopId}` : ''}`
logger.info('Edge selected:', {
edgeId: edge.id,
sourceId: edge.source,
targetId: edge.target,
sourceNodeParent: sourceNode?.parentId,
targetNodeParent: targetNode?.parentId,
parentLoopId,
contextId,
})
setSelectedEdgeInfo({
id: edge.id,
parentLoopId,
@@ -1223,11 +1190,6 @@ function WorkflowContent() {
parentLoopId,
onDelete: (edgeId: string) => {
// Log deletion for debugging
logger.info('Deleting edge:', {
edgeId,
fromSelection: selectedEdgeInfo?.id === edgeId,
contextId: edgeContextId,
})
// Only delete this specific edge
removeEdge(edgeId)
@@ -1245,12 +1207,6 @@ function WorkflowContent() {
useEffect(() => {
const handleKeyDown = (event: KeyboardEvent) => {
if ((event.key === 'Delete' || event.key === 'Backspace') && selectedEdgeInfo) {
logger.info('Keyboard shortcut edge deletion:', {
edgeId: selectedEdgeInfo.id,
parentLoopId: selectedEdgeInfo.parentLoopId,
contextId: selectedEdgeInfo.contextId,
})
// Only delete the specific selected edge
removeEdge(selectedEdgeInfo.id)
setSelectedEdgeInfo(null)