mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-01-30 21:09:53 +00:00
[feat] Improve UX for disabled node packs in Manager dialog (#5478)
* [feat] Improve UX for disabled node packs in Manager dialog - Hide "Update All" button when only disabled packs have updates - Add tooltip on "Update All" hover to indicate disabled nodes won't be updated - Disable version selector and show tooltip for disabled node packs - Filter updates to only show enabled packs in the update queue - Add visual indicators (opacity, cursor) for disabled pack cards - Add comprehensive test coverage for new functionality This improves the user experience by clearly indicating which packs can be updated and preventing confusion about disabled packs. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * chore: missing nodes description added * test: test code modified --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1,5 +1,11 @@
|
||||
<template>
|
||||
<Button unstyled :class="buttonStyle" :disabled="disabled" @click="onClick">
|
||||
<Button
|
||||
v-bind="$attrs"
|
||||
unstyled
|
||||
:class="buttonStyle"
|
||||
:disabled="disabled"
|
||||
@click="onClick"
|
||||
>
|
||||
<slot></slot>
|
||||
</Button>
|
||||
</template>
|
||||
@@ -20,6 +26,10 @@ interface IconButtonProps extends BaseButtonProps {
|
||||
onClick: (event: Event) => void
|
||||
}
|
||||
|
||||
defineOptions({
|
||||
inheritAttrs: false
|
||||
})
|
||||
|
||||
const {
|
||||
size = 'md',
|
||||
type = 'secondary',
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
<template>
|
||||
<Button unstyled :class="buttonStyle" :disabled="disabled" @click="onClick">
|
||||
<Button
|
||||
v-bind="$attrs"
|
||||
unstyled
|
||||
:class="buttonStyle"
|
||||
:disabled="disabled"
|
||||
@click="onClick"
|
||||
>
|
||||
<slot v-if="iconPosition !== 'right'" name="icon"></slot>
|
||||
<span>{{ label }}</span>
|
||||
<slot v-if="iconPosition === 'right'" name="icon"></slot>
|
||||
@@ -18,6 +24,10 @@ import {
|
||||
getButtonTypeClasses
|
||||
} from '@/types/buttonTypes'
|
||||
|
||||
defineOptions({
|
||||
inheritAttrs: false
|
||||
})
|
||||
|
||||
interface IconTextButtonProps extends BaseButtonProps {
|
||||
iconPosition?: 'left' | 'right'
|
||||
label: string
|
||||
|
||||
@@ -1,5 +1,11 @@
|
||||
<template>
|
||||
<Button unstyled :class="buttonStyle" :disabled="disabled" @click="onClick">
|
||||
<Button
|
||||
v-bind="$attrs"
|
||||
unstyled
|
||||
:class="buttonStyle"
|
||||
:disabled="disabled"
|
||||
@click="onClick"
|
||||
>
|
||||
<span>{{ label }}</span>
|
||||
</Button>
|
||||
</template>
|
||||
@@ -21,6 +27,10 @@ interface TextButtonProps extends BaseButtonProps {
|
||||
onClick: () => void
|
||||
}
|
||||
|
||||
defineOptions({
|
||||
inheritAttrs: false
|
||||
})
|
||||
|
||||
const {
|
||||
size = 'md',
|
||||
type = 'primary',
|
||||
|
||||
@@ -2,8 +2,8 @@
|
||||
<NoResultsPlaceholder
|
||||
class="pb-0"
|
||||
icon="pi pi-exclamation-circle"
|
||||
title="Some Nodes Are Missing"
|
||||
message="When loading the graph, the following node types were not found"
|
||||
:title="$t('loadWorkflowWarning.missingNodesTitle')"
|
||||
:message="$t('loadWorkflowWarning.missingNodesDescription')"
|
||||
/>
|
||||
<MissingCoreNodesMessage :missing-core-nodes="missingCoreNodes" />
|
||||
<ListBox
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { VueWrapper, mount } from '@vue/test-utils'
|
||||
import { createPinia } from 'pinia'
|
||||
import PrimeVue from 'primevue/config'
|
||||
import Tooltip from 'primevue/tooltip'
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { nextTick } from 'vue'
|
||||
import { createI18n } from 'vue-i18n'
|
||||
@@ -31,11 +32,14 @@ const mockInstalledPacks = {
|
||||
'installed-pack': { ver: '2.0.0' }
|
||||
}
|
||||
|
||||
const mockIsPackEnabled = vi.fn(() => true)
|
||||
|
||||
vi.mock('@/stores/comfyManagerStore', () => ({
|
||||
useComfyManagerStore: vi.fn(() => ({
|
||||
installedPacks: mockInstalledPacks,
|
||||
isPackInstalled: (id: string) =>
|
||||
!!mockInstalledPacks[id as keyof typeof mockInstalledPacks]
|
||||
!!mockInstalledPacks[id as keyof typeof mockInstalledPacks],
|
||||
isPackEnabled: mockIsPackEnabled
|
||||
}))
|
||||
}))
|
||||
|
||||
@@ -60,6 +64,7 @@ describe('PackVersionBadge', () => {
|
||||
beforeEach(() => {
|
||||
mockToggle.mockReset()
|
||||
mockHide.mockReset()
|
||||
mockIsPackEnabled.mockReturnValue(true) // Reset to default enabled state
|
||||
})
|
||||
|
||||
const mountComponent = ({
|
||||
@@ -79,6 +84,9 @@ describe('PackVersionBadge', () => {
|
||||
},
|
||||
global: {
|
||||
plugins: [PrimeVue, createPinia(), i18n],
|
||||
directives: {
|
||||
tooltip: Tooltip
|
||||
},
|
||||
stubs: {
|
||||
Popover: PopoverStub,
|
||||
PackVersionSelectorPopover: true
|
||||
@@ -229,4 +237,63 @@ describe('PackVersionBadge', () => {
|
||||
expect(mockHide).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('disabled state', () => {
|
||||
beforeEach(() => {
|
||||
mockIsPackEnabled.mockReturnValue(false) // Set all packs as disabled for these tests
|
||||
})
|
||||
|
||||
it('adds disabled styles when pack is disabled', () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const badge = wrapper.find('[role="text"]') // role changes to "text" when disabled
|
||||
expect(badge.exists()).toBe(true)
|
||||
expect(badge.classes()).toContain('cursor-not-allowed')
|
||||
expect(badge.classes()).toContain('opacity-60')
|
||||
})
|
||||
|
||||
it('does not show chevron icon when disabled', () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const chevronIcon = wrapper.find('.pi-chevron-right')
|
||||
expect(chevronIcon.exists()).toBe(false)
|
||||
})
|
||||
|
||||
it('does not show update arrow when disabled', () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const updateIcon = wrapper.find('.pi-arrow-circle-up')
|
||||
expect(updateIcon.exists()).toBe(false)
|
||||
})
|
||||
|
||||
it('does not toggle popover when clicked while disabled', async () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const badge = wrapper.find('[role="text"]') // role changes to "text" when disabled
|
||||
expect(badge.exists()).toBe(true)
|
||||
await badge.trigger('click')
|
||||
|
||||
// Since it's disabled, the popover should not be toggled
|
||||
expect(mockToggle).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('has correct tabindex when disabled', () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const badge = wrapper.find('[role="text"]') // role changes to "text" when disabled
|
||||
expect(badge.exists()).toBe(true)
|
||||
expect(badge.attributes('tabindex')).toBe('-1')
|
||||
})
|
||||
|
||||
it('does not respond to keyboard events when disabled', async () => {
|
||||
const wrapper = mountComponent()
|
||||
|
||||
const badge = wrapper.find('[role="text"]') // role changes to "text" when disabled
|
||||
expect(badge.exists()).toBe(true)
|
||||
await badge.trigger('keydown.enter')
|
||||
await badge.trigger('keydown.space')
|
||||
|
||||
expect(mockToggle).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -1,21 +1,28 @@
|
||||
<template>
|
||||
<div>
|
||||
<div
|
||||
class="inline-flex items-center gap-1 rounded-2xl text-xs cursor-pointer py-1"
|
||||
:class="{ 'bg-gray-100 dark-theme:bg-neutral-700 px-1.5': fill }"
|
||||
aria-haspopup="true"
|
||||
role="button"
|
||||
tabindex="0"
|
||||
@click="toggleVersionSelector"
|
||||
@keydown.enter="toggleVersionSelector"
|
||||
@keydown.space="toggleVersionSelector"
|
||||
v-tooltip.top="
|
||||
isDisabled ? $t('manager.enablePackToChangeVersion') : null
|
||||
"
|
||||
class="inline-flex items-center gap-1 rounded-2xl text-xs py-1"
|
||||
:class="{
|
||||
'bg-gray-100 dark-theme:bg-neutral-700 px-1.5': fill,
|
||||
'cursor-pointer': !isDisabled,
|
||||
'cursor-not-allowed opacity-60': isDisabled
|
||||
}"
|
||||
:aria-haspopup="!isDisabled"
|
||||
:role="isDisabled ? 'text' : 'button'"
|
||||
:tabindex="isDisabled ? -1 : 0"
|
||||
@click="!isDisabled && toggleVersionSelector($event)"
|
||||
@keydown.enter="!isDisabled && toggleVersionSelector($event)"
|
||||
@keydown.space="!isDisabled && toggleVersionSelector($event)"
|
||||
>
|
||||
<i
|
||||
v-if="isUpdateAvailable"
|
||||
class="pi pi-arrow-circle-up text-blue-600 text-xs"
|
||||
/>
|
||||
<span>{{ installedVersion }}</span>
|
||||
<i class="pi pi-chevron-right text-xxs" />
|
||||
<i v-if="!isDisabled" class="pi pi-chevron-right text-xxs" />
|
||||
</div>
|
||||
|
||||
<Popover
|
||||
@@ -61,6 +68,11 @@ const popoverRef = ref()
|
||||
|
||||
const managerStore = useComfyManagerStore()
|
||||
|
||||
const isInstalled = computed(() => managerStore.isPackInstalled(nodePack?.id))
|
||||
const isDisabled = computed(
|
||||
() => isInstalled.value && !managerStore.isPackEnabled(nodePack?.id)
|
||||
)
|
||||
|
||||
const installedVersion = computed(() => {
|
||||
if (!nodePack.id) return 'nightly'
|
||||
const version =
|
||||
|
||||
@@ -1,5 +1,8 @@
|
||||
<template>
|
||||
<IconTextButton
|
||||
v-tooltip.top="
|
||||
hasDisabledUpdatePacks ? $t('manager.disabledNodesWontUpdate') : null
|
||||
"
|
||||
v-bind="$attrs"
|
||||
type="transparent"
|
||||
:label="$t('manager.updateAll')"
|
||||
@@ -24,8 +27,9 @@ import type { components } from '@/types/comfyRegistryTypes'
|
||||
|
||||
type NodePack = components['schemas']['Node']
|
||||
|
||||
const { nodePacks } = defineProps<{
|
||||
const { nodePacks, hasDisabledUpdatePacks } = defineProps<{
|
||||
nodePacks: NodePack[]
|
||||
hasDisabledUpdatePacks?: boolean
|
||||
}>()
|
||||
|
||||
const isUpdating = ref<boolean>(false)
|
||||
|
||||
@@ -34,7 +34,8 @@
|
||||
/>
|
||||
<PackUpdateButton
|
||||
v-if="isUpdateAvailableTab && hasUpdateAvailable"
|
||||
:node-packs="updateAvailableNodePacks"
|
||||
:node-packs="enabledUpdateAvailableNodePacks"
|
||||
:has-disabled-update-packs="hasDisabledUpdatePacks"
|
||||
/>
|
||||
</div>
|
||||
<div class="flex mt-3 text-sm">
|
||||
@@ -103,8 +104,11 @@ const { t } = useI18n()
|
||||
const { missingNodePacks, isLoading, error } = useMissingNodes()
|
||||
|
||||
// Use the composable to get update available nodes
|
||||
const { hasUpdateAvailable, updateAvailableNodePacks } =
|
||||
useUpdateAvailableNodes()
|
||||
const {
|
||||
hasUpdateAvailable,
|
||||
enabledUpdateAvailableNodePacks,
|
||||
hasDisabledUpdatePacks
|
||||
} = useUpdateAvailableNodes()
|
||||
|
||||
const hasResults = computed(
|
||||
() => searchQuery.value?.trim() && searchResults?.length
|
||||
|
||||
@@ -44,9 +44,24 @@ export const useUpdateAvailableNodes = () => {
|
||||
return filterOutdatedPacks(installedPacks.value)
|
||||
})
|
||||
|
||||
// Check if there are any outdated packs
|
||||
// Filter only enabled outdated packs
|
||||
const enabledUpdateAvailableNodePacks = computed(() => {
|
||||
return updateAvailableNodePacks.value.filter((pack) =>
|
||||
comfyManagerStore.isPackEnabled(pack.id)
|
||||
)
|
||||
})
|
||||
|
||||
// Check if there are any enabled outdated packs
|
||||
const hasUpdateAvailable = computed(() => {
|
||||
return updateAvailableNodePacks.value.length > 0
|
||||
return enabledUpdateAvailableNodePacks.value.length > 0
|
||||
})
|
||||
|
||||
// Check if there are disabled packs with updates
|
||||
const hasDisabledUpdatePacks = computed(() => {
|
||||
return (
|
||||
updateAvailableNodePacks.value.length >
|
||||
enabledUpdateAvailableNodePacks.value.length
|
||||
)
|
||||
})
|
||||
|
||||
// Automatically fetch installed pack data when composable is used
|
||||
@@ -58,7 +73,9 @@ export const useUpdateAvailableNodes = () => {
|
||||
|
||||
return {
|
||||
updateAvailableNodePacks,
|
||||
enabledUpdateAvailableNodePacks,
|
||||
hasUpdateAvailable,
|
||||
hasDisabledUpdatePacks,
|
||||
isLoading,
|
||||
error
|
||||
}
|
||||
|
||||
@@ -193,6 +193,8 @@
|
||||
"updateSelected": "Update Selected",
|
||||
"updateAll": "Update All",
|
||||
"updatingAllPacks": "Updating all packages",
|
||||
"disabledNodesWontUpdate": "Disabled nodes will not be updated",
|
||||
"enablePackToChangeVersion": "Enable this pack to change versions",
|
||||
"license": "License",
|
||||
"nightlyVersion": "Nightly",
|
||||
"latestVersion": "Latest",
|
||||
@@ -1470,6 +1472,8 @@
|
||||
"missingModelsMessage": "When loading the graph, the following models were not found"
|
||||
},
|
||||
"loadWorkflowWarning": {
|
||||
"missingNodesTitle": "Some Nodes Are Missing",
|
||||
"missingNodesDescription": "When loading the graph, the following node types were not found.\nThis may also happen if your installed version is lower and that node type can’t be found.",
|
||||
"outdatedVersion": "Some nodes require a newer version of ComfyUI (current: {version}). Please update to use all nodes.",
|
||||
"outdatedVersionGeneric": "Some nodes require a newer version of ComfyUI. Please update to use all nodes.",
|
||||
"coreNodesFromVersion": "Requires ComfyUI {version}:"
|
||||
|
||||
@@ -63,12 +63,14 @@ describe('useUpdateAvailableNodes', () => {
|
||||
const mockStartFetchInstalled = vi.fn()
|
||||
const mockIsPackInstalled = vi.fn()
|
||||
const mockGetInstalledPackVersion = vi.fn()
|
||||
const mockIsPackEnabled = vi.fn()
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
|
||||
// Default setup
|
||||
mockIsPackInstalled.mockReturnValue(true)
|
||||
mockIsPackEnabled.mockReturnValue(true) // Default: all packs are enabled
|
||||
mockGetInstalledPackVersion.mockImplementation((id: string) => {
|
||||
switch (id) {
|
||||
case 'pack-1':
|
||||
@@ -100,7 +102,8 @@ describe('useUpdateAvailableNodes', () => {
|
||||
|
||||
mockUseComfyManagerStore.mockReturnValue({
|
||||
isPackInstalled: mockIsPackInstalled,
|
||||
getInstalledPackVersion: mockGetInstalledPackVersion
|
||||
getInstalledPackVersion: mockGetInstalledPackVersion,
|
||||
isPackEnabled: mockIsPackEnabled
|
||||
} as any)
|
||||
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
@@ -357,4 +360,127 @@ describe('useUpdateAvailableNodes', () => {
|
||||
expect(mockIsPackInstalled).toHaveBeenCalledWith('pack-4')
|
||||
})
|
||||
})
|
||||
|
||||
describe('enabledUpdateAvailableNodePacks', () => {
|
||||
it('returns only enabled packs with updates', () => {
|
||||
mockIsPackEnabled.mockImplementation((id: string) => {
|
||||
// pack-1 is disabled
|
||||
return id !== 'pack-1'
|
||||
})
|
||||
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0], mockInstalledPacks[1]]),
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { updateAvailableNodePacks, enabledUpdateAvailableNodePacks } =
|
||||
useUpdateAvailableNodes()
|
||||
|
||||
// pack-1 has updates but is disabled
|
||||
expect(updateAvailableNodePacks.value).toHaveLength(1)
|
||||
expect(updateAvailableNodePacks.value[0].id).toBe('pack-1')
|
||||
|
||||
// enabledUpdateAvailableNodePacks should be empty
|
||||
expect(enabledUpdateAvailableNodePacks.value).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('returns all packs when all are enabled', () => {
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0]]), // pack-1: outdated
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { updateAvailableNodePacks, enabledUpdateAvailableNodePacks } =
|
||||
useUpdateAvailableNodes()
|
||||
|
||||
expect(updateAvailableNodePacks.value).toHaveLength(1)
|
||||
expect(enabledUpdateAvailableNodePacks.value).toHaveLength(1)
|
||||
expect(enabledUpdateAvailableNodePacks.value[0].id).toBe('pack-1')
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasDisabledUpdatePacks', () => {
|
||||
it('returns true when there are disabled packs with updates', () => {
|
||||
mockIsPackEnabled.mockImplementation((id: string) => {
|
||||
// pack-1 is disabled
|
||||
return id !== 'pack-1'
|
||||
})
|
||||
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0]]), // pack-1: outdated
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { hasDisabledUpdatePacks } = useUpdateAvailableNodes()
|
||||
|
||||
expect(hasDisabledUpdatePacks.value).toBe(true)
|
||||
})
|
||||
|
||||
it('returns false when all packs with updates are enabled', () => {
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0]]), // pack-1: outdated
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { hasDisabledUpdatePacks } = useUpdateAvailableNodes()
|
||||
|
||||
expect(hasDisabledUpdatePacks.value).toBe(false)
|
||||
})
|
||||
|
||||
it('returns false when no packs have updates', () => {
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[1]]), // pack-2: up to date
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { hasDisabledUpdatePacks } = useUpdateAvailableNodes()
|
||||
|
||||
expect(hasDisabledUpdatePacks.value).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('hasUpdateAvailable with disabled packs', () => {
|
||||
it('returns false when only disabled packs have updates', () => {
|
||||
mockIsPackEnabled.mockReturnValue(false) // All packs disabled
|
||||
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0]]), // pack-1: outdated
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { hasUpdateAvailable } = useUpdateAvailableNodes()
|
||||
|
||||
expect(hasUpdateAvailable.value).toBe(false)
|
||||
})
|
||||
|
||||
it('returns true when at least one enabled pack has updates', () => {
|
||||
mockIsPackEnabled.mockImplementation((id: string) => {
|
||||
// Only pack-1 is enabled
|
||||
return id === 'pack-1'
|
||||
})
|
||||
|
||||
mockUseInstalledPacks.mockReturnValue({
|
||||
installedPacks: ref([mockInstalledPacks[0]]), // pack-1: outdated
|
||||
isLoading: ref(false),
|
||||
error: ref(null),
|
||||
startFetchInstalled: mockStartFetchInstalled
|
||||
} as any)
|
||||
|
||||
const { hasUpdateAvailable } = useUpdateAvailableNodes()
|
||||
|
||||
expect(hasUpdateAvailable.value).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user