[Admin] Added Route Parameters to MenuNodeAdmin #109
Conversation
src/Admin/Menu/MenuNodeAdmin.php
Outdated
| 'uri' => array('link'), | ||
| 'content' => array('content', TreeSelectType::class), | ||
| ), | ||
| 'placeholder' => 'auto', | ||
| 'required' => false, | ||
| )) | ||
| ->add('link', TextType::class, array('required' => false, 'mapped' => false)) | ||
| ->add('routeParameters', KeyValueType::class, array( |
There was a problem hiding this comment.
currently the key-value bundle is an optional dependency. even if we would make it a hard dependency, the user still needs to load the bundle in the kernel.
i think we should have an options array in the constructor to choose whether route parameters should be editable. then the DI extension can check whether the keyvaluebundle is loaded and complain if its not. the seo bundle form type does this, see the form and the DI extension and configuration
can you please add the configuration for this?
There was a problem hiding this comment.
@BadPixxel any chances to get this one ready. Or shall we finish it?
There was a problem hiding this comment.
Thansk for you feedback. I'll do it, just need time.
Question... shall we filter only route parameters or whole route configuration options?
There was a problem hiding this comment.
i would only make the route parameters optional. the rest works without the key-value bundle
|
I finaly used cmf_sonata_phpcr_admin_integration.menu.extension.menu_options.advanced as it already check presence of burgov/key-value-form-bundle on DI |
|
the problem with this is that the configuration is specifically about the menu options extension. i think we need to have a separate config option for this. but we can refactor MenuAdminFactory to have a method to check availability of the burgov bundle |
…ect BurgovKeyValueForm Bundle.
| @@ -34,6 +34,11 @@ | |||
| protected $menuRoot; | |||
|
|
|||
| /** | |||
| * @var bool | |||
| */ | |||
| protected $useBurgovKeyValueForm; | |||
There was a problem hiding this comment.
why is this on the base class? it is only used in one place, lets move it to the specific class until we need it in more places.
| @@ -38,6 +38,7 @@ public function addConfiguration(NodeBuilder $builder) | |||
| { | |||
| $builder | |||
| ->booleanNode('recursive_breadcrumbs')->defaultTrue()->end() | |||
| ->booleanNode('use_burgov_keyvalue_form')->defaultFalse()->end() | |||
There was a problem hiding this comment.
plesae make this a true/false/auto option node like in the seo configuration
| { | ||
| $bundles = $container->getParameter('kernel.bundles'); | ||
|
|
||
| $container->setParameter('cmf_sonata_phpcr_admin_integration.menu.use_burgov_keyvalue_form', isset($bundles['BurgovKeyValueFormBundle']) ); |
There was a problem hiding this comment.
this ignores the value set in the configuration. please use the logic as in the SEO DI extension to check for the bundle if the value is auto, and fail if set to true and burgov is missing
Added support of routes parameters in Menu Nodes Admin Page.
