vue-simple-suggest icon indicating copy to clipboard operation
vue-simple-suggest copied to clipboard

Search is performed on initial load without any interaction (since upgrading from 1.9.5 to 1.10.1)

Open robjbrain opened this issue 5 years ago • 3 comments

I'm submitting a ...

  • [x] bug report

What is the current behavior?

I have a table with 30 rows which has a vue-simple-suggest input in one column of each row (so 30 instances). The contents of the input is preloaded and the request is assigned to :list and @select is used to fetch new selections.

This has only started occuring since upgrading from 1.9.5 to 1.10.1

:list="getResults()"
value-attribute="id"
display-attribute="name"
:value="value"
@select="onSelect"

When the table renders getResults() is called 30 times once for each component. Without any interaction with the page.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem

This is my exact component, its just a wrapper really that emits its own select event which is handled elsewhere and doesn't seem to make a difference.

<template>
    <vue-simple-suggest
        ref="input"
        :list="getResults"
        :max-suggestions="10"
        :min-length="2"
        :debounce="350"
        :filter-by-query="false"
        mode="input"
        value-attribute="id"
        display-attribute="name"
        :value="value"
        @select="onSelect"
        @focus="$event.target.select()"
        :placeholder="placeholder"
        :styles="suggestStyles"
        :controls="shortcuts"
        @blur="onBlur"
        autocomplete="no"
    >
        <div slot="suggestion-item" slot-scope="{ suggestion, query }">
            <div class="item">
                {{ suggestion.name }} ({{ suggestion.fm_id }})
                <small v-if="suggestion.deleted_at" class="text-danger">Deleted: {{ localDate(suggestion.deleted_at) }}</small>
                <br />
                <small class="text-muted">
                    {{ suggestion.type }}
                    <template v-if="suggestion.breadcrumbs">
                        | {{ suggestion.breadcrumbs.map(r => r.name).join(' > ') }}
                    </template>
                </small>
            </div>
        </div>
    </vue-simple-suggest>
</template>

<script>
    import VueSimpleSuggest from 'vue-simple-suggest';
    import {recordsApi} from "../api";

    export default {
        props: {
            value: {
                default: ''
            },
            recordTypes: {
                type: Array,
                default: () => []
            },
            placeholder: {
                type: String,
                default: 'Search for a record...'
            },
            resetOnBlur: {
                type: Boolean,
                default: false
            },
            userOwns: {
                type: Boolean,
                default: null
            }
        },
        components: {
            VueSimpleSuggest
        },
        data() {
            return {
                selectedRecord: null,
                suggestStyles: {
                    vueSimpleSuggest: "position-relative",
                    inputWrapper: "",
                    defaultInput : 'form-control',
                    suggestions: "position-absolute list-group",
                    suggestItem: "list-group-item"
                },
                shortcuts: {
                    selectionUp: [38],
                    selectionDown: [40],
                    select: [13],
                    hideList: [27],
                    autocomplete: [13]
                }
            }
        },
        methods: {
            getResults(search_term) {
                return recordsApi
                    .search({
                        search_term: search_term,
                        types: this.recordTypes.length > 0 ? this.recordTypes : null,
                        my_records: this.userOwns || null
                    })
                    .then(response => response.data.data)
                    .catch(error => this.toastFormErrors(error))
            },
            onSelect (suggestion) {
                this.selectedRecord = suggestion
                this.$emit('select', suggestion)
            },
            onBlur() {
                if (this.resetOnBlur) {
                    this.$refs.input.text = this.selectedRecord ? this.selectedRecord.name : null
                }
            },
            focus() {
                this.$refs.input.inputElement.focus()
            }
        }
    }
</script>

What is the expected behavior?

getResults() should only be called when there is some sort of interaction with the component.

How are you importing Vue-simple-suggest?

  • [* ] ES6 (import VueSimpleSuggest from 'vue-simple-suggest')

Please tell us about your environment:

  • Vue.js Version: 2.6.10
  • Vue-simple-suggest version: 1.10.1
  • Browser: Chrome 78
  • Language: ES6

robjbrain avatar Nov 14 '19 04:11 robjbrain

From what I can tell it stems from the watcher on value calling updateTextOutside

value: {
      handler(current) {
        if (typeof current !== 'string') {
          current = this.displayProperty(current)
        }
        this.updateTextOutside(current)
      },
      immediate: true
    }

It then goes through:

Line:166 updateTextOutside Line:528 this.research() Line: 539: this.getSuggestions(this.text) Line: 581: result = (await this.list(value)) || []

As i'm reading it, it looks like every time the value of the input is changed then it performs a new search and there's no way around this. But it seems perfectly valid to change the display text (or to change the value if you're using an object or something) without performing a search. Surely a search should only be performed when some sort of interaction occurs.

Here is a JS Fiddle: https://jsfiddle.net/5awsth47/

If you click the "Set value React" or "Set value Vue" buttons to set the value of the input a search will be performed ("Called Suggestion List" is logged to the console).

I made a fake promise to replicate an API call, I think its sufficient to demonstrate the behaviour.

robjbrain avatar Nov 14 '19 05:11 robjbrain

this.updateTextOutside(current) was added to the watcher in the PR #186 to fix issue #185

if research on value change is not desirable then we can set some flag in the watcher and check it in the showSuggestions to determine if we need to make research

shrpne avatar Nov 18 '19 14:11 shrpne

So nearly 4 years later I finally fixed this bug for myself with a rather hacky extend of the original.

Basically you want to add this to data()

data() {
        return {
            text: typeof this.value === 'object'
                ? this.getPropertyByAttribute(this.value, this.displayAttribute)
                : this.value,
        }
    },

Annoyingly this then leads an API call being made as soon as you click or focus on the input.

To get around this you want to extend the prepareEventHandlers() and onFocus() methods to remove calls to this.showSuggestions

Here's a full component that extends the original:

<script>
import VueSimpleSuggest from 'vue-simple-suggest';
export default {
    extends: VueSimpleSuggest,
    data() {
        return {
            text: typeof this.value === 'object'
                ? this.getPropertyByAttribute(this.value, this.displayAttribute)
                : this.value,
        }
    },
    methods: {
        prepareEventHandlers(enable) {
            const binder = this[enable ? 'on' : 'off']
            const keyEventsList = {
                //@modified by Rob
                //click: this.showSuggestions,
                keydown: this.onKeyDown,
                keyup: this.onListKeyUp
            }
            const eventsList = Object.assign({
                blur: this.onBlur,
                focus: this.onFocus,
                input: this.onInput,
            }, keyEventsList)

            for (const event in eventsList) {
                this.input[binder](event, eventsList[event])
            }

            const listenerBinder = enable ? 'addEventListener' : 'removeEventListener'

            for (const event in keyEventsList) {
                this.inputElement[listenerBinder](event, keyEventsList[event])
            }
        },
        onFocus (e) {
            this.isInFocus = true

            // Only emit, if it was a native input focus
            if (e && !this.isFalseFocus) {
                this.$emit('focus', e)
            }

            // Show list only if the item has not been clicked (isFalseFocus indicates that click was made earlier)
            if (!this.isClicking && !this.isFalseFocus) {
                //@modified by Rob
                //this.showSuggestions()
            }

            this.isFalseFocus = false
        },
    }
}
</script>

It's a horrible hack but it looks like this package is abandoned so it'll do for now.

robjbrain avatar Jul 21 '23 23:07 robjbrain