Skip to content

Conversation

@nikhilsaikethe
Copy link
Contributor

@nikhilsaikethe nikhilsaikethe commented Sep 29, 2025

User description

This PR enhances the pipelines UI


PR Type

Enhancement


Description

  • Revamped pipeline node sidebar UI

  • Added inline function definition viewer

  • Improved node/edge interactions and deletion UX

  • Updated icons, styles, and dark mode


Diagram Walkthrough

flowchart LR
  Sidebar["NodeSidebar.vue UI/UX revamp"] -- styled nodes & tooltips --> Editor["PipelineEditor.vue visuals"]
  Editor -- keyboard edge delete & headers --> Flow["PipelineFlow.vue interactions"]
  Flow -- edge click help notification --> Edges["CustomEdge/EdgeWithButton cleanup"]
  AssocFn["AssociateFunction.vue function preview"] -- RAF/RBF guidelines --> Users["Better user guidance"]
Loading

File Walkthrough

Relevant files
Enhancement
8 files
AssociateFunction.vue
Show selected function definition and RAF/RBF note             
+205/-1 
NodeSidebar.vue
Redesign draggable nodes with custom tooltips                       
+285/-13
PipelineEditor.vue
Visual updates, new icons, and keyboard edge delete           
+185/-22
PipelineView.vue
Update assets and scoped styles for nodes                               
+61/-50 
CustomEdge.vue
Remove on-edge delete button; style and cleanup                   
+9/-47   
CustomNode.vue
Hover actions, handle styling, labels truncation, UX polish
+704/-342
EdgeWithButton.vue
Remove edge button UI; keep keyboard deletion                       
+14/-29 
PipelineFlow.vue
Edge click help notification and event handling                   
+85/-3   

@github-actions github-actions bot added ☢️ Bug Something isn't working labels Sep 29, 2025
@github-actions github-actions bot changed the title fix: pipeline UI revamp fix: pipeline UI revamp Sep 29, 2025
@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Runtime Errors

The template uses clearTimeout and setTimeout directly in attribute bindings while showButtons is a ref; also hideButtonsTimeout is a plain variable not typed/reactive. Verify no undefined or stale timeout references at runtime and consider wrapping hover handlers to avoid inline side effects.

<div v-show="showButtons" class="node-action-buttons" :style="{ '--node-color': getNodeColor(io_type) }" @mouseenter="clearTimeout(hideButtonsTimeout)" @mouseleave="hideButtonsTimeout = setTimeout(() => showButtons = false, 200)">

  <q-btn
    flat
    round
    dense
    icon="delete"
    size="0.6em"
    @click.stop="deleteNode(id)"
    class="node-action-btn delete-btn"
    @mouseenter="showDeleteTooltip = true"
    @mouseleave="showDeleteTooltip = false"
  />
  <div v-if="showDeleteTooltip" class="custom-tooltip delete-tooltip" style="left: 15px;">
    Delete Node
    <div class="tooltip-arrow delete-arrow"></div>
  </div>
</div>
UX Consistency

Hover edge-color logic mutates edge.style/markerEnd directly; ensure Vue Flow reacts to style mutations and that edges reset correctly when rapidly moving between nodes. Potential stale styles if edges are shared or pipelineObj updates asynchronously.

// Function to update edge colors on node hover
const updateEdgeColors = (nodeId, color, reset = false) => {
  if (pipelineObj.currentSelectedPipeline?.edges) {
    pipelineObj.currentSelectedPipeline.edges.forEach(edge => {
      if (edge.source === nodeId) {
        if (reset) {
          // Reset to default color
          edge.style = { 
            ...edge.style, 
            stroke: '#6b7280', 
            strokeWidth: 2 
          };
          edge.markerEnd = {
            ...edge.markerEnd,
            color: '#6b7280'
          };
        } else {
          // Apply node color to both edge and arrow
          edge.style = { 
            ...edge.style, 
            stroke: color, 
            strokeWidth: 2 
          };
          edge.markerEnd = {
            ...edge.markerEnd,
            color: color
          };
        }
      }
    });
  }
};
Global State Cleanup

Edge help notification uses timeouts stored in a local variable; confirm proper cleanup on component unmount and no overlapping timers causing flicker. Also ensure @edge-click handler doesn’t log excessively in production.

const vueFlowRef = ref(null);
const isCanvasEmpty = computed(() => pipelineObj.currentSelectedPipeline.nodes.length === 0);
const showEdgeHelpNotification = ref(false);
let notificationTimeout = null;

const { setViewport, getSelectedEdges, addSelectedEdges, removeSelectedEdges, removeEdges } = useVueFlow()

