bug #787 [Autocomplete] Limiting fancy updating to non-multiple selects (weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[Autocomplete] Limiting fancy updating to non-multiple selects

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | Fix #785
| License       | MIT

Autocomplete uses TomSelect, which is great. Unfortunately, it does a lot of weird things internally. For example, when you select an item, it actually reorders the `option` elements. Also, if you change the options in a `<select>`, it is very difficult to get TomSelect to "reset" itself and update to show those new options.

In an earlier PR, I actually DID (through a lot of pain), make Autocomplete smart enough so that, if *anything* external changes the `<option>` elements inside of the original `<select>`, the `TomSelect` instance would reinitialize to show the new options. This is especially challenging since TomSelect itself is rearranging and changing elements in an odd way.

However, when it comes to `<select multiple>`, its behavior seems WAY too complex to "fix". Thus, if you have a `<select multiple` and you change some of its `<option>` elements, you will get the default TomSelect behavior: it will NOT update it show the new options.

This is all more relevant when using TomSelect with LiveComponents.

tl;dr

* Autocomplete + LiveComponents "should" (thank to an earlier PR) work great together... except for `multiple` selects
* For `multiple` selects, I've waved the white flag. To make it work in 95% of the cases, however, we add a `data-live-ignore` so that LiveComponents doesn't touch the TomSelect widget after it's initialized.

Commits
-------

669ed87 [Autocomplete] Limiting fancy updating to non-multiple selects

Author: weaverryan

src/Autocomplete/CHANGELOG.md

@@ -2,12 +2,11 @@
 
 ## 2.8.0
 
-
--   The autocomplete will now update if any of the `option` elements inside of
+-   The autocomplete now watches for update to any `option` elements inside of
     it change, including the empty / placeholder element. Additionally, if the
     `select` or `input` element's `disabled` attribute changes, the autocomplete
