fix(kselect): fix items watcher and only update if changed [KHCP-4425] (

#750) * fix(kselect): fix items watcher and only update if changed * fix(kselect): trigger popover to redraw
Kong · Aug 9, 2022 · a80958b · a80958b
1 parent c87b91f
commit a80958b
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 17 deletions.
diff --git a/packages/KPop/KPop.vue b/packages/KPop/KPop.vue
@@ -339,6 +339,12 @@ export default {
       this.$emit('opened')
     },
 
+    updatePopper () {
+      if (this.popper && typeof this.popper.update === 'function') {
+        this.popper.update()
+      }
+    },
+
     async createInstance () {
       // destroy any previous poppers before creating new one
       this.destroy()
@@ -369,7 +375,7 @@ export default {
       })
 
       await this.$nextTick()
-      this.popper.update()
+      this.updatePopper()
     },
 
     handleClick (e) {

diff --git a/packages/KSelect/KSelect.spec.js b/packages/KSelect/KSelect.spec.js
@@ -219,13 +219,12 @@ describe('KSelect', () => {
 
   it('works in autosuggest mode', async () => {
     const onQueryChange = jest.fn()
-    const items = []
     const wrapper = mount(KSelect, {
       propsData: {
         testMode: true,
         autosuggest: true,
         loading: false,
-        items
+        items: []
       },
       listeners: {
         'query-change': onQueryChange
@@ -241,8 +240,10 @@ describe('KSelect', () => {
     await tick(wrapper.vm, 1)
     expect(wrapper.find('[data-testid="k-select-loading"]').exists()).toBe(true)
 
-    items.push({ label: 'Label 1', value: 'label1' })
-    wrapper.setProps({ loading: false })
+    const items = [{ label: 'Label 1', value: 'label1' }]
+
+    await tick(wrapper.vm, 1)
+    wrapper.setProps({ items, loading: false })
     await tick(wrapper.vm, 1)
     expect(wrapper.find('[data-testid="k-select-loading"]').exists()).toBe(false)
     expect(wrapper.find('[data-testid="k-select-item-label1"]').html()).toEqual(expect.stringContaining('Label 1'))

diff --git a/packages/KSelect/KSelect.vue b/packages/KSelect/KSelect.vue
@@ -34,6 +34,7 @@
       </div>
       <KToggle v-slot="{toggle, isToggled}">
         <KPop
+          ref="popper"
           v-bind="boundKPopAttributes"
           :on-popover-click="() => {
             toggle()
@@ -122,7 +123,7 @@
               class="k-select-list ma-0 pa-0"
             >
               <slot
-                :items="items"
+                :items="selectItems"
                 :is-open="isToggled"
                 name="items"
               >
@@ -366,7 +367,7 @@ export default {
     },
     filteredItems: function () {
       // For autosuggest, items don't need to be filtered internally
-      return this.autosuggest ? this.items : this.filterFunc({ items: this.selectItems, query: this.filterStr })
+      return this.autosuggest ? this.selectItems : this.filterFunc({ items: this.selectItems, query: this.filterStr })
     },
     placeholderText () {
       if (this.placeholder) {
@@ -393,11 +394,20 @@ export default {
   },
   watch: {
     items: {
-      handler () {
-        // items need keys
-        this.selectItems = this.items
+      handler (newValue, oldValue) {
+        // Only trigger the watcher if items actually change
+        if (JSON.stringify(newValue) === JSON.stringify(oldValue)) {
+          return
+        }
+
+        this.selectItems = JSON.parse(JSON.stringify(this.items))
 
         for (let i = 0; i < this.selectItems.length; i++) {
+          // Ensure each item has a `selected` property
+          if (!Object.hasOwn(this.selectItems[i], 'selected')) {
+            this.selectItems[i].selected = false
+          }
+
           this.selectItems[i].key = `${this.selectItems[i].label.replace(' ', '-').replace(/[^a-z0-9-_]/gi, '')}-${i}`
           if (this.selectItems[i].value === this.value || this.selectItems[i].selected) {
             this.selectItems[i].selected = true
@@ -409,10 +419,18 @@ export default {
             }
           }
 
-          if (this.selectedItem && this.selectedItem.value === this.items[i].value) {
-            this.items[i].selected = true
+          if (this.selectedItem && this.selectedItem.value === this.selectItems[i].value) {
+            this.selectItems[i].selected = true
           }
         }
+
+        // Trigger an update to the popper element to cause the popover to redraw
+        // This prevents the popover from displaying "detached" from the KSelect
+        if (this.$refs.popper && typeof this.$refs.popper.updatePopper === 'function') {
+          this.$nextTick(() => {
+            this.$refs.popper.updatePopper()
+          })
+        }
       },
       immediate: true
     },
@@ -433,11 +451,13 @@ export default {
       this.selectItems.forEach(anItem => {
         if (anItem.key === item.key) {
           anItem.selected = true
-          anItem.key += '-selected'
+          anItem.key = anItem.key.includes('-selected') ? anItem.key : `${anItem.key}-selected`
           this.selectedItem = anItem
         } else if (anItem.selected) {
-          delete anItem.selected
-          anItem.key = anItem.key.split('-selected')[0]
+          anItem.selected = false
+          anItem.key = anItem.key.replace(/-selected/gi, '')
+        } else {
+          anItem.selected = false
         }
       })
       this.filterStr = this.appearance === 'dropdown' ? '' : item.label
@@ -448,8 +468,8 @@ export default {
     },
     clearSelection () {
       this.selectItems.forEach(anItem => {
-        delete anItem.selected
-        anItem.key = anItem.key.split('-selected')[0]
+        anItem.selected = false
+        anItem.key = anItem.key.replace(/-selected/gi, '')
       })
       this.selectedItem = null
       // this 'input' event must be emitted for v-model binding to work properly