Skip to content
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

Fix #342 fix canvas component #342

Merged
merged 10 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions app/components/chat.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { cn } from "@/lib/utils";
import { TypeAnimation } from "react-type-animation";
import { DropdownMenu, DropdownMenuContent, DropdownMenuTrigger } from "@/components/ui/dropdown-menu";
import { prepareArg } from "../utils";
import { NodeObject } from "react-force-graph-2d";

type PathData = {
nodes: any[]
Expand Down Expand Up @@ -84,6 +85,7 @@ interface Props {
isPathResponse: boolean | undefined
setIsPathResponse: (isPathResponse: boolean | undefined) => void
setData: Dispatch<SetStateAction<GraphData>>
chartRef: any
}

const SUGGESTIONS = [
Expand All @@ -105,7 +107,7 @@ const RemoveLastPath = (messages: Message[]) => {
return messages
}

export function Chat({ repo, path, setPath, graph, selectedPathId, isPathResponse, setIsPathResponse, setData }: Props) {
export function Chat({ repo, path, setPath, graph, selectedPathId, isPathResponse, setIsPathResponse, setData, chartRef }: Props) {

// Holds the messages in the chat
const [messages, setMessages] = useState<Message[]>([]);
Expand All @@ -131,7 +133,6 @@ export function Chat({ repo, path, setPath, graph, selectedPathId, isPathRespons
const p = paths.find((path) => [...path.links, ...path.nodes].some((e: any) => e.id === selectedPathId))

if (!p) return

handelSetSelectedPath(p)
}, [selectedPathId])

Expand All @@ -154,6 +155,9 @@ export function Chat({ repo, path, setPath, graph, selectedPathId, isPathRespons
}, [isPathResponse])

const handelSetSelectedPath = (p: PathData) => {
const chart = chartRef.current

if (!chart) return
setSelectedPath(prev => {
if (prev) {
if (isPathResponse && paths.some((path) => [...path.nodes, ...path.links].every((e: any) => [...prev.nodes, ...prev.links].some((e: any) => e.id === e.id)))) {
Expand Down Expand Up @@ -206,6 +210,9 @@ export function Chat({ repo, path, setPath, graph, selectedPathId, isPathRespons
});
}
setData({ ...graph.Elements })
setTimeout(() => {
chart.zoomToFit(1000, 150, (n: NodeObject<Node>) => p.nodes.some(node => node.id === n.id));
}, 0)
}

// A function that handles the change event of the url input box
Expand Down Expand Up @@ -262,6 +269,10 @@ export function Chat({ repo, path, setPath, graph, selectedPathId, isPathRespons
}

const handleSubmit = async () => {
const chart = chartRef.current

if (!chart) return

setSelectedPath(undefined)

if (!path?.start?.id || !path.end?.id) return
Expand Down Expand Up @@ -297,6 +308,9 @@ export function Chat({ repo, path, setPath, graph, selectedPathId, isPathRespons
setPath(undefined)
setIsPathResponse(true)
setData({ ...graph.Elements })
setTimeout(() => {
chart.zoomToFit(1000, 150, (n: NodeObject<Node>) => formattedPaths.some(p => p.nodes.some(node => node.id === n.id)));
}, 0)
}

const getTip = (disabled = false) =>
Expand Down
44 changes: 23 additions & 21 deletions app/components/code-graph.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { Checkbox } from '@/components/ui/checkbox';
import dynamic from 'next/dynamic';
import { Position } from "./graphView";
import { prepareArg } from '../utils';
import { NodeObject } from "react-force-graph-2d";

const GraphView = dynamic(() => import('./graphView'));

Expand Down Expand Up @@ -208,34 +209,35 @@ export function CodeGraph({
nodes.forEach((node) => {
node.expand = expand
})

setSelectedObj(undefined)
setData({ ...graph.Elements })
}

const handelSearchSubmit = (node: any) => {
const n = { name: node.properties.name, id: node.id }

let chartNode = graph.Elements.nodes.find(n => n.id == node.id)

if (!chartNode?.visible) {
if (!chartNode) {
chartNode = graph.extend({ nodes: [node], edges: [] }).nodes[0]
} else {
chartNode.visible = true
setCooldownTicks(undefined)
setCooldownTime(1000)
}
graph.visibleLinks(true, [chartNode.id])
}

setSearchNode(n)
setData({ ...graph.Elements })

const chart = chartRef.current

if (chart) {
chart.centerAt(chartNode.x, chartNode.y, 1000);
const n = { name: node.properties.name, id: node.id }

let chartNode = graph.Elements.nodes.find(n => n.id == node.id)

if (!chartNode?.visible) {
if (!chartNode) {
chartNode = graph.extend({ nodes: [node], edges: [] }).nodes[0]
} else {
chartNode.visible = true
setCooldownTicks(undefined)
setCooldownTime(1000)
}
graph.visibleLinks(true, [chartNode!.id])
setData({ ...graph.Elements })
}

setSearchNode(n)
setTimeout(() => {
chart.zoomToFit(1000, 150, (n: NodeObject<Node>) => n.id === chartNode!.id);
}, 0)
}
}

Expand Down
87 changes: 37 additions & 50 deletions app/components/graphView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ export default function GraphView({
}: Props) {

const parentRef = useRef<HTMLDivElement>(null)

const lastClick = useRef<Date>(new Date())
useEffect(() => {
setCooldownTime(4000)
setCooldownTicks(undefined)
Expand All @@ -78,18 +78,7 @@ export default function GraphView({
setSelectedObjects([])
}

const handelNodeClick = (node: Node, evt: MouseEvent) => {
if (isShowPath) {
setPath(prev => {
if (!prev?.start?.name || (prev.end?.name && prev.end?.name !== "")) {
return ({ start: { id: Number(node.id), name: node.name } })
} else {
return ({ end: { id: Number(node.id), name: node.name }, start: prev.start })
}
})
return
}

const handelNodeRightClick = (node: Node, evt: MouseEvent) => {
if (evt.ctrlKey) {
if (selectedObjects.some(obj => obj.id === node.id)) {
setSelectedObjects(selectedObjects.filter(obj => obj.id !== node.id))
Expand All @@ -111,26 +100,45 @@ export default function GraphView({
setSelectedPathId(link.id)
}

const handelNodeRightClick = async (node: Node) => {
const expand = !node.expand
if (expand) {
const elements = await onFetchNode([node.id])
const handelNodeClick = async (node: Node) => {
const now = new Date()

if (elements.nodes.length === 0) {
toast({
title: `No neighbors found`,
description: `No neighbors found`,
})
return
const isDoubleClick = now.getTime() - lastClick.current.getTime() < 1000
lastClick.current = now

if (isDoubleClick) {
const expand = !node.expand

if (expand) {
const elements = await onFetchNode([node.id])

if (elements.nodes.length === 0) {
toast({
title: `No neighbors found`,
description: `No neighbors found`,
})

return
}
} else {
deleteNeighbors([node]);
}
} else {
deleteNeighbors([node]);
}

node.expand = expand
node.expand = expand

setSelectedObj(undefined)
setData({ ...graph.Elements })
setSelectedObj(undefined)
setData({ ...graph.Elements })

} else if (isShowPath) {
setPath(prev => {
if (!prev?.start?.name || (prev.end?.name && prev.end?.name !== "")) {
return ({ start: { id: Number(node.id), name: node.name } })
} else {
return ({ end: { id: Number(node.id), name: node.name }, start: prev.start })
}
})
return
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

🛠️ Refactor suggestion

Improve error handling and fix naming convention

The review comment is accurate. The code needs the suggested improvements:

  1. The function name typo should be fixed ('handel' → 'handle')
  2. The toast message is indeed duplicated and should be more descriptive
  3. The implementation lacks multi-edge validation during edge creation, which should be addressed as per the PR objective
🔗 Analysis chain

Improve error handling and fix naming convention

The node click handler needs improvements:

  1. Function name has a typo ('handel' → 'handle')
  2. Toast error message is duplicated
  3. The implementation doesn't address the PR objective of checking multi-edges on create

Apply these changes:

-    const handelNodeClick = async (node: Node) => {
+    const handleNodeClick = async (node: Node) => {
         const now = new Date()
         const isDoubleClick = now.getTime() - lastClick.current.getTime() < 1000
         lastClick.current = now
         
         if (isDoubleClick) {
             const expand = !node.expand
 
             if (expand) {
                 const elements = await onFetchNode([node.id])
 
                 if (elements.nodes.length === 0) {
                     toast({
                         title: `No neighbors found`,
-                        description: `No neighbors found`,
+                        description: `No connected nodes found for ${node.name}`,
                     })
 
                     return
                 }

The PR objective mentions checking for multi-edges on create, but this implementation seems to handle it during render. Let's verify the implementation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for multi-edge checking logic in the codebase
rg -l "multi.*edge|parallel.*edge" --type ts --type tsx

Length of output: 85


Script:

#!/bin/bash
# Search for edge-related logic with broader terms
rg -l "edge|createEdge|addEdge|validateEdge" --type ts

# Search for edge type definitions
ast-grep --pattern 'type $_ = {
  $$$
  edge$_
  $$$
}'

# Search for edge-related functions
ast-grep --pattern 'function $_($$$) {
  $$$
  edge$_
  $$$
}'

Length of output: 436


Script:

#!/bin/bash
# Check edge-related code in model.ts
rg "edge|Edge" -A 5 -B 5 app/components/model.ts

# Look for edge creation/validation in code-graph.tsx
rg "edge|Edge" -A 5 -B 5 app/components/code-graph.tsx

# Search for specific edge validation or creation functions
ast-grep --pattern 'function $_($$$) {
  $$$
  new Edge($$$)
  $$$
}'

Length of output: 4937

}

return (
Expand Down Expand Up @@ -221,20 +229,7 @@ export default function GraphView({

if (!start.x || !start.y || !end.x || !end.y) return

const sameNodesLinks = graph.Elements.links.filter(l => (l.source.id === start.id && l.target.id === end.id) || (l.target.id === start.id && l.source.id === end.id))
const index = sameNodesLinks.findIndex(l => l.id === link.id) || 0
const even = index % 2 === 0
let curve

if (start.id === end.id) {
if (even) {
curve = Math.floor(-(index / 2)) - 3
} else {
curve = Math.floor((index + 1) / 2) + 2
}

link.curve = curve * 0.1

const radius = NODE_SIZE * link.curve * 6.2;
const angleOffset = -Math.PI / 4; // 45 degrees offset for text alignment
const textX = start.x + radius * Math.cos(angleOffset);
Expand All @@ -244,14 +239,6 @@ export default function GraphView({
ctx.translate(textX, textY);
ctx.rotate(-angleOffset);
} else {
if (even) {
curve = Math.floor(-(index / 2))
} else {
curve = Math.floor((index + 1) / 2)
}

link.curve = curve * 0.1

const midX = (start.x + end.x) / 2 + (end.y - start.y) * (link.curve / 2);
const midY = (start.y + end.y) / 2 + (start.x - end.x) * (link.curve / 2);

Expand Down
67 changes: 62 additions & 5 deletions app/components/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ const COLORS_ORDER = [
"#80E6E6",
]

export function getCategoryColorValue(index: number): string {
export function getCategoryColorValue(index: number = 0): string {
return COLORS_ORDER[index % COLORS_ORDER.length]
}

Expand Down Expand Up @@ -176,13 +176,43 @@ export class Graph {
return
}

let sourceId = edgeData.src_node;
let destinationId = edgeData.dest_node
let source = this.nodesMap.get(edgeData.src_node)
let target = this.nodesMap.get(edgeData.dest_node)

if (!source) {
source = {
id: edgeData.src_node,
name: edgeData.src_node,
color: getCategoryColorValue(),
category: "",
expand: false,
visible: true,
collapsed,
isPath: !!path,
isPathSelected: path?.start?.id === edgeData.src_node || path?.end?.id === edgeData.src_node
}
this.nodesMap.set(edgeData.src_node, source)
}

if (!target) {
target = {
id: edgeData.dest_node,
name: edgeData.dest_node,
color: getCategoryColorValue(),
category: "",
expand: false,
visible: true,
collapsed,
isPath: !!path,
isPathSelected: path?.start?.id === edgeData.dest_node || path?.end?.id === edgeData.dest_node
}
this.nodesMap.set(edgeData.dest_node, target)
}

link = {
id: edgeData.id,
source: sourceId,
target: destinationId,
source,
target,
label: edgeData.relation,
visible: true,
expand: false,
Expand All @@ -196,6 +226,33 @@ export class Graph {
newElements.links.push(link)
})

newElements.links.forEach(link => {
const start = link.source
const end = link.target
const sameNodesLinks = this.Elements.links.filter(l => (l.source.id === start.id && l.target.id === end.id) || (l.target.id === start.id && l.source.id === end.id))
const index = sameNodesLinks.findIndex(l => l.id === link.id) ?? 0
const even = index % 2 === 0
let curve

if (start.id === end.id) {
if (even) {
curve = Math.floor(-(index / 2)) - 3
} else {
curve = Math.floor((index + 1) / 2) + 2
}
} else {
if (even) {
curve = Math.floor(-(index / 2))
} else {
curve = Math.floor((index + 1) / 2)
}

}

link.curve = curve * 0.1
})


return newElements
}

Expand Down
1 change: 1 addition & 0 deletions app/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,7 @@ export default function Home() {
<PanelResizeHandle />
<Panel className="border-l min-w-[420px]" defaultSize={30} >
<Chat
chartRef={chartRef}
setPath={setPath}
path={path}
repo={graph.Id}
Expand Down
1 change: 1 addition & 0 deletions tailwind.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ module.exports = {
animation: {
"accordion-down": "accordion-down 0.2s ease-out",
"accordion-up": "accordion-up 0.2s ease-out",
'spin': 'spin 1s linear',
},
},
},
Expand Down
Loading