ui(viewer): update graph interface with grid, zoom, and glow effects#101
ui(viewer): update graph interface with grid, zoom, and glow effects#101rohitg00 merged 1 commit intorohitg00:mainfrom
Conversation
📝 WalkthroughWalkthroughAll changes are within a single HTML file that enhances the graph viewer's visual presentation and interactive capabilities. Updates include tooltip styling with translucent glass effects, new floating zoom and recenter UI controls, refined sidebar layout, improved node radius scaling, and expanded graph interactions featuring double-click node selection, canvas grid background, focus-driven edge/node rendering with emphasis effects, hover proximity detection, and per-type edge coloring. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/viewer/index.html (1)
1313-1323:⚠️ Potential issue | 🟡 MinorRadius scaling only runs on the initial graph load.
The new size formula is applied in
initGraph()only. AfterexpandNode(), inserted nodes still keep their fixed radius and existing nodes never get resized for their new degree, so the double-click expansion flow makes the sizing semantics stale almost immediately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/viewer/index.html` around lines 1313 - 1323, The node radius is only computed in initGraph(), so after expandNode() new nodes get default radii and existing nodes are not resized; update the code to centralize radius logic (e.g., a computeNodeRadius(node, deg) helper) and call it both when you build graphSim.nodes in initGraph() and after expandNode() (or inside the function that inserts/updates nodes) to recalc and assign r using the current edgeMap/degree values; ensure any code that maps state.graph.nodes (the block producing id,type,name,x,y,vx,vy,r) uses this helper so newly inserted nodes and existing nodes get correct sizes.
🧹 Nitpick comments (1)
src/viewer/index.html (1)
696-697: The tooltip fade won’t actually animate.
display: none→display: blockbypasses the newopacitytransition, and the hidden state never definesopacity: 0. If the glass tooltip is meant to ease in/out, keep it rendered and toggleopacity/visibilityinstead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/viewer/index.html` around lines 696 - 697, The tooltip CSS uses display to show/hide which prevents the opacity transition; update the .graph-tooltip rule to keep it rendered (remove display:none), set initial opacity: 0 and visibility: hidden (and keep pointer-events: none), and change .graph-tooltip.visible to set opacity: 1, visibility: visible (and enable pointer-events if needed) so the opacity transition on .graph-tooltip applies and the tooltip fades in/out smoothly; refer to the .graph-tooltip and .graph-tooltip.visible selectors to make these edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/viewer/index.html`:
- Around line 1631-1645: The hover-based focus can persist because hoverNodeId
is computed from stale graphSim.mouseX/mouseY and doesn't check pointer presence
or panning; update the hover detection in the block that computes hoverNodeId
(references: hoverNodeId, graphSim.mouseX, graphSim.mouseY, graphSim.canvas,
graphSim.dragNode, focusNodeId) to first verify the pointer is inside the canvas
bounds (compare mouseX/mouseY to rect left/top/width/height) and that the
simulator is not currently panning (check graphSim.isPanning or equivalent), or
explicitly clear/invalidate stored mouse coordinates on mouseleave; also ensure
focusNodeId uses selectedId || hoverNodeId only when hoverNodeId is valid and
not during panning so stale highlights cannot appear while dragging.
- Around line 1733-1737: The glow is being applied for every visible node
because of the `|| !searchActive` clause; change the condition so glow is only
applied for selected/hovered nodes or explicit search hits. Replace the current
check that uses `matchesSearch && !isFocusFaded && (isSelected || isHovered ||
!searchActive)` with a predicate that first ensures `!isFocusFaded` and then
either `(isSelected || isHovered)` OR `(searchActive && matchesSearch)` so that
when no search is active only selected/hovered nodes get shadow, and when a
search is active only matching nodes (or selected/hovered) get shadow; update
the block around `matchesSearch`, `isFocusFaded`, `isSelected`, `isHovered`, and
`searchActive` and leave the `ctx.shadowColor`/`ctx.shadowBlur` logic unchanged.
- Around line 1723-1726: The code computes isFocusFaded by calling
graphSim.edges.some(...) inside the per-node render loop (using focusNodeId,
n.id, graphSim.edges), causing O(nodes×edges) work each frame; instead, in the
frame/render function precompute a Set of neighbor IDs for the current
focusNodeId by scanning graphSim.edges once (add the opposite endpoint when an
edge.sourceNodeId === focusNodeId or edge.targetNodeId === focusNodeId), then
replace the per-node some(...) with a constant-time lookup like
!neighborSet.has(n.id) (and keep the existing falsy focusNodeId handling).
---
Outside diff comments:
In `@src/viewer/index.html`:
- Around line 1313-1323: The node radius is only computed in initGraph(), so
after expandNode() new nodes get default radii and existing nodes are not
resized; update the code to centralize radius logic (e.g., a
computeNodeRadius(node, deg) helper) and call it both when you build
graphSim.nodes in initGraph() and after expandNode() (or inside the function
that inserts/updates nodes) to recalc and assign r using the current
edgeMap/degree values; ensure any code that maps state.graph.nodes (the block
producing id,type,name,x,y,vx,vy,r) uses this helper so newly inserted nodes and
existing nodes get correct sizes.
---
Nitpick comments:
In `@src/viewer/index.html`:
- Around line 696-697: The tooltip CSS uses display to show/hide which prevents
the opacity transition; update the .graph-tooltip rule to keep it rendered
(remove display:none), set initial opacity: 0 and visibility: hidden (and keep
pointer-events: none), and change .graph-tooltip.visible to set opacity: 1,
visibility: visible (and enable pointer-events if needed) so the opacity
transition on .graph-tooltip applies and the tooltip fades in/out smoothly;
refer to the .graph-tooltip and .graph-tooltip.visible selectors to make these
edits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| // --- Hover node detection for focus effect --- | ||
| var hoverNodeId = null; | ||
| if (!graphSim.dragNode && graphSim.canvas) { | ||
| var rect = graphSim.canvas.getBoundingClientRect(); | ||
| var hx = (graphSim.mouseX - rect.left - graphSim.panX) / graphSim.zoom; | ||
| var hy = (graphSim.mouseY - rect.top - graphSim.panY) / graphSim.zoom; | ||
| for (var hi = graphSim.nodes.length - 1; hi >= 0; hi--) { | ||
| var hn = graphSim.nodes[hi]; | ||
| if (!state.graph.filters[hn.type]) continue; | ||
| var hdx = hn.x - hx, hdy = hn.y - hy; | ||
| if (hdx * hdx + hdy * hdy < hn.r * hn.r + 25) { hoverNodeId = hn.id; break; } | ||
| } | ||
| } | ||
| var focusNodeId = selectedId || hoverNodeId; | ||
|
|
There was a problem hiding this comment.
Hover focus can stick after mouseleave and during panning.
hoverNodeId is recomputed from the last stored mouseX/mouseY, but those coordinates are never invalidated here when the pointer leaves the canvas, and this path cannot see isPanning. The result is stale focus/glow on the last hovered node and focus highlights while the canvas is being dragged.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/viewer/index.html` around lines 1631 - 1645, The hover-based focus can
persist because hoverNodeId is computed from stale graphSim.mouseX/mouseY and
doesn't check pointer presence or panning; update the hover detection in the
block that computes hoverNodeId (references: hoverNodeId, graphSim.mouseX,
graphSim.mouseY, graphSim.canvas, graphSim.dragNode, focusNodeId) to first
verify the pointer is inside the canvas bounds (compare mouseX/mouseY to rect
left/top/width/height) and that the simulator is not currently panning (check
graphSim.isPanning or equivalent), or explicitly clear/invalidate stored mouse
coordinates on mouseleave; also ensure focusNodeId uses selectedId ||
hoverNodeId only when hoverNodeId is valid and not during panning so stale
highlights cannot appear while dragging.
| var isFocusFaded = focusNodeId && n.id !== focusNodeId && !graphSim.edges.some(function(ed) { | ||
| return (ed.sourceNodeId === focusNodeId && ed.targetNodeId === n.id) || | ||
| (ed.targetNodeId === focusNodeId && ed.sourceNodeId === n.id); | ||
| }); |
There was a problem hiding this comment.
Avoid scanning the full edge list for every node on every frame.
This graphSim.edges.some(...) call sits inside the node render loop, so the new focus check is O(nodes × edges) on every animation frame. That will get noticeably expensive as the graph grows. Precompute the focused node’s neighbors once per frame and reuse that lookup.
♻️ Possible fix
+ var focusNeighbors = new Set();
+ if (focusNodeId) {
+ graphSim.edges.forEach(function(ed) {
+ if (ed.sourceNodeId === focusNodeId) focusNeighbors.add(ed.targetNodeId);
+ if (ed.targetNodeId === focusNodeId) focusNeighbors.add(ed.sourceNodeId);
+ });
+ }
+
graphSim.nodes.forEach(function(n) {
if (!state.graph.filters[n.type]) return;
var color = NODE_COLORS[n.type] || '#666666';
var isSelected = selectedId === n.id;
var isHovered = hoverNodeId === n.id;
var matchesSearch = !searchActive || n.name.toLowerCase().includes(graphSearchTerm);
- var isFocusFaded = focusNodeId && n.id !== focusNodeId && !graphSim.edges.some(function(ed) {
- return (ed.sourceNodeId === focusNodeId && ed.targetNodeId === n.id) ||
- (ed.targetNodeId === focusNodeId && ed.sourceNodeId === n.id);
- });
+ var isFocusFaded = focusNodeId && n.id !== focusNodeId && !focusNeighbors.has(n.id);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var isFocusFaded = focusNodeId && n.id !== focusNodeId && !graphSim.edges.some(function(ed) { | |
| return (ed.sourceNodeId === focusNodeId && ed.targetNodeId === n.id) || | |
| (ed.targetNodeId === focusNodeId && ed.sourceNodeId === n.id); | |
| }); | |
| var focusNeighbors = new Set(); | |
| if (focusNodeId) { | |
| graphSim.edges.forEach(function(ed) { | |
| if (ed.sourceNodeId === focusNodeId) focusNeighbors.add(ed.targetNodeId); | |
| if (ed.targetNodeId === focusNodeId) focusNeighbors.add(ed.sourceNodeId); | |
| }); | |
| } | |
| graphSim.nodes.forEach(function(n) { | |
| if (!state.graph.filters[n.type]) return; | |
| var color = NODE_COLORS[n.type] || '#666666'; | |
| var isSelected = selectedId === n.id; | |
| var isHovered = hoverNodeId === n.id; | |
| var matchesSearch = !searchActive || n.name.toLowerCase().includes(graphSearchTerm); | |
| var isFocusFaded = focusNodeId && n.id !== focusNodeId && !focusNeighbors.has(n.id); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/viewer/index.html` around lines 1723 - 1726, The code computes
isFocusFaded by calling graphSim.edges.some(...) inside the per-node render loop
(using focusNodeId, n.id, graphSim.edges), causing O(nodes×edges) work each
frame; instead, in the frame/render function precompute a Set of neighbor IDs
for the current focusNodeId by scanning graphSim.edges once (add the opposite
endpoint when an edge.sourceNodeId === focusNodeId or edge.targetNodeId ===
focusNodeId), then replace the per-node some(...) with a constant-time lookup
like !neighborSet.has(n.id) (and keep the existing falsy focusNodeId handling).
| // Glow effect | ||
| if (matchesSearch && !isFocusFaded && (isSelected || isHovered || !searchActive)) { | ||
| ctx.shadowColor = color; | ||
| ctx.shadowBlur = isSelected ? 14 : (isDense ? 3 : 6); | ||
| ctx.shadowBlur = isSelected ? 20 : isHovered ? 16 : (isDense ? 4 : 8); | ||
| } |
There was a problem hiding this comment.
The idle state now applies glow to every visible node.
Because of the || !searchActive clause, the normal “no search” state sends every non-faded node through the shadow path. That makes the graph look permanently highlighted and adds extra shadow work to the hot render loop. Glow should stay opt-in for selected/hovered nodes, or explicit search hits only.
♻️ Possible fix
- if (matchesSearch && !isFocusFaded && (isSelected || isHovered || !searchActive)) {
+ if (matchesSearch && !isFocusFaded && (isSelected || isHovered)) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Glow effect | |
| if (matchesSearch && !isFocusFaded && (isSelected || isHovered || !searchActive)) { | |
| ctx.shadowColor = color; | |
| ctx.shadowBlur = isSelected ? 14 : (isDense ? 3 : 6); | |
| ctx.shadowBlur = isSelected ? 20 : isHovered ? 16 : (isDense ? 4 : 8); | |
| } | |
| // Glow effect | |
| if (matchesSearch && !isFocusFaded && (isSelected || isHovered)) { | |
| ctx.shadowColor = color; | |
| ctx.shadowBlur = isSelected ? 20 : isHovered ? 16 : (isDense ? 4 : 8); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/viewer/index.html` around lines 1733 - 1737, The glow is being applied
for every visible node because of the `|| !searchActive` clause; change the
condition so glow is only applied for selected/hovered nodes or explicit search
hits. Replace the current check that uses `matchesSearch && !isFocusFaded &&
(isSelected || isHovered || !searchActive)` with a predicate that first ensures
`!isFocusFaded` and then either `(isSelected || isHovered)` OR `(searchActive &&
matchesSearch)` so that when no search is active only selected/hovered nodes get
shadow, and when a search is active only matching nodes (or selected/hovered)
get shadow; update the block around `matchesSearch`, `isFocusFaded`,
`isSelected`, `isHovered`, and `searchActive` and leave the
`ctx.shadowColor`/`ctx.shadowBlur` logic unchanged.
This PR updates the graph UI to make it look and feel better.
Changes made:
1.Added floating Zoom buttons (+ / - / Recenter).
2.Added a faint grid background to the canvas.
3.Made selected nodes and edges glow.
4.Updated the tooltip to have a modern glass effect.
5.You can now double-click a node to quickly expand its neighbors.
Closes #98
Summary by CodeRabbit
New Features
Improvements