Fix multi Async option issues

[w3-3w]

The first two commits of this PR fixes #1542 and #1561 respectively.
The 3rd ~ 5th commit resolves an inconsistency problem as described in commit message.

As in #1289 , clearing options after selection for multi Async has been a default behavior. I introduced a new prop clearOptionsOnSelection , which defaults to true. For those who still need to keep options on selection, set it to false and it will behave as it did in v1.0.0-rc.2.

[coveralls]

Coverage decreased (-0.2%) to 92.45% when pulling 7cb42ae on w3-3w:feature/add_option_for_multiAsync into 93c3009 on JedWatson:master.

[coveralls]

Coverage decreased (-0.3%) to 92.336% when pulling 58e6e4b on w3-3w:feature/add_option_for_multiAsync into 93c3009 on JedWatson:master.

[gazpachu]

I've tested this PR and even if I set clearOptionsOnSelection={false}, the options in the select get removed when I start a new search

[w3-3w]

@gazpachu By start a new search did you mean clicking somewhere outside the component and focusing on it again? Maybe the third commit is questionable, but I think it is better to clear options on blur, because the keyword typed for search will be cleared on blur, or else it creates inconsistency on a second focus between keyword and options.

[gazpachu]

@w3-3w the options on a multi select shouldn't be cleared ever. Only when the user clicks on the 'x' icon. ATM, in rc2 and rc3, the options are gone whenever a new search starts. The only become visible again if the search starts with the same letter as the options that were previously selected. For this reason, I had to disable the search funtionality.

I'm not sure if this is exactly the same issue you guys are talking about in this thread.

[gazpachu]

No need to click outside the select, the issue happens just by starting a new search after having added a few options on the async multi select.

[gazpachu]

@w3-3w Nevermind my previous messages. I've found the issue. It turns out that I was reloading all the options because of the input parameter was changing, so I stopped passing the input parameter to the API endpoint and now the search works fine. It doesn't remove any option.

loadOptions={input => this.loadData(filter.attributes.url, input)}

return axios.get('${API_URL}${API_PATH}${endpoint}?query=').then((response) => {

[coveralls]

Coverage increased (+0.4%) to 93.146% when pulling cd4044d on w3-3w:feature/add_option_for_multiAsync into a0e5855 on JedWatson:master.

[hsingh23]

Does this fix the complete issue here: #1514?

[w3-3w]

@hsingh23 No. This PR focuses on option display behaviors.

[amit034]

any progress?

[picolll]

Can someone merge this PR please. I'm waiting for it for a long time now.

[ndbroadbent]

This would be awesome, thanks @w3-3w! It would be great if this could be merged (but there's
a merge conflict now)

[ndbroadbent]

@w3-3w i'm using your branch, and the clearOptionsOnSelection option doesn't seem to do anything for me. The options are hidden as soon as I select one.

[ndbroadbent]

I've tried downgrading to 1.0.0-rc.2, but then there's a serious bug when using multiple Selects. They all seem to share the same options instead of loading them individually.

[begincalendar]

@w3-3w Thank you for creating this PR.

I have some constructive feedback for you:

• I don't think you should conflate two issues with this (or any PR). As far as I can tell you are trying to fix Async Multiselect, options dropdown hides every time an option selected #1542 and Select.Async list disappear when two selected #1561 (which are the same issue, so that's fine), but then you also appear to be adding functionality to clear options when the field is blurred. I think the latter part should be stripped out into another PR. What do you think?
• You appear to be doing some refactoring and bug fixing, as well as adding functionality. Related to the previous point, I think refactoring, bug fixing and feature addition should be split into separate PRs. Yes it takes more work on the part of the contributor, but it makes life easier for reviewers of PRs and also for future maintainers/contributors reviewing (master branch) squashed merge commit history. Would you agree?

[JedWatson]

Thanks for the PR @w3-3w

I believe all of the fixes you've included here have been implemented separately (and slightly differently, see the changelog) but this has been a great reference.

For what it's worth in the future, separate PRs for each feature will make it easier to merge 🙂