Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Aug 9, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion packages/KPop/KPop.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -369,7 +375,7 @@ export default {
})

await this.$nextTick()
this.popper.update()
this.updatePopper()
},

handleClick (e) {
Expand Down
9 changes: 5 additions & 4 deletions packages/KSelect/KSelect.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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'))
Expand Down
44 changes: 32 additions & 12 deletions packages/KSelect/KSelect.vue
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
</div>
<KToggle v-slot="{toggle, isToggled}">
<KPop
ref="popper"
v-bind="boundKPopAttributes"
:on-popover-click="() => {
toggle()
Expand Down Expand Up @@ -122,7 +123,7 @@
class="k-select-list ma-0 pa-0"
>
<slot
:items="items"
:items="selectItems"
:is-open="isToggled"
name="items"
>
Expand Down Expand Up @@ -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) {
Expand All @@ -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
Expand All @@ -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
},
Expand All @@ -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
Expand All @@ -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
Expand Down