-    instance will update accordingly. This makes Autocomplete work perfectly inside
-    of a LiveComponent.
+    instance will update accordingly. This makes Autocomplete work correctly inside
+    of a LiveComponent. This functionality does _not_ work for `multiple` selects.
 
 -   Added support for using [OptionGroups](https://tom-select.js.org/examples/optgroups/).
 

src/Autocomplete/assets/dist/controller.d.ts

@@ -46,4 +46,5 @@ export default class extends Controller {
     private startMutationObserver;
     private stopMutationObserver;
     private onMutations;
+    private requiresLiveIgnore;
 }

src/Autocomplete/assets/dist/controller.js

@@ -30,10 +30,21 @@ class default_1 extends Controller {
         this.isObserving = false;
     }
     initialize() {
-        if (!this.mutationObserver) {
-            this.mutationObserver = new MutationObserver((mutations) => {
-                this.onMutations(mutations);
-            });
+        if (this.requiresLiveIgnore()) {
+            this.element.setAttribute('data-live-ignore', '');
+            if (this.element.id) {
+                const label = document.querySelector(`label[for="${this.element.id}"]`);
+                if (label) {
+                    label.setAttribute('data-live-ignore', '');
+                }
+            }
+        }
+        else {
+            if (!this.mutationObserver) {
+                this.mutationObserver = new MutationObserver((mutations) => {
+                    this.onMutations(mutations);
+                });
+            }
         }
     }
     connect() {
@@ -118,7 +129,7 @@ class default_1 extends Controller {
         }
     }
     startMutationObserver() {
-        if (!this.isObserving) {
+        if (!this.isObserving && this.mutationObserver) {
             this.mutationObserver.observe(this.element, {
                 childList: true,
                 subtree: true,
@@ -129,7 +140,7 @@ class default_1 extends Controller {
         }
     }
     stopMutationObserver() {
-        if (this.isObserving) {
+        if (this.isObserving && this.mutationObserver) {
             this.mutationObserver.disconnect();
             this.isObserving = false;
         }
@@ -200,6 +211,9 @@ class default_1 extends Controller {
             this.updateTomSelectPlaceholder();
         }
     }
+    requiresLiveIgnore() {
+        return this.element instanceof HTMLSelectElement && this.element.multiple;
+    }
 }
 _default_1_instances = new WeakSet(), _default_1_getCommonConfig = function _default_1_getCommonConfig() {
     const plugins = {};
@@ -218,12 +232,19 @@ _default_1_instances = new WeakSet(), _default_1_getCommonConfig = function _def
             return `<div class="no-results">${this.noResultsFoundTextValue}</div>`;
         },
     };
+    const requiresLiveIgnore = this.requiresLiveIgnore();
     const config = {
         render,
         plugins,
         onItemAdd: () => {
             this.tomSelect.setTextboxValue('');
         },
+        onInitialize: function () {
+            if (requiresLiveIgnore) {
+                const tomSelect = this;
+                tomSelect.wrapper.setAttribute('data-live-ignore', '');
+            }
+        },
         closeAfterSelect: true,
     };
     if (!this.selectElement && !this.urlValue) {

src/Autocomplete/assets/src/controller.ts

@@ -36,10 +36,27 @@ export default class extends Controller {
     private isObserving = false;
 
     initialize() {
-        if (!this.mutationObserver) {
-            this.mutationObserver = new MutationObserver((mutations: MutationRecord[]) => {
-                this.onMutations(mutations);
-            });
+        if (this.requiresLiveIgnore()) {
+            // unfortunately, TomSelect does enough weird things that, for a
+            // multi select, if the HTML in the `<select>` element changes,
+            // we can't reliably update TomSelect to see those changes. So,
+            // as a workaround, we tell LiveComponents to entirely ignore trying
+            // to update this item
+            this.element.setAttribute('data-live-ignore', '');
+            if (this.element.id) {
+                const label = document.querySelector(`label[for="${this.element.id}"]`);
+                if (label) {
+                    label.setAttribute('data-live-ignore', '');
+                }
+            }
+        } else {
+            // for non-multiple selects, we use a MutationObserver to update
+            // the TomSelect instance if the options themselves change
+            if (!this.mutationObserver) {
+                this.mutationObserver = new MutationObserver((mutations: MutationRecord[]) => {
+                    this.onMutations(mutations);
+                });
+            }
         }
     }
 
@@ -88,13 +105,22 @@ export default class extends Controller {
             },
         };
 
+        const requiresLiveIgnore = this.requiresLiveIgnore();
+
         const config: RecursivePartial<TomSettings> = {
             render,
             plugins,
             // clear the text input after selecting a value
             onItemAdd: () => {
                 this.tomSelect.setTextboxValue('');
             },
+            // see initialize() method for explanation
+            onInitialize: function () {
+                if (requiresLiveIgnore) {
+                    const tomSelect = this as any;
+                    tomSelect.wrapper.setAttribute('data-live-ignore', '');
+                }
+            },
             closeAfterSelect: true,
         };
 
@@ -300,7 +326,7 @@ export default class extends Controller {
     }
 
     private startMutationObserver(): void {
-        if (!this.isObserving) {
+        if (!this.isObserving && this.mutationObserver) {
             this.mutationObserver.observe(this.element, {
                 childList: true,
                 subtree: true,
@@ -312,7 +338,7 @@ export default class extends Controller {
     }
 
     private stopMutationObserver(): void {
-        if (this.isObserving) {
+        if (this.isObserving && this.mutationObserver) {
             this.mutationObserver.disconnect();
             this.isObserving = false;
         }
@@ -404,4 +430,8 @@ export default class extends Controller {
             this.updateTomSelectPlaceholder();
         }
     }
+
+    private requiresLiveIgnore(): boolean {
+        return this.element instanceof HTMLSelectElement && this.element.multiple;
+    }
 }

src/Autocomplete/assets/test/controller.test.ts

@@ -227,6 +227,29 @@ describe('AutocompleteController', () => {
         expect(tomSelect.settings.shouldLoad('')).toBeTruthy()
     })
 
+    it('adds work-around for live-component & multiple select', async () => {
+        const { container } = await startAutocompleteTest(`
+            <div>
+                <label for="the-select" data-testid="main-element-label">Select something</label>
+                <select
+                    id="the-select"
+                    data-testid="main-element"
+                    data-controller="check autocomplete"
+                    multiple
+                ></select>
+            </div>
+        `);
+
+        expect(getByTestId(container, 'main-element')).toHaveAttribute('data-live-ignore');
+        expect(getByTestId(container, 'main-element-label')).toHaveAttribute('data-live-ignore');
+        const tsDropdown = container.querySelector('.ts-wrapper');
+
+        await waitFor(() => {
+            expect(tsDropdown).not.toBeNull();
+        });
+        expect(tsDropdown).toHaveAttribute('data-live-ignore');
+    });
+
     it('loads new pages on scroll', async () => {
         document.addEventListener('autocomplete:pre-connect', (event: any) => {
             const options = (event.detail as AutocompletePreConnectOptions).options;

src/Autocomplete/doc/index.rst

@@ -560,6 +560,19 @@ consider registering the needed type extension ``AutocompleteChoiceTypeExtension
         // ... your tests
     }
 
+Known Issue when using with Live Component
+------------------------------------------
+
+You *can* use autocomplete inside of a `Live Component`_: the autocomplete JavaScript
+widget should work normally and even update if your element changes (e.g. if you
+add or change ``<option>`` elements. Internally, a ``MutationObserver`` inside
+the UX autocomplete controller detects these changes and forwards them to TomSelect.
+
+However, if you use the ``multiple`` option, due to complexities in TomSelect, the
+autocomplete widget *will* work, but it will not update if you change any options.
+For example, if your change the "options" for a ``select`` during re-render, those
+will not update on the frontend.
+
 Backward Compatibility promise
 ------------------------------
 
@@ -572,3 +585,4 @@ the Symfony framework: https://symfony.com/doc/current/contributing/code/bc.html
 .. _`controller.ts`: https://github.com/symfony/ux/blob/2.x/src/Autocomplete/assets/src/controller.ts
 .. _`Tom Select Render Templates`: https://tom-select.js.org/docs/#render-templates
 .. _`Tom Select Option Group`: https://tom-select.js.org/examples/optgroups/
+.. _`Live Component`: https://symfony.com/bundles/ux-live-component/current/index.html

ux.symfony.com/src/Form/MealPlannerForm.php

@@ -68,7 +68,7 @@ public function configureOptions(OptionsResolver $resolver)
     private function getAvailableFoodChoices(string $meal): array
     {
         $foods = match ($meal) {
-            self::MEAL_BREAKFAST => ['Eggs 🍳', 'Bacon 🥓', 'Strawberries 🍓', 'Croissant 🥐', 'Other', 'AnOther'],
+            self::MEAL_BREAKFAST => ['Eggs 🍳', 'Bacon 🥓', 'Strawberries 🍓', 'Croissant 🥐'],
             self::MEAL_SECOND_BREAKFAST => ['Bagel 🥯', 'Kiwi 🥝', 'Avocado 🥑', 'Waffles 🧇'],
             self::MEAL_ELEVENSES => ['Pancakes 🥞', 'Salad 🥙', 'Tea ☕️'],
             self::MEAL_LUNCH => ['Sandwich 🥪', 'Cheese 🧀', 'Sushi 🍱'],