From 08bfc1020d61ca37ecf6f473ef4f028aa3cf3d87 Mon Sep 17 00:00:00 2001
From: Ruben Fiszel <ruben@rubenfiszel.com>
Date: Thu, 8 Dec 2022 17:18:24 +0100
Subject: [PATCH] UX flow improvements

---
 .../src/lib/components/FlowBuilder.svelte     |   4 +-
 .../src/lib/components/FlowGraphViewer.svelte | 179 ++++++++++--------
 .../lib/components/FlowPreviewContent.svelte  |  15 +-
 .../lib/components/FlowStatusViewer.svelte    |  24 ++-
 frontend/src/lib/components/FlowViewer.svelte |   3 +-
 .../flows/content/FlowPathViewer.svelte       |   7 +-
 .../flows/header/FlowPreviewButtons.svelte    |   4 +-
 .../flows/map/FlowErrorHandlerItem.svelte     |  10 +-
 .../src/lib/components/graph/FlowGraph.svelte |  19 +-
 9 files changed, 147 insertions(+), 118 deletions(-)

diff --git a/frontend/src/lib/components/FlowBuilder.svelte b/frontend/src/lib/components/FlowBuilder.svelte
index 78c1c81342..b8c4908b0f 100644
--- a/frontend/src/lib/components/FlowBuilder.svelte
+++ b/frontend/src/lib/components/FlowBuilder.svelte
@@ -190,7 +190,9 @@
 
 <Drawer bind:this={flowViewer} size="1200px">
 	<DrawerContent title="View Graph" on:close={flowViewer.closeDrawer} noPadding>
-		<FlowGraphViewer flow={$flowStore} overflowAuto />
+		<div class="overflow-hidden h-full w-full">
+			<FlowGraphViewer flow={$flowStore} />
+		</div>
 	</DrawerContent>
 </Drawer>
 
diff --git a/frontend/src/lib/components/FlowGraphViewer.svelte b/frontend/src/lib/components/FlowGraphViewer.svelte
index b444d2216e..fc98e1c1d2 100644
--- a/frontend/src/lib/components/FlowGraphViewer.svelte
+++ b/frontend/src/lib/components/FlowGraphViewer.svelte
@@ -10,6 +10,7 @@
 	import ObjectViewer from './propertyPicker/ObjectViewer.svelte'
 	import typescript from 'svelte-highlight/languages/typescript'
 	import { cleanExpr } from './flows/utils'
+	import FlowPathViewer from './flows/content/FlowPathViewer.svelte'
 	export let flow: {
 		summary: string
 		description?: string
@@ -17,8 +18,11 @@
 		schema?: any
 	}
 	export let overflowAuto = false
+	export let noSide = false
+
 	let stepDetail: FlowModule | undefined = undefined
 	let codeViewer: Drawer
+	let topHeight = 0
 </script>
 
 <Drawer bind:this={codeViewer} size="900px">
@@ -72,99 +76,106 @@
 		{/if}
 	</DrawerContent>
 </Drawer>
-<div class="grid grid-cols-3 w-full h-full">
-	<div class="h-full col-span-2 w-full border border-gray-700" class:overflow-auto={overflowAuto}>
+<div class="grid grid-cols-3 w-full">
+	<div
+		bind:clientHeight={topHeight}
+		class="{noSide ? 'col-span-3' : 'col-span-2'} w-full border border-gray-400 h-screen"
+		class:overflow-auto={overflowAuto}
+	>
 		<FlowGraph
+			minHeight={topHeight}
 			modules={flow?.value?.modules}
 			failureModule={flow?.value?.failure_module}
 			on:click={(e) => (stepDetail = e.detail)}
 		/>
 	</div>
