Merge pull request #39 from lordmathis/feat/instance-dialog

feat: Redesign create/edit instance dialog

webui/src/components/InstanceDialog.tsx

@@ -1,7 +1,5 @@
 import React, { useState, useEffect } from "react";
 import { Button } from "@/components/ui/button";
-import { Input } from "@/components/ui/input";
-import { Label } from "@/components/ui/label";
 import {
   Dialog,
   DialogContent,
@@ -11,13 +9,9 @@ import {
   DialogTitle,
 } from "@/components/ui/dialog";
 import { BackendType, type CreateInstanceOptions, type Instance } from "@/types/instance";
-import { getAdvancedFields, getAdvancedBackendFields } from "@/lib/zodFormUtils";
-import { ChevronDown, ChevronRight, Terminal } from "lucide-react";
 import ParseCommandDialog from "@/components/ParseCommandDialog";
-import AutoRestartConfiguration from "@/components/instance/AutoRestartConfiguration";
-import BasicInstanceFields from "@/components/instance/BasicInstanceFields";
-import BackendConfiguration from "@/components/instance/BackendConfiguration";
-import AdvancedInstanceFields from "@/components/instance/AdvancedInstanceFields";
+import InstanceSettingsCard from "@/components/instance/InstanceSettingsCard";
+import BackendConfigurationCard from "@/components/instance/BackendConfigurationCard";
 
 interface InstanceDialogProps {
   open: boolean;
@@ -36,13 +30,9 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
 
   const [instanceName, setInstanceName] = useState("");
   const [formData, setFormData] = useState<CreateInstanceOptions>({});
-  const [showAdvanced, setShowAdvanced] = useState(false);
   const [nameError, setNameError] = useState("");
   const [showParseDialog, setShowParseDialog] = useState(false);
 
-  // Get field lists dynamically from the type
-  const advancedFields = getAdvancedFields();
-  const advancedBackendFields = getAdvancedBackendFields(formData.backend_type);
 
   // Reset form when dialog opens/closes or when instance changes
   useEffect(() => {
@@ -60,7 +50,6 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
           backend_options: {},
         });
       }
-      setShowAdvanced(false); // Always start with basic view
       setNameError(""); // Reset any name errors
     }
   }, [open, instance]);
@@ -151,9 +140,6 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
     onOpenChange(false);
   };
 
