From e863d6778f267090c29800f152e4370d58fc88df Mon Sep 17 00:00:00 2001 From: Francesco Torchia Date: Tue, 9 Sep 2025 16:43:35 +0200 Subject: [PATCH] [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 --- shell/components/form/LabeledSelect.vue | 16 +-- shell/components/form/Select.vue | 23 +-- .../form/__tests__/LabeledSelect.test.ts | 133 +++++++++++++++++ .../components/form/__tests__/Select.test.ts | 134 ++++++++++++++++++ shell/utils/select.js | 24 ---- 5 files changed, 288 insertions(+), 42 deletions(-) diff --git a/shell/components/form/LabeledSelect.vue b/shell/components/form/LabeledSelect.vue index 96e0c1fe28..dd64e47b4c 100644 --- a/shell/components/form/LabeledSelect.vue +++ b/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) }} -
+
{{ getOptionLabel(option.label) }}
diff --git a/shell/components/form/__tests__/LabeledSelect.test.ts b/shell/components/form/__tests__/LabeledSelect.test.ts index bd0ef87ffe..f94e27b5b7 100644 --- a/shell/components/form/__tests__/LabeledSelect.test.ts +++ b/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); + }); + }); }); diff --git a/shell/components/form/__tests__/Select.test.ts b/shell/components/form/__tests__/Select.test.ts index e6d91c1365..4a0eb0e0d4 100644 --- a/shell/components/form/__tests__/Select.test.ts +++ b/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; @@ -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); + }); + }); }); diff --git a/shell/utils/select.js b/shell/utils/select.js index 9aa2c5a16b..ccad091a1f 100644 --- a/shell/utils/select.js +++ b/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