From f7dbbf1c3e5e689b4abd7464d020f7b75d1f102e Mon Sep 17 00:00:00 2001
From: Emir Karabeg <emirkarabeg@berkeley.edu>
Date: Thu, 13 Feb 2025 13:34:50 -0800
Subject: [PATCH] Fix: eval-input input ref and styling

---
 .../sub-block/components/eval-input.tsx       | 172 ++++++------------
 1 file changed, 53 insertions(+), 119 deletions(-)

diff --git a/app/w/[id]/components/workflow-block/components/sub-block/components/eval-input.tsx b/app/w/[id]/components/workflow-block/components/sub-block/components/eval-input.tsx
index 92978c862a..38cdeadfe2 100644
--- a/app/w/[id]/components/workflow-block/components/sub-block/components/eval-input.tsx
+++ b/app/w/[id]/components/workflow-block/components/sub-block/components/eval-input.tsx
@@ -1,10 +1,9 @@
 import { useRef, useState } from 'react'
-import { ChevronDown, ChevronUp, Plus, Trash } from 'lucide-react'
+import { Plus, Trash } from 'lucide-react'
 import { Button } from '@/components/ui/button'
 import { Input } from '@/components/ui/input'
 import { Label } from '@/components/ui/label'
 import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip'
-import { cn } from '@/lib/utils'
 import { useSubBlockValue } from '../hooks/use-sub-block-value'
 
 interface EvalMetric {
@@ -22,38 +21,26 @@ interface EvalInputProps {
   subBlockId: string
 }
 
+// Default values
+const DEFAULT_METRIC: EvalMetric = {
+  id: crypto.randomUUID(),
+  name: '',
+  description: '',
+  range: { min: 0, max: 1 },
+}
+
 export function EvalInput({ blockId, subBlockId }: EvalInputProps) {
+  // State hooks
   const [value, setValue] = useSubBlockValue<EvalMetric[]>(blockId, subBlockId)
-  const containerRef = useRef<HTMLDivElement>(null)
-
-  // Initialize with default metrics if value is null
-  const metrics = value || [
-    {
-      id: crypto.randomUUID(),
-      name: '',
-      description: '',
-      range: { min: 0, max: 1 },
-    },
-  ]
+  const metrics = value || [DEFAULT_METRIC]
 
+  // Metric operations
   const addMetric = () => {
     const newMetric: EvalMetric = {
+      ...DEFAULT_METRIC,
       id: crypto.randomUUID(),
-      name: '',
-      description: '',
-      range: { min: 0, max: 1 },
     }
     setValue([...metrics, newMetric])
-
-    // Focus the new metric's name input after a short delay
-    setTimeout(() => {
-      const newInput = containerRef.current?.querySelector(
-        `[data-metric-id="${newMetric.id}"] input[name="name"]`
-      ) as HTMLInputElement
-      if (newInput) {
-        newInput.focus()
-      }
-    }, 0)
   }
 
   const removeMetric = (id: string) => {
@@ -61,17 +48,9 @@ export function EvalInput({ blockId, subBlockId }: EvalInputProps) {
     setValue(metrics.filter((metric) => metric.id !== id))
   }
 
+  // Update handlers
   const updateMetric = (id: string, field: keyof EvalMetric, value: any) => {
-    setValue(
-      metrics.map((metric) =>
-        metric.id === id
-          ? {
-              ...metric,
-              [field]: value,
-            }
-          : metric
-      )
-    )
+    setValue(metrics.map((metric) => (metric.id === id ? { ...metric, [field]: value } : metric)))
   }
 
   const updateRange = (id: string, field: 'min' | 'max', value: string) => {
@@ -80,18 +59,15 @@ export function EvalInput({ blockId, subBlockId }: EvalInputProps) {
         metric.id === id
           ? {
               ...metric,
-              range: {
-                ...metric.range,
-                [field]: value,
-              },
+              range: { ...metric.range, [field]: value },
             }
           : metric
       )
     )
   }
 
+  // Validation handlers
   const handleRangeBlur = (id: string, field: 'min' | 'max', value: string) => {
-    // Allow any number including negatives, just remove non-numeric characters except - and .
     const sanitizedValue = value.replace(/[^\d.-]/g, '')
     const numValue = parseFloat(sanitizedValue)
 
@@ -110,94 +86,52 @@ export function EvalInput({ blockId, subBlockId }: EvalInputProps) {
     )
   }
 
-  const moveMetric = (id: string, direction: 'up' | 'down') => {
-    const index = metrics.findIndex((metric) => metric.id === id)
-    if (
-      (direction === 'up' && index === 0) ||
-      (direction === 'down' && index === metrics.length - 1)
-    )
-      return
+  // Metric header
+  const renderMetricHeader = (metric: EvalMetric, index: number) => (
+    <div className="flex h-10 items-center justify-between bg-card px-3 rounded-t-lg border-b">
+      <span className="text-sm font-medium">Metric {index + 1}</span>
+      <div className="flex items-center gap-1">
+        <Tooltip>
+          <TooltipTrigger asChild>
+            <Button variant="ghost" size="sm" onClick={addMetric} className="h-8 w-8">
+              <Plus className="h-4 w-4" />
+              <span className="sr-only">Add Metric</span>
+            </Button>
+          </TooltipTrigger>
+          <TooltipContent>Add Metric</TooltipContent>
+        </Tooltip>
 
-    const newMetrics = [...metrics]
-    const targetIndex = direction === 'up' ? index - 1 : index + 1
-    ;[newMetrics[index], newMetrics[targetIndex]] = [newMetrics[targetIndex], newMetrics[index]]
-    setValue(newMetrics)
-  }
+        <Tooltip>
+          <TooltipTrigger asChild>
+            <Button
+              variant="ghost"
+              size="sm"
+              onClick={() => removeMetric(metric.id)}
+              disabled={metrics.length === 1}
+              className="h-8 w-8 text-destructive hover:text-destructive"
+            >
+              <Trash className="h-4 w-4" />
+              <span className="sr-only">Delete Metric</span>
+            </Button>
+          </TooltipTrigger>
+          <TooltipContent>Delete Metric</TooltipContent>
+        </Tooltip>
+      </div>
+    </div>
+  )
 
+  // Main render
   return (
-    <div className="space-y-2" ref={containerRef}>
+    <div className="space-y-2">
       {metrics.map((metric, index) => (
         <div
           key={metric.id}
           data-metric-id={metric.id}
           className="overflow-visible rounded-lg border bg-background group relative"
         >
-          <div className="flex h-10 items-center justify-between bg-card px-3 rounded-t-lg border-b">
-            <span className="text-sm font-medium">Metric {index + 1}</span>
-            <div className="flex items-center gap-1">
-              <Tooltip>
-                <TooltipTrigger asChild>
-                  <Button variant="ghost" size="sm" onClick={addMetric} className="h-8 w-8">
-                    <Plus className="h-4 w-4" />
-                    <span className="sr-only">Add Metric</span>
-                  </Button>
-                </TooltipTrigger>
-                <TooltipContent>Add Metric</TooltipContent>
-              </Tooltip>
+          {renderMetricHeader(metric, index)}
 
-              <div className="flex items-center">
-                <Tooltip>
-                  <TooltipTrigger asChild>
-                    <Button
-                      variant="ghost"
-                      size="sm"
-                      onClick={() => moveMetric(metric.id, 'up')}
-                      disabled={index === 0}
-                      className="h-8 w-8"
-                    >
-                      <ChevronUp className="h-4 w-4" />
-                      <span className="sr-only">Move Up</span>
-                    </Button>
-                  </TooltipTrigger>
-                  <TooltipContent>Move Up</TooltipContent>
-                </Tooltip>
-
-                <Tooltip>
-                  <TooltipTrigger asChild>
-                    <Button
-                      variant="ghost"
-                      size="sm"
-                      onClick={() => moveMetric(metric.id, 'down')}
-                      disabled={index === metrics.length - 1}
-                      className="h-8 w-8"
-                    >
-                      <ChevronDown className="h-4 w-4" />
-                      <span className="sr-only">Move Down</span>
-                    </Button>
-                  </TooltipTrigger>
-                  <TooltipContent>Move Down</TooltipContent>
-                </Tooltip>
-              </div>
-
-              <Tooltip>
-                <TooltipTrigger asChild>
-                  <Button
-                    variant="ghost"
-                    size="sm"
-                    onClick={() => removeMetric(metric.id)}
-                    disabled={metrics.length === 1}
-                    className="h-8 w-8 text-destructive hover:text-destructive"
-                  >
-                    <Trash className="h-4 w-4" />
-                    <span className="sr-only">Delete Metric</span>
-                  </Button>
-                </TooltipTrigger>
-                <TooltipContent>Delete Metric</TooltipContent>
-              </Tooltip>
-            </div>
-          </div>
-
-          <div className="px-4 py-3 space-y-2">
+          <div className="px-3 pt-2 pb-3 space-y-2">
             <div className="space-y-1">
               <Label>Name</Label>
               <Input