Skip to content

[Autocomplete] Fix regression where placeholder change didn't cause a reset #1513

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

Merged

Conversation

weaverryan
Copy link
Member

Q A
Bug fix? yes
New feature? no
Issues None
License MIT

Introduced here - https://github.com/symfony/ux/pull/1502/files#diff-655bee3f1f3d92a2b984a8599df80a4f31a842d5f04fd3d186f8e3161ce49c0bR429 - when fixing the bug:

A) If there was no empty <option value""> element, then each time a value was selected, it incorrectly looked like the options had changed, triggering a reset. This is because TomSelect adds a <option value=""> is there was not one at the start.

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Feb 17, 2024
@weaverryan weaverryan merged commit e8fce4d into symfony:2.x Feb 20, 2024
@weaverryan weaverryan deleted the autocomplete-fix-placeholder-reset branch February 20, 2024 14:54
@@ -199,6 +199,14 @@ class default_1 extends Controller {
areOptionsEquivalent(newOptions) {
const filteredOriginalOptions = this.originalOptions.filter((option) => option.value !== '');
const filteredNewOptions = newOptions.filter((option) => option.value !== '');
const originalPlaceholderOption = this.originalOptions.find((option) => option.value === '');
const newPlaceholderOption = newOptions.find((option) => option.value === '');
console.log(originalPlaceholderOption, newPlaceholderOption);
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, but why we need to log here in prod?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants