-
Notifications
You must be signed in to change notification settings - Fork 447
[feat] Add HuggingFace model import support #7540
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
ba3ccff
2340ae2
fbc93b1
713a723
0f1cd03
c8273c6
6eb268c
e0b727b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,13 @@ | |
| > | ||
| <!-- Step 1: Enter URL --> | ||
| <UploadModelUrlInput | ||
| v-if="currentStep === 1" | ||
| v-if="currentStep === 1 && flags.huggingfaceModelImportEnabled" | ||
| v-model="wizardData.url" | ||
| :error="uploadError" | ||
| class="flex-1" | ||
| /> | ||
| <UploadModelUrlInputCivitai | ||
| v-else-if="currentStep === 1" | ||
| v-model="wizardData.url" | ||
| :error="uploadError" | ||
| /> | ||
|
Comment on lines
6
to
16
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add consistent styling to both conditional rendering paths. The feature-flagged Apply this diff to maintain consistent layout: <UploadModelUrlInputCivitai
v-else-if="currentStep === 1"
v-model="wizardData.url"
:error="uploadError"
+ class="flex-1"
/>🤖 Prompt for AI Agents |
||
|
|
@@ -46,14 +52,17 @@ | |
| <script setup lang="ts"> | ||
| import { onMounted } from 'vue' | ||
| import { useFeatureFlags } from '@/composables/useFeatureFlags' | ||
| import UploadModelConfirmation from '@/platform/assets/components/UploadModelConfirmation.vue' | ||
| import UploadModelFooter from '@/platform/assets/components/UploadModelFooter.vue' | ||
| import UploadModelProgress from '@/platform/assets/components/UploadModelProgress.vue' | ||
| import UploadModelUrlInput from '@/platform/assets/components/UploadModelUrlInput.vue' | ||
| import UploadModelUrlInputCivitai from '@/platform/assets/components/UploadModelUrlInputCivitai.vue' | ||
| import { useModelTypes } from '@/platform/assets/composables/useModelTypes' | ||
| import { useUploadModelWizard } from '@/platform/assets/composables/useUploadModelWizard' | ||
| import { useDialogStore } from '@/stores/dialogStore' | ||
| const { flags } = useFeatureFlags() | ||
| const dialogStore = useDialogStore() | ||
| const { modelTypes, fetchModelTypes } = useModelTypes() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,29 @@ | ||
| <template> | ||
| <div class="flex items-center gap-2 p-4 font-bold"> | ||
| <img src="/assets/images/civitai.svg" class="size-4" /> | ||
| <span>{{ $t('assetBrowser.uploadModelFromCivitai') }}</span> | ||
| <img | ||
| v-if="!flags.huggingfaceModelImportEnabled" | ||
| src="/assets/images/civitai.svg" | ||
| class="size-4" | ||
| /> | ||
| <span>{{ $t(titleKey) }}</span> | ||
| <span | ||
| class="rounded-full bg-white px-1.5 py-0 text-xxs font-inter font-semibold uppercase text-black" | ||
| > | ||
| {{ $t('g.beta') }} | ||
| </span> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import { computed } from 'vue' | ||
|
|
||
| import { useFeatureFlags } from '@/composables/useFeatureFlags' | ||
|
|
||
| const { flags } = useFeatureFlags() | ||
|
|
||
| const titleKey = computed(() => { | ||
| return flags.huggingfaceModelImportEnabled | ||
| ? 'assetBrowser.uploadModelGeneric' | ||
| : 'assetBrowser.uploadModelFromCivitai' | ||
| }) | ||
| </script> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ | |
| size="md" | ||
| data-attr="upload-model-step1-continue-button" | ||
| :disabled="!canFetchMetadata || isFetchingMetadata" | ||
| @click="emit('fetchMetadata')" | ||
| :on-click="() => emit('fetchMetadata')" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial 🧩 Analysis chain🏁 Script executed: # Find IconTextButton.vue and examine its props/emits
fd -e vue IconTextButton src/components/button/
cat src/components/button/IconTextButton.vue
# Also check UploadModelFooter.vue context
cat src/platform/assets/components/UploadModelFooter.vue | head -80Repository: Comfy-Org/ComfyUI_frontend Length of output: 3799 IconTextButton uses an The Consider refactoring to: const handleFetchMetadata = () => emit('fetchMetadata')
const handleUpload = () => emit('upload')Then use 🤖 Prompt for AI Agents |
||
| > | ||
| <template #icon> | ||
| <i | ||
|
|
@@ -58,7 +58,7 @@ | |
| size="md" | ||
| data-attr="upload-model-step2-confirm-button" | ||
| :disabled="!canUploadModel || isUploading" | ||
| @click="emit('upload')" | ||
| :on-click="() => emit('upload')" | ||
| > | ||
| <template #icon> | ||
| <i | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,28 +1,74 @@ | ||
| <template> | ||
| <div class="flex flex-col gap-6 text-sm text-muted-foreground"> | ||
| <div class="flex flex-col gap-2"> | ||
| <p class="m-0"> | ||
| {{ $t('assetBrowser.uploadModelDescription1') }} | ||
| </p> | ||
| <ul class="list-disc space-y-1 pl-5 mt-0"> | ||
| <li v-html="$t('assetBrowser.uploadModelDescription2')" /> | ||
| <li v-html="$t('assetBrowser.uploadModelDescription3')" /> | ||
| </ul> | ||
| <div | ||
| class="flex flex-col justify-between h-full gap-6 text-sm text-muted-foreground" | ||
| > | ||
| <div class="flex flex-col gap-6"> | ||
| <div class="flex flex-col gap-2"> | ||
| <p class="m-0 text-white"> | ||
| {{ $t('assetBrowser.uploadModelDescription1Generic') }} | ||
| </p> | ||
| <div class="m-0"> | ||
| <p class="m-0"> | ||
| {{ $t('assetBrowser.uploadModelDescription2Generic') }} | ||
| </p> | ||
| <span class="inline-flex items-center gap-1 flex-wrap mt-2"> | ||
| <!-- eslint-disable @intlify/vue-i18n/no-raw-text --> | ||
| <span class="inline-flex items-center gap-1"> | ||
| <img | ||
| src="/assets/images/civitai.svg" | ||
| alt="Civitai" | ||
| class="w-4 h-4" | ||
| /> | ||
| <a | ||
| href="https://civitai.com" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="text-muted underline" | ||
| > | ||
| Civitai</a | ||
| ><span>,</span> | ||
| </span> | ||
| <span class="inline-flex items-center gap-1"> | ||
| <img | ||
| src="/assets/images/hf-logo.svg" | ||
| alt="Hugging Face" | ||
| class="w-4 h-4" | ||
| /> | ||
| <a | ||
| href="https://huggingface.co" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| class="text-muted underline" | ||
| > | ||
| Hugging Face | ||
| </a> | ||
| </span> | ||
| <!-- eslint-enable @intlify/vue-i18n/no-raw-text --> | ||
| </span> | ||
|
Comment on lines
+14
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick | 🔵 Trivial Consider i18n for brand name presentation. While "Civitai" and "Hugging Face" are proper nouns, the presentation structure (including punctuation and formatting) may vary across locales. The current implementation uses Consider moving this to an i18n template for better localization control: // In src/locales/en/main.json
"supportedPlatforms": "Supported platforms: <a href='https://civitai.com'>Civitai</a>, <a href='https://huggingface.co'>Hugging Face</a>"Then in template: <p v-html="$t('assetBrowser.supportedPlatforms')"></p>This approach allows locales to control the presentation format while keeping brand names intact. As per coding guidelines, all user-facing strings should use vue-i18n. |
||
| </div> | ||
| </div> | ||
|
|
||
| <div class="flex flex-col gap-2"> | ||
| <InputText | ||
| v-model="url" | ||
| autofocus | ||
| :placeholder="$t('assetBrowser.genericLinkPlaceholder')" | ||
| class="w-full bg-secondary-background border-0 p-4" | ||
| data-attr="upload-model-step1-url-input" | ||
| /> | ||
| <p v-if="error" class="text-xs text-error"> | ||
| {{ error }} | ||
| </p> | ||
| <p | ||
| v-else | ||
| class="text-white" | ||
| v-html="$t('assetBrowser.maxFileSize')" | ||
| ></p> | ||
| </div> | ||
| </div> | ||
|
|
||
| <div class="flex flex-col gap-2"> | ||
| <label class="mb-0" v-html="$t('assetBrowser.civitaiLinkLabel')"> </label> | ||
| <InputText | ||
| v-model="url" | ||
| autofocus | ||
| :placeholder="$t('assetBrowser.civitaiLinkPlaceholder')" | ||
| class="w-full bg-secondary-background border-0 p-4" | ||
| data-attr="upload-model-step1-url-input" | ||
| /> | ||
| <p v-if="error" class="text-xs text-error"> | ||
| {{ error }} | ||
| </p> | ||
| <p v-else v-html="$t('assetBrowser.civitaiLinkExample')"></p> | ||
| <div class="text-sm text-muted"> | ||
| {{ $t('assetBrowser.uploadModelHelpFooterText') }} | ||
| </div> | ||
| </div> | ||
| </template> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| <template> | ||
| <div class="flex flex-col gap-6 text-sm text-muted-foreground"> | ||
| <div class="flex flex-col gap-2"> | ||
| <p class="m-0"> | ||
| {{ $t('assetBrowser.uploadModelDescription1') }} | ||
| </p> | ||
| <ul class="list-disc space-y-1 pl-5 mt-0"> | ||
| <li v-html="$t('assetBrowser.uploadModelDescription2')" /> | ||
| <li v-html="$t('assetBrowser.uploadModelDescription3')" /> | ||
| </ul> | ||
| </div> | ||
|
|
||
| <div class="flex flex-col gap-2"> | ||
| <label class="mb-0" v-html="$t('assetBrowser.civitaiLinkLabel')"> </label> | ||
| <InputText | ||
| v-model="url" | ||
| autofocus | ||
| :placeholder="$t('assetBrowser.civitaiLinkPlaceholder')" | ||
| class="w-full bg-secondary-background border-0 p-4" | ||
| data-attr="upload-model-step1-url-input" | ||
| /> | ||
| <p v-if="error" class="text-xs text-error"> | ||
| {{ error }} | ||
| </p> | ||
| <p v-else v-html="$t('assetBrowser.civitaiLinkExample')"></p> | ||
| </div> | ||
| </div> | ||
| </template> | ||
|
|
||
| <script setup lang="ts"> | ||
| import InputText from 'primevue/inputtext' | ||
| import { computed } from 'vue' | ||
| const props = defineProps<{ | ||
| modelValue: string | ||
| error?: string | ||
| }>() | ||
| const emit = defineEmits<{ | ||
| 'update:modelValue': [value: string] | ||
| }>() | ||
| const url = computed({ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: could use defineModel here
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. on url* |
||
| get: () => props.modelValue, | ||
| set: (value: string) => emit('update:modelValue', value) | ||
| }) | ||
| </script> | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid embedded HTML in i18n strings.
Line 2308 contains embedded HTML markup in the translation string:
This pattern was previously flagged and marked as addressed (commits 6e291e6 to ae03cc9), but has reappeared in the new keys. Embedding HTML in translations breaks i18n best practices because:
Refactor to use template parameters:
Then apply the styling in the Vue component that uses this string. For example:
Or use i18n's built-in formatting to inject styled content.
🤖 Prompt for AI Agents