-  const toggleAdvanced = () => {
-    setShowAdvanced(!showAdvanced);
-  };
 
   const handleCommandParsed = (parsedOptions: CreateInstanceOptions) => {
     setFormData(prev => ({
@@ -189,91 +175,25 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
         </DialogHeader>
 
         <div className="flex-1 overflow-y-auto">
-          <div className="grid gap-6 py-4">
-            {/* Instance Name - Special handling since it's not in CreateInstanceOptions */}
-            <div className="grid gap-2">
-              <Label htmlFor="name">
-                Instance Name <span className="text-red-500">*</span>
-              </Label>
-              <Input
-                id="name"
-                value={instanceName}
-                onChange={(e) => handleNameChange(e.target.value)}
-                placeholder="my-instance"
-                disabled={isEditing} // Don't allow name changes when editing
-                className={nameError ? "border-red-500" : ""}
-              />
-              {nameError && <p className="text-sm text-red-500">{nameError}</p>}
-              <p className="text-sm text-muted-foreground">
-                Unique identifier for the instance
-              </p>
-            </div>
-
-            {/* Auto Restart Configuration Section */}
-            <AutoRestartConfiguration
+          <div className="space-y-6 py-4">
+            {/* Instance Settings Card */}
+            <InstanceSettingsCard
+              instanceName={instanceName}
+              nameError={nameError}
+              isEditing={isEditing}
               formData={formData}
+              onNameChange={handleNameChange}
               onChange={handleFieldChange}
             />
 
-            {/* Basic Fields */}
-            <BasicInstanceFields
-              formData={formData}
-              onChange={handleFieldChange}
-            />
-
-            {/* Backend Configuration Section */}
-            <BackendConfiguration
+            {/* Backend Configuration Card */}
+            <BackendConfigurationCard
               formData={formData}
               onBackendFieldChange={handleBackendFieldChange}
-              showAdvanced={showAdvanced}
+              onChange={handleFieldChange}
+              onParseCommand={() => setShowParseDialog(true)}
             />
 
-            {/* Advanced Fields Toggle */}
-            <div className="border-t pt-4">
-              <div className="flex items-center justify-between">
-                <Button
-                  variant="outline"
-                  onClick={() => setShowParseDialog(true)}
-                  className="flex items-center gap-2"
-                >
-                  <Terminal className="h-4 w-4" />
-                  Parse Command
-                </Button>
-                
-                <Button
-                  variant="ghost"
-                  onClick={toggleAdvanced}
-                  className="flex items-center gap-2 p-0 h-auto font-medium"
-                >
-                  {showAdvanced ? (
-                    <ChevronDown className="h-4 w-4" />
-                  ) : (
-                    <ChevronRight className="h-4 w-4" />
-                  )}
-                  Advanced Configuration
-                  <span className="text-muted-foreground text-sm font-normal">
-                    (
-                    {
-                      advancedFields.filter(
-                        (f) =>
-                          !["max_restarts", "restart_delay", "backend_options"].includes(f as string)
-                      ).length + advancedBackendFields.length
-                    }{" "}
-                    options)
-                  </span>
-                </Button>
-              </div>
-            </div>
-
-            {/* Advanced Fields */}
-            {showAdvanced && (
-              <div className="space-y-4 pl-6 border-l-2 border-muted">
-                <AdvancedInstanceFields
-                  formData={formData}
-                  onChange={handleFieldChange}
-                />
-              </div>
-            )}
           </div>
         </div>
 
@@ -299,6 +219,7 @@ const InstanceDialog: React.FC<InstanceDialogProps> = ({
         open={showParseDialog}
         onOpenChange={setShowParseDialog}
         onParsed={handleCommandParsed}
+        backendType={formData.backend_type || BackendType.LLAMA_CPP}
       />
     </Dialog>
   );

webui/src/components/ParseCommandDialog.tsx

@@ -17,15 +17,16 @@ interface ParseCommandDialogProps {
   open: boolean;
   onOpenChange: (open: boolean) => void;
   onParsed: (options: CreateInstanceOptions) => void;
+  backendType: BackendTypeValue;
 }
 
 const ParseCommandDialog: React.FC<ParseCommandDialogProps> = ({
   open,
   onOpenChange,
   onParsed,
+  backendType,
 }) => {
   const [command, setCommand] = useState('');
-  const [backendType, setBackendType] = useState<BackendTypeValue>(BackendType.LLAMA_CPP);
   const [loading, setLoading] = useState(false);
   const [error, setError] = useState<string | null>(null);
 
@@ -75,7 +76,6 @@ const ParseCommandDialog: React.FC<ParseCommandDialogProps> = ({
   const handleOpenChange = (open: boolean) => {
     if (!open) {
       setCommand('');
-      setBackendType(BackendType.LLAMA_CPP);
       setError(null);
     }
     onOpenChange(open);
@@ -103,17 +103,13 @@ const ParseCommandDialog: React.FC<ParseCommandDialogProps> = ({
 
         <div className="space-y-4">
           <div>
-            <Label htmlFor="backend-type">Backend Type</Label>
-            <select
-              id="backend-type"
-              value={backendType}
-              onChange={(e) => setBackendType(e.target.value as BackendTypeValue)}
-              className="flex h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-sm ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50"
-            >
-              <option value={BackendType.LLAMA_CPP}>Llama Server</option>
-              <option value={BackendType.MLX_LM}>MLX LM</option>
-              <option value={BackendType.VLLM}>vLLM</option>
-            </select>
+            <Label className="text-sm font-medium">Backend Type:
+              <span className="font-normal text-muted-foreground">
+                {backendType === BackendType.LLAMA_CPP && 'Llama Server (llama_cpp)'}
+                {backendType === BackendType.MLX_LM && 'MLX LM (mlx_lm)'}
+                {backendType === BackendType.VLLM && 'vLLM (vllm)'}
+              </span>
+            </Label>
           </div>
 
           <div>

webui/src/components/__tests__/InstanceModal.test.tsx

@@ -280,29 +280,6 @@ afterEach(() => {
     })
   })
 
-  describe('Advanced Fields Toggle', () => {
-    it('shows advanced fields when toggle clicked', async () => {
-      const user = userEvent.setup()
-      
-      render(
-        <InstanceDialog
-          open={true}
-          onOpenChange={mockOnOpenChange}
-          onSave={mockOnSave}
-        />
-      )
-
-      // Advanced fields should be hidden initially
-      expect(screen.queryByText(/Advanced Configuration/)).toBeInTheDocument()
-      
-      // Click to expand
-      await user.click(screen.getByText(/Advanced Configuration/))
-
-      // Should show more configuration options
-      // Note: Specific fields depend on zodFormUtils configuration
-      // We're testing the toggle behavior, not specific fields
-    })
-  })
 
   describe('Form Data Handling', () => {
     it('cleans up undefined values before submission', async () => {

webui/src/components/instance/AdvancedInstanceFields.tsx

@@ -1,98 +0,0 @@
-import React from 'react'
-import type { CreateInstanceOptions } from '@/types/instance'
-import { getAdvancedFields, basicFieldsConfig } from '@/lib/zodFormUtils'
-import { getFieldType } from '@/schemas/instanceOptions'
-import TextInput from '@/components/form/TextInput'
-import NumberInput from '@/components/form/NumberInput'
-import CheckboxInput from '@/components/form/CheckboxInput'
-import ArrayInput from '@/components/form/ArrayInput'
-
-interface AdvancedInstanceFieldsProps {
-  formData: CreateInstanceOptions
-  onChange: (key: keyof CreateInstanceOptions, value: any) => void
-}
-
-const AdvancedInstanceFields: React.FC<AdvancedInstanceFieldsProps> = ({
-  formData,
-  onChange
-}) => {
-  const advancedFields = getAdvancedFields()
-
-  const renderField = (fieldKey: keyof CreateInstanceOptions) => {
-    const config = basicFieldsConfig[fieldKey as string] || { label: fieldKey }
-    const fieldType = getFieldType(fieldKey)
-
-    switch (fieldType) {
-      case 'boolean':
-        return (
-          <CheckboxInput
-            key={fieldKey}
-            id={fieldKey}
-            label={config.label}
-            value={formData[fieldKey] as boolean | undefined}
-            onChange={(value) => onChange(fieldKey, value)}
-            description={config.description}
-          />
-        )
-
-      case 'number':
-        return (
-          <NumberInput
-            key={fieldKey}
-            id={fieldKey}
-            label={config.label}
-            value={formData[fieldKey] as number | undefined}
-            onChange={(value) => onChange(fieldKey, value)}
-            placeholder={config.placeholder}
-            description={config.description}
-          />
-        )
-
-      case 'array':
-        return (
-          <ArrayInput
-            key={fieldKey}
-            id={fieldKey}
-            label={config.label}
-            value={formData[fieldKey] as string[] | undefined}
-            onChange={(value) => onChange(fieldKey, value)}
-            placeholder={config.placeholder}
-            description={config.description}
-          />
-        )
-
-      default:
-        return (
-          <TextInput
-            key={fieldKey}
-            id={fieldKey}
-            label={config.label}
-            value={formData[fieldKey] as string | number | undefined}
-            onChange={(value) => onChange(fieldKey, value)}
-            placeholder={config.placeholder}
-            description={config.description}
-          />
-        )
-    }
-  }
-
-  // Filter out restart options and backend_options (handled separately)
-  const fieldsToRender = advancedFields.filter(
-    fieldKey => !['max_restarts', 'restart_delay', 'backend_options'].includes(fieldKey as string)
-  )
-
-  if (fieldsToRender.length === 0) {
-    return null
-  }
-
-  return (
-    <div className="space-y-4">
-      <h4 className="text-md font-medium">Advanced Instance Configuration</h4>
-      {fieldsToRender
-        .sort()
-        .map(renderField)}
-    </div>
-  )
-}
-
-export default AdvancedInstanceFields

webui/src/components/instance/BackendConfigurationCard.tsx

@@ -0,0 +1,117 @@
+import React, { useState } from 'react'
+import { BackendType, type CreateInstanceOptions } from '@/types/instance'
+import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'
+import { Button } from '@/components/ui/button'
+import { Terminal, ChevronDown, ChevronRight } from 'lucide-react'
+import { getBasicBackendFields, getAdvancedBackendFields } from '@/lib/zodFormUtils'
+import BackendFormField from '@/components/BackendFormField'
+import SelectInput from '@/components/form/SelectInput'
+
+interface BackendConfigurationCardProps {
+  formData: CreateInstanceOptions
+  onBackendFieldChange: (key: string, value: unknown) => void
+  onChange: (key: keyof CreateInstanceOptions, value: unknown) => void
+  onParseCommand: () => void
+}
+
+const BackendConfigurationCard: React.FC<BackendConfigurationCardProps> = ({
+  formData,
+  onBackendFieldChange,
+  onChange,
+  onParseCommand
+}) => {
+  const [showAdvanced, setShowAdvanced] = useState(false)
+  const basicBackendFields = getBasicBackendFields(formData.backend_type)
+  const advancedBackendFields = getAdvancedBackendFields(formData.backend_type)
+
+  return (
+    <Card>
+      <CardHeader>
+        <CardTitle>Backend Configuration</CardTitle>
+      </CardHeader>
+      <CardContent className="space-y-6">
+        {/* Backend Type Selection */}
+        <SelectInput
+          id="backend_type"
+          label="Backend Type"
+          value={formData.backend_type || BackendType.LLAMA_CPP}
+          onChange={(value) => onChange('backend_type', value)}
+          options={[
+            { value: BackendType.LLAMA_CPP, label: 'Llama Server (llama_cpp)' },
+            { value: BackendType.MLX_LM, label: 'MLX LM (mlx_lm)' },
+            { value: BackendType.VLLM, label: 'vLLM (vllm)' }
+          ]}
+          description="Select the backend server type"
+        />
+
+        {/* Parse Command Section */}
+        <div className="flex flex-col gap-2">
+          <Button
+            variant="outline"
+            onClick={onParseCommand}
+            className="flex items-center gap-2 w-fit"
+          >
+            <Terminal className="h-4 w-4" />
+            Parse Command
+          </Button>
+          <p className="text-sm text-muted-foreground">
+            Import settings from your backend command
+          </p>
+        </div>
+
+        {/* Basic Backend Options */}
+        {basicBackendFields.length > 0 && (
+          <div className="space-y-4">
+            <h3 className="text-md font-medium">Basic Backend Options</h3>
+            {basicBackendFields.map((fieldKey) => (
+              <BackendFormField
+                key={fieldKey}
+                fieldKey={fieldKey}
+                value={(formData.backend_options as Record<string, unknown>)?.[fieldKey] as string | number | boolean | string[] | undefined}
+                onChange={onBackendFieldChange}
+              />
+            ))}
+          </div>
+        )}
+
+        {/* Advanced Backend Options */}
+        {advancedBackendFields.length > 0 && (
+          <div className="space-y-4">
+            <Button
+              variant="ghost"
+              onClick={() => setShowAdvanced(!showAdvanced)}
+              className="flex items-center gap-2 p-0 h-auto font-medium"
+            >
+              {showAdvanced ? (
+                <ChevronDown className="h-4 w-4" />
+              ) : (
+                <ChevronRight className="h-4 w-4" />
+              )}
+              Advanced Backend Options
+              <span className="text-muted-foreground text-sm font-normal">
+                ({advancedBackendFields.length} options)
+              </span>
+            </Button>
+
+            {showAdvanced && (
+              <div className="space-y-4 pl-6 border-l-2 border-muted">
+                {advancedBackendFields
+                  .sort()
+                  .map((fieldKey) => (
+                    <BackendFormField
+                      key={fieldKey}
+                      fieldKey={fieldKey}
+                      value={(formData.backend_options as Record<string, unknown>)?.[fieldKey] as string | number | boolean | string[] | undefined}
+                      onChange={onBackendFieldChange}
+                    />
+                  ))}
+              </div>
+            )}
+          </div>
+        )}
+      </CardContent>
+    </Card>
+  )
+}
+
+export default BackendConfigurationCard

webui/src/components/instance/InstanceSettingsCard.tsx

@@ -0,0 +1,84 @@
+import React from 'react'
+import type { CreateInstanceOptions } from '@/types/instance'
+import { Card, CardContent, CardHeader, CardTitle } from '@/components/ui/card'
+import { Label } from '@/components/ui/label'
+import { Input } from '@/components/ui/input'
+import AutoRestartConfiguration from '@/components/instance/AutoRestartConfiguration'
+import NumberInput from '@/components/form/NumberInput'
+import CheckboxInput from '@/components/form/CheckboxInput'
+
+interface InstanceSettingsCardProps {
+  instanceName: string
+  nameError: string
+  isEditing: boolean
+  formData: CreateInstanceOptions
+  onNameChange: (name: string) => void
+  onChange: (key: keyof CreateInstanceOptions, value: unknown) => void
+}
+
+const InstanceSettingsCard: React.FC<InstanceSettingsCardProps> = ({
+  instanceName,
+  nameError,
+  isEditing,
+  formData,
+  onNameChange,
+  onChange
+}) => {
+  return (
+    <Card>
+      <CardHeader>
+        <CardTitle>Instance Settings</CardTitle>
+      </CardHeader>
+      <CardContent className="space-y-6">
+        {/* Instance Name */}
+        <div className="grid gap-2">
+          <Label htmlFor="name">
+            Instance Name <span className="text-red-500">*</span>
+          </Label>
+          <Input
+            id="name"
+            value={instanceName}
+            onChange={(e) => onNameChange(e.target.value)}
+            placeholder="my-instance"
+            disabled={isEditing}
+            className={nameError ? "border-red-500" : ""}
+          />
+          {nameError && <p className="text-sm text-red-500">{nameError}</p>}
+          <p className="text-sm text-muted-foreground">
+            Unique identifier for the instance
+          </p>
+        </div>
+
+        {/* Auto Restart Configuration */}
+        <AutoRestartConfiguration
+          formData={formData}
+          onChange={onChange}
+        />
+
+        {/* Basic Instance Options */}
+        <div className="space-y-4">
+          <h3 className="text-lg font-medium">Basic Instance Options</h3>
+
+          <NumberInput
+            id="idle_timeout"
+            label="Idle Timeout (minutes)"
+            value={formData.idle_timeout}
+            onChange={(value) => onChange('idle_timeout', value)}
+            placeholder="30"
+            description="Minutes before stopping an idle instance"
+          />
+
+          <CheckboxInput
+            id="on_demand_start"
+            label="On Demand Start"
+            value={formData.on_demand_start}
+            onChange={(value) => onChange('on_demand_start', value)}
+            description="Start instance only when needed"
+          />
+        </div>
+      </CardContent>
+    </Card>
+  )
+}
+
+export default InstanceSettingsCard