Will there be issues if multiple instances are present on a page?

[scottadamsmith]

• vue-autosuggest version: 1.7.1-2

Problem description:

I was reviewing some of the code as the ensureItemVisible capability doesn't appear to be working for me. I noticed that in several places, there are attempts to use document.querySelector to a specific element from within the autocomplete. It is often using classes/ids that would be the same for multiple instances and some that are not within the consumers control.

Suggested solution:

Should these document.querySelector() be replaced with this.$el.querySelector()?

[darrenjennings]

@scottadamsmith here is a demo where everything seems to be working https://codesandbox.io/s/2olxlv9q9r

However, I see your point and this could be an issue if multiple vue-autosuggest containers were in the open state. We should target only the results container of the instance in question. The simplest fix is to assume (as the rest of the component does) that the id of the component itself (componentAttrIdAutosuggest) is unique and therefore can be used to prepend all the current querySelectors. I accept PRs and it is Hacktoberfest if you're feeling up to it!

[scottadamsmith]

Thanks for the quick response, it is appreciated!

In your example, I tried the following

• tabbed to first field, typed "f"
• tabbed to next field, typed "f"

At that point, both results were present and I was focused in the second field. But if I pressed down arrow, it started moving through the first instance items.

Interesting you mention the componentAttrIdAutosuggest. I had been actively trying to omit that since I didn't have one at my disposal and would have to generate one. I found if you set it to null, it would omit it. But after reviewing the code, I see one of the aria labeledby attribute will likely not work properly without an ID.

I can certainly update selectors to use the ID, but I would still recommend using this.$el.querySelector. In my mind it is a bit simpler and a consistent mechanism provided by Vue for searching within the instance. It also eliminates a direct reference to document, which I find makes unit testing easier. Though in this case, there would still be document references remaining for some of the event listeners.

Any concerns if I proceed with a PR moving to this.$el.querySelector()?

[darrenjennings]

I have to know the id for accessibility, as you pointed out. Only concerns would just be to confirm that this.$el is the container, not the li or any sub elements of the results. Also, refs return the node HTMLElement, if you wanted to explore using refs. Appreciate the feedback!

[scottadamsmith]

Just wanted to follow up as it has been a few days. I 100% plan to do this, it's just been a busy week. I may try to do it some night this week or perhaps over the weekend.