Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Autocomplete][LiveComponent] Select box doesn't reset when used in a live component #1500

Open
pierredup opened this issue Feb 15, 2024 · 11 comments · May be fixed by #1505 or #1512
Open

[Autocomplete][LiveComponent] Select box doesn't reset when used in a live component #1500

pierredup opened this issue Feb 15, 2024 · 11 comments · May be fixed by #1505 or #1512

Comments

@pierredup
Copy link
Contributor

When using the Autocomplete component in a form as part of a LiveComponent, the field is not reset when re-rendering the component (E.G when submitting the form). The previous selected values are still selected.

Here is an example using a multi-select dropdown, with a custom query builder that removes the selected items from the list:

Screen.Recording.2024-02-15.at.15.21.08.mov

When changing the form to an Autocomplete type, and selecting items, it doesn't get removed from the select

Screen.Recording.2024-02-15.at.15.24.12.mov

Currently in order to reset the autocomplete dropdown, I'm emitting a custom browser event with a custom controller on the select with the following which seems to work fine:

import { Controller } from '@hotwired/stimulus';
import TomSelect from 'tom-select';
import { TomInput, TomSettings } from 'tom-select/dist/types/types';

/* stimulusFetch: 'lazy' */
export default class extends Controller {
    connect(): void {
        document.addEventListener('select:clear', (): void => {
            if ((this.element as TomInput).tomselect) {
                const settings: TomSettings|undefined = (this.element as TomInput).tomselect?.settings;
                (this.element as TomInput).tomselect?.destroy();
                new TomSelect(<TomInput>this.element, (settings as TomSettings));
            }
        });
    }
}
@weaverryan
Copy link
Member

Hey @pierredup!

Ok, let's see if we can fix this :). The underlying problem is that TomSelect does NOT like to reset itself when things change on the underlying select and option elements. So, ignoring TomSelect, if I understand things correctly, this is the situation:

A) A <select multiple> appears on the page with 10 options.
B) The user selects 2 of these: Pan Jan and John Roberts and the submits the form / triggers the LiveAction.
C) When then LiveComponent re-renders, the <select multiple> still on the page. But now it only has 8 options. Is that correct? And, perhaps also important, even if it did still have the original 10 options, Pan Jan and John Roberts would no longer be selected (if you try this iteration, I expect the problem will remain).

The autocomplete live controller has logic to try to determine when TomSelect needs to be reset - https://github.com/symfony/ux/blob/2.x/src/Autocomplete/assets/src/controller.ts#L398-L401

It looks like we may need more there. We already check to see if the options are different between the existing <select> and the new <select>. So if you start with 10 options and there are only 8 on re-render, I would expect that this would trigger a reset. Is it? Or is something going wrong there?

Additionally, it seems like we should perhaps look at the "selected" values before vs after. And if those are different, also trigger a reset.

Let me know if this can help you dig a bit deeper about the issue. I have #1502 open which makes changes in this same area, but I don't think it addresses your issue.

@pierredup
Copy link
Contributor Author

pierredup commented Feb 15, 2024

When then LiveComponent re-renders, the <select multiple> still on the page. But now it only has 8 options. Is that correct?

Yes that's correct.

perhaps also important, even if it did still have the original 10 options, Pan Jan and John Roberts would no longer be selected (if you try this iteration, I expect the problem will remain)

Correct again :)

So if you start with 10 options and there are only 8 on re-render, I would expect that this would trigger a reset. Is it? Or is something going wrong there?

When the component re-renders, the select is empty (since it lazy loads the options when opening the select for the first time). So I'm not sure if it will be possible to know which items are selected

Additionally, it seems like we should perhaps look at the "selected" values before vs after. And if those are different, also trigger a reset.

I don't set any default form values, so selecting any option won't have it selected by default on re-render (or page reload).

@pierredup
Copy link
Contributor Author

When using #1322, the only thing that is different from the rendered content, is the data-symfony--ux-autocomplete--autocomplete-url-value attribute (since the extra_options hash is different since I'm sending the already selected users as an extra option, similar to the initial test case mentioned in #1322), but that looks like it doesn't trigger anything to change (even doing a search in the autocomplete field sends the old extra_options hash instead of the new one)

@weaverryan
Copy link
Member

I don't set any default form values, so selecting any option won't have it selected by default on re-render (or page reload).

So, if you have 2 items selected, then you submit the form and the new html renders either an empty select (meaning, of course, that zero options are selected), that should trigger TomSelect to reset I think.

Btw, another solution in general to think about is leveraging morphing events. For example, turbo 8 dispatches several events during morphing and at some point, live components will do the same to match. Perhaps leveraging those somehow might be easier than a mutation observer. They could allow us, at least, to easily know which attributes are being modified on which elements in a more standard way.

@pierredup
Copy link
Contributor Author

if you have 2 items selected, then you submit the form and the new html renders either an empty select (meaning, of course, that zero options are selected), that should trigger TomSelect to reset I think.

This does not happen, because there is nothing to trigger the reset. The mutation observer doesn't run in this scenario, so there is nothing in the Autocomplete controller that would trigger the reset.

another solution in general to think about is leveraging morphing events

Cool, will have a look at this

@weaverryan
Copy link
Member

