From 7a801fadd5c3ba82886df0c27458608776fe9ba8 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Thu, 15 Feb 2024 10:53:50 -0500 Subject: [PATCH] [Autocomplete] Fix 2 bugs where TomSelect would reset when not necessary --- src/Autocomplete/assets/dist/controller.js | 16 +++++-- src/Autocomplete/assets/src/controller.ts | 18 ++++++-- .../assets/test/controller.test.ts | 42 +++++++++++++++++++ 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/src/Autocomplete/assets/dist/controller.js b/src/Autocomplete/assets/dist/controller.js index 49587145268..5659d9bcc17 100644 --- a/src/Autocomplete/assets/dist/controller.js +++ b/src/Autocomplete/assets/dist/controller.js @@ -116,6 +116,7 @@ class default_1 extends Controller { } resetTomSelect() { if (this.tomSelect) { + this.dispatchEvent('before-reset', { tomSelect: this.tomSelect }); this.stopMutationObserver(); const currentHtml = this.element.innerHTML; const currentValue = this.tomSelect.getValue(); @@ -143,6 +144,7 @@ class default_1 extends Controller { subtree: true, attributes: true, characterData: true, + attributeOldValue: true, }); this.isObserving = true; } @@ -164,7 +166,11 @@ class default_1 extends Controller { break; } if (mutation.target === this.element && mutation.attributeName === 'multiple') { - requireReset = true; + const isNowMultiple = this.element.hasAttribute('multiple'); + const wasMultiple = mutation.oldValue === 'multiple'; + if (isNowMultiple !== wasMultiple) { + requireReset = true; + } break; } break; @@ -191,12 +197,14 @@ class default_1 extends Controller { }); } areOptionsEquivalent(newOptions) { - if (this.originalOptions.length !== newOptions.length) { + const filteredOriginalOptions = this.originalOptions.filter((option) => option.value !== ''); + const filteredNewOptions = newOptions.filter((option) => option.value !== ''); + if (filteredOriginalOptions.length !== filteredNewOptions.length) { return false; } const normalizeOption = (option) => `${option.value}-${option.text}-${option.group}`; - const originalOptionsSet = new Set(this.originalOptions.map(normalizeOption)); - const newOptionsSet = new Set(newOptions.map(normalizeOption)); + const originalOptionsSet = new Set(filteredOriginalOptions.map(normalizeOption)); + const newOptionsSet = new Set(filteredNewOptions.map(normalizeOption)); return (originalOptionsSet.size === newOptionsSet.size && [...originalOptionsSet].every((option) => newOptionsSet.has(option))); } diff --git a/src/Autocomplete/assets/src/controller.ts b/src/Autocomplete/assets/src/controller.ts index 580552a25e1..bc3812b7a82 100644 --- a/src/Autocomplete/assets/src/controller.ts +++ b/src/Autocomplete/assets/src/controller.ts @@ -325,6 +325,7 @@ export default class extends Controller { private resetTomSelect(): void { if (this.tomSelect) { + this.dispatchEvent('before-reset', { tomSelect: this.tomSelect }); this.stopMutationObserver(); // Grab the current HTML then restore it after destroying TomSelect @@ -358,6 +359,7 @@ export default class extends Controller { subtree: true, attributes: true, characterData: true, + attributeOldValue: true, }); this.isObserving = true; } @@ -384,7 +386,11 @@ export default class extends Controller { } if (mutation.target === this.element && mutation.attributeName === 'multiple') { - requireReset = true; + const isNowMultiple = this.element.hasAttribute('multiple'); + const wasMultiple = mutation.oldValue === 'multiple'; + if (isNowMultiple !== wasMultiple) { + requireReset = true; + } break; } @@ -419,14 +425,18 @@ export default class extends Controller { } private areOptionsEquivalent(newOptions: Array<{ value: string; text: string; group: string | null }>): boolean { - if (this.originalOptions.length !== newOptions.length) { + // remove the empty option, which is added by TomSelect so may be missing from new options + const filteredOriginalOptions = this.originalOptions.filter((option) => option.value !== ''); + const filteredNewOptions = newOptions.filter((option) => option.value !== ''); + + if (filteredOriginalOptions.length !== filteredNewOptions.length) { return false; } const normalizeOption = (option: { value: string; text: string; group: string | null }) => `${option.value}-${option.text}-${option.group}`; - const originalOptionsSet = new Set(this.originalOptions.map(normalizeOption)); - const newOptionsSet = new Set(newOptions.map(normalizeOption)); + const originalOptionsSet = new Set(filteredOriginalOptions.map(normalizeOption)); + const newOptionsSet = new Set(filteredNewOptions.map(normalizeOption)); return ( originalOptionsSet.size === newOptionsSet.size && diff --git a/src/Autocomplete/assets/test/controller.test.ts b/src/Autocomplete/assets/test/controller.test.ts index 440e2004014..f7c5060f412 100644 --- a/src/Autocomplete/assets/test/controller.test.ts +++ b/src/Autocomplete/assets/test/controller.test.ts @@ -815,4 +815,46 @@ describe('AutocompleteController', () => { }); expect(getSelectedValues()).toEqual(['2', '3']); }); + + it('does not trigger a reset when the style of "multiple" attribute changes', async () => { + const { container } = await startAutocompleteTest(` + + `); + + let wasReset = false; + container.addEventListener('autocomplete:before-reset', () => { + wasReset = true; + }); + + const selectElement = getByTestId(container, 'main-element') as HTMLSelectElement; + selectElement.setAttribute('multiple', 'multiple'); + // wait for the mutation observe + await shortDelay(10); + expect(wasReset).toBe(false); + }); + + it('does not trigger a reset based on the extra, empty select', async () => { + const { container, tomSelect } = await startAutocompleteTest(` + + `); + + let wasReset = false; + container.addEventListener('autocomplete:before-reset', () => { + wasReset = true; + }); + + tomSelect.addItem('2'); + // wait for the mutation observe + await shortDelay(10); + expect(wasReset).toBe(false); + }); });