From 289cbe428e7f327a254ee91027148f808a58a4dc Mon Sep 17 00:00:00 2001
From: bymyself <cbyrne@comfy.org>
Date: Fri, 28 Feb 2025 14:27:39 -0700
Subject: [PATCH] move validation to form

---
 .../nodeModelsMetadata/ModelForm.vue          | 65 ++++++++-----
 .../nodeModelsMetadata/NodeModelsPopover.vue  | 97 ++++++-------------
 src/locales/en/main.json                      |  6 +-
 src/utils/litegraphUtil.ts                    | 50 ++++++----
 4 files changed, 100 insertions(+), 118 deletions(-)

diff --git a/src/components/graph/selectionToolbox/nodeModelsMetadata/ModelForm.vue b/src/components/graph/selectionToolbox/nodeModelsMetadata/ModelForm.vue
index 9eed9d535..e0afc7483 100644
--- a/src/components/graph/selectionToolbox/nodeModelsMetadata/ModelForm.vue
+++ b/src/components/graph/selectionToolbox/nodeModelsMetadata/ModelForm.vue
@@ -1,10 +1,9 @@
 <template>
   <Form
-    :id="isFirst ? 'node-models-form' : undefined"
-    @submit="$emit('submit', $event)"
-    :resolver="zodResolver(zModelFile)"
-    :initial-values="model"
     v-slot="$form"
+    :resolver="zodResolver(zModelFile)"
+    :initial-values="node.properties?.models?.[index]"
+    @submit="handleSubmit"
   >
     <div class="flex flex-col gap-2">
       <div class="p-2 surface-ground rounded-lg">
@@ -19,7 +18,7 @@
         <div class="flex flex-col gap-2">
           <InputGroup>
             <InputGroupAddon>
-              <i class="pi pi-file-pdf" />
+              <i class="pi pi-file" />
             </InputGroupAddon>
             <FormField v-slot="$field" name="name">
               <IftaLabel>
@@ -29,7 +28,7 @@
                   class="h-full"
                 />
                 <label :for="`model-name-${index}`">
-                  {{ $t('nodeMetadata.models.fields.name') }}
+                  {{ $t('nodeMetadata.models.fields.filename') }}
                 </label>
               </IftaLabel>
             </FormField>
@@ -82,7 +81,7 @@
             severity="danger"
             text
             size="small"
-            @click="$emit('remove')"
+            @click="emit('remove')"
           />
           <Button
             v-if="isLast"
@@ -90,25 +89,23 @@
             icon="pi pi-plus"
             text
             size="small"
-            @click="$emit('add')"
+            @click="emit('add')"
+          />
+          <Button
+            icon="pi pi-check"
+            severity="primary"
+            size="small"
+            :disabled="!$form.valid"
+            type="submit"
           />
         </div>
-        <Button
-          v-if="showSaveButton"
-          type="submit"
-          icon="pi pi-check"
-          severity="primary"
-          size="small"
-          v-tooltip="$t('nodeMetadata.models.save')"
-          form="node-models-form"
-        />
       </div>
     </div>
   </Form>
 </template>
 
 <script setup lang="ts">
-import { Form, FormField, type FormSubmitEvent } from '@primevue/forms'
+import { Form, FormField } from '@primevue/forms'
 // @ts-expect-error https://github.com/primefaces/primevue/issues/6722
 import { zodResolver } from '@primevue/forms/resolvers/zod'
 import { InputGroup } from 'primevue'
@@ -117,20 +114,40 @@ import IftaLabel from 'primevue/iftalabel'
 import InputGroupAddon from 'primevue/inputgroupaddon'
 import InputText from 'primevue/inputtext'
 import Message from 'primevue/message'
+import { useToast } from 'primevue/usetoast'
+import { computed } from 'vue'
+import { useI18n } from 'vue-i18n'
 
 import { type ModelFile, zModelFile } from '@/schemas/comfyWorkflowSchema'
+import { app } from '@/scripts/app'
 