// Handle edge click events
const onEdgeClick = (event) => {
  console.log('Edge click event triggered')

  // Clear any existing timeout
  if (notificationTimeout) {
    clearTimeout(notificationTimeout)
    notificationTimeout = null
  }

  // Always show notification on edge click (even if already visible)
  console.log('Showing notification for edge click')
  showEdgeHelpNotification.value = true
  console.log('Notification state set to:', showEdgeHelpNotification.value)

  // Auto-hide notification after 3.5 seconds 
  notificationTimeout = setTimeout(() => {
    console.log('Auto-hiding notification')
    showEdgeHelpNotification.value = false
    notificationTimeout = null
  }, 3500)
}

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Select edge on click for deletion

The @edge-click handler only shows a help toast and doesn’t select the clicked edge,
which breaks expected UX for keyboard deletion. Programmatically select the clicked
edge using the composable to ensure Delete/Backspace applies correctly.

web/src/plugins/pipelines/PipelineFlow.vue [33-46]

 <VueFlow
   @drop="onDrop"
   ref="vueFlowRef"
   v-model:nodes="pipelineObj.currentSelectedPipeline.nodes"
   v-model:edges="pipelineObj.currentSelectedPipeline.edges"
   @node-change="onNodeChange"
   @nodes-change="onNodesChange"
   @edges-change="onEdgesChange"
   @edge-click="onEdgeClick"
   @connect="onConnect"
   @dragover="onDragOver"
   :default-viewport="{ zoom: 1.5 }"
   :min-zoom="0.2"
 >
 ...
-const onEdgeClick = (event) => {
-  console.log('Edge click event triggered')
-  ...
-  showEdgeHelpNotification.value = true
-  ...
-}
+const { addSelectedEdges, removeSelectedEdges } = useVueFlow();
 
+const onEdgeClick = (_evt, edge) => {
+  // Select the clicked edge so keyboard deletion applies to it
+  removeSelectedEdges(); // clear previous selection
+  addSelectedEdges([edge.id]);
+
+  // Show help notification
+  if (notificationTimeout) {
+    clearTimeout(notificationTimeout);
+    notificationTimeout = null;
+  }
+  showEdgeHelpNotification.value = true;
+  notificationTimeout = setTimeout(() => {
+    showEdgeHelpNotification.value = false;
+    notificationTimeout = null;
+  }, 3500);
+};
+
Suggestion importance[1-10]: 8

__

Why: Ensuring the clicked edge is programmatically selected improves UX and makes keyboard deletion reliable. The change directly complements the new @edge-click handler and uses useVueFlow APIs accurately.

Medium
Possible issue
Make edge color updates reactive

Mutating pipelineObj.currentSelectedPipeline.edges in-place may not trigger Vue
reactivity, causing edge colors to not update reliably. Build new edge objects and
replace the array to ensure updates propagate. This also avoids accidental shared
object mutations.

web/src/plugins/pipelines/CustomNode.vue [96-113]

 const updateEdgeColors = (nodeId, color, reset = false) => {
-  if (pipelineObj.currentSelectedPipeline?.edges) {
-    pipelineObj.currentSelectedPipeline.edges.forEach(edge => {
-      if (edge.source === nodeId) {
-        if (reset) {
-          // Reset to default color
-          edge.style = { 
-            ...edge.style, 
-            stroke: '#6b7280', 
-            strokeWidth: 2 
-          };
-          edge.markerEnd = {
-            ...edge.markerEnd,
-            color: '#6b7280'
-          };
-        } else {
-          // Apply node color to both edge and arrow
-          edge.style = { 
-            ...edge.style, 
-            stroke: color, 
-            strokeWidth: 2 
-          };
-          edge.markerEnd = {
-            ...edge.markerEnd,
-            color: color
-          };
-        }
-      }
-    });
-  }
+  const edges = pipelineObj.currentSelectedPipeline?.edges;
+  if (!edges) return;
+
+  const updated = edges.map(edge => {
+    if (edge.source !== nodeId) return edge;
+    const stroke = reset ? '#6b7280' : (color || '#6b7280');
+    const style = { ...(edge.style || {}), stroke, strokeWidth: 2 };
+    const markerEnd = { ...(edge.markerEnd || {}), color: stroke };
+    return { ...edge, style, markerEnd };
+  });
+
+  // Replace array to trigger reactivity
+  pipelineObj.currentSelectedPipeline.edges = updated;
 };
Suggestion importance[1-10]: 7

__

Why: Correctly notes that in-place mutation of edges may not trigger Vue reactivity; replacing the array is safer. The improved code aligns with the snippet and would improve reliability, though it's a maintainability fix rather than critical.

