fix: address code review findings for React Flow UI integration
- Use screenToFlowPosition() for drop coordinates (fixes zoom/pan bug) - Remove duplicate selection border from DeviceNode (BaseNode handles it) - Add w-full to GroupNode for proper container sizing - Remove unused 'selected' destructuring from DeviceNode Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -25,7 +25,7 @@ function TooltipRow({ label, value }: { label: string; value: string | null | un
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
function DeviceNodeComponent({ data, selected }: NodeProps) {
|
function DeviceNodeComponent({ data }: NodeProps) {
|
||||||
const nodeData = data as unknown as DeviceNodeData
|
const nodeData = data as unknown as DeviceNodeData
|
||||||
const { icon: Icon, color } = getDeviceRenderConfig(nodeData.deviceType, nodeData.category)
|
const { icon: Icon, color } = getDeviceRenderConfig(nodeData.deviceType, nodeData.category)
|
||||||
const status = (nodeData.properties?.status || 'unknown') as NodeStatus
|
const status = (nodeData.properties?.status || 'unknown') as NodeStatus
|
||||||
@@ -38,7 +38,7 @@ function DeviceNodeComponent({ data, selected }: NodeProps) {
|
|||||||
<NodeStatusIndicator status={status}>
|
<NodeStatusIndicator status={status}>
|
||||||
<NodeTooltip>
|
<NodeTooltip>
|
||||||
<NodeTooltipTrigger>
|
<NodeTooltipTrigger>
|
||||||
<BaseNode className={`min-w-[120px] group ${selected ? 'border-accent' : ''}`}>
|
<BaseNode className="min-w-[120px] group">
|
||||||
<BaseNodeHeader className="flex-col gap-1 items-center py-3 px-4">
|
<BaseNodeHeader className="flex-col gap-1 items-center py-3 px-4">
|
||||||
<Icon size={28} style={{ color }} />
|
<Icon size={28} style={{ color }} />
|
||||||
<BaseNodeHeaderTitle className="text-center text-xs">
|
<BaseNodeHeaderTitle className="text-center text-xs">
|
||||||
|
|||||||
@@ -45,7 +45,7 @@ export function GroupNode({ data, selected }: NodeProps) {
|
|||||||
return (
|
return (
|
||||||
<BaseNode
|
<BaseNode
|
||||||
className={cn(
|
className={cn(
|
||||||
'h-full min-h-[200px] min-w-[300px] overflow-hidden rounded-lg bg-elevated/30 border-default/50',
|
'h-full w-full min-h-[200px] min-w-[300px] overflow-hidden rounded-lg bg-elevated/30 border-default/50',
|
||||||
selected && 'border-accent',
|
selected && 'border-accent',
|
||||||
)}
|
)}
|
||||||
>
|
>
|
||||||
|
|||||||
@@ -25,7 +25,7 @@ import type { DeviceNodeData } from '@/components/network/nodes/DeviceNode'
|
|||||||
function DiagramEditorInner() {
|
function DiagramEditorInner() {
|
||||||
const { id } = useParams<{ id: string }>()
|
const { id } = useParams<{ id: string }>()
|
||||||
const navigate = useNavigate()
|
const navigate = useNavigate()
|
||||||
const { getNodes, fitView } = useReactFlow()
|
const { getNodes, fitView, screenToFlowPosition } = useReactFlow()
|
||||||
|
|
||||||
const [diagramId, setDiagramId] = useState<string | null>(id || null)
|
const [diagramId, setDiagramId] = useState<string | null>(id || null)
|
||||||
const [name, setName] = useState('Untitled Diagram')
|
const [name, setName] = useState('Untitled Diagram')
|
||||||
@@ -201,13 +201,7 @@ function DiagramEditorInner() {
|
|||||||
const onDrop = useCallback((event: React.DragEvent) => {
|
const onDrop = useCallback((event: React.DragEvent) => {
|
||||||
event.preventDefault()
|
event.preventDefault()
|
||||||
|
|
||||||
const reactFlowBounds = (event.target as HTMLElement).closest('.react-flow')?.getBoundingClientRect()
|
const position = screenToFlowPosition({ x: event.clientX, y: event.clientY })
|
||||||
if (!reactFlowBounds) return
|
|
||||||
|
|
||||||
const position = {
|
|
||||||
x: event.clientX - reactFlowBounds.left,
|
|
||||||
y: event.clientY - reactFlowBounds.top,
|
|
||||||
}
|
|
||||||
|
|
||||||
// Handle device drops
|
// Handle device drops
|
||||||
const deviceRaw = event.dataTransfer.getData('application/reactflow-device')
|
const deviceRaw = event.dataTransfer.getData('application/reactflow-device')
|
||||||
@@ -256,7 +250,7 @@ function DiagramEditorInner() {
|
|||||||
setNodes(nds => [...nds, newNode])
|
setNodes(nds => [...nds, newNode])
|
||||||
setIsDirty(true)
|
setIsDirty(true)
|
||||||
}
|
}
|
||||||
}, [setNodes])
|
}, [setNodes, screenToFlowPosition])
|
||||||
|
|
||||||
const handleNodeUpdate = useCallback((nodeId: string, updates: Partial<DeviceNodeData>) => {
|
const handleNodeUpdate = useCallback((nodeId: string, updates: Partial<DeviceNodeData>) => {
|
||||||
setNodes(nds => nds.map(n => {
|
setNodes(nds => nds.map(n => {
|
||||||
|
|||||||
Reference in New Issue
Block a user