Make element-search-dialog easier to customize

[klesaulnier]

No description provided.

[sBouzols]

Why do we need to expose this prop to the owner of ElementSearchDialog ?
Maybe we could set a local state to true when onSearchTermChange is called then set it to false when elementsFound changes ?
I think It's complex if we let the component owner manage this state no ?
At least name is loading as the Autocomplete prop ?

[klesaulnier]

I think it's way better to handle "isLoading" when we make the fetch, otherwise we will have some troubles handling errors or edge cases

for "isLoading" naming, I see your point, but I think it's more clear to call it "isLoading"
Can we ask a 3rd person to decide with us ?

[souissimai]

?on which case we can have troubles or errors when using state instead of props

[sBouzols]

When testing I saw that we show the "loading..." text when input field is empty (enter a character, then remove it), that doesn't happen at the moment in gridstudy

[klesaulnier]

When testing I saw that we show the "loading..." text when input field is empty (enter a character, then remove it), that doesn't happen at the moment in gridstudy

Before, emptying the field would make the suggestions empty, now we are displaying the history
But you're right, maybe we should display them without having to way for the debounce to actually execute if the use input is empty

[sBouzols]

When testing I saw that we show the "loading..." text when input field is empty (enter a character, then remove it), that doesn't happen at the moment in gridstudy

Before, emptying the field would make the suggestions empty, now we are displaying the history But you're right, maybe we should display them without having to way for the debounce to actually execute if the use input is empty

Yex I think it could be a problem if we fetch something with an empty string you know.
I think we want to avoid this for any onSearchTermChange callback prop then maybe just don't call it in this component if searchTerm is empty ?

[sBouzols]

Suggested change

	return searchTermDisabled || searchTermDisableReason
	? searchTermDisableReason
	: searchTerm;
	return searchTermDisabled
	? searchTermDisableReason
	: searchTerm;

[sBouzols]

and add a default value for searchTermDisableReason maybe ?

[klesaulnier]

Hmm I'm not sure here, he we could have a behaviour where searchTermDisabled is "true" and searchTermDisableReason is empty

It means we would disable the component, but still display "searchTerm" in the textInput

This was the previous behaviour if I'm not wrong

[sBouzols]

Code Review OK
Test OK
Console warning check OK