Skip to content

[Autocomplete] Tom Select Options Plugins #2657

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.

Sign up for GitHub

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

Closed
wants to merge 4 commits into from
Closed

[Autocomplete] Tom Select Options Plugins #2657

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
74e8584
Fix #2656
luchidalgo Mar 25, 2025
cc350f2
Fix CS
luchidalgo Mar 25, 2025
c9e1dfb
Fix CS
luchidalgo Mar 25, 2025
9562beb
Add compled src
luchidalgo Mar 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/Autocomplete/assets/dist/controller.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,11 @@ _default_1_instances = new WeakSet(), _default_1_getCommonConfig = function _def
if (this.urlValue) {
plugins.virtual_scroll = {};
}
if ('plugins' in this.tomSelectOptionsValue && Array.isArray(this.tomSelectOptionsValue.plugins)) {
this.tomSelectOptionsValue.plugins.forEach((pluginName) => {
plugins[pluginName] = {};
});
}
const render = {
no_results: () => {
return `<div class="no-results">${this.noResultsFoundTextValue}</div>`;
Expand Down Expand Up @@ -288,7 +293,7 @@ _default_1_instances = new WeakSet(), _default_1_getCommonConfig = function _def
if (!this.selectElement && !this.urlValue) {
config.shouldLoad = () => false;
}
return __classPrivateFieldGet(this, _default_1_instances, "m", _default_1_mergeObjects).call(this, config, this.tomSelectOptionsValue);
return __classPrivateFieldGet(this, _default_1_instances, "m", _default_1_mergeObjects).call(this, this.tomSelectOptionsValue, config);
}, _default_1_createAutocomplete = function _default_1_createAutocomplete() {
const config = __classPrivateFieldGet(this, _default_1_instances, "m", _default_1_mergeObjects).call(this, __classPrivateFieldGet(this, _default_1_instances, "m", _default_1_getCommonConfig).call(this), {
maxOptions: this.getMaxOptions(),
Expand Down
8 changes: 7 additions & 1 deletion src/Autocomplete/assets/src/controller.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ export default class extends Controller {
plugins.virtual_scroll = {};
}

if ('plugins' in this.tomSelectOptionsValue && Array.isArray(this.tomSelectOptionsValue.plugins)) {
this.tomSelectOptionsValue.plugins.forEach((pluginName) => {
plugins[pluginName] = {};
});
}
Comment on lines +152 to +156
Copy link
Member

@Kocal Kocal Mar 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given examples (https://tom-select.js.org/plugins/caret-position/ and https://tom-select.js.org/plugins/checkbox-options/) it looks like plugins can also be a key-value object.

Can't we use a more simple solution like this?

Suggested change
if ('plugins' in this.tomSelectOptionsValue && Array.isArray(this.tomSelectOptionsValue.plugins)) {
this.tomSelectOptionsValue.plugins.forEach((pluginName) => {
plugins[pluginName] = {};
});
}
plugins = this.tomSelectOptionsValue.plugins || [];

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is treaky : https://tom-select.js.org/docs/plugins/ says we can put just plugin names (simple array) or plugin with options (object in js side). Also with simple assignment we override plugins configuration added line 138 to 150


const render: Partial<TomTemplates> = {
no_results: () => {
return `<div class="no-results">${this.noResultsFoundTextValue}</div>`;
Expand Down Expand Up @@ -214,7 +220,7 @@ export default class extends Controller {
config.shouldLoad = () => false;
}

return this.#mergeObjects(config, this.tomSelectOptionsValue);
return this.#mergeObjects(this.tomSelectOptionsValue, config);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not really sure about that, it looks like a BC no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

By doing this, we prevent user-provided options from overwriting the configuration created within this function, especially for plugins.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As @Kocal said, it is BC break. By looking at the code, the original idea is: if a user provides some config values, these values always win over default config

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I can revert this change and merge the config with a copy of tomSelectOptionsValue, without the plugins previously merged by a specific process, in order to avoid them being replaced ?

}

#createAutocomplete(): TomSelect {
Expand Down
Loading