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

initial pass at a 'no results found' implementation. #44

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mattzollinhofer
Copy link
Owner

This PR is not ready for merging as there some issues:

  1. Doesn't handle 'minMatchedCharacters'. It needs to handle initial
    typing and also backspacing

  2. Do we want an additional config for whether to show this message at
    all? Is there a class of users who will not want to show this message?

  3. What about a custom message? Do we want to provide access to the
    characters the user has already typed as part of a custom message?

Thoughts? Suggestions?

There some issues though:
1) Doesn't handle 'minMatchedCharacters'. It needs to handle initial
typing and also backspacing

2) Do we want an additional config for whether to show this message at
all? Is there a class of users who will not want to show this message?

3) What about a custom message? Do we want to provide access to the
characters the user has already typed as part of a custom message?
@darrinmn9
Copy link
Contributor

Here are my thoughts:

  1. if query does not meet minMatchingChars, I don't see a reason to show a no results found message. As the prop docs state, minMatchingChars -> Minimum matching characters in query before the typeahead list appears. If that minimum isnt met, typeahead list shouldnt appear and nor should a No results found state. "No results found" tends to imply, I attempted to find a match, but couldnt. Whereas if the query doesn't meet minMatchingChars, the component never attempted to find/show a match in the typeahead.

2/3. While it might seem convenient to provide a default string for the noResultsText prop, more likely than not, users will want their own message. Especially since the text "No results found." cannot be localized, some users might want to capitalize each word, some users may not want the . at the end, some may want to use a less vague word than results given the specific context of what the user is typing (i.e. countries, github users, etc). Why not just default the prop to an empty '' and force the user to explicitly pass the message they want (which they can localize). This approach would be consistent with the current component API that doesn't provide a default fallback for the placeholder message.

Is there a class of users who will not want to show this message?

Definitely. If noResultsInfo.length === 0 (AKA user doesn't explicitly pass a string), you don't show the message at all.

Do we want to provide access to the
characters the user has already typed as part of a custom message?

This is pretty trivial for the user to do themselves since they have access to v-model="query", I'm not sure if taking on the additional complexity internally will be much value... especially as I mentioned, imo, most users will want to override message this anyways.

@mattzollinhofer
Copy link
Owner Author

Yeah, cool. I'm on board with everything you said. Thanks for the thoughtfulness.

I started this PR a whiiiiile ago. If you're interested in using it as a starting point feel free. If you'd rather start over that's cool too of course.

@darrinmn9
Copy link
Contributor

@mattzollinhofer yea i will create a new PR (while taking inspiration from this), but lets keep this PR open till mine is ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants