Add option to return all items when querying for an empty string about autosuggest-trie HOT 9 CLOSED

Go ahead! I'll be happy to accept a PR.

from autosuggest-trie.

I just wanted to say that I'm actually content with how it went. It's my first open source contribution, at least a mental one, No problem, I also noticed that when implementing this, that it probably does too much, as you said. It's a valid point. I will continue to use my workaround in the app.

Thank you for being a considerate project maintainer! It was a pleasure

from autosuggest-trie.

@Gelio Thanks a lot for a properly submitted issue!

Your suggestion makes sense, but I'm curious to know what stops you from doing something like this?

if (query.trim() === '') {
  items = allItems;
} else {
  items = trie.getMatches(query);
}

from autosuggest-trie.

@moroshko thank you for your time! Actually, that's the workaround I have been using for now. I just thought it would be a good enhancement to this library, as people who use it for similar purposes as me would have it already implemented as a handy declarative option, instead of that workaround.

If you decide it isn't needed I will accept it and hold no grudge. I just mean to help

from autosuggest-trie.

@Gelio How about this:

createTrie(items, textKey, { blankQueryMatches: allItems })

?

Note that the API changed a bit in v2.0. I'd appreciate your feedback on the new API.

from autosuggest-trie.

from autosuggest-trie.

@moroshko I've seen the new API, looks solid, extensibility is nicely included now. I believe what you proposed with that blankQueryMatches option also is valid. Are you going to implement this yourself, or may I add this (along with necessary tests) and make a pull request?

This would be my first open source contribution, so if you would give me the opportunity I would be grateful :) no worries, I understand if you want to do it yourself

from autosuggest-trie.

I'm wondering if we should slice the array if a limit is specified when using a blank query. The same goes for sorting the array - do we sort it when using a blank query?

To my mind, it should return all the items specified when creating a trie, however, they should be sorted. Therefore it can sort the items at the beginning if the comparator function is provided.

from autosuggest-trie.

Hmm.. Now I feel that blankQueryMatches only introduces an unnecessary complexity to the surface area of this library, without adding much value in return.

It doesn't make sense to me to limit the blank query matches. Same with sorting. I think this logic should live outside of this library.

When I think more about this, I feel like this:

if (query.trim() === '') {
  items = allItems; // you can slice or limit here, it's totally up to you!
} else {
  items = trie.getMatches(query);
}

is actually the right way to use autosuggest-trie (not a workaround).

The purpose of this library is to get matches based on a query. If you don't have a query (i.e. the query is blank) you shouldn't be trying to get matches. That's my mental model. I hope it makes sense.

I'll have to pass on this one. Sorry if I wasted your time. Somehow, I hope it was a positive experience anyway.

I just want to thank you again for taking the time to raise the issue, propose and implement a solution, and having a positive attitude. Please don't let this experience to stop you from contributing to Open Source in the future.

from autosuggest-trie.