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

Fix bugs in ChoiceType's name #715

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

ukeloop
Copy link
Contributor

@ukeloop ukeloop commented Sep 13, 2023

@rudiedirkx
Copy link
Collaborator

Yeeaah, but tmp is so ugly... =( I rather just get rid of ChoiceType and ParentType and create a breaking 2.x with a very simple upgrade path. It's always problems with these fields 😠

I'm okay with this PR, because I don't use ChoiceType, so I know it definitely won't break my code, but I'm not sure enough about ChoiceType ever.

@rudiedirkx
Copy link
Collaborator

LOL what a complainer #324 :D

@Eloar
Copy link
Contributor

Eloar commented Dec 6, 2023

@ukeloop why tmp, why not internal if those options are not supposed to be exposed in view?

@ukeloop
Copy link
Contributor Author

ukeloop commented Dec 23, 2023

@Eloar

See that:

if ($this->getOption('attr.multiple') && !$this->getOption('tmp.multipleBracesSet')) {
$this->name = $this->name . '[]';
$this->setOption('tmp.multipleBracesSet', true);
}

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

Successfully merging this pull request may close these issues.

3 participants