Medium
Scope and safely clean key handler

Attaching a global keydown handler on window can delete edges even when the editor
is not focused, and multiple mounts risk stale global state. Scope deletion to when
the VueFlow canvas is focused and remove the global (window as any) reference to
avoid leaks.

web/src/components/pipeline/PipelineEditor.vue [519-558]

 onMounted(async () => {
   window.addEventListener("beforeunload", beforeUnloadHandler);
-  
-  // Add keyboard handler for edge deletion
-  const handleKeydown = (event) => {
+
+  const handleKeydown = (event: KeyboardEvent) => {
+    // Only act when an input is not focused and editor container is active
+    const active = document.activeElement as HTMLElement | null;
+    const isTypingTarget = active && (active.tagName === 'INPUT' || active.tagName === 'TEXTAREA' || active.isContentEditable);
+    if (isTypingTarget) return;
+
     if (event.key === 'Delete' || event.key === 'Backspace') {
-      const selectedEdges = getSelectedEdges.value
-      
+      const selectedEdges = getSelectedEdges.value;
       if (selectedEdges.length > 0) {
-        event.preventDefault()
-        const edgeIds = selectedEdges.map(edge => edge.id)
-        removeEdges(edgeIds)
+        event.preventDefault();
+        removeEdges(selectedEdges.map(e => e.id));
       }
     }
-  }
-  
-  window.addEventListener("keydown", handleKeydown);
-  
-  // Store handler reference for cleanup
-  (window as any).pipelineKeydownHandler = handleKeydown;
-  
+  };
+
+  // Attach to document to allow focus checks; keep local reference
+  document.addEventListener("keydown", handleKeydown);
+
+  onUnmounted(() => {
+    document.removeEventListener("keydown", handleKeydown);
+  });
+
   pipelineDestinationsList.value = await getPipelineDestinations();
+  usedStreamsListResponse.value = await getUsedStreamsList();
   ...
 });
 
-onUnmounted(() => {
-  window.removeEventListener("beforeunload", beforeUnloadHandler);
-  
-  // Cleanup keyboard handler
-  if ((window as any).pipelineKeydownHandler) {
-    window.removeEventListener("keydown", (window as any).pipelineKeydownHandler);
-  }
-});
-
Suggestion importance[1-10]: 6

__

Why: Scoping key handling and avoiding a global window reference reduces side effects and leaks. The idea is sound and relevant, but it’s a UX refinement rather than a bug fix, and the proposed code changes structure (using onUnmounted inside onMounted) which may need slight adjustment.

Low

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR enhances the pipeline UI with significant visual and functional improvements. The revamp includes:

Key Changes:

  • Enhanced Node Styling: New color schemes, hover effects, and improved typography for better visual hierarchy
  • Interactive Node Actions: Hover-to-show delete buttons with tooltips and edge color highlighting
  • Keyboard Edge Deletion: Replaced click-to-delete with keyboard shortcuts (Delete/Backspace) for better UX
  • Function Definition Display: New component showing function code in AssociateFunction dialog
  • Visual Polish: Updated icons, improved spacing, better dark mode support, and enhanced tooltips
  • Condition Truncation: Implements 30-character limit for condition display as per requirements

Technical Implementation:

  • Clean separation of concerns with dedicated Vue components
  • Proper event handling and state management through composables
  • Responsive design with dark/light theme support
  • Good use of Vue 3 Composition API patterns

Minor Issues:

  • Console.log statements left in development code (should be removed)
  • Otherwise solid implementation following Vue.js best practices

Confidence Score: 4/5

  • This PR is safe to merge with only minor cleanup needed for console statements
  • Score reflects high-quality UI improvements with comprehensive functionality, proper Vue.js patterns, and good responsive design. Only minor style issues with debug statements prevent a perfect score
  • PipelineFlow.vue and PipelineEditor.vue need console.log statements removed before production deployment

Important Files Changed

File Analysis

Filename        Score        Overview
web/src/components/pipeline/PipelineEditor.vue 4/5 Major UI revamp with new styling, keyboard edge deletion, and JSON editor integration - no critical issues found
web/src/components/pipeline/NodeForm/AssociateFunction.vue 4/5 Function association dialog with new function definition display - minor styling improvements
web/src/plugins/pipelines/CustomNode.vue 4/5 Enhanced node component with hover effects, action buttons, and edge color updates - well-implemented
web/src/plugins/pipelines/PipelineFlow.vue 4/5 Main pipeline flow with keyboard edge deletion and notification system - solid functionality

Sequence Diagram

