[2.12.3] Fix LabeledSelect and Select dropdown behavior

- fix LabeledSelect component when removing tagged elements
- remove onClickOption to avoid duplicate update:value emit from LabeledSelect.vue and Select.vue
- fix Select component when removing tagged elements

Signed-off-by: Francesco Torchia <francesco.torchia@suse.com>

shell/components/form/LabeledSelect.vue

@@ -4,7 +4,7 @@ import LabeledFormElement from '@shell/mixins/labeled-form-element';
 import { get } from '@shell/utils/object';
 import { LabeledTooltip } from '@components/LabeledTooltip';
 import VueSelectOverrides from '@shell/mixins/vue-select-overrides';
-import { onClickOption, calculatePosition } from '@shell/utils/select';
+import { calculatePosition } from '@shell/utils/select';
 import { generateRandomAlphaString } from '@shell/utils/string';
 import LabeledSelectPagination from '@shell/components/form/labeled-select-utils/labeled-select-pagination';
 import { LABEL_SELECT_NOT_OPTION_KINDS } from '@shell/types/components/labeledSelect';
@@ -169,14 +169,19 @@ export default {
   },
 
   methods: {
-    // Ensure we only focus on open, otherwise we re-open on close
-    clickSelect() {
+    clickSelect(event) {
       if (this.mode === _VIEW || this.loading === true || this.disabled === true) {
         return;
       }
 
+      // Ensure we don't toggle when clicking the clear button on multi-select
+      if (this.$attrs.multiple && event?.target.className === 'vs__deselect') {
+        return;
+      }
+
       this.isOpen = !this.isOpen;
 
+      // Ensure we only focus on open, otherwise we re-open on close
       if (this.isOpen) {
         this.focusSearch();
       }
@@ -262,10 +267,6 @@ export default {
 
     get,
 
-    onClickOption(option, event) {
-      onClickOption.call(this, option, event);
-    },
-
     dropdownShouldOpen(instance, forceOpen = false) {
       if (!this.isOpen) {
         return false;
@@ -428,7 +429,6 @@ export default {
           v-else
           class="vs__option-kind"
           :class="{ 'has-icon' : hasGroupIcon}"
-          @mousedown="(e) => onClickOption(option, e)"
         >
           {{ getOptionLabel(option) }}
           <i

shell/components/form/Select.vue

@@ -4,7 +4,7 @@ import LabeledFormElement from '@shell/mixins/labeled-form-element';
 import VueSelectOverrides from '@shell/mixins/vue-select-overrides';
 import { generateRandomAlphaString } from '@shell/utils/string';
 import { LabeledTooltip } from '@components/LabeledTooltip';
-import { onClickOption, calculatePosition } from '@shell/utils/select';
+import { calculatePosition } from '@shell/utils/select';
 import { _VIEW } from '@shell/config/query-params';
 import { useClickOutside } from '@shell/composables/useClickOutside';
 import { ref } from 'vue';
@@ -94,7 +94,11 @@ export default {
     isLangSelect: {
       type:    Boolean,
       default: false
-    }
+    },
+    loading: {
+      default: false,
+      type:    Boolean
+    },
   },
   setup() {
     const select = ref(null);
@@ -134,14 +138,19 @@ export default {
       calculatePosition(dropdownList, component, width, this.placement);
     },
 
-    // Ensure we only focus on open, otherwise we re-open on close
     clickSelect(ev) {
       if (this.mode === _VIEW || this.loading === true || this.disabled === true) {
         return;
       }
 
+      // Ensure we don't toggle when clicking the clear button on multi-select
+      if (this.$attrs.multiple && ev?.target.className === 'vs__deselect') {
+        return;
+      }
+
       this.isOpen = !this.isOpen;
 
+      // Ensure we only focus on open, otherwise we re-open on close
       if (this.isOpen) {
         this.focusSearch(ev);
       }
@@ -163,9 +172,6 @@ export default {
 
     get,
 
-    onClickOption(option, event) {
-      onClickOption.call(this, option, event);
-    },
     selectable(opt) {
       // Lets you disable options that are used
       // for headings on groups of options.
@@ -352,10 +358,7 @@ export default {
       <template
         #option="option"
       >
-        <div
-          :lang="isLangSelect ? option.value : undefined"
-          @mousedown="(e) => onClickOption(option, e)"
-        >
+        <div :lang="isLangSelect ? option.value : undefined">
           {{ getOptionLabel(option.label) }}
         </div>
       </template>

shell/components/form/__tests__/LabeledSelect.test.ts

@@ -291,4 +291,137 @@ describe('component: LabeledSelect', () => {
     expect(spyFocus).toHaveBeenCalled();
     expect(spyPreventDefault).not.toHaveBeenCalled();
   });
+
+  describe('function: clickSelect', () => {
+    it('should open dropdown when clickSelect is called and not disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  false,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(true);
+    });
+
+    it('should not open dropdown when clickSelect is called and disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: true,
+          loading:  false,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when loading is true', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  true,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when mode is _VIEW', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  false,
+          mode:     _VIEW
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not clear value if disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          multiple: true,
+          disabled: true,
+          mode:     _EDIT
+        }
+      });
+
+      const clearBtn = wrapper.find('.vs__deselect');
+
+      expect(clearBtn.exists()).toBe(true);
+
+      await clearBtn.trigger('mousedown');
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.emitted('update:value')).toBeUndefined();
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when remove button is clicked', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(LabeledSelect, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          multiple: true,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      const clearBtn = wrapper.find('.vs__deselect');
+
+      await clearBtn.trigger('mousedown');
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.emitted('update:value')).toBeUndefined();
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+  });
 });

shell/components/form/__tests__/Select.test.ts

@@ -1,6 +1,7 @@
 import { shallowMount, mount } from '@vue/test-utils';
 import { defineComponent } from 'vue';
 import Select from '@shell/components/form/Select.vue';
+import { _EDIT, _VIEW } from '@shell/config/query-params';
 
 const SelectComponent = Select as ReturnType<typeof defineComponent>;
 
@@ -100,4 +101,137 @@ describe('select.vue', () => {
     expect(spyFocus).toHaveBeenCalled();
     expect(spyPreventDefault).not.toHaveBeenCalled();
   });
+
+  describe('function: clickSelect', () => {
+    it('should open dropdown when clickSelect is called and not disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  false,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(true);
+    });
+
+    it('should not open dropdown when clickSelect is called and disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: true,
+          loading:  false,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when loading is true', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  true,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when mode is _VIEW', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          disabled: false,
+          loading:  false,
+          mode:     _VIEW
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      wrapper.vm.clickSelect();
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not clear value if disabled', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          multiple: true,
+          disabled: true,
+          mode:     _EDIT
+        }
+      });
+
+      const clearBtn = wrapper.find('.vs__deselect');
+
+      expect(clearBtn.exists()).toBe(true);
+
+      await clearBtn.trigger('mousedown');
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.emitted('update:value')).toBeUndefined();
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+
+    it('should not open dropdown when remove button is clicked', async() => {
+      const label = 'Foo';
+      const value = 'foo';
+      const wrapper = mount(Select, {
+        props: {
+          value,
+          options:  [{ label, value }],
+          multiple: true,
+          mode:     _EDIT
+        }
+      });
+
+      expect(wrapper.vm.isOpen).toBe(false);
+
+      const clearBtn = wrapper.find('.vs__deselect');
+
+      await clearBtn.trigger('mousedown');
+      await wrapper.vm.$nextTick();
+
+      expect(wrapper.emitted('update:value')).toBeUndefined();
+      expect(wrapper.vm.isOpen).toBe(false);
+    });
+  });
 });

shell/utils/select.js

@@ -1,27 +1,3 @@
-export function onClickOption(option, e) {
-  if (!this.$attrs.multiple) {
-    return;
-  }
-
-  const getValue = (opt) => (this.optionKey ? this.get(opt, this.optionKey) : this.getOptionLabel(opt));
-  const optionValue = getValue(option);
-  const value = this.value || [];
-  const optionIndex = value.findIndex((option) => getValue(option) === optionValue);
-
-  if (optionIndex < 0) {
-    return;
-  }
-
-  this.value.splice(optionIndex, 1);
-
-  this.$emit('update:value', this.value);
-  e.preventDefault();
-  e.stopPropagation();
-
-  if (this.closeOnSelect) {
-    this.$refs['select-input'].closeSearchOptions();
-  }
-}
 
 // This is a simpler positionner for the dropdown for a select control
 // We used to use popper for these, but it does not suppotr fractional pixel placements which