-defineProps<{
-  model: ModelFile
+const toast = useToast()
+const { t } = useI18n()
+
+const { nodeId, index } = defineProps<{
   index: number
-  isFirst: boolean
   isLast: boolean
-  showSaveButton: boolean
+  nodeId: string | number
 }>()
 
-defineEmits<{
-  (e: 'submit', event: FormSubmitEvent): void
+const emit = defineEmits<{
   (e: 'remove'): void
   (e: 'add'): void
 }>()
+
+const node = computed(() => app.graph.getNodeById(nodeId))
+
+const handleSubmit = (event: { values: ModelFile; valid: boolean }) => {
+  if (!event.valid) return
+
+  node.value.properties ||= {}
+  node.value.properties.models ||= []
+  node.value.properties.models[index] = event.values
+
+  toast.add({
+    severity: 'success',
+    summary: t('nodeMetadata.models.success'),
+    life: 3000
+  })
+}
 </script>
diff --git a/src/components/graph/selectionToolbox/nodeModelsMetadata/NodeModelsPopover.vue b/src/components/graph/selectionToolbox/nodeModelsMetadata/NodeModelsPopover.vue
index 39aa432a9..3c365ada1 100644
--- a/src/components/graph/selectionToolbox/nodeModelsMetadata/NodeModelsPopover.vue
+++ b/src/components/graph/selectionToolbox/nodeModelsMetadata/NodeModelsPopover.vue
@@ -1,8 +1,8 @@
 <template>
-  <template v-if="selectedModelNode">
+  <template v-if="node">
     <div ref="contentRef" class="flex flex-col overflow-y-auto px-4 py-2">
-      <div class="mb-2">
-        {{ $t('nodeMetadata.models.title') }}
+      <div class="mb-2 flex items-center justify-between">
+        <span>{{ $t('nodeMetadata.models.title') }}</span>
       </div>
       <div class="flex flex-col gap-2">
         <template
@@ -12,12 +12,10 @@
           <ModelForm
             :model="model"
             :index="index"
-            :is-first="index === 0"
+            :node-id="node.id"
             :is-last="isLastModel(index)"
-            :show-save-button="shouldShowSaveButton(index)"
-            @submit="submitModel(index, $event)"
             @remove="removeModel(index)"
-            @add="nodeModels.push({ name: '', url: '', directory: '' })"
+            @add="addEmptyModel"
           />
         </template>
         <div v-if="nodeModels.length === 0" class="flex items-center">
@@ -27,17 +25,9 @@
               icon="pi pi-plus"
               text
               size="small"
-              @click="nodeModels.push({ name: '', url: '', directory: '' })"
+              @click="addEmptyModel"
             />
           </div>
-          <Button
-            type="submit"
-            icon="pi pi-check"
-            severity="primary"
-            size="small"
-            v-tooltip="$t('nodeMetadata.models.save')"
-            form="node-models-form"
-          />
         </div>
       </div>
     </div>
@@ -45,25 +35,34 @@
 </template>
 
 <script setup lang="ts">
-import type { FormSubmitEvent } from '@primevue/forms'
+import { LGraphNode } from '@comfyorg/litegraph'
 import { useResizeObserver } from '@vueuse/core'
 import Button from 'primevue/button'
-import { useToast } from 'primevue/usetoast'
-import { computed, ref } from 'vue'
-import { useI18n } from 'vue-i18n'
+import { onMounted, ref } from 'vue'
 
 import ModelForm from '@/components/graph/selectionToolbox/nodeModelsMetadata/ModelForm.vue'
 import type { ModelFile } from '@/schemas/comfyWorkflowSchema'
-import { app } from '@/scripts/app'
 import { useCanvasStore } from '@/stores/graphStore'
 import { isModelNode } from '@/utils/litegraphUtil'
 
-const { t } = useI18n()
-const toast = useToast()
 const canvasStore = useCanvasStore()
 
 const contentRef = ref<HTMLElement>()
+const nodeModels = ref<ModelFile[]>([])
+const node = ref<LGraphNode | null>(null)
+
+const isLastModel = (index: number) => index === nodeModels.value.length - 1
 const formatMaxHeight = (top: number) => `calc(100vh - ${top}px)`
+
+const addEmptyModel = () => {
+  nodeModels.value.push({ name: '', url: '', directory: '' })
+}
+const removeModel = (index: number) => {
+  nodeModels.value.splice(index, 1)
+  const models = node.value?.properties?.models as ModelFile[]
+  if (models) models.splice(index, 1)
+}
+
 useResizeObserver(contentRef, () => {
   if (contentRef.value) {
     contentRef.value.style.maxHeight = formatMaxHeight(
@@ -72,52 +71,12 @@ useResizeObserver(contentRef, () => {
   }
 })
 
-const selectedModelNode = computed(() => {
+onMounted(() => {
   const nodes = canvasStore.selectedItems.filter(isModelNode)
-  if (!nodes.length) return null
-  return app.graph.getNodeById(nodes[0].id)
+  node.value = nodes[0]
+
+  if (node.value?.properties?.models) {
+    nodeModels.value = [...(node.value.properties.models as ModelFile[])]
+  }
 })
-
-const nodeModels = ref<ModelFile[]>([
-  ...((selectedModelNode.value?.properties?.models as ModelFile[]) ?? [])
-])
-
-const isEmpty = computed(() => nodeModels.value.length === 0)
-const isLastModel = (index: number) => index === nodeModels.value.length - 1
-const shouldShowSaveButton = (index: number) =>
-  isLastModel(index) || isEmpty.value
-
-const updateNodeProperties = () => {
-  if (!selectedModelNode.value) return
-  if (!selectedModelNode.value.properties) {
-    selectedModelNode.value.properties = {}
-  }
-  selectedModelNode.value.properties.models = nodeModels.value
-}
-
-const removeModel = (index: number) => {
-  nodeModels.value.splice(index, 1)
-  updateNodeProperties()
-}
-
-const submitModel = (index: number, event: FormSubmitEvent) => {
-  if (event.valid) {
-    try {
-      nodeModels.value[index] = event.values
-      updateNodeProperties()
-      toast.add({
-        severity: 'success',
-        summary: t('nodeMetadata.models.modelUpdated'),
-        life: 3000
-      })
-    } catch (error) {
-      toast.add({
-        severity: 'error',
-        summary: t('nodeMetadata.models.modelUpdateFailed'),
-        detail: error instanceof Error ? error.message : String(error),
-        life: 3000
-      })
-    }
-  }
-}
 </script>
diff --git a/src/locales/en/main.json b/src/locales/en/main.json
index 7531ed7b7..8eda2ef71 100644
--- a/src/locales/en/main.json
+++ b/src/locales/en/main.json
@@ -83,13 +83,11 @@
   "nodeMetadata": {
     "models": {
       "title": "Edit Node Models Metadata",
-      "save": "Save Models Metadata",
       "add": "Add Model",
       "remove": "Remove Model",
-      "modelUpdated": "Model updated",
-      "modelUpdateFailed": "Invalid model metadata. Node not updated.",
+      "success": "Node models metadata updated",
       "fields": {
-        "name": "Name",
+        "filename": "Filename",
         "directory": "Directory",
         "url": "URL"
       }
diff --git a/src/utils/litegraphUtil.ts b/src/utils/litegraphUtil.ts
index aa1ce0bc1..d09e0ac96 100644
--- a/src/utils/litegraphUtil.ts
+++ b/src/utils/litegraphUtil.ts
@@ -8,11 +8,33 @@ import {
 import type { IComboWidget } from '@comfyorg/litegraph/dist/types/widgets'
 import _ from 'lodash'
 
+import { ModelFile } from '@/schemas/comfyWorkflowSchema'
+
 type ImageNode = LGraphNode & { imgs: HTMLImageElement[] | undefined }
 type VideoNode = LGraphNode & {
   videoContainer: HTMLElement | undefined
   imgs: HTMLVideoElement[] | undefined
 }
+type ModelNode = LGraphNode & {
+  properties: {
+    models?: ModelFile[]
+  }
+}
+
+const MODEL_OUTPUT_TYPES = new Set([
+  'MODEL',
+  'CLIP',
+  'VAE',
+  'CONTROL_NET',
+  'UPSCALE_MODEL',
+  'CLIP_VISION',
+  'STYLE_MODEL',
+  'GLIGEN',
+  'HOOKS',
+  'UPSCALE_MODEL',
+  'PHOTOMAKER',
+  'SAM_MODEL'
+])
 
 export function isImageNode(node: LGraphNode | undefined): node is ImageNode {
   if (!node) return false
@@ -31,6 +53,13 @@ export function isAudioNode(node: LGraphNode | undefined): boolean {
   return !!node && node.previewMediaType === 'audio'
 }
 
+export const isModelNode = (node: unknown): node is ModelNode => {
+  if (!isLGraphNode(node)) return false
+  if (node.properties?.models) return true
+  if (!node.outputs?.length) return false
+  return node.outputs.some((output) => MODEL_OUTPUT_TYPES.has(`${output.type}`))
+}
+
 export function addToComboValues(widget: IComboWidget, value: string) {
   if (!widget.options) widget.options = { values: [] }
   if (!widget.options.values) widget.options.values = []
@@ -46,27 +75,6 @@ export const isLGraphNode = (item: unknown): item is LGraphNode => {
 export const isLGraphGroup = (item: unknown): item is LGraphGroup => {
   return item instanceof LGraphGroup
 }
-
-const modelOutputTypes = new Set([
-  'MODEL',
-  'CLIP',
-  'VAE',
-  'CONTROL_NET',
-  'UPSCALE_MODEL',
-  'CLIP_VISION',
-  'STYLE_MODEL',
-  'GLIGEN',
-  'HOOKS',
-  'UPSCALE_MODEL',
-  'PHOTOMAKER',
-  'SAM_MODEL'
-])
-
-export const isModelNode = (node: LGraphNode): boolean => {
-  if (!node?.outputs?.length) return false
-  return node.outputs.some((output) => modelOutputTypes.has(`${output.type}`))
-}
-
 /**
  * Get the color option of all canvas items if they are all the same.
  * @param items - The items to get the color option of.