Jelajahi Sumber

fix: give Viewing and Pinning separate, honest entry points

Viewing and pinning are different intents - viewing is a temporary,
read-only look at one past run; pinning is a deliberate, lasting
choice to bring a run's outputs onto the live canvas while continuing
to edit. They shared a single pin slot under the hood, and "View"
silently persisted a pin as a side effect: leaving Viewing left the
pin behind even though nobody asked to keep it, which is why "Clear
pinned data" needed to exist everywhere and why the header's "Pinned"
tag could appear for a run someone only glanced at.

Added isPinKept to the pin store. "Pin Data" and "Keep Pinned" (in the
banner, while viewing) set it; "View" does not - it borrows the pin
slot only to have something to show on the read-only canvas. Returning
to the editor now clears an unkept pin automatically, so a plain view
leaves no residue; a kept pin is left alone. The header's "Pinned" tag
and "Clear pinned data" only appear once something is actually kept,
so the screen never claims a persisted pin nobody asked for. The two
"close" affordances (the results panel's X and the banner's "Return to
Editor") now both go through the same handler, so they cannot drift
apart again.

Also: "Pin Data" now opens the results panel the same way Execute
does (previously the panel was forced open by the pin's mere
existence, which also meant the "Results" toggle did nothing while
pinned - clicking it flipped a flag that an OR condition ignored).
Results is now a real, working toggle in every mode.

Renamed the banner's "Pin This Data" callback to onKeepPinned to match
what it actually does now.
fszontagh 1 bulan lalu
induk
melakukan
a24607c0c4

+ 11 - 6
webui/src/components/workflow/EditorHeader.tsx

@@ -18,6 +18,7 @@ interface EditorHeaderProps {
   isViewingExecution: boolean
   executingNodeId: string | null
   pinnedExecution: ExecutionDetail | null
+  isPinKept: boolean
   showExecutionPanel: boolean
   hasExecutionData: boolean
   isActive: boolean
@@ -101,6 +102,7 @@ export function EditorHeader({
   isViewingExecution,
   executingNodeId,
   pinnedExecution,
+  isPinKept,
   showExecutionPanel,
   hasExecutionData,
   isActive,
@@ -358,19 +360,22 @@ export function EditorHeader({
         <button
           onClick={onShowExecutionList}
           className={`flex items-center gap-1.5 px-2 py-1 rounded-lg ${
-            pinnedExecution
+            pinnedExecution && isPinKept
               ? 'bg-green-100 dark:bg-green-900/30 text-green-700 dark:text-green-400'
               : 'text-gray-600 dark:text-gray-400 hover:bg-gray-100 dark:hover:bg-slate-700'
           }`}
-          title="View execution history"
+          title="Browse execution history"
         >
           <History className="w-4 h-4" />
           Executions
         </button>
-        {/* Clear pinned data: reachable in every mode where there is a pin to
-            clear, viewing included - it only clears the pin store, so there is
-            nothing about Viewing mode that makes it unsafe here. */}
-        {pinnedExecution && (
+        {/* Clear pinned data: reachable in every mode where there is a kept
+            pin to clear, viewing included - it only clears the pin store, so
+            there is nothing about Viewing mode that makes it unsafe here.
+            Not shown for a pin that only exists to power Viewing mode itself
+            (isPinKept false) - that one clears on its own when the user
+            leaves, since nobody asked to keep it. */}
+        {pinnedExecution && isPinKept && (
           <button
             onClick={onUnpinExecution}
             className="flex items-center gap-1 px-1.5 py-1 text-xs text-green-700 dark:text-green-400 hover:text-red-600 dark:hover:text-red-400 hover:bg-red-50 dark:hover:bg-red-900/20 rounded-lg"

+ 7 - 4
webui/src/components/workflow/ExecutionViewerBanner.tsx

@@ -4,7 +4,10 @@ import type { ExecutionDetail } from '../../api/workflows'
 interface ExecutionViewerBannerProps {
   execution: ExecutionDetail
   onReturnToEditor: () => void
-  onPinData: () => void
+  // Promotes this view into a kept pin. Viewing alone leaves nothing behind
+  // when you return to the editor - this is the one deliberate way to change
+  // that for this run's data.
+  onKeepPinned: () => void
 }
 
 function formatTimestamp(timestamp: number): string {
@@ -19,7 +22,7 @@ function formatTimestamp(timestamp: number): string {
 export function ExecutionViewerBanner({
   execution,
   onReturnToEditor,
-  onPinData,
+  onKeepPinned,
 }: ExecutionViewerBannerProps) {
   const isCompleted = execution.status === 'completed'
   const StatusIcon = isCompleted ? CheckCircle : XCircle
@@ -49,8 +52,8 @@ export function ExecutionViewerBanner({
         <div className="flex items-center gap-3">
           {isCompleted && (
             <button
-              onClick={onPinData}
-              title="This execution's data is already pinned - return to the live editor and keep it pinned"
+              onClick={onKeepPinned}
+              title="Keep this run's outputs pinned on the canvas after you return to the editor - without this, viewing leaves nothing behind"
               className="flex items-center gap-1.5 px-3 py-1.5 bg-white/20 hover:bg-white/30 rounded-lg text-sm font-medium transition-colors"
             >
               <Pin className="w-4 h-4" />

+ 38 - 17
webui/src/pages/WorkflowEditorPage.tsx

@@ -418,9 +418,11 @@ function WorkflowEditorInner() {
     pinnedNodeData,
     nodeChangeStatus,
     isViewingExecution,
+    isPinKept,
     pinExecution,
     unpinExecution,
     setViewingExecution,
+    keepPin,
     recomputeChangeStatus,
   } = useExecutionPinStore()
 
@@ -550,27 +552,42 @@ function WorkflowEditorInner() {
     return { currentNodes, currentConnections }
   }, [nodes, edges])
 
-  // Handle viewing an execution in read-only mode
+  // Handle viewing an execution in read-only mode. Viewing is a temporary,
+  // read-only look at one past run - it borrows the pin slot to have
+  // something to show on the canvas, but it is not a "kept" pin: leaving
+  // Viewing without saying otherwise (see handleReturnToEditor) clears it,
+  // so a look at old history doesn't quietly leave something pinned behind.
   const handleViewExecution = useCallback((execution: ExecutionDetail) => {
     const { currentNodes, currentConnections } = getCurrentWorkflowData()
-    pinExecution(execution, currentNodes, currentConnections)
+    pinExecution(execution, currentNodes, currentConnections, false)
     setViewingExecution(true) // Must be called AFTER pinExecution (which resets it to false)
     // Keep execution list open for continuous navigation
     showToast('info', 'Viewing execution results')
   }, [getCurrentWorkflowData, pinExecution, setViewingExecution, showToast])
 
-  // Handle pinning execution data
+  // Handle pinning execution data. This is the one way to bring a run's
+  // outputs onto the live canvas while continuing to edit - a deliberate,
+  // lasting choice, unlike Viewing.
   const handlePinExecution = useCallback((execution: ExecutionDetail) => {
     const { currentNodes, currentConnections } = getCurrentWorkflowData()
-    pinExecution(execution, currentNodes, currentConnections)
+    pinExecution(execution, currentNodes, currentConnections, true)
+    // Opens the results panel the same way Execute does, so every action
+    // that produces something worth looking at behaves the same way.
+    setShowExecutionPanel(true)
     // Keep execution list open for continuous navigation
     showToast('success', 'Execution data pinned')
   }, [getCurrentWorkflowData, pinExecution, showToast])
 
-  // Return from viewing to editor
+  // Return from viewing to editor. A plain view leaves no residue: if the
+  // user never asked to keep this pin (via "Keep Pinned" in the banner),
+  // leaving clears it, so the header cannot end up showing "Pinned" for a
+  // pin nobody deliberately asked to keep.
   const handleReturnToEditor = useCallback(() => {
     setViewingExecution(false)
-  }, [setViewingExecution])
+    if (!isPinKept) {
+      unpinExecution()
+    }
+  }, [setViewingExecution, isPinKept, unpinExecution])
 
   // The pinned/unchanged-vs-changed diff is computed once at pin time. If the
   // user keeps editing after pinning (a node is live-editable in Pinned-not-
@@ -2838,9 +2855,12 @@ function WorkflowEditorInner() {
         <ExecutionViewerBanner
           execution={pinnedExecution}
           onReturnToEditor={handleReturnToEditor}
-          onPinData={() => {
+          onKeepPinned={() => {
+            // The one way a view is promoted into a kept pin. Without this,
+            // returning to the editor would clear it (see handleReturnToEditor).
+            keepPin()
             setViewingExecution(false)
-            showToast('success', 'Execution data pinned')
+            showToast('success', 'Kept - this run\'s outputs will stay on the canvas until you clear them')
           }}
         />
       )}
@@ -2865,7 +2885,8 @@ function WorkflowEditorInner() {
         isViewingExecution={isViewingExecution}
         executingNodeId={executingNodeId}
         pinnedExecution={pinnedExecution}
-        showExecutionPanel={showExecutionPanel || isViewingExecution || !!pinnedExecution}
+        isPinKept={isPinKept}
+        showExecutionPanel={showExecutionPanel || isViewingExecution}
         hasExecutionData={!!(effectiveExecutionState.executionId || Object.keys(effectiveExecutionState.nodeStates).length > 0)}
         isActive={!!workflow?.active}
         isTogglingActive={toggleActiveMutation.isPending}
@@ -2938,7 +2959,7 @@ function WorkflowEditorInner() {
             // of the nodes it was copied from.
             pointerRef.current = reactFlowInstance.screenToFlowPosition({ x: e.clientX, y: e.clientY })
           }}
-          className={`flex-1 overflow-hidden ${(showExecutionPanel || isViewingExecution || pinnedExecution) ? 'w-2/3' : 'w-full'}`}>
+          className={`flex-1 overflow-hidden ${(showExecutionPanel || isViewingExecution) ? 'w-2/3' : 'w-full'}`}>
           <ReactFlow
             nodes={isViewingExecution && viewedNodes ? viewedNodes : nodes}
             edges={isViewingExecution && viewedEdges ? viewedEdges : edges}
@@ -3035,7 +3056,7 @@ function WorkflowEditorInner() {
         </div>
 
         {/* Execution results panel */}
-        {(showExecutionPanel || isViewingExecution || pinnedExecution) && (
+        {(showExecutionPanel || isViewingExecution) && (
           <ExecutionResultsPanel
             executionState={effectiveExecutionState}
             nodes={isViewingExecution && viewedNodes ? viewedNodes : nodes}
@@ -3044,14 +3065,14 @@ function WorkflowEditorInner() {
             selectedIterations={selectedIterations}
             isViewingExecution={isViewingExecution}
             onClose={() => {
-              // Closing here means the same thing as the banner's "Return to
-              // Editor": stop looking at the read-only snapshot, but leave the
-              // pin alone. Clearing pinned data is a separate, deliberate
-              // action (the "Clear" control), reachable in every mode where
-              // something is pinned - not a side effect of closing this panel.
+              // Closing here means exactly the same thing as the banner's
+              // "Return to Editor" - both go through handleReturnToEditor, so
+              // the two closes cannot drift apart again. An ephemeral view
+              // pin is cleared; a kept one is left alone, same as leaving via
+              // the banner.
               setShowExecutionPanel(false)
               if (isViewingExecution) {
-                setViewingExecution(false)
+                handleReturnToEditor()
               }
             }}
             onToggleNodeExpansion={toggleNodeExpansion}

+ 30 - 2
webui/src/stores/executionPinStore.ts

@@ -8,14 +8,35 @@ interface ExecutionPinState {
   pinnedNodeData: Record<string, any>
   nodeChangeStatus: Record<string, NodeChangeStatus>
   isViewingExecution: boolean
+  /**
+   * Whether the current pin is a deliberate, lasting choice ("Pin Data", or
+   * "Keep Pinned" while viewing) versus an implementation detail of Viewing
+   * mode (which reuses this same slot to have something to show on screen).
+   * Viewing and pinning are different intents - browsing a past run
+   * read-only versus bringing its outputs onto the live canvas while still
+   * editing - and this flag is what lets the UI tell them apart: the header
+   * only shows a "Pinned" tag and a way to clear it when this is true, so
+   * leaving a plain view behind doesn't look like it left something pinned
+   * that nobody asked to keep.
+   */
+  isPinKept: boolean
 
+  /**
+   * @param kept Pass true for a pin the user asked for directly ("Pin Data").
+   *   Pass false when this is only being pinned to have something to view
+   *   read-only - it does not count as a kept pin until keepPin() says so.
+   */
   pinExecution: (
     execution: ExecutionDetail,
     currentNodes: WorkflowNode[],
-    currentConnections: Connection[]
+    currentConnections: Connection[],
+    kept: boolean
   ) => void
   unpinExecution: () => void
   setViewingExecution: (viewing: boolean) => void
+  /** Promotes the current pin (usually an ephemeral one set up for Viewing)
+   * into a kept one. This is the one way "View" turns into "Pin". */
+  keepPin: () => void
   getPinnedOutput: (nodeId: string) => any | null
   getNodeExecution: (nodeId: string) => NodeExecution | null
   /**
@@ -135,8 +156,9 @@ export const useExecutionPinStore = create<ExecutionPinState>((set, get) => ({
   pinnedNodeData: {},
   nodeChangeStatus: {},
   isViewingExecution: false,
+  isPinKept: false,
 
-  pinExecution: (execution, currentNodes, currentConnections) => {
+  pinExecution: (execution, currentNodes, currentConnections, kept) => {
     const nodeChangeStatus = analyzeChanges(
       execution,
       currentNodes,
@@ -149,6 +171,7 @@ export const useExecutionPinStore = create<ExecutionPinState>((set, get) => ({
       pinnedNodeData,
       nodeChangeStatus,
       isViewingExecution: false,
+      isPinKept: kept,
     })
   },
 
@@ -158,6 +181,7 @@ export const useExecutionPinStore = create<ExecutionPinState>((set, get) => ({
       pinnedNodeData: {},
       nodeChangeStatus: {},
       isViewingExecution: false,
+      isPinKept: false,
     })
   },
 
@@ -165,6 +189,10 @@ export const useExecutionPinStore = create<ExecutionPinState>((set, get) => ({
     set({ isViewingExecution: viewing })
   },
 
+  keepPin: () => {
+    set((state) => (state.pinnedExecution ? { isPinKept: true } : state))
+  },
+
   getPinnedOutput: (nodeId) => {
     const state = get()
     return state.pinnedNodeData[nodeId] ?? null