sequenceDiagram
    participant User
    participant NodeSidebar
    participant PipelineFlow
    participant CustomNode
    participant NodeDialog
    participant PipelineEditor

    Note over User, PipelineEditor: Pipeline UI Revamp - Node Management Flow

    User->>NodeSidebar: Drag node from sidebar
    NodeSidebar->>PipelineFlow: onDragStart with node data
    User->>PipelineFlow: Drop node on canvas
    PipelineFlow->>PipelineFlow: onDrop - create new node
    PipelineFlow->>CustomNode: Render node with new styling

    User->>CustomNode: Hover over node
    CustomNode->>CustomNode: Show action buttons & highlight edges
    CustomNode->>PipelineFlow: Update connected edge colors

    User->>CustomNode: Click on node to edit
    CustomNode->>PipelineEditor: editNode(id)
    PipelineEditor->>NodeDialog: Open appropriate dialog
    alt Function Node
        NodeDialog->>NodeDialog: Show function selection with definition
    else Stream Node
        NodeDialog->>NodeDialog: Show stream configuration
    else Condition Node
        NodeDialog->>NodeDialog: Show condition builder
    end

    User->>NodeDialog: Configure node settings
    NodeDialog->>PipelineEditor: Save node data
    PipelineEditor->>PipelineFlow: Update pipeline state

    User->>PipelineFlow: Click edge
    PipelineFlow->>PipelineFlow: Show delete notification
    User->>PipelineFlow: Press Delete/Backspace key
    PipelineFlow->>PipelineFlow: Remove selected edge

    User->>CustomNode: Hover over delete button
    CustomNode->>CustomNode: Show delete tooltip
    User->>CustomNode: Click delete button
    CustomNode->>PipelineEditor: Delete node confirmation
    PipelineEditor->>PipelineFlow: Remove node from canvas

    User->>PipelineEditor: Click Save button
    PipelineEditor->>PipelineEditor: Validate pipeline structure
    PipelineEditor->>PipelineEditor: Save pipeline to backend
Loading

8 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

Comment on lines 153 to 168
console.log('Edge click event triggered')
// Clear any existing timeout
if (notificationTimeout) {
clearTimeout(notificationTimeout)
notificationTimeout = null
}
// Always show notification on edge click (even if already visible)
console.log('Showing notification for edge click')
showEdgeHelpNotification.value = true
console.log('Notification state set to:', showEdgeHelpNotification.value)
// Auto-hide notification after 3.5 seconds
notificationTimeout = setTimeout(() => {
console.log('Auto-hiding notification')
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Remove debug console.log statements before production deployment

Suggested change
console.log('Edge click event triggered')
// Clear any existing timeout
if (notificationTimeout) {
clearTimeout(notificationTimeout)
notificationTimeout = null
}
// Always show notification on edge click (even if already visible)
console.log('Showing notification for edge click')
showEdgeHelpNotification.value = true
console.log('Notification state set to:', showEdgeHelpNotification.value)
// Auto-hide notification after 3.5 seconds
notificationTimeout = setTimeout(() => {
console.log('Auto-hiding notification')
const onEdgeClick = (event) => {
// Clear any existing timeout
if (notificationTimeout) {
clearTimeout(notificationTimeout)
notificationTimeout = null
}
// Always show notification on edge click (even if already visible)
showEdgeHelpNotification.value = true
// Auto-hide notification after 3.5 seconds
notificationTimeout = setTimeout(() => {
showEdgeHelpNotification.value = false
notificationTimeout = null
}, 3500)
}
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/plugins/pipelines/PipelineFlow.vue
Line: 153:168

Comment:
style: Remove debug console.log statements before production deployment

```suggestion
    const onEdgeClick = (event) => {
      // Clear any existing timeout
      if (notificationTimeout) {
        clearTimeout(notificationTimeout)
        notificationTimeout = null
      }
      
      // Always show notification on edge click (even if already visible)
      showEdgeHelpNotification.value = true
      
      // Auto-hide notification after 3.5 seconds 
      notificationTimeout = setTimeout(() => {
        showEdgeHelpNotification.value = false
        notificationTimeout = null
      }, 3500)
    }
```

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes sure I will remove that

@nikhilsaikethe nikhilsaikethe force-pushed the ui-revamp branch 2 times, most recently from fae40fc to d4db0d0 Compare September 29, 2025 15:24
@nikhilsaikethe nikhilsaikethe merged commit ad34aaa into main Sep 30, 2025
30 of 31 checks passed
@nikhilsaikethe nikhilsaikethe deleted the ui-revamp branch September 30, 2025 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

☢️ Bug Something isn't working Review effort 4/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants