[AdminBundle] Remove id of nested form item containers #2049
Conversation
There was a problem hiding this comment.
Hi @SpadXIII, your PR needs some changes
- This PR seems to need a milestone of a patch release.
There was a problem hiding this comment.
Hi @SpadXIII, your PR passed all our requirements.
Thank you for contributing!
There was a problem hiding this comment.
Hi @SpadXIII,
It seems that this ID on nested forms is indeed not used by any JS. Only the _pagepart-editor.js is using that ID and that only relate to the page part itself. Good catch.
* 5.0: recompile assets update changelog Remove id of nested form item containers (#2049) Fix empty url in admin list (#2071) [MediaBundle] fixed BackgroundFilterLoader issue (#2028) [MediaBundle] Skip test if ghostscript is not installed (#2066) [NodeBundle] Removed unused use of the VarDumper class (#2067) #2050 Fixed issue in refreshing ck editor (don't do a destroy if the instance is already initializing) (#2064) [KunstmaanTranslatorBundle]: export all translations, use correct locales (#2055) [AdminListBundle] fixed form options from AdaptSimpleFormEvent in admin list (#2056) [GeneratorBundle] Fix xpath for publish modal in behat tests (#2052) [AdminBundle] fixed BC break in CreateUserCommand (#2043) remove hardcopy of ckeditor (#2041)
|
@Devolicious @SpadXIII - actually this change breaks ability to sort nested collections in pageparts using Move Up and Move Down buttons. |
|
@genjusz I'm not sure if it's specifically my fix or if there's structurally something not working properly with the javascripts and/or container ids Nested containers would get the same ids as their parents, which breaks other parts :) |
|
It's specifically this fix, because it's removing element id ( Of course I agree that also JavaScript needs to be reworked to fix both issues. IMO different element id should be used or preferably class, that can be retrived from DOM by for example |
* 5.1: update changelog updated changelog [AdminBundle] fix error state for select2 & wysiwyg fields (#2352) [AdminBundle] Revert PR #2049 changes (#2283) [ArticleBundle] Fixed missing sprintf with classname in deprecation message (#2334) [GeneratorBundle] Fix incorrect plural of certain table names (#2277) [Admin][Cache][Config][Generator][Pagepart] fix some routes that didn't use the configurable admin prefix (#2266) [AllBundles] Remove php 7.3 check as php-cs-fixer supports it now [AdminBundle] Update _form-group.scss (#2199)
Nested item containers conflict with page parts, because they use the same id: #-pp-container
As far as I could tell, the id of this element is never used, but the id of page part container elements is.
I have a page part with a (one to many) collection field. When the page has multiple page parts, deleting the second one actually triggers the removal of the second item of the collection field, while also marks the page part for deletion. When saving, this results in a error: Call to a member function setContext() on null at vendor/kunstmaan/bundles-cms/src/Kunstmaan/PagePartBundle/Repository/PagePartRefRepository.php:195.