Skip to content
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

[LiveComponents] Bug with required select with empty placeholders #2425

Closed
FoxCie opened this issue Dec 4, 2024 · 0 comments · Fixed by #2426
Closed

[LiveComponents] Bug with required select with empty placeholders #2425

FoxCie opened this issue Dec 4, 2024 · 0 comments · Fixed by #2426
Labels
Bug Bug Fix Status: Needs Review Needs to be reviewed

Comments

@FoxCie
Copy link

FoxCie commented Dec 4, 2024

Hello,
the last version (2.22.0) fixed bug #1332 that selects the first value of select fields when they are required. This works great, even with placeholders, as they are selected if present, which is the intended behaviour. Unfortunately, due to loose comparison, if there is an empty placeholder (that is, a placeholder defined as the empty string), it is detected as a field without placeholder.

The error comes from the line here :

&& !$child->vars['placeholder']
with :

!$child->vars['placeholder']

Casting the placeholder to a boolean leads to the empty string being considered as an absence of placeholder, this should be tested this way I think :

$child['placeholder'] === null

To reproduce the bug, try a ChoiceType field that is required, with an empty placeholder (['placeholder => ''] in the options array of the form), and see that it takes the first value when changing another field of the form.

Not a big deal, but I had to investigate why the behaviour was different in the previous version, so I might not be the only one.

@FoxCie FoxCie added the Bug Bug Fix label Dec 4, 2024
@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Dec 4, 2024
FoxCie pushed a commit to FoxCie/symfony_ux that referenced this issue Dec 5, 2024
Fix for a bug that arises when using an empty placeholder in a required select field, that is wrongly treated as an absence of placeholder by LiveComponents.
@Kocal Kocal closed this as completed in 6008eed Dec 7, 2024
smnandre added a commit that referenced this issue Jan 25, 2025
…ues()` with edge cases (smnandre)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[LiveComponent] Fix `ComponentWithFormTrait::extractFormValues()` with edge cases

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | Fix #2487
| License       | MIT

#2403 fixed an old bug to simulate browser behaviour when a select field has no option selected,  no placeholder, and is required (it then uses the first option)
#2425 and #2426 fixed the way we handled empty strings as placeholders

In #2487 `@maciazek` explained us how a custom type with a "choices" option became unusable since the last changes.

Also.. while I was reading all this I realized we returned wrong values here when "option groups" were used.. leading to other problems.

I updated the code of `extractFormValues()` method to:
* check if most of the caracteristic keys added in `ChoiceType::buildView` were defined in the `$view->vars`, to ensure we are dealing with a `<select>` field built by a `ChoiceType`
* fetch recursively the value of the first displayed option (even with groups)

Commits
-------

b7f1ba4 [LiveComponent] Fix `ComponentWithFormTrait::extractFormValues()` with edge cases
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Bug Fix Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants