Always render suggestions

[thibaudcolas]

Hey hey,

I have a specific use case where I need the list of suggestions to always be rendered. I expected this to be the case if shouldRenderSuggestions always returned true, but things are a bit more complicated.

In reality, for suggestions to be rendered the criteria are:

• isFocused, and
• !isCollapsed, and
• suggestions.length > 0, and
• Result from shouldRenderSuggestions

(see https://github.com/moroshko/react-autosuggest/blob/master/src/Autosuggest.js#L177)

I think it makes perfect sense to check for "are there suggestions" and I don't know what isCollapsed is about. For isFocused however, this seems like a sensible default but not necessarily something that this component should have opinions about. react-autosuggest strikes me as rather unopinionated in other parts of its API.

I can see at least three ways to tackle this:

• Have a configuration option to define whether isFocused (and isCollapsed?) should be checked. This seems like the simplest choice because it's fully backward compatible, but not the best IMHO because shouldRenderSuggestions's name suggests that it should be responsible for this.
• Pass isFocused to shouldRenderSuggestions, and let the component's user decide whether they want to take it into account or not. Seems like the option that aligns best with the existing API. It's just an extra parameter, and its logic is somewhat similar to that of giving a reason for onSuggestionsUpdateRequested and leaving it up to the user to decide what they want to do. However this isn't backwards compatible.
• If you don't think this use case is worth supporting, then at least update shouldRenderSuggestions's documentation to make it clear that suggestions are only rendered when input field is focused.

In those three cases I'm happy to contribute a PR, but I think this is worth discussing in advance :)

[sush-g]

@ThibWeb +1 Even I was wishing to have that feature.

[moroshko]

@ThibWeb Thanks for the detailed issue!

Could you explain a bit more what do you mean by:

I need the list of suggestions to always be rendered

I assume when you focus on the field for the first time, you'd like the suggestions to appear.
Do you still want to filter suggestions as you type, or you have a static list to present to users regardless of what they type?

I modified the Basic example to always show suggestions on focus. Is that close to what you are trying to achieve?

[thibaudcolas]

Sure. By "always' I mean when the component renders the suggestions render too.

For my project, the UX thinking is that people shouldn't need to type text and filter things in order to pick a suggestion. It is relevant for us to show the full list, so we do just that and leave them the opportunity to either type a search term or scroll through the whole list and click. Autocompletion / filtering is the main means of interaction but in our case it's not the only one.

Here is a prototype where it is implemented (click on one of the buttons to display the autocomplete UI): https://springload.github.io/react-search-example/. To compare with the modified basic example: like this, except the list should be displayed regardless of whether the field is focused or not.

Thinking about it more, this "always open" seems to make the most sense combined with a modal as in my example. I don't see how this would be useful for fields that are always visible on the page, where picking a value doesn't "close" anything like the modal does.

[moroshko]

@ThibWeb I can see why in your use case it makes sense to keep the suggestions always visible to the user. However, the input field in your case feels more like a filter than a traditional autosuggest. As you pointed out, it makes sense to keep the suggestions always visible only when selecting a suggestion takes you somewhere else (in your case, it's closing the modal, but it could just be a normal link to another page like here).

From the options you listed above, I like the second suggestion (passing isFocused to shouldRenderSuggestions). The only problem with this is, that the default implementation of shouldRenderSuggestions will have to be more complex to take isFocused into account. The main use case for shouldRenderSuggestions is to control the minimal number of characters that trigger suggestions, which is 1 by default. So, if all user wants is to change it to be 2, they need to start worrying about isFocused now, which isn't great.

From implementation point of view, it will probably be easiest to just add a new prop, say alwaysRenderSuggestions={true}. But, obviously, adding a new prop to support this "edge case" isn't ideal from API point of view.

What would you do if you were the maintainer of react-autosuggest?

[thibaudcolas]

This is a very legitimate, and tough question :) I've been thinking about it but am still undecided – will get back to you on the issue.

It's hard to accommodate both more control over how things work and sane defaults for the things that 90% of the users wouldn't want to deal with. Thinking about it!

[selaux]

We also need this feature for a modal overlay on mobile. I'm not sure about how to implement this either. Our goal would be to basically set isOpen && !isCollapsed to true. We did this by encapsulating it into a prop of type function that is called at this point. Looking back, this decision was not good, since the difference between this prop (which we called shouldOpenSuggestions) and shouldRenderSuggestions is not clear. Maybe an additional boolean prop would be the better solution.

[colllin]

I'm needing this too.

What if you just always render this.props.suggestions if the input is focused?

This would make it so that the simplest use case (<Autosuggest suggestions={['a', 'static', 'array']} />) always renders that array when the input is focused.

And if the typical usage is to implement this.props.onSuggestionsUpdateRequested to filter suggestions based on the input value, then people can easily set the suggestions to an empty array when the input value is empty:

    onSuggestionsUpdateRequested = (data) => this.setState({
        suggestions: this.getSuggestions(data.value),
    });

    getSuggestions(value) {
        const inputValue = value.trim();
        if (!inputValue) return [];

        const inputRegexp = new RegExp('^'+inputValue, 'i');
        return _.filter(list, (item) => inputRegexp.test(item.name));
    }

[colllin]

Sorry, shouldRenderSuggestions={() => true} totally works. I had tried that but on the wrong <Autosuggest>.

[thibaudcolas]

Ok – my project has evolved a bit and I took some time to think about this.

I still think there is a valid but probably uncommon use case for "always render suggestions". I tried to make this work on my project with autoFocus on the input field, but this isn't enough to support the UI I want to build. For some people like @colllin, inputProps={{autoFocus: true}} and shouldRenderSuggestions={() => true} might be enough.

With this in mind, I have two propositions. They are both based on the premise that because of shouldRenderSuggestions's name, it should be responsible for this:

• Pass isFocused or similar to shouldRenderSuggestions to give as much control as possible to the library user. This is the "cleanest" solution IMHO, but projects that already use shouldRenderSuggestions will need to use those extra parameters. This is a big breaking change that should be conveyed with a major version increase, and appropriate upgrade documentation.
• Change shouldRenderSuggestions to be a function or a boolean value (shouldRenderSuggestions={true}). I don't like the dynamic nature of JS too much, but it might be the right thing to do in this specific case. Examples:
  • shouldRenderSuggestions={true} does what its name implies – suggestions are always rendered.
  • shouldRenderSuggestions={false} as well, even though this is useless
  • shouldRenderSuggestions={(value) => [...]} is the same as it is now – projects that only care about the length of value are happy, it's still slightly confusing that returning true wouldn't always render the suggestion (but we now have a way to do this).

The only issue I see with this second proposition is that counter-intuitively I don't find it as future-proof. If other use cases come up, it will be quite simple to add extra parameters to shouldRenderSuggestions. With a boolean value you don't get that flexibility. This solution also has a higher cost of implementation, but nothing off the chart. This isn't a breaking change, it could be released in a minor version.

---

Of those two propositions, I think I like the second one better – it supports the uncommon use case we're discussing, without impacting the most common ones. It requires more implementation & documentation, but does not increase the API surface as much as another config flag. It improves the semantics of the shouldRenderSuggestions name.

If we decide not to support this altogether, or go for the first proposition, the documentation of shouldRenderSuggestions should mention this caveat:

-By default, suggestions are rendered when input field isn't blank. Feel free to override this behaviour.
+When the field is focused, controls if suggestions are rendered. By default, suggestions are rendered when input field isn't blank. Feel free to override this behaviour.

[moroshko]

@ThibWeb Thanks a lot for your feedback and suggestions!

I think the first option makes the common use case of shouldRenderSuggestions much more complex. I don't believe that this is an acceptable compromise.

With the second option, I find it very strange that shouldRenderSuggestions={true} and shouldRenderSuggestions={() => true} behave very differently.

I'm starting to lean towards introducing a new prop, say modalMode={true}. It doesn't affect the common use case, and can be easily managed with proper documentation.

What do you think?

@selaux Your feedback would also be appreciated!

[selaux]

@moroshko: Here are my two cents for the suggestions

• I agree that extending shouldRenderSuggestions would be the cleanest way and I don't think that it would make the common use case much more complex (it would come down to adding isFocused &&). The big drawback that I see here is that this will require users to change their current implementations to include this, so there would be a new major version for this fringe use-case.
• The second option seems weird to me since, as you already pointed out, booleans behave differently than functions that return booleans. Only if combined with option 1 this would make sense to me and then it would not really be needed anymore.
• I would also tend to the option using a new prop. But I wouldn't call it modalMode, since it is unclear what a modal mode is, but rather something in the likes of renderSuggestionsWithoutFocus.

[moroshko]

How about alwaysRenderSuggestions instead of renderSuggestionsWithoutFocus?

I think renderSuggestionsWithoutFocus leads to more questions like:

If I set renderSuggestionsWithoutFocus={true}, what happens "with focus"?
Is there a similar renderSuggestionsWithFocus?

Any volunteers to implement this?

[thibaudcolas]

I volunteer! alwaysRenderSuggestions sounds good.

[moroshko]

Thanks @ThibWeb 👍

[thibaudcolas]

This use case should now be tackled with the work done in #155. I'd love to get more eyes onto this PR to make sure it does the right thing and is tested enough.