[Autocomplete] SortableJS Sorting Issue

[rodnaph]

Issue

When an autocomplete control is used inside a sortable list using SortableJS, then the value of the autocomplete is lost when sorting elements.

Description

This seems to be caused by the disconnect method, which...

1. stops mutation observers
2. destroys the TomSelect control

But, if the TomSelect control value has been modified then this value is now lost, and when the control is re-initialized after sorting the original value is set.

This issue is not present when using a plain (ie. non-ux-autocomplete) TomSelect control with SortableJS. eg. https://jsfiddle.net/mqwspcf3/

Solution

I've been able to create a workaround by flushing the current TomSelect value to the underlying control on disconnect like so (not suggesting this as actual solution, just demonstrating problem)...

    disconnect() {
        this.stopMutationObserver();

        // Flush current TomSelect value to underlying select control.
        const value = this.tomSelect.getValue();
        this.element.querySelectorAll('option').forEach((option) => {
            option.removeAttribute('selected');

            if (option.value === value) {
                option.setAttribute('selected', 'selected');
            }
        });

        this.tomSelect.destroy();
    }

I'm not sure if this is expected behaviour, but it does seem to cause an issue by default which is surprising. Any feedback appreciated, thanks.

[weaverryan]

Interesting. So, if I understand correctly, the MutationObserver is not the problem. It is the this.tomSelect.destroy() that causes the problem, right? Could it make sense just to remove that? Iirc, we added it just to do responsible cleanup, but perhaps that was overeager.

See #1219

[rodnaph]

Thanks for the reply - I'll try to create a reproducer, with minimal cases with TomSelect and UX, and get back to you.

[rodnaph]

Repo, info and reproduction steps in README: https://github.com/owsy/sortablejs-symfony-ux-autocomplete

I've not tried to debug this further yet but will take a look as soon as I have time.

[weaverryan]

@rodnaph THANK you. But is the repos private? I got a 404

[rodnaph]

@weaverryan Gah, schoolboy... apologies I thought I made it public, it's visible now. 👍

[smnandre]

Well, there is something weird, and i'm not sure if our script should handle that or Sortable...

but big picture, the "disconnect" is triggered by Stimulus on drag:start, and the connect is immediately trigger in an other dom position....

But while "in the air" by Sortable, TomSelect seems to have not synchronized yet the value to the initial field... ... so when the drop ends, we already have resetted the value / options.

(and i lost time with required / not, multiple / not and data-attributes haha)

[smnandre]

And it does work with your patch @weaverryan

So it's indeed something with the destroy beeing called before the value is sync back into the HTML

Update:

i confirm as soon the element is injected in another place in the DOM, the select has no value.

[rodnaph]

@smnandre Thanks for taking the time to look at this.

Well, there is something weird, and i'm not sure if our script should handle that or Sortable...

I was also not sure where the issue lies, if autocomplete even needs to change, but as a plain TomSelect with SortableJS does not have this issue I figured perhaps that's a sign it should be supported here.

But while "in the air" by Sortable, TomSelect seems to have not synchronized yet the value to the initial field...

My initial attempt to work around this was to force TomSelect to flush the value onchange, but it does not have any public methods for this (as you can see from the workaround code in my description). And again, the plain TomSelect does work...

Anyway, thanks to anyone spending time on this, I realise it's something of an edge-case in as much as it's support for one particular other library, but SortableJS has been suggested in other threads for the basis for a UX sortable component, so might be useful to understand what's going on.

[smnandre]

SortableJS has been suggested in other threads for the basis for a https://github.com/symfony/ux/issues/1071, so might be useful to understand what's going on.

💯

And even more, if than happens there, that probably happen with other librairies, so it's something to look at :)

If the @weaverryan solution (not destroying the instance) has no other consequences than "philosophical" ones, i'd say we use it for now (with a warning/information ?)

[smnandre]

I spent some time to look for a solution, and...

• the bug comes from TomSelect (see https://github.com/orchidjs/tom-select/issues/562 )
• we could "manually" store the correct value in the input (see example[^1]) .. but it's not cleaner than "not destroying" ...

So i'm even more for your solution @weaverryan ...

(if we really wanted to avoid memory loss, we could add a "timer" and destroy the instance if the element has not been re-connected after some time....)

---

[^1] : something like

    // PSEUDO CODE (does not handle multiple options...)
    disconnect() {
        this.stopMutationObserver();
        const value = this.element.value;
        this.tomSelect.destroy();
        this.element.value = value;
    }