mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-21 07:14:11 +00:00
fix: disable job asset actions when preview output is missing (#8700)
### Motivation - Prevent context-menu actions from operating on stale or non-existent assets when a completed job has no `previewOutput`, since `Inspect asset`, `Add to current workflow`, and `Download` should be inactive in that case. - Also ensure the `Inspect asset` entry is disabled when the optional inspect callback is not provided to avoid unexpected behavior. ### Description - Added an optional `disabled?: boolean` field to the `MenuEntry` type returned by `useJobMenu` and computed `hasPreviewAsset` to detect when `taskRef.previewOutput` is present. - Mark `inspect-asset`, `add-to-current`, and `download` entries as `disabled` when the preview is missing (and also when the inspect callback is missing for `inspect-asset`), and keep `delete` omitted when no preview exists. - Updated `JobContextMenu.vue` to pass `:disabled="entry.disabled"` to the rendered `Button` and to short-circuit the action emit when an entry is disabled. - Expanded `useJobMenu` unit tests to assert enabled states when a preview exists and disabled states when preview or inspect handler is missing. ### Testing - Ran `pnpm vitest src/composables/queue/useJobMenu.test.ts` and all tests passed (36 tests). - Ran `pnpm lint` and the linter reported no warnings or errors. - Ran `pnpm typecheck` (`vue-tsc --noEmit`) and type checking completed without errors. ------ [Codex Task](https://chatgpt.com/codex/tasks/task_e_69864ed56e408330b88c3e9def1b5fb5) ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8700-fix-disable-job-asset-actions-when-preview-output-is-missing-2ff6d73d365081b8b72ccadf0ae43e9d) by [Unito](https://www.unito.io)
This commit is contained in:
81
src/components/queue/job/JobContextMenu.test.ts
Normal file
81
src/components/queue/job/JobContextMenu.test.ts
Normal file
@@ -0,0 +1,81 @@
|
||||
import { mount } from '@vue/test-utils'
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import JobContextMenu from '@/components/queue/job/JobContextMenu.vue'
|
||||
import type { MenuEntry } from '@/composables/queue/useJobMenu'
|
||||
|
||||
const buttonStub = {
|
||||
props: {
|
||||
disabled: {
|
||||
type: Boolean,
|
||||
default: false
|
||||
}
|
||||
},
|
||||
template: `
|
||||
<div
|
||||
class="button-stub"
|
||||
:data-disabled="String(disabled)"
|
||||
>
|
||||
<slot />
|
||||
</div>
|
||||
`
|
||||
}
|
||||
|
||||
const createEntries = (): MenuEntry[] => [
|
||||
{ key: 'enabled', label: 'Enabled action', onClick: vi.fn() },
|
||||
{
|
||||
key: 'disabled',
|
||||
label: 'Disabled action',
|
||||
disabled: true,
|
||||
onClick: vi.fn()
|
||||
},
|
||||
{ kind: 'divider', key: 'divider-1' }
|
||||
]
|
||||
|
||||
const mountComponent = (entries: MenuEntry[]) =>
|
||||
mount(JobContextMenu, {
|
||||
props: { entries },
|
||||
global: {
|
||||
stubs: {
|
||||
Popover: {
|
||||
template: '<div class="popover-stub"><slot /></div>'
|
||||
},
|
||||
Button: buttonStub
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
describe('JobContextMenu', () => {
|
||||
it('passes disabled state to action buttons', () => {
|
||||
const wrapper = mountComponent(createEntries())
|
||||
|
||||
const buttons = wrapper.findAll('.button-stub')
|
||||
expect(buttons).toHaveLength(2)
|
||||
expect(buttons[0].attributes('data-disabled')).toBe('false')
|
||||
expect(buttons[1].attributes('data-disabled')).toBe('true')
|
||||
})
|
||||
|
||||
it('emits action for enabled entries', async () => {
|
||||
const entries = createEntries()
|
||||
const wrapper = mountComponent(entries)
|
||||
|
||||
await wrapper.findAll('.button-stub')[0].trigger('click')
|
||||
|
||||
expect(wrapper.emitted('action')).toEqual([[entries[0]]])
|
||||
})
|
||||
|
||||
it('does not emit action for disabled entries', async () => {
|
||||
const wrapper = mountComponent([
|
||||
{
|
||||
key: 'disabled',
|
||||
label: 'Disabled action',
|
||||
disabled: true,
|
||||
onClick: vi.fn()
|
||||
}
|
||||
])
|
||||
|
||||
await wrapper.get('.button-stub').trigger('click')
|
||||
|
||||
expect(wrapper.emitted('action')).toBeUndefined()
|
||||
})
|
||||
})
|
||||
@@ -26,6 +26,7 @@
|
||||
variant="textonly"
|
||||
size="sm"
|
||||
:aria-label="entry.label"
|
||||
:disabled="entry.disabled"
|
||||
@click="onEntry(entry)"
|
||||
>
|
||||
<i
|
||||
@@ -68,6 +69,7 @@ function hide() {
|
||||
}
|
||||
|
||||
function onEntry(entry: MenuEntry) {
|
||||
if (entry.kind === 'divider' || entry.disabled) return
|
||||
emit('action', entry)
|
||||
}
|
||||
|
||||
|
||||
@@ -743,6 +743,17 @@ describe('useJobMenu', () => {
|
||||
'd3',
|
||||
'delete'
|
||||
])
|
||||
|
||||
expect(
|
||||
findActionEntry(jobMenuEntries.value, 'inspect-asset')?.disabled
|
||||
).toBe(false)
|
||||
expect(
|
||||
findActionEntry(jobMenuEntries.value, 'add-to-current')?.disabled
|
||||
).toBe(false)
|
||||
expect(findActionEntry(jobMenuEntries.value, 'download')?.disabled).toBe(
|
||||
false
|
||||
)
|
||||
|
||||
const inspectEntry = findActionEntry(jobMenuEntries.value, 'inspect-asset')
|
||||
await inspectEntry?.onClick?.()
|
||||
expect(inspectSpy).toHaveBeenCalledWith(currentItem.value)
|
||||
@@ -760,6 +771,7 @@ describe('useJobMenu', () => {
|
||||
await nextTick()
|
||||
const inspectEntry = findActionEntry(jobMenuEntries.value, 'inspect-asset')
|
||||
expect(inspectEntry?.onClick).toBeUndefined()
|
||||
expect(inspectEntry?.disabled).toBe(true)
|
||||
})
|
||||
|
||||
it('omits delete asset entry when no preview exists', async () => {
|
||||
@@ -767,6 +779,15 @@ describe('useJobMenu', () => {
|
||||
setCurrentItem(createJobItem({ state: 'completed', taskRef: {} }))
|
||||
|
||||
await nextTick()
|
||||
expect(
|
||||
findActionEntry(jobMenuEntries.value, 'inspect-asset')?.disabled
|
||||
).toBe(true)
|
||||
expect(
|
||||
findActionEntry(jobMenuEntries.value, 'add-to-current')?.disabled
|
||||
).toBe(true)
|
||||
expect(findActionEntry(jobMenuEntries.value, 'download')?.disabled).toBe(
|
||||
true
|
||||
)
|
||||
expect(jobMenuEntries.value.some((entry) => entry.key === 'delete')).toBe(
|
||||
false
|
||||
)
|
||||
|
||||
@@ -29,6 +29,7 @@ export type MenuEntry =
|
||||
key: string
|
||||
label: string
|
||||
icon?: string
|
||||
disabled?: boolean
|
||||
onClick?: () => void | Promise<void>
|
||||
}
|
||||
| { kind: 'divider'; key: string }
|
||||
@@ -239,13 +240,14 @@ export function useJobMenu(
|
||||
const item = currentMenuItem()
|
||||
const state = item?.state
|
||||
if (!state) return []
|
||||
const hasDeletableAsset = !!item?.taskRef?.previewOutput
|
||||
const hasPreviewAsset = !!item?.taskRef?.previewOutput
|
||||
if (state === 'completed') {
|
||||
return [
|
||||
{
|
||||
key: 'inspect-asset',
|
||||
label: st('queue.jobMenu.inspectAsset', 'Inspect asset'),
|
||||
icon: 'icon-[lucide--zoom-in]',
|
||||
disabled: !hasPreviewAsset || !onInspectAsset,
|
||||
onClick: onInspectAsset
|
||||
? () => {
|
||||
const item = currentMenuItem()
|
||||
@@ -260,12 +262,14 @@ export function useJobMenu(
|
||||
'Add to current workflow'
|
||||
),
|
||||
icon: 'icon-[comfy--node]',
|
||||
disabled: !hasPreviewAsset,
|
||||
onClick: addOutputLoaderNode
|
||||
},
|
||||
{
|
||||
key: 'download',
|
||||
label: st('queue.jobMenu.download', 'Download'),
|
||||
icon: 'icon-[lucide--download]',
|
||||
disabled: !hasPreviewAsset,
|
||||
onClick: downloadPreviewAsset
|
||||
},
|
||||
{ kind: 'divider', key: 'd1' },
|
||||
@@ -289,7 +293,7 @@ export function useJobMenu(
|
||||
onClick: copyJobId
|
||||
},
|
||||
{ kind: 'divider', key: 'd3' },
|
||||
...(hasDeletableAsset
|
||||
...(hasPreviewAsset
|
||||
? [
|
||||
{
|
||||
key: 'delete',
|
||||
|
||||
Reference in New Issue
Block a user