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

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

Open
wants to merge 4 commits into
base: 2.x
Choose a base branch
from

Conversation

smnandre
Copy link
Member

@smnandre smnandre commented Jan 8, 2025

Q A
Bug fix? yes
New feature? no
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)

@maciazek
Copy link

maciazek commented Jan 9, 2025

I can confirm that this patch resolved my specific case with custom form type and "choices" option. Thank you.

@norkunas
Copy link
Contributor

norkunas commented Jan 9, 2025

Something is more going on here.
I was getting An exception has been thrown during the rendering of a template ("Warning: Undefined array key "expanded""). and so I've tried this patch, then started to get:
An exception has been thrown during the rendering of a template ("Warning: Undefined array key """).

While #2490 works for me

@maciazek
Copy link

maciazek commented Jan 9, 2025

Something is more going on here. I was getting An exception has been thrown during the rendering of a template ("Warning: Undefined array key "expanded""). and so I've tried this patch, then started to get: An exception has been thrown during the rendering of a template ("Warning: Undefined array key """).

While #2490 works for me

I think it could happen when form has some ChoiceType field, which has 'choices' set as empty array. In that case
https://github.com/smnandre/ux/blob/cd353fc7a0689b9cee5da252f977acb420176d96/src/LiveComponent/src/ComponentWithFormTrait.php#L269
returns true, and then the error itself comes from:
https://github.com/smnandre/ux/blob/cd353fc7a0689b9cee5da252f977acb420176d96/src/LiveComponent/src/ComponentWithFormTrait.php#L284

Seems that some additional check is needed (if 'choices' is array and if it is not empty?)

@smnandre
Copy link
Member Author

Entirely my fault here.

As you can see here i checked for the "choices" existence.
https://github.com/smnandre/ux/blob/cd353fc7a0689b9cee5da252f977acb420176d96/src/LiveComponent/src/ComponentWithFormTrait.php#L275C33-L275C34

I decided later on to "optimize" and moved the check first, adding an isset in the operation when the key was not defined.... and losing then my original goal: ensure there is one value.

@smnandre
Copy link
Member Author

This should work better now. Thank you very much to both of you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[LiveComponent] Undefined array key "placeholder" after upgrading to 2.22.x
4 participants