-	<div class="w-full border-r border-b border-t border-gray-700 min-h-[150px] p-2 overflow-auto">
-		{#if stepDetail == undefined}
-			<span class="font-black text-lg w-full my-4">
-				<span>Click on a step to see its details</span>
-			</span>
-		{:else}
-			<div class="font-black text-lg w-full mb-6"
-				>Step {stepDetail.id ?? ''}<span class="ml-2 font-normal">{stepDetail.summary || ''}</span
-				></div
-			>
-			{#if stepDetail.value.type == 'identity'}
-				<div> An identity step return as output its input </div>
-			{:else if stepDetail.value.type == 'rawscript'}
-				<div class="text-2xs mb-4">
-					<h3 class="mb-2">Step Inputs</h3>
-					<InputTransformsViewer
-						inputTransforms={stepDetail?.value?.input_transforms ??
-							stepDetail?.input_transforms ??
-							{}}
-					/>
-				</div>
-
-				<h3 class="mb-2"
-					>Code &nbsp; <button class="text-gray-600 text-xs" on:click={codeViewer.openDrawer}
-						>Expand</button
-					>
-				</h3>
-				<span class="!text-xs">
-					<HighlightCode language={stepDetail.value.language} code={stepDetail.value.content} />
+	{#if !noSide}
+		<div class="w-full border-r border-b border-t border-gray-400 min-h-[150px] p-2 overflow-auto">
+			{#if stepDetail == undefined}
+				<span class="font-black text-lg w-full my-4">
+					<span>Click on a step to see its details</span>
 				</span>
-			{:else if stepDetail.value.type == 'script'}
-				<div class="mb-4">
-					<a
-						rel="noreferrer"
-						target="_blank"
-						href={scriptPathToHref(stepDetail?.value?.path ?? '')}
-						class=""
-					>
-						<IconedPath path={stepDetail?.value?.path ?? ''} />
-					</a>
-				</div>
-				<div class="text-2xs mb-4">
-					<h3 class="mb-2">Step Inputs</h3>
-					<InputTransformsViewer
-						inputTransforms={stepDetail?.value?.input_transforms ??
-							stepDetail?.input_transforms ??
-							{}}
-					/>
-				</div>
-				{#if stepDetail.value.path.startsWith('hub/')}
-					<div class="mt-6">
-						<h3
-							>Code&nbsp; <button class="text-gray-600 text-xs" on:click={codeViewer.openDrawer}
-								>Expand</button
-							>
-						</h3>
-						<iframe
-							class="w-full h-full  text-sm"
-							title="embedded script from hub"
-							frameborder="0"
-							src="https://hub.windmill.dev/embed/script/{stepDetail.value?.path?.substring(4)}"
+			{:else}
+				<div class="font-black text-lg w-full mb-6"
+					>Step {stepDetail.id ?? ''}<span class="ml-2 font-normal">{stepDetail.summary || ''}</span
+					></div
+				>
+				{#if stepDetail.value.type == 'identity'}
+					<div> An identity step return as output its input </div>
+				{:else if stepDetail.value.type == 'rawscript'}
+					<div class="text-2xs mb-4">
+						<h3 class="mb-2">Step Inputs</h3>
+						<InputTransformsViewer
+							inputTransforms={stepDetail?.value?.input_transforms ??
+								stepDetail?.input_transforms ??
+								{}}
 						/>
 					</div>
-				{/if}
-			{:else if stepDetail.value.type == 'forloopflow'}
-				<p
-					>For loop with iterator: {#if stepDetail.value.iterator.type == 'static'}<ObjectViewer
-							json={stepDetail.value.iterator.value}
-						/>{:else}
-						<span class="text-xs">
-							<Highlight
-								offsetTop={0}
-								language={typescript}
-								code={cleanExpr(stepDetail.value.iterator.expr)}
+
+					<h3 class="mb-2"
+						>Code &nbsp; <button class="text-gray-600 text-xs" on:click={codeViewer.openDrawer}
+							>Expand</button
+						>
+					</h3>
+					<span class="!text-xs">
+						<HighlightCode language={stepDetail.value.language} code={stepDetail.value.content} />
+					</span>
+				{:else if stepDetail.value.type == 'script'}
+					<div class="mb-4">
+						<a
+							rel="noreferrer"
+							target="_blank"
+							href={scriptPathToHref(stepDetail?.value?.path ?? '')}
+							class=""
+						>
+							<IconedPath path={stepDetail?.value?.path ?? ''} />
+						</a>
+					</div>
+					<div class="text-2xs mb-4">
+						<h3 class="mb-2">Step Inputs</h3>
+						<InputTransformsViewer
+							inputTransforms={stepDetail?.value?.input_transforms ??
+								stepDetail?.input_transforms ??
+								{}}
+						/>
+					</div>
+					{#if stepDetail.value.path.startsWith('hub/')}
+						<div class="mt-6">
+							<h3
+								>Code&nbsp; <button class="text-gray-600 text-xs" on:click={codeViewer.openDrawer}
+									>Expand</button
+								>
+							</h3>
+							<iframe
+								class="w-full h-full  text-sm"
+								title="embedded script from hub"
+								frameborder="0"
+								src="https://hub.windmill.dev/embed/script/{stepDetail.value?.path?.substring(4)}"
 							/>
-						</span>
-					{/if}</p
-				>
-			{:else if stepDetail.value.type == 'branchall'}
-				<p>Run all branches</p>
-			{:else if stepDetail.value.type == 'branchone'}
-				<p>Run one branch based on a predicate</p>
-			{:else if stepDetail.value.type == 'flow'}
-				<p>Run a workspace flow</p>
+						</div>
+					{/if}
+				{:else if stepDetail.value.type == 'forloopflow'}
+					<p
+						>For loop with iterator: {#if stepDetail.value.iterator.type == 'static'}<ObjectViewer
+								json={stepDetail.value.iterator.value}
+							/>{:else}
+							<span class="text-xs">
+								<Highlight
+									offsetTop={0}
+									language={typescript}
+									code={cleanExpr(stepDetail.value.iterator.expr)}
+								/>
+							</span>
+						{/if}</p
+					>
+				{:else if stepDetail.value.type == 'branchall'}
+					<p>Run all branches</p>
+				{:else if stepDetail.value.type == 'branchone'}
+					<p>Run one branch based on a predicate</p>
+				{:else if stepDetail.value.type == 'flow'}
+					<FlowPathViewer noSide path={stepDetail.value.path} />
+				{/if}
 			{/if}
-		{/if}
-	</div>
+		</div>
+	{/if}
 </div>
diff --git a/frontend/src/lib/components/FlowPreviewContent.svelte b/frontend/src/lib/components/FlowPreviewContent.svelte
index f7c508c37e..ad611ec217 100644
--- a/frontend/src/lib/components/FlowPreviewContent.svelte
+++ b/frontend/src/lib/components/FlowPreviewContent.svelte
@@ -94,17 +94,12 @@
 
 <div class="flex divide-y flex-col space-y-2 h-screen bg-white px-6 py-2 w-full">
 	<div class="flex flex-row justify-between w-full items-center gap-x-2">
-		<Button
-			variant="border"
-			size="lg"
-			color="dark"
-			btnClasses="!p-0 !w-16 !h-full"
-			on:click={() => {
-				dispatch('close')
-			}}
+		<button
+			on:click={() => dispatch('close')}
+			class="hover:bg-gray-200 bg-gray-100 rounded-full w-8 h-8 flex items-center justify-center transition-all"
 		>
-			<Icon data={faClose} />
-		</Button>
+			<Icon data={faClose} class="text-gray-500" />
+		</button>
 
 		{#if isRunning}
 			<Button
diff --git a/frontend/src/lib/components/FlowStatusViewer.svelte b/frontend/src/lib/components/FlowStatusViewer.svelte
index 1fffba8400..aad9989d73 100644
--- a/frontend/src/lib/components/FlowStatusViewer.svelte
+++ b/frontend/src/lib/components/FlowStatusViewer.svelte
@@ -14,6 +14,7 @@
 	import { FlowGraph, type GraphModuleState } from './graph'
 	import ModuleStatus from './ModuleStatus.svelte'
 	import { displayDate, truncateRev } from '$lib/utils'
+	import JobArgs from './JobArgs.svelte'
 
 	const dispatch = createEventDispatcher()
 
@@ -338,9 +339,8 @@
 		</div>
 	</div>
 	{#if job.raw_flow && !isListJob}
-		<div class="{selected != 'graph' ? 'hidden' : ''} mx-10 mt-4">
-			<div class="border" />
-			<div class="grid grid-cols-3">
+		<div class="{selected != 'graph' ? 'hidden' : ''} mt-4">
+			<div class="grid grid-cols-3 border border-gray-300">
 				<div class="col-span-2 bg-gray-50">
 					<div class="flex flex-col">
 						{#each Object.values(retry_status) as count}
@@ -355,16 +355,30 @@
 						on:click={(e) => {
 							if (e.detail.id) {
 								selectedNode = e.detail.id
+							} else if (e.detail == 'End') {
+								selectedNode = 'end'
+							} else if (e.detail == 'Start') {
+								selectedNode = 'start'
 							}
 						}}
 						modules={job.raw_flow?.modules ?? []}
 						failureModule={job.raw_flow?.failure_module}
 					/>
 				</div>
-				<div class="border-l border-gray-400 pt-1">
+				<div class="border-l border-gray-400 pt-1 overflow-hidden">
 					{#if selectedNode}
 						{@const node = localFlowModuleStates[selectedNode]}
-						{#if node}
+						{#if selectedNode == 'end'}
+							<FlowJobResult noBorder col result={job['result'] ?? {}} logs={job.logs ?? ''} />
+						{:else if selectedNode == 'start'}
+							{#if job.args}
+								<div class="p-2">
+									<JobArgs args={job.args} />
+								</div>
+							{:else}
+								<p class="p-2">No arguments</p>
+							{/if}
+						{:else if node}
 							<div class="px-2 flex gap-2 min-w-0 ">
 								<ModuleStatus type={node.type} scheduled_for={node['scheduled_for']} />
 								{#if node.job_id}
diff --git a/frontend/src/lib/components/FlowViewer.svelte b/frontend/src/lib/components/FlowViewer.svelte
index 8536ec1a59..9de4cb8b36 100644
--- a/frontend/src/lib/components/FlowViewer.svelte
+++ b/frontend/src/lib/components/FlowViewer.svelte
@@ -17,6 +17,7 @@
 		schema?: any
 	}
 	export let initialOpen: number | undefined = undefined
+	export let noSide = false
 
 	$: flowFiltered = {
 		summary: flow.summary,
@@ -76,7 +77,7 @@
 				{:else}
 					<div class="text-gray-700 text-xs italic mb-4">No inputs</div>
 				{/if}
-				<FlowGraphViewer {flow} overflowAuto />
+				<FlowGraphViewer {noSide} {flow} overflowAuto />
 			</div>
 		</TabContent>
 		<TabContent value="json">
diff --git a/frontend/src/lib/components/flows/content/FlowPathViewer.svelte b/frontend/src/lib/components/flows/content/FlowPathViewer.svelte
index 7287bb3212..109e833c0f 100644
--- a/frontend/src/lib/components/flows/content/FlowPathViewer.svelte
+++ b/frontend/src/lib/components/flows/content/FlowPathViewer.svelte
@@ -1,10 +1,11 @@
 <script lang="ts">
 	import Skeleton from '$lib/components/common/skeleton/Skeleton.svelte'
-	import FlowViewer from '$lib/components/FlowViewer.svelte'
+	import FlowGraphViewer from '$lib/components/FlowGraphViewer.svelte'
 	import { FlowService, type Flow } from '$lib/gen'
 	import { workspaceStore } from '$lib/stores'
 
 	export let path: string
+	export let noSide = false
 
 	let flow: Flow | undefined = undefined
 	async function loadFlow(path: string) {
@@ -14,9 +15,9 @@
 	$: path && loadFlow(path)
 </script>
 
-<div class="flex flex-col flex-1 h-full overflow-auto p-2">
+<div class="flex flex-col flex-1 h-full overflow-auto">
 	{#if flow}
-		<FlowViewer {flow} />
+		<FlowGraphViewer {noSide} {flow} />
 	{:else}
 		<Skeleton layout={[[40]]} />
 	{/if}
diff --git a/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte b/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte
index 73b52e0eaf..b7e091d6c6 100644
--- a/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte
+++ b/frontend/src/lib/components/flows/header/FlowPreviewButtons.svelte
@@ -26,7 +26,7 @@
 			'inputs',
 			'schedules',
 			'failure'
-		].includes($selectedId) || $selectedId.includes('branch')
+		].includes($selectedId) || $selectedId?.includes('branch')
 </script>
 
 <div class="flex flex-row-reverse justify-between items-center gap-x-2">
@@ -60,7 +60,7 @@
 	{/if}
 </div>
 
-<Drawer bind:open={previewOpen} size="1200px">
+<Drawer bind:open={previewOpen} size="75%">
 	<FlowPreviewContent
 		open={previewOpen}
 		bind:previewMode
diff --git a/frontend/src/lib/components/flows/map/FlowErrorHandlerItem.svelte b/frontend/src/lib/components/flows/map/FlowErrorHandlerItem.svelte
index 80b590c964..100a6fe2af 100644
--- a/frontend/src/lib/components/flows/map/FlowErrorHandlerItem.svelte
+++ b/frontend/src/lib/components/flows/map/FlowErrorHandlerItem.svelte
@@ -12,7 +12,7 @@
 	const { select, selectedId } = getContext<FlowEditorContext>('FlowEditorContext')
 
 	function onToggle() {
-		if ($flowStore.value.failure_module) {
+		if ($flowStore?.value?.failure_module) {
 			$flowStore.value.failure_module = undefined
 			// By default, we return to settings when disabling the failure module
 			select('settings')
@@ -34,7 +34,7 @@
 <!-- svelte-ignore a11y-click-events-have-key-events -->
 <div
 	on:click={() => {
-		if ($flowStore.value.failure_module) {
+		if ($flowStore?.value?.failure_module) {
 			select('failure')
 		} else {
 			onToggle()
@@ -42,7 +42,7 @@
 	}}
 	class={classNames(
 		'border rounded-sm px-2 py-1 bg-white text-sm border-gray-400 cursor-pointer flex flex-col overflow-x-hidden ',
-		$selectedId.includes('failure') ? 'outline outline-offset-1 outline-2 outline-slate-900' : ''
+		$selectedId?.includes('failure') ? 'outline outline-offset-1 outline-2 outline-slate-900' : ''
 	)}
 >
 	<div class=" flex justify-between items-center flex-wrap">
@@ -51,12 +51,12 @@
 			<span class="font-bold text-xs">Error handler</span>
 		</div>
 		<div class="-my-1">
-			<Toggle checked={Boolean($flowStore.value.failure_module)} on:change={onToggle} />
+			<Toggle checked={Boolean($flowStore?.value?.failure_module)} on:change={onToggle} />
 		</div>
 	</div>
 
 	<div class="w-full truncate block">
-		{#if Boolean($flowStore.value.failure_module)}
+		{#if Boolean($flowStore?.value?.failure_module)}
 			<span>
 				{$flowStore.value.failure_module?.summary ||
 					($flowStore.value.failure_module?.value.type === 'rawscript'
diff --git a/frontend/src/lib/components/graph/FlowGraph.svelte b/frontend/src/lib/components/graph/FlowGraph.svelte
index 8b4e4b1fba..fc10d8354d 100644
--- a/frontend/src/lib/components/graph/FlowGraph.svelte
+++ b/frontend/src/lib/components/graph/FlowGraph.svelte
@@ -34,7 +34,6 @@
 	let edges: Edge[] = []
 	let width: number, height: number
 
-	let loadedFlows: Record<string, FlowModule[]> = {}
 	let errorHandlers: Record<string, number> = {}
 
 	let dispatch = createEventDispatcher()
@@ -55,13 +54,13 @@
 		}
 		nestedNodes = nodes = []
 
-		nestedNodes.push(createVirtualNode(getParentIds(), 'Flow start'))
+		nestedNodes.push(createVirtualNode(getParentIds(), 'Start'))
 
 		modules.forEach((m) => {
 			const item = getConvertedFlowModule(m)
 			item && nestedNodes.push(item)
 		})
-		nestedNodes.push(createVirtualNode(getParentIds(), 'Flow end'))
+		nestedNodes.push(createVirtualNode(getParentIds(), 'End'))
 
 		if (!flowModuleStates) {
 			if (failureModule) nestedNodes.push(createErrorHandler(failureModule))
@@ -430,9 +429,15 @@
 			},
 			width: NODE.width,
 			height: NODE.height,
-			borderColor: '#999',
-			bgColor: '#d4e4ff',
-			parentIds
+			borderColor: selectedNode == label ? 'black' : '#999',
+			bgColor: selectedNode == label ? '#f5f5f5' : '#d4e4ff',
+			parentIds,
+			clickCallback: (node) => {
+				if (!notSelectable) {
+					selectedNode = label
+				}
+				dispatch('click', label)
+			}
 		}
 	}
 
@@ -459,7 +464,7 @@
 			width: NODE.width,
 			height: NODE.height,
 			bgColor: selectedNode == mod.id ? '#f5f5f5' : getStateColor(flowModuleStates?.[mod.id]?.type),
-			borderColor: '#999',
+			borderColor: selectedNode == mod.id ? 'black' : '#999',
 			parentIds: parent_module ? [parent_module] : [],
 			clickCallback: (node) => {
 				if (!notSelectable) {