Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/components/ui/button/button.variants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export const buttonVariants = cva({
md: 'h-8 rounded-lg p-2 text-xs',
lg: 'h-10 rounded-lg px-4 py-2 text-sm',
icon: 'size-8',
'icon-sm': 'size-5 p-0'
'icon-sm': 'size-5 p-0',
unset: ''
}
},

Expand Down
2 changes: 1 addition & 1 deletion src/components/ui/tags-input/TagsInput.vue
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ onClickOutside(rootEl, () => {
<i
v-if="!disabled && !isEditing"
aria-hidden="true"
class="icon-[lucide--square-pen] absolute bottom-2 right-2 size-4 text-muted-foreground"
class="icon-[lucide--square-pen] absolute bottom-2 right-2 size-4 text-muted-foreground transition-opacity opacity-0 group-hover:opacity-100"
/>
</TagsInputRoot>
</template>
2 changes: 2 additions & 0 deletions src/locales/en/main.json
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@
"reportIssueTooltip": "Submit the error report to Comfy Org",
"reportSent": "Report Submitted",
"copyToClipboard": "Copy to Clipboard",
"copyAll": "Copy All",
"openNewIssue": "Open New Issue",
"showReport": "Show Report",
"imageFailedToLoad": "Image failed to load",
Expand Down Expand Up @@ -2420,6 +2421,7 @@
"selectModelPrompt": "Select a model to see its information",
"basicInfo": "Basic Info",
"displayName": "Display Name",
"editDisplayName": "Edit display name",
"fileName": "File Name",
"source": "Source",
"viewOnSource": "View on {source}",
Expand Down
7 changes: 5 additions & 2 deletions src/platform/assets/components/modelInfo/ModelInfoField.vue
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
<template>
<div class="flex flex-col gap-1 px-4 py-2 text-sm text-muted-foreground">
<span>{{ label }}</span>
<div class="flex flex-col gap-2 px-4 py-2 text-sm text-base-foreground">
<div class="flex items-center justify-between relative">
<span>{{ label }}</span>
<slot name="label-action" />
</div>
<slot />
</div>
</template>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import { mount } from '@vue/test-utils'
import { createTestingPinia } from '@pinia/testing'
import { describe, expect, it } from 'vitest'
import { describe, expect, it, vi } from 'vitest'
import { createI18n } from 'vue-i18n'

import type { AssetDisplayItem } from '@/platform/assets/composables/useAssetBrowser'

import ModelInfoPanel from './ModelInfoPanel.vue'

vi.mock('@/composables/useCopyToClipboard', () => ({
useCopyToClipboard: () => ({
copyToClipboard: vi.fn()
})
}))
Comment on lines +10 to +14
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Consider using vi.hoisted() to enable assertions on clipboard calls.

The current mock prevents runtime errors but doesn't expose copyToClipboard for test assertions. Since the PR adds click-to-copy functionality, tests should verify the behavior works correctly.

♻️ Proposed refactor to enable per-test assertions
+const mockCopyToClipboard = vi.hoisted(() => vi.fn())
+
-vi.mock('@/composables/useCopyToClipboard', () => ({
-  useCopyToClipboard: () => ({
-    copyToClipboard: vi.fn()
-  })
-}))
+vi.mock('@/composables/useCopyToClipboard', () => ({
+  useCopyToClipboard: () => ({
+    copyToClipboard: mockCopyToClipboard
+  })
+}))

Then add tests for the copy functionality:

describe('Trigger Phrases Copy', () => {
  beforeEach(() => {
    mockCopyToClipboard.mockClear()
  })

  it('copies individual trigger phrase when clicked', async () => {
    const asset = createMockAsset({
      user_metadata: { trained_words: ['trigger1', 'trigger2'] }
    })
    const wrapper = mountPanel(asset)
    // Find and click the copy button for a phrase
    // Assert: expect(mockCopyToClipboard).toHaveBeenCalledWith('trigger1')
  })

  it('copies all trigger phrases when copy-all button clicked', async () => {
    const asset = createMockAsset({
      user_metadata: { trained_words: ['trigger1', 'trigger2'] }
    })
    const wrapper = mountPanel(asset)
    // Find and click the copy-all button
    // Assert: expect(mockCopyToClipboard).toHaveBeenCalledWith('trigger1 trigger2')
  })
})

Based on learnings, using vi.hoisted() allows per-test manipulation of mock state, which is essential for verifying clipboard interactions.

Would you like me to generate complete test cases for the new copy-to-clipboard functionality?

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
vi.mock('@/composables/useCopyToClipboard', () => ({
useCopyToClipboard: () => ({
copyToClipboard: vi.fn()
})
}))
const mockCopyToClipboard = vi.hoisted(() => vi.fn())
vi.mock('@/composables/useCopyToClipboard', () => ({
useCopyToClipboard: () => ({
copyToClipboard: mockCopyToClipboard
})
}))
🤖 Prompt for AI Agents
In `@src/platform/assets/components/modelInfo/ModelInfoPanel.test.ts` around lines
10 - 14, Replace the inline mock in ModelInfoPanel.test.ts with a hoisted mock
so tests can assert clipboard calls: use vi.hoisted to create and expose a
shared mock function (e.g., mockCopyToClipboard) while still returning
useCopyToClipboard -> { copyToClipboard: mockCopyToClipboard }, then in your
tests import/reference that exposed mockCopyToClipboard, call
mockCopyToClipboard.mockClear() in beforeEach and assert expectations like
expect(mockCopyToClipboard).toHaveBeenCalledWith('trigger1') for single-phrase
clicks and expect(mockCopyToClipboard).toHaveBeenCalledWith('trigger1 trigger2')
for the copy-all button; target the useCopyToClipboard and copyToClipboard
symbols when making these changes.


const i18n = createI18n({
legacy: false,
locale: 'en',
Expand Down
84 changes: 66 additions & 18 deletions src/platform/assets/components/modelInfo/ModelInfoPanel.vue
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,29 @@
</span>
</template>
<ModelInfoField :label="t('assetBrowser.modelInfo.displayName')">
<EditableText
:model-value="displayName"
:is-editing="isEditingDisplayName"
:class="cn('break-all', !isImmutable && 'text-base-foreground')"
@dblclick="isEditingDisplayName = !isImmutable"
@edit="handleDisplayNameEdit"
@cancel="isEditingDisplayName = false"
/>
<div class="group flex justify-between">
<EditableText
:model-value="displayName"
:is-editing="isEditingDisplayName"
:class="cn('break-all text-muted-foreground flex-auto')"
@dblclick="isEditingDisplayName = !isImmutable"
@edit="handleDisplayNameEdit"
@cancel="isEditingDisplayName = false"
/>
<Button
v-if="!isImmutable && !isEditingDisplayName"
size="icon-sm"
variant="muted-textonly"
class="transition-opacity opacity-0 group-hover:opacity-100"
:aria-label="t('assetBrowser.modelInfo.editDisplayName')"
@click="isEditingDisplayName = !isImmutable"
>
<i class="icon-[lucide--square-pen] self-center size-4" />
</Button>
</div>
</ModelInfoField>
<ModelInfoField :label="t('assetBrowser.modelInfo.fileName')">
<span class="break-all">{{ asset.name }}</span>
<span class="break-all text-muted-foreground">{{ asset.name }}</span>
</ModelInfoField>
<ModelInfoField
v-if="sourceUrl"
Expand Down Expand Up @@ -51,7 +63,7 @@
</span>
</template>
<ModelInfoField :label="t('assetBrowser.modelInfo.modelType')">
<Select v-model="selectedModelType" :disabled="isImmutable">
<Select v-if="!isImmutable" v-model="selectedModelType">
<SelectTrigger class="w-full">
<SelectValue
:placeholder="t('assetBrowser.modelInfo.selectModelType')"
Expand All @@ -67,6 +79,12 @@
</SelectItem>
</SelectContent>
</Select>
<div v-else class="p-2 text-sm text-muted-foreground">
{{
modelTypes.find((o) => o.value === selectedModelType)?.name ??
t('assetBrowser.unknown')
}}
</div>
</ModelInfoField>
<ModelInfoField :label="t('assetBrowser.modelInfo.compatibleBaseModels')">
<TagsInput
Expand Down Expand Up @@ -124,14 +142,31 @@
v-if="triggerPhrases.length > 0"
:label="t('assetBrowser.modelInfo.triggerPhrases')"
>
<div class="flex flex-wrap gap-1">
<span
<template #label-action>
<Button
variant="muted-textonly"
size="icon-sm"
:title="t('g.copyAll')"
:aria-label="t('g.copyAll')"
class="p-0"
@click="copyToClipboard(triggerPhrases.join(', '))"
>
<i class="icon-[lucide--copy] size-4 min-w-4 min-h-4 opacity-60" />
</Button>
</template>
<div class="flex flex-wrap gap-1 pt-1">
<Button
v-for="phrase in triggerPhrases"
:key="phrase"
class="rounded px-2 py-0.5 text-xs"
variant="muted-textonly"
size="unset"
:title="t('g.copyToClipboard')"
class="text-pretty whitespace-normal text-left text-xs"
@click="copyToClipboard(phrase)"
>
{{ phrase }}
</span>
<i class="icon-[lucide--copy] size-4 min-w-4 min-h-4 opacity-60" />
</Button>
</div>
</ModelInfoField>
<ModelInfoField
Expand Down Expand Up @@ -170,7 +205,9 @@ import { computed, ref, useTemplateRef, watch } from 'vue'
import { useI18n } from 'vue-i18n'

import EditableText from '@/components/common/EditableText.vue'
import { useCopyToClipboard } from '@/composables/useCopyToClipboard'
import PropertiesAccordionItem from '@/components/rightSidePanel/layout/PropertiesAccordionItem.vue'
import Button from '@/components/ui/button/Button.vue'
import Select from '@/components/ui/select/Select.vue'
import SelectContent from '@/components/ui/select/SelectContent.vue'
import SelectItem from '@/components/ui/select/SelectItem.vue'
Expand Down Expand Up @@ -201,6 +238,7 @@ import { cn } from '@/utils/tailwindUtil'
import ModelInfoField from './ModelInfoField.vue'

const { t } = useI18n()
const { copyToClipboard } = useCopyToClipboard()

const descriptionTextarea = useTemplateRef<HTMLTextAreaElement>(
'descriptionTextarea'
Expand All @@ -219,6 +257,7 @@ const assetsStore = useAssetsStore()
const { modelTypes } = useModelTypes()

const pendingUpdates = ref<AssetUserMetadata>({})
const pendingModelType = ref<string | undefined>(undefined)
const isEditingDisplayName = ref(false)

const isImmutable = computed(() => asset.is_immutable ?? true)
Expand All @@ -239,10 +278,17 @@ watch(
}
)

watch(
() => asset.tags,
() => {
pendingModelType.value = undefined
}
)

const debouncedFlushMetadata = useDebounceFn(() => {
if (isImmutable.value) return
assetsStore.updateAssetMetadata(
asset.id,
asset,
{ ...(asset.user_metadata ?? {}), ...pendingUpdates.value },
cacheKey
)
Expand All @@ -267,7 +313,7 @@ const debouncedSaveModelType = useDebounceFn((newModelType: string) => {
const newTags = asset.tags
.filter((tag) => tag !== currentModelType)
.concat(newModelType)
assetsStore.updateAssetTags(asset.id, newTags, cacheKey)
assetsStore.updateAssetTags(asset, newTags, cacheKey)
}, 500)

const baseModels = computed({
Expand All @@ -288,9 +334,11 @@ const userDescription = computed({
})

const selectedModelType = computed({
get: () => getAssetModelType(asset) ?? undefined,
get: () => pendingModelType.value ?? getAssetModelType(asset) ?? undefined,
set: (value: string | undefined) => {
if (value) debouncedSaveModelType(value)
if (!value) return
pendingModelType.value = value
debouncedSaveModelType(value)
}
})
Comment on lines 336 to 343
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Remove redundant ?? undefined.

The trailing ?? undefined on line 338 is redundant. If both pendingModelType.value and getAssetModelType(asset) are nullish, the expression already evaluates to undefined.

Proposed fix
 const selectedModelType = computed({
-  get: () => pendingModelType.value ?? getAssetModelType(asset) ?? undefined,
+  get: () => pendingModelType.value ?? getAssetModelType(asset),
   set: (value: string | undefined) => {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const selectedModelType = computed({
get: () => getAssetModelType(asset) ?? undefined,
get: () => pendingModelType.value ?? getAssetModelType(asset) ?? undefined,
set: (value: string | undefined) => {
if (value) debouncedSaveModelType(value)
if (!value) return
pendingModelType.value = value
debouncedSaveModelType(value)
}
})
const selectedModelType = computed({
get: () => pendingModelType.value ?? getAssetModelType(asset),
set: (value: string | undefined) => {
if (!value) return
pendingModelType.value = value
debouncedSaveModelType(value)
}
})
🤖 Prompt for AI Agents
In `@src/platform/assets/components/modelInfo/ModelInfoPanel.vue` around lines 337
- 344, The getter for selectedModelType contains a redundant "?? undefined";
update the computed getter in selectedModelType to return pendingModelType.value
?? getAssetModelType(asset) (remove the trailing "?? undefined") so the
expression naturally yields undefined when both operands are nullish; ensure
this change is made in the computed definition that references pendingModelType,
getAssetModelType, and asset.

</script>
10 changes: 10 additions & 0 deletions src/platform/assets/schemas/assetSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ const zAssetUserMetadata = z.object({

export type AssetUserMetadata = z.infer<typeof zAssetUserMetadata>

export const tagsOperationResultSchema = z.object({
total_tags: z.array(z.string()),
added: z.array(z.string()).optional(),
removed: z.array(z.string()).optional(),
already_present: z.array(z.string()).optional(),
not_present: z.array(z.string()).optional()
})

export type TagsOperationResult = z.infer<typeof tagsOperationResultSchema>

// Legacy interface for backward compatibility (now aligned with Zod schema)
export interface ModelFolderInfo {
name: string
Expand Down
68 changes: 66 additions & 2 deletions src/platform/assets/services/assetService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import { st } from '@/i18n'
import {
assetItemSchema,
assetResponseSchema,
asyncUploadResponseSchema
asyncUploadResponseSchema,
tagsOperationResultSchema
} from '@/platform/assets/schemas/assetSchema'
import type {
AssetItem,
Expand All @@ -14,7 +15,8 @@ import type {
AssetUpdatePayload,
AsyncUploadResponse,
ModelFile,
ModelFolder
ModelFolder,
TagsOperationResult
} from '@/platform/assets/schemas/assetSchema'
import { api } from '@/scripts/api'
import { useModelToNodeStore } from '@/stores/modelToNodeStore'
Expand Down Expand Up @@ -471,6 +473,66 @@ function createAssetService() {
return await res.json()
}

/**
* Add tags to an asset
* @param id - The asset ID (UUID)
* @param tags - Tags to add
* @returns Promise<TagsOperationResult>
*/
async function addAssetTags(
id: string,
tags: string[]
): Promise<TagsOperationResult> {
const res = await api.fetchApi(`${ASSETS_ENDPOINT}/${id}/tags`, {
method: 'POST',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ tags })
})

if (!res.ok) {
throw new Error(
`Unable to add tags to asset ${id}: Server returned ${res.status}`
)
}

const result = await res.json()
const parseResult = tagsOperationResultSchema.safeParse(result)
if (!parseResult.success) {
throw fromZodError(parseResult.error)
}
return parseResult.data
}

/**
* Remove tags from an asset
* @param id - The asset ID (UUID)
* @param tags - Tags to remove
* @returns Promise<TagsOperationResult>
*/
async function removeAssetTags(
id: string,
tags: string[]
): Promise<TagsOperationResult> {
const res = await api.fetchApi(`${ASSETS_ENDPOINT}/${id}/tags`, {
method: 'DELETE',
headers: { 'Content-Type': 'application/json' },
body: JSON.stringify({ tags })
})

if (!res.ok) {
throw new Error(
`Unable to remove tags from asset ${id}: Server returned ${res.status}`
)
}

const result = await res.json()
const parseResult = tagsOperationResultSchema.safeParse(result)
if (!parseResult.success) {
throw fromZodError(parseResult.error)
}
return parseResult.data
}

/**
* Uploads an asset asynchronously using the /api/assets/download endpoint
* Returns immediately with either the asset (if already exists) or a task to track
Expand Down Expand Up @@ -546,6 +608,8 @@ function createAssetService() {
getAssetsByTag,
deleteAsset,
updateAsset,
addAssetTags,
removeAssetTags,
getAssetMetadata,
uploadAssetFromUrl,
uploadAssetFromBase64,
Expand Down
Loading