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 loading new item list after adding new created element on sortable collections #8202

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

Conversation

BA-JBI
Copy link

@BA-JBI BA-JBI commented Aug 15, 2024

Subject

This Pull request closes two issues related to sortable collections as described in #8201

I am targeting this branch, because this patches an existing issue.

Closes #8201

Changelog

### Fixed
- Assert new created items on sortable collections are added in javascript correctly after creation in modal
- Avoid clearing selections when loading new choice list after reating a new model in sortable collections

src/Form/Extension/ChoiceTypeExtension.php Outdated Show resolved Hide resolved
Comment on lines +343 to +345
var selections = jQuery('#{{ id }}').val().split(',');
selections.push(data.objectId);
jQuery('#{{ id }}').val(selections.filter((val) => val.length > 0).join(','));
Copy link
Member

Choose a reason for hiding this comment

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

If entities use string as id, the usage of , could conflict no ?

Comment on lines +45 to +46
if (str_contains($data[0], ',')) {
$event->setData(explode(',', $data[0]));
Copy link
Member

Choose a reason for hiding this comment

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

While it's work for your usage (with the javascript),
I assume it will introduce a bug if someone use this Extension/FormType in another situation for texte with string like

['Foo, bar']

It will become ['Foo', 'bar'] instead of staying the same...

I dunno if there is a way to avoid such thing...

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the correct approach here would be to have multiple hidden inputs, one for each id. Like the ModelAutocompleteType does it.

This would probably be more work, tho.

Another approach would be adding a special prefix to the value, that then can be checked.

Co-authored-by: Vincent Langlet <VincentLanglet@users.noreply.github.com>
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.

Model Type: New created entity does not get selected on sortable collections
3 participants