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] Check if accessed vars exist #2490

Closed
wants to merge 1 commit into from

Conversation

maciazek
Copy link

@maciazek maciazek commented Jan 7, 2025

Q A
Bug fix? yes
New feature? no
Issues Fix #2487
License MIT

In my case, checking 'placeholder' for existence is sufficient. But I can imagine some very extreme custom form type which - besides 'choices' - has 'placeholder' option too. In that case, when 'placeholder' is set to false, then 'multiple' option need to be checked for existence, and so on... Considering the most pessimistic scenario, I think at least 'placeholder', 'multiple' and 'expanded' options should be checked.

I'm not sure about the rest - if there could be a situation where 'required', 'disabled' or 'value' option is nonexistent?

@carsonbot carsonbot added Bug Bug Fix LiveComponent Status: Needs Review Needs to be reviewed labels Jan 7, 2025
@smnandre
Copy link
Member

smnandre commented Jan 8, 2025

Hey @maciazek during my reading of all this I found another bug here...

As I wanted to make some changes in your PR... I ended by opening a new PR for both: #2491

If you want to have a look at it and confirm it does fix your problem?

Thank you very much for opening this one!

@maciazek
Copy link
Author

maciazek commented Jan 8, 2025

Of course, I'll check it in my workplace tomorrow as I'm out of office today. Thanks!

@smnandre
Copy link
Member

smnandre commented Jan 8, 2025

I'm closing this one but I'd like to thank you again for stepping up, explaining your case precisely and kindly, then opening a PR to fix the issue.

🙇‍♂️

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
3 participants