mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-02-07 08:30:06 +00:00
[backport cloud/1.38] fix: prevent image/video preview reset on dynamic widget addition (#8493)
Backport of #8366 to `cloud/1.38` Automatically created by backport workflow. ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8493-backport-cloud-1-38-fix-prevent-image-video-preview-reset-on-dynamic-widget-addition-2f86d73d36508192b27fc55a29d6d02e) by [Unito](https://www.unito.io) Co-authored-by: Christian Byrne <cbyrne@comfy.org> Co-authored-by: Subagent 5 <subagent@example.com> Co-authored-by: Amp <amp@ampcode.com>
This commit is contained in:
@@ -158,7 +158,15 @@ const hasMultipleVideos = computed(() => props.imageUrls.length > 1)
|
||||
// Watch for URL changes and reset state
|
||||
watch(
|
||||
() => props.imageUrls,
|
||||
(newUrls) => {
|
||||
(newUrls, oldUrls) => {
|
||||
// Only reset state if URLs actually changed (not just array reference)
|
||||
const urlsChanged =
|
||||
!oldUrls ||
|
||||
newUrls.length !== oldUrls.length ||
|
||||
newUrls.some((url, i) => url !== oldUrls[i])
|
||||
|
||||
if (!urlsChanged) return
|
||||
|
||||
// Reset current index if it's out of bounds
|
||||
if (currentIndex.value >= newUrls.length) {
|
||||
currentIndex.value = 0
|
||||
@@ -169,7 +177,7 @@ watch(
|
||||
videoError.value = false
|
||||
showLoader.value = newUrls.length > 0
|
||||
},
|
||||
{ deep: true, immediate: true }
|
||||
{ immediate: true }
|
||||
)
|
||||
|
||||
// Event handlers
|
||||
|
||||
@@ -308,4 +308,80 @@ describe('ImagePreview', () => {
|
||||
expect(imgElement.exists()).toBe(true)
|
||||
expect(imgElement.attributes('alt')).toBe('Node output 2')
|
||||
})
|
||||
|
||||
describe('URL change detection', () => {
|
||||
it('should NOT reset loading state when imageUrls prop is reassigned with identical URLs', async () => {
|
||||
vi.useFakeTimers()
|
||||
try {
|
||||
const urls = ['/api/view?filename=test.png&type=output']
|
||||
const wrapper = mountImagePreview({ imageUrls: urls })
|
||||
|
||||
// Simulate image load completing
|
||||
const img = wrapper.find('img')
|
||||
await img.trigger('load')
|
||||
await nextTick()
|
||||
|
||||
// Verify loader is hidden after load
|
||||
expect(wrapper.find('[aria-busy="true"]').exists()).toBe(false)
|
||||
|
||||
// Reassign with new array reference but same content
|
||||
await wrapper.setProps({ imageUrls: [...urls] })
|
||||
await nextTick()
|
||||
|
||||
// Advance past the 250ms delayed loader timeout
|
||||
await vi.advanceTimersByTimeAsync(300)
|
||||
await nextTick()
|
||||
|
||||
// Loading state should NOT have been reset - aria-busy should still be false
|
||||
// because the URLs are identical (just a new array reference)
|
||||
expect(wrapper.find('[aria-busy="true"]').exists()).toBe(false)
|
||||
} finally {
|
||||
vi.useRealTimers()
|
||||
}
|
||||
})
|
||||
|
||||
it('should reset loading state when imageUrls prop changes to different URLs', async () => {
|
||||
const urls = ['/api/view?filename=test.png&type=output']
|
||||
const wrapper = mountImagePreview({ imageUrls: urls })
|
||||
|
||||
// Simulate image load completing
|
||||
const img = wrapper.find('img')
|
||||
await img.trigger('load')
|
||||
await nextTick()
|
||||
|
||||
// Verify loader is hidden
|
||||
expect(wrapper.find('[aria-busy="true"]').exists()).toBe(false)
|
||||
|
||||
// Change to different URL
|
||||
await wrapper.setProps({
|
||||
imageUrls: ['/api/view?filename=different.png&type=output']
|
||||
})
|
||||
await nextTick()
|
||||
|
||||
// After 250ms timeout, loading state should be reset (aria-busy="true")
|
||||
// We can check the internal state via the Skeleton appearing
|
||||
// or wait for the timeout
|
||||
await new Promise((resolve) => setTimeout(resolve, 300))
|
||||
await nextTick()
|
||||
|
||||
expect(wrapper.find('[aria-busy="true"]').exists()).toBe(true)
|
||||
})
|
||||
|
||||
it('should handle empty to non-empty URL transitions correctly', async () => {
|
||||
const wrapper = mountImagePreview({ imageUrls: [] })
|
||||
|
||||
// No preview initially
|
||||
expect(wrapper.find('.image-preview').exists()).toBe(false)
|
||||
|
||||
// Add URLs
|
||||
await wrapper.setProps({
|
||||
imageUrls: ['/api/view?filename=test.png&type=output']
|
||||
})
|
||||
await nextTick()
|
||||
|
||||
// Preview should appear
|
||||
expect(wrapper.find('.image-preview').exists()).toBe(true)
|
||||
expect(wrapper.find('img').exists()).toBe(true)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@@ -176,7 +176,15 @@ const imageAltText = computed(() => `Node output ${currentIndex.value + 1}`)
|
||||
// Watch for URL changes and reset state
|
||||
watch(
|
||||
() => props.imageUrls,
|
||||
(newUrls) => {
|
||||
(newUrls, oldUrls) => {
|
||||
// Only reset state if URLs actually changed (not just array reference)
|
||||
const urlsChanged =
|
||||
!oldUrls ||
|
||||
newUrls.length !== oldUrls.length ||
|
||||
newUrls.some((url, i) => url !== oldUrls[i])
|
||||
|
||||
if (!urlsChanged) return
|
||||
|
||||
// Reset current index if it's out of bounds
|
||||
if (currentIndex.value >= newUrls.length) {
|
||||
currentIndex.value = 0
|
||||
@@ -188,7 +196,7 @@ watch(
|
||||
imageError.value = false
|
||||
if (newUrls.length > 0) startDelayedLoader()
|
||||
},
|
||||
{ deep: true, immediate: true }
|
||||
{ immediate: true }
|
||||
)
|
||||
|
||||
// Event handlers
|
||||
|
||||
@@ -551,6 +551,12 @@ const showAdvancedState = customRef((track, trigger) => {
|
||||
}
|
||||
})
|
||||
|
||||
const hasVideoInput = computed(() => {
|
||||
return (
|
||||
lgraphNode.value?.inputs?.some((input) => input.type === 'VIDEO') ?? false
|
||||
)
|
||||
})
|
||||
|
||||
const nodeMedia = computed(() => {
|
||||
const newOutputs = nodeOutputs.nodeOutputs[nodeOutputLocatorId.value]
|
||||
const node = lgraphNode.value
|
||||
@@ -560,13 +566,9 @@ const nodeMedia = computed(() => {
|
||||
const urls = nodeOutputs.getNodeImageUrls(node)
|
||||
if (!urls?.length) return undefined
|
||||
|
||||
// Determine media type from previewMediaType or fallback to input slot types
|
||||
// Note: Despite the field name "images", videos are also included in outputs
|
||||
// TODO: fix the backend to return videos using the videos key instead of the images key
|
||||
const hasVideoInput = node.inputs?.some((input) => input.type === 'VIDEO')
|
||||
const type =
|
||||
node.previewMediaType === 'video' ||
|
||||
(!node.previewMediaType && hasVideoInput)
|
||||
(!node.previewMediaType && hasVideoInput.value)
|
||||
? 'video'
|
||||
: 'image'
|
||||
|
||||
|
||||
Reference in New Issue
Block a user