FluentAutoComplete: Allow the ability to trigger the search options via code.

[StevenRasmussen]

Pull Request

📖 Description

I have the need to be able to trigger the auto complete to perform the search operation in code. This refactors the code and exposes a new method called InvokeOptionsSearchAsync. Feel free to suggest a better name as I'm not completely sold on it.

✅ Checklist

General

• I have added tests for my changes.
• I have tested my changes.
• I have updated the project documentation to reflect my changes.
• I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

• I have added a new component
• I have added Unit Tests for my new component
• I have modified an existing component
• I have validated the Unit Tests for an existing component

[dvoituron]

I don't really understand why you need to trigger a search via the code. It's a user component where it's the user who should initiate the search process.

In your code, you split the InputHandlerAsync method into two parts and your unit test assumes that the first part is always executed... which might not always be the case. For example, the start of the new InvokeOptionsSearchAsync method uses ValueText which should have been assigned before it was used. This will no longer be the case.
I think this modification is very risky and could lead to undesirable behavior.

[StevenRasmussen]

@dvoituron - Thanks for taking a look. I can see why you might not understand the need to trigger this manually. The test I provided was not really indicative of the real-world example that I need it for. Here is an image of the real-world problem that I am trying to solve:

BTW - I already branched the code and have this running locally with the example above and it seems to be working without any issues.

Regarding your concern about the splitting of the method: I will look at this again, but this seemed like the best place to split it based on my use-case. The existing functionality should not be impacted at all by this change since it follows the exact same path as before. If there is any risk, it would just be by those invoking this method manually. I will get back to you on my thoughts regarding where I split the method.

[vnbaaij]

I don't think you should actually do a search if a value is selected from the first dropdown.

Filtering and/or limiting the list of options available to the AutoComplete based on that choice seems logical but displaying the whole list of possible options then immediately does not.

To me the logical step would be to set focus to the Autocomplete and then let the user initiate the interaction (as Denis already said)

[StevenRasmussen]

@vnbaaij - The combobox is in the header template of the autocomplete dropdown. I have attached a video demonstrating the desired behavior. When the user starts searching for an item, the autocomplete opens and the combobox is available. Once they change say from 'Accounts' to 'Contacts', you notice that the items are refreshed. This is when I am calling the new InvokeOptionsSearchAsync method. If this method does not exist, then the user needs to close the dropdown and either 1) Clear what they have already typed, or 2) Type some additional text, or delete a character, just to trigger the OOB search method to be invoked.

[StevenRasmussen]

@dvoituron - I thought more on your comment regarding the splitting of the InputHandlerAsync method and I still believe that the PR represents what the desired outcome should be. Your comment is 100% accurate:

your unit test assumes that the first part is always executed... which might not always be the case. For example, the start of the new InvokeOptionsSearchAsync method uses ValueText which should have been assigned before it was used. This will no longer be the case.

That being said, that is the desired outcome. IMO - the ValueText should remain decoupled from the InvokeOptionsSearchAsync method which should absolutely take into account the value of the ValueText which was set previously. The intent of the InvokeOptionsSearchAsync method is to take the current state of the text input and then perform the search operation again. If/when the ValueText is changed, whether by typing something or by updating it directly in code, then that should be taken into account by the InvokeOptionsSearchAsync method. As I mentioned earlier, there should be zero risk of affecting the current behavior since the logic is executed in the same manner as the current implementation. This simply exposes the Search method for public consumption. If you feel there is a better way to organize this code, then please explain a little more in detail and I'd be happy to update the PR.

Stepping back from my particular use-case, is there any reason either from a technical perspective or from an API perspective that this type of functionality should not be allowed? I feel like this could be loosely compared to the FluentDataGrid control when an items provider is used. The FluentDataGrid provides the RefreshDataAsync method which I think is analogous to what I'm asking for with this control. Anyway, I appreciate your consideration of this functionality, and I especially appreciate the time and effort that has been put into this amazing library!

[vnbaaij]

@vnbaaij - The combobox is in the header template of the autocomplete dropdown. I have attached a video demonstrating the desired behavior. When the user starts searching for an item, the autocomplete opens and the combobox is available. Once they change say from 'Accounts' to 'Contacts', you notice that the items are refreshed. This is when I am calling the new InvokeOptionsSearchAsync method. If this method does not exist, then the user needs to close the dropdown and either 1) Clear what they have already typed, or 2) Type some additional text, or delete a character, just to trigger the OOB search method to be invoked.

Stepping back from my particular use-case, is there any reason either from a technical perspective or from an API perspective that this type of functionality should not be allowed? I feel like this could be loosely compared to the FluentDataGrid control when an items provider is used. The FluentDataGrid provides the RefreshDataAsync method which I think is analogous to what I'm asking for with this control. Anyway, I appreciate your consideration of this functionality, and I especially appreciate the time and effort that has been put into this amazing library!

Steven (@StevenRasmussen) I combined two of your responses here to (hopefully) clarify why we are reponding as we are. (hint: it is not because we like giving you a hard time 😁. On the contrary, we welcome your contributions very much!)

The first part of your response highlights perfectly fine what your use case is. I understand why you therefore want to have the functionality change you proposed. I (still) think there are different/other ways that you could solve this UI issue but that doesn't really matter in this context..

That brings us to the second part...We need (and want) to try to guard against making every component in the library work for every situation developers come up with. It remains very hard to determine what a small change in one part of a component can mean for another part of that same component or even in another component (that might make use of it). We just don't have the bandwidth to manage all of that, in spite of all the unit tests we already have in place.

So it is not that we have technical or API blockers per se. We just need to be careful and thoughtful about the changes we do make. But as you say, the change here is quite simple. We can take this in as 'low hanging fruit' once the docs cover and explain the situation that can arise with the ValueText

[StevenRasmussen]

@vnbaaij - Thanks for your response. I can certainly appreciate your situation with trying to maintain this library with limited resources and certainly you should try to guard against trying to support every scenario and only accept changes that improve the library more broadly for everyone. I feel this is one of those types of changes... and certainly that is always up for debate ;) I have updated the comments on the new method to better explain that it uses the ValueText that may have changed in a previous operation. I'm happy to do more if requested... I'm just not quite sure what you're looking for as far as documentation of that. Thanks again for your consideration of this PR.

[StevenRasmussen]

@vnbaaij - Checking in to see if you required any additional changes on this PR. Thanks!