[Manager] Fix version selector popover not closing when selecting different pack (#4176)

Author: christian-byrne

src/components/dialog/content/manager/PackVersionBadge.test.ts

@@ -62,6 +62,7 @@ describe('PackVersionBadge', () => {
     return mount(PackVersionBadge, {
       props: {
         nodePack: mockNodePack,
+        isSelected: false,
         ...props
       },
       global: {
@@ -162,4 +163,58 @@ describe('PackVersionBadge', () => {
     // Verify that the hide method was called
     expect(mockHide).toHaveBeenCalled()
   })
+
+  describe('selection state changes', () => {
+    it('closes the popover when card is deselected', async () => {
+      const wrapper = mountComponent({
+        props: { isSelected: true }
+      })
+
+      // Change isSelected from true to false
+      await wrapper.setProps({ isSelected: false })
+      await nextTick()
+
+      // Verify that the hide method was called
+      expect(mockHide).toHaveBeenCalled()
+    })
+
+    it('does not close the popover when card is selected', async () => {
+      const wrapper = mountComponent({
+        props: { isSelected: false }
+      })
+
+      // Change isSelected from false to true
+      await wrapper.setProps({ isSelected: true })
+      await nextTick()
+
+      // Verify that the hide method was NOT called
+      expect(mockHide).not.toHaveBeenCalled()
+    })
+
+    it('does not close the popover when isSelected remains false', async () => {
+      const wrapper = mountComponent({
+        props: { isSelected: false }
+      })
+
+      // Change isSelected from false to false (no change)
+      await wrapper.setProps({ isSelected: false })
+      await nextTick()
+
+      // Verify that the hide method was NOT called
+      expect(mockHide).not.toHaveBeenCalled()
+    })
+
+    it('does not close the popover when isSelected remains true', async () => {
+      const wrapper = mountComponent({
+        props: { isSelected: true }
+      })
+
+      // Change isSelected from true to true (no change)
+      await wrapper.setProps({ isSelected: true })
+      await nextTick()
+
+      // Verify that the hide method was NOT called
+      expect(mockHide).not.toHaveBeenCalled()
+    })
+  })
 })

src/components/dialog/content/manager/PackVersionBadge.vue

@@ -33,7 +33,7 @@
 <script setup lang="ts">
 import Button from 'primevue/button'
 import Popover from 'primevue/popover'
-import { computed, ref } from 'vue'
+import { computed, ref, watch } from 'vue'
 
 import PackVersionSelectorPopover from '@/components/dialog/content/manager/PackVersionSelectorPopover.vue'
 import { useComfyManagerStore } from '@/stores/comfyManagerStore'
@@ -43,8 +43,9 @@ import { isSemVer } from '@/utils/formatUtil'
 
 const TRUNCATED_HASH_LENGTH = 7
 
-const { nodePack } = defineProps<{
+const { nodePack, isSelected } = defineProps<{
   nodePack: components['schemas']['Node']
+  isSelected: boolean
 }>()
 
 const popoverRef = ref()
@@ -69,4 +70,14 @@ const toggleVersionSelector = (event: Event) => {
 const closeVersionSelector = () => {
   popoverRef.value.hide()
 }
+
+// If the card is unselected, automatically close the version selector popover
+watch(
+  () => isSelected,
+  (isSelected, wasSelected) => {
+    if (wasSelected && !isSelected) {
+      closeVersionSelector()
+    }
+  }
+)
 </script>

src/components/dialog/content/manager/PackVersionSelectorPopover.test.ts

@@ -191,6 +191,100 @@ describe('PackVersionSelectorPopover', () => {
     expect(mockGetPackVersions).toHaveBeenCalledWith(newNodePack.id)
   })
 
+  describe('nodePack.id changes', () => {
+    it('re-fetches versions when nodePack.id changes', async () => {
+      // Set up the mock for the initial fetch
+      mockGetPackVersions.mockResolvedValueOnce(defaultMockVersions)
+
+      const wrapper = mountComponent()
+      await waitForPromises()
+
+      // Verify initial fetch
+      expect(mockGetPackVersions).toHaveBeenCalledTimes(1)
+      expect(mockGetPackVersions).toHaveBeenCalledWith(mockNodePack.id)
+
+      // Set up the mock for the second fetch
+      const newVersions = [
+        { version: '2.0.0', createdAt: '2023-06-01' },
+        { version: '1.9.0', createdAt: '2023-05-01' }
+      ]
+      mockGetPackVersions.mockResolvedValueOnce(newVersions)
+
+      // Update the nodePack with a new ID
+      const newNodePack = {
+        ...mockNodePack,
+        id: 'different-pack',
+        name: 'Different Pack'
+      }
+      await wrapper.setProps({ nodePack: newNodePack })
+      await waitForPromises()
+
+      // Should fetch versions for the new nodePack
+      expect(mockGetPackVersions).toHaveBeenCalledTimes(2)
+      expect(mockGetPackVersions).toHaveBeenLastCalledWith(newNodePack.id)
+
+      // Check that new versions are displayed
+      const listbox = wrapper.findComponent(Listbox)
+      const options = listbox.props('options')!
+      expect(options.some((o) => o.value === '2.0.0')).toBe(true)
+      expect(options.some((o) => o.value === '1.9.0')).toBe(true)
+    })
+
+    it('does not re-fetch when nodePack changes but id remains the same', async () => {
+      // Set up the mock for the initial fetch
+      mockGetPackVersions.mockResolvedValueOnce(defaultMockVersions)
+
+      const wrapper = mountComponent()
+      await waitForPromises()
+
+      // Verify initial fetch
+      expect(mockGetPackVersions).toHaveBeenCalledTimes(1)
+
+      // Update the nodePack with same ID but different properties
+      const updatedNodePack = {
+        ...mockNodePack,
+        name: 'Updated Test Pack',
+        description: 'New description'
+      }
+      await wrapper.setProps({ nodePack: updatedNodePack })
+      await waitForPromises()
+
+      // Should NOT fetch versions again
+      expect(mockGetPackVersions).toHaveBeenCalledTimes(1)
+    })
+
+    it('maintains selected version when switching to a new pack', async () => {
+      // Set up the mock for the initial fetch
+      mockGetPackVersions.mockResolvedValueOnce(defaultMockVersions)
+
+      const wrapper = mountComponent()
+      await waitForPromises()
+
+      // Select a specific version
+      const listbox = wrapper.findComponent(Listbox)
+      await listbox.setValue('0.9.0')
+      expect(listbox.props('modelValue')).toBe('0.9.0')
+
+      // Set up the mock for the second fetch
+      mockGetPackVersions.mockResolvedValueOnce([
+        { version: '3.0.0', createdAt: '2023-07-01' },
+        { version: '0.9.0', createdAt: '2023-04-01' }
+      ])
+
+      // Update to a new pack that also has version 0.9.0
+      const newNodePack = {
+        id: 'another-pack',
+        name: 'Another Pack',
+        latest_version: { version: '3.0.0' }
+      }
+      await wrapper.setProps({ nodePack: newNodePack })
+      await waitForPromises()
+
+      // Selected version should remain the same if available
+      expect(listbox.props('modelValue')).toBe('0.9.0')
+    })
+  })
+
   describe('Unclaimed GitHub packs handling', () => {
     it('falls back to nightly when no versions exist', async () => {
       // Set up the mock to return versions

src/components/dialog/content/manager/PackVersionSelectorPopover.vue

@@ -62,7 +62,7 @@ import { whenever } from '@vueuse/core'
 import Button from 'primevue/button'
 import Listbox from 'primevue/listbox'
 import ProgressSpinner from 'primevue/progressspinner'
-import { onMounted, onUnmounted, ref } from 'vue'
+import { onMounted, ref } from 'vue'
 import { useI18n } from 'vue-i18n'
 
 import ContentDivider from '@/components/common/ContentDivider.vue'
@@ -161,9 +161,11 @@ const onNodePackChange = async () => {
 }
 
 whenever(
-  () => nodePack,
-  () => {
-    void onNodePackChange()
+  () => nodePack.id,
+  (nodePackId, oldNodePackId) => {
+    if (nodePackId !== oldNodePackId) {
+      void onNodePackChange()
+    }
   },
   { deep: true, immediate: true }
 )
@@ -182,8 +184,4 @@ const handleSubmit = async () => {
   isQueueing.value = false
   emit('submit')
 }
-
-onUnmounted(() => {
-  managerStore.installPack.clear()
-})
 </script>

src/components/dialog/content/manager/infoPanel/InfoPanel.vue

@@ -32,7 +32,7 @@
             />
           </MetadataRow>
           <MetadataRow :label="t('manager.version')">
-            <PackVersionBadge :node-pack="nodePack" />
+            <PackVersionBadge :node-pack="nodePack" :is-selected="true" />
           </MetadataRow>
         </div>
         <div class="mb-6 overflow-hidden">
@@ -118,7 +118,15 @@ const onNodePackChange = () => {
   y.value = 0
 }
 
-whenever(() => nodePack, onNodePackChange, { immediate: true, deep: true })
+whenever(
+  () => nodePack.id,
+  (nodePackId, oldNodePackId) => {
+    if (nodePackId !== oldNodePackId) {
+      onNodePackChange()
+    }
+  },
+  { immediate: true }
+)
 </script>
 <style scoped>
 .hidden-scrollbar {

src/components/dialog/content/manager/packCard/PackCard.vue

@@ -70,7 +70,10 @@
                     >
                       <i class="pi pi-arrow-circle-up text-blue-600" />
                     </div>
-                    <PackVersionBadge :node-pack="nodePack" />
+                    <PackVersionBadge
+                      :node-pack="nodePack"
+                      :is-selected="isSelected"
+                    />
                   </div>
                   <div
                     v-if="formattedLatestVersionDate"