Cool, will have a look at this

Awesome :). Though, I'm still going back and forth on this - it's just a suggestion.

This does not happen, because there is nothing to trigger the reset. The mutation observer doesn't run in this scenario, so there is nothing in the Autocomplete controller that would trigger the reset.

Ah yes, because the MutationObserver is NOT initialized for a "remote data" autocomplete. Tbh, I'm not sure that was entirely intentional by me. Certainly, it doesn't make sense to have it running for <option> elements, but it does make sense to have it watch for disabled. And we also need to react/reset to the "selected" options changing, like after a form submit. Like all of morphing (and TomSelect is one of the most difficult), I'm not sure yet the best way to handle this stuff, but I'd like to figure it out. One idea (which may or may not help) with TomSelect is to stop relying on TomSelect to use the <option> elements and instead pass those in as a Stimulus value then pass the value to the options config of TomSelect - https://tom-select.js.org/docs/ - that would allow us to use a value callback, instead of watching the option elements. Perhaps there could even be a value for which item(s) are currently selected. And on change, we make sure we "reset" the selected values on TomSelect if they are different (to do this, I think we would need to hook into TomSelect so that whenever the user selects an option, we update this "value". That way, if you selected 2 options, the data attribute for that value would be updated by us. Then, if we morphed and the value still showed 2 options, there would be no change. But if changed to 0 selected, we'd know.

I'm writing this as my son just woke up - so not 100% thought-out yet. But this is a problem that has been on my mind a lot lately to get solved correctly.

@weaverryan
Copy link
Member

FYI - I have #1512 as a WIP to address more of the issues on a (hopefully) final & robust way. I'll be gone for several days. If you are eager & want to play with that code and keep going, please feel free :). I added several quick things (and a page to test them on in ux.symfony.com), but was only starting to test things and work out the bugs.

@bastien70
Copy link
Contributor

bastien70 commented Mar 25, 2024

Hi ! I don't know if the problem has been resolved since, but I'm here to explain the same type of problem and the solution put in place:

I have a page in which I list items, and a button allowing me to open a modal containing an addition form for my entity.

Everything is managed with LiveComponent.

The form contains an EntityType field with 'autocomplete' => true option to make it ajax . When the form is submitted, the entity is created, and the form is reset.

    #[LiveAction]
    public function save(EntityManagerInterface $entityManager, Request $request): void
    {
        $this->submitForm();

        /** @var MeetingPoint $meetingPoint */
        $meetingPoint = $this->getForm()->getData();

        if ($this->meeting) {
            $meetingPoint->setMeeting($this->meeting);
        }

        $entityManager->persist($meetingPoint);
        $entityManager->flush();

        $this->resetForm();
        $this->dispatchBrowserEvent('modal:close');
        $this->emit('meeting-planning:refresh');
    }

Except that if I click again on the add button to display the creation form, the autocomplete field was not reset. Worse, even when selecting a new value, and submitting the form, it kept the old value.

Capture.video.du.2024-03-25.09-58-15.webm

As you see in the video, the 1st time I select a user, but when I open the modal a second time, that same user is still selected, and if I take another one instead and submit the form, it is still the one that I had previously selected which is chosen. Even weirder, if I open the modal again, and click on the cross (to the right of the field) to clear the select, and I choose a new one, if I submit, it actually takes one other than the one I want.

I was inspired by the method of the author of this issue in order to create a controller-stimulus that I add to my components using an ajax field.

tomselect-reset-controller.js

import { Controller } from '@hotwired/stimulus';
import TomSelect from 'tom-select';
import { getComponent } from '@symfony/ux-live-component';

/* stimulusFetch: 'lazy' */
export default class extends Controller {
    async initialize() {
        this.component = await getComponent(this.element);

        window.addEventListener('select:clear', (event) => {
            var elements = this.element.querySelectorAll('.tomselected');

            elements.forEach(element => {
                if (element.tomselect) {
                    var settings = element.tomselect.settings;
                    element.tomselect.destroy();
                    new TomSelect(element, settings);
                }
            });
        });
    }
}

This controller will listen on the "select:clear" event, retrieve the autocomplete fields using the class added automatically by the "autocomplete => true" attribute, and reset the field.

Now in my template :

<div {{ attributes.add(stimulus_controller('tomselect-reset')) }}>

// form ...
</div>

In my component :

//...
$this->resetForm();
$this->dispatchBrowserEvent('select:clear'); // will reset the select
$this->dispatchBrowserEvent('modal:close');
$this->emit('meeting-planning:refresh');

And now :

Capture.video.du.2024-03-25.10-09-55.webm

Has this type of problem been resolved since then, @weaverryan ?

@antoniovj1
Copy link

@pierredup I'm not sure which approach (#1505 #1512) is better, but with a little guidance I can help you to test the best option and solve this 💪

@pierredup
Copy link
Contributor Author

@antoniovj1 As far as I can tell, both #1505 and #1512 are required, since it fixes different issues (although I haven't checked if #1512 fixes the same issue addressed in #1505, so maybe that is a first starting point)

@antoniovj1
Copy link

I've had a few days very busy 🥵 Your PR @pierredup seems perfect to me. I'm going to check the @weaverryan one to see if I can contribute something or not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants