Blazored / Typeahead

Typeahead control for Blazor applications
https://blazored.github.io/Typeahead/
MIT License
446 stars 103 forks source link

Typeahead search text dissapearing from search box after displaying results in v 4.7.0 #288

Open pradeep-w opened 2 years ago

pradeep-w commented 2 years ago

Please see attached screenshots. This behavior came after updating to version 4.7.0

`

<BlazoredTypeahead SearchMethod="SearchCDMS" MinimumLength="1" Debounce="1000" placeholder="start typing to get results..." @bind-Value="selectedItem">

                                            <NotFoundTemplate>
                                                <div class="bg-dark">
                                                    <p>No results found. </p>
                                                </div>

                                            </NotFoundTemplate>
                                            <SelectedTemplate>
                                                <div class="bg-dark">

                                                    @if (selectedItem != null && !string.IsNullOrWhiteSpace(selectedItem.phn_id))
                                                    {
                                                        <PhnResultComponent PhnWithEncounters="@selectedItem"> </PhnResultComponent>

                                                    }
                                                </div>
                                            </SelectedTemplate>
                                            <ResultTemplate>
                                                <div class="bg-dark">

                                                    @if (context != null)
                                                    {
                                                        <PhnResultComponent PhnWithEncounters="@context"> </PhnResultComponent>

                                                    }
                                                </div>
                                            </ResultTemplate>

                                        </BlazoredTypeahead>

                                    </div>`
pradeep-w commented 2 years ago

4 6 0 behaviour 4 7 0 behaviour

chrissainty commented 2 years ago

Have you seen the discussion here?

Others have reported similar issues but it's always turned out to be a fix in their code base.

Kazbek commented 2 years ago

@chrissainty I have same issue after updating Blazored.Modal from 6.0.1 to 7.0.0 right now. Typeahead was 4.7.0 for long time and worked good.

BeyondDefinition commented 2 years ago

I can confirm this issue occurs together with Blazored.Modal 7.0.0. I am unable to type text in the Typeahead box as the characters I type/paste are immediately removed.

  • I've brought my modal code down to a minimum (typeahead box only, static list of results), which does not fix the issue.
  • The dropdown function with the arrow down button does work.
  • It works fine when using Typeahead outside modal dialogs.
  • Reverting Blazored.Modal back to 6.0.1 fixes the issue inside modal dialogs.

~Later I will attempt to set up a minimal reproduction sample.~

Edit: I cannot reproduce it in a minimal reproduction sample. I will follow up by rechecking the (updated?) package setup requirements and further stripping down my solution. I also suspect it has something to do with the solution template initially started with .net5 and I might have missed an upgrade step.

SSchulze1989 commented 2 years ago

Can confirm I see the same issue with Blazored.Modal 7.0.0 and Typeahead 4.7.0. Though this is my first use of Typeahead and I might be doing something wrong.

I don't know if I started from net5.0 template anymore but I am going to check the.

I did some more digging. Compared my app to the net6.0 blazor Server app template and found no significant differences besides what I added for my app. Tried with a new minimal reproduceable example but just like @BeyondDefinition could not reproduce the issue.

Will try to copy my code into a clean repo and see if it works then...

SSchulze1989 commented 2 years ago

I could actually narrow it down to the BlazoredModalIstance being passed to the Modal content as [CascadingParameter] Trying this minimal example I could reproduce the issue

<h3>ModalWithTypeahead</h3>

<BlazoredTypeahead @bind-Value=PersonSelected SearchMethod=SearchPeople placholder="Search by name...">
    <SelectedTemplate Context=person>
        @person?.Firstname @person?.Lastname
    </SelectedTemplate>
    <ResultTemplate Context=person>
        @person.Firstname @person.Lastname
    </ResultTemplate>
</BlazoredTypeahead>

<BlazoredTypeahead @bind-Values=PeopleSelected SearchMethod=SearchPeople placeholder="Search by name ...">
    <SelectedTemplate Context=person>
        @person.Firstname @person.Lastname
    </SelectedTemplate>
    <ResultTemplate Context=person>
        @person.Firstname @person.Lastname
    </ResultTemplate>
</BlazoredTypeahead>

@code {
    [CascadingParameter]
    public BlazoredModalInstance? ModalInstance { get; set; }
    private IEnumerable<Person> PeopleAvaialable { get; set; } = Array.Empty<Person>();
    private Person? PersonSelected { get; set; }
    private IList<Person> PeopleSelected { get; set; } = new List<Person>();

    protected override void OnInitialized()
    {
        var people = new List<Person>();
        for (int i=0; i<10; i++)
        {
            people.Add(new() { Firstname = "Person", Lastname = $"Number{i + 1}" });
        }
        PeopleAvaialable = people;
    }

    private Task<IEnumerable<Person>> SearchPeople(string searchString)
    {
        return Task.FromResult(PeopleAvaialable
            .Where(x => x.Firstname.Contains(searchString) || x.Lastname.Contains(searchString)));
    }

    private class Person
    {
        public string Firstname { get; set; } = string.Empty;
        public string Lastname { get; set; } = string.Empty;
    }
}

If you remove

    [CascadingParameter]
    public BlazoredModalInstance? ModalInstance { get; set; }

Typeahead performs as expected.

Edit: Going back to Typeahead 4.6.0 solves this issue entirely as well as going back to Modal 6.0.1

SSchulze1989 commented 2 years ago

Looking at the changes from #217 the problem is rather obvious:

protected override void OnParametersSet()
{
    Initialize();
}

private void Initialize()
{
    SearchText = "";
    IsShowingSuggestions = false;
    IsShowingMask = Value != null;
}

The component Initialize() Method is called everytime in OnParameterSet() but this method can fire multiple times, even when the component is active. It seems that including

    [CascadingParameter]
    public BlazoredModalInstance? ModalInstance { get; set; }

is creating such a condition, where the parameter will bubble down and the OnParametersSet() Method is called after every input.

I think the necessity of the Initialize call should be looked at and it needs to be prevented to fire repeatedly after the component has been initialized already.

BeyondDefinition commented 2 years ago

Thanks @SSchulze1989 for your insights.

I'd like to mention that with Blazored.Modal v7.0.0 the OnParametersSet in Typeahead is probably (indirectly) triggered after every input because the setter of [CascadingParameter] ModalInstance is called every time you type a letter, which was not the case with v6.0.1. The setter is even triggered for changes on a regular InputText or input, with or without value binding.

Is there a way to identify why that happens? The call stack in the setter is kinda useless. Step out doesn't work. I tried setting various breakpoints in Blazored.Modal but that didn't get me closer to the answer.

If preferred I can report this issue separately on Blazored.Modal.

SSchulze1989 commented 2 years ago

Good question. I don't know if that is the desired behaviour on Blazored.Modal side or not.

But in any case a component should be able to handle multiple calls to OnParametersSet() gracefully without affecting current user input as there are countless other situations that can trigger it.

SSchulze1989 commented 2 years ago

Opened a pr that should fix this issue without affecting the one where the selected value would not disappear after being set from outside.

BeyondDefinition commented 2 years ago

Great job!

I figured out what is causing the frequent rerenders of BlazoredModal. Apparently the onkeyup and onkeydown handlers in FocusTrap sets the CascadedValue every time any keyup/keydown DOM event is generated in a child component, which rerenders the component twice for every letter, causing OnParametersSet to be called in Typeahead. If you remove these 2 event handlers, the Typeahead component also works as intended.

I'm not sure if this needs to be fixed, but it does seem to generate some overhead. Perhaps it's possible to disable event propagation. What is your opinion?

Kazbek commented 2 years ago

Also may be this make v7 much slower than 6.0.1. On previouse version modals closing instantly and on latest they have delay like 0.5 second.

SSchulze1989 commented 2 years ago

I think this is a separate issue and should definitely be tracked on the Blazored.Modam side.

BeyondDefinition commented 2 years ago

Also may be this make v7 much slower than 6.0.1. On previouse version modals closing instantly and on latest they have delay like 0.5 second.

That is by design in v7.

I think this is a separate issue and should definitely be tracked on the Blazored.Modam side.

Agreed, see https://github.com/Blazored/Modal/issues/459