symfony-cmf / simple-cms-bundle

UNMAINTAINED - A more-than-simple cms based on the cmf components
http://cmf.symfony.com/
49 stars 45 forks source link

Allow to configure menu options #114

Closed dbu closed 10 years ago

dbu commented 10 years ago

E.g. allow to set the display flag. Probably in a new group. We could also try to move the "Advanced" group into a "Advanced URL Options" group.

benglass commented 10 years ago

We would also want to allow management of all of the menu related options which are described in the docs

mkoosej commented 10 years ago

I'm building this into the admin class. What is your suggestion for all the attributes fields? it's too broad and I think we have to be more specific of what we want to expose to user. Each attributes field can be a list of key/value items if we want to let user have full control.

benglass commented 10 years ago

Since these are html attributes i dont think its realistic for us to try to constrain the possible values so we want to just allow key/value pair with add/delete. I believe there is already field type functionality for this, see

https://github.com/doctrine/DoctrinePHPCRBundle/pull/120 https://github.com/Burgov/KeyValueFormBundle/

@dbu is it problematic to provide admin functionality to edit these attribute fields using the key value form type which seems to be optional?

dbu commented 10 years ago

the html attributes things? it seems we do not allow to edit them in the menu admin https://github.com/symfony-cmf/MenuBundle/blob/master/Admin/AbstractMenuNodeAdmin.php - its a bit of an edge case if we even want to expose that to an admin - they are only useful together with css code.

but then again, one can always extend this admin class and remove the fields, or build ones own admin based on this, so if you want to add them i am ok with that. the burgov key value seems ideal for that. just make sure we don't create a hard dependency on the burgov bundle.

maybe have a configuration option for the admin whether those should be editable with true or false. when set to true, check in the DI class if burgov is available and fail early otherwise. something like

if ($config['sonata_admin']['edit_attributes']) {
    $bundles = $container->getParameter('kernel.bundles');
    if (!isset($bundles['BurgovKeyValueFormBundle'])) {
        throw new InvalidConfigurationException('To edit menu attributes, you need the burgov/key-value-bundle in your project.');
    }
    ... set the parameter accordingly
dbu commented 10 years ago

and lets default edit_attributes to false.

if we get that nice, we could also port it to the MenuBundle then, its the same thing there i assume.

benglass commented 10 years ago

Perhaps this would be a good use case for an admin extension which adds these fields?

MenuNodeHtmlAttributeExtension which could optionally be enabled. Then it could live in the MenuBundle and just be enablable for both bundles.

dbu commented 10 years ago

actually yes, sounds like an excellent idea.

mkoosej commented 10 years ago

I'm building the extension for the menu bundle. I assume I have to add the service definition to admin.xml file of the menu bundle. What is the recommended way of make it optionally enabled. Is it something the user should manage in their own config.yml file?

benglass commented 10 years ago

@mkoosej you will want to allow users to enable/disable this feature via configuration, similar to the way that CoreBundle implements publish workflow. There is a boolean config option for enabling it that triggers the extension to load the xml file with the admin extension definition.

https://github.com/symfony-cmf/CoreBundle/blob/master/DependencyInjection/CmfCoreExtension.php#L268

https://github.com/symfony-cmf/CoreBundle/blob/93ea17fd833ae044714e6cb3a73d9a80e814dbb5/Resources/config/publish-workflow.xml

The user is responsible for enabling the extension for the correct admins in their config.yml but instructions are added to the docs

http://symfony.com/doc/current/cmf/bundles/core/publish_workflow.html#editing-publication-information-publish-workflow-sonata-admin-extension

benglass commented 10 years ago

We should probably create an interface which the subject classes can implement to take advantage of these methods. This method should encapsulate the shared publish methods which MenuBundle Menus and SimpleCmsPage Page's share and be called something like MenuOptionsInterface

Then the user can apply the admin based on the interface or using any of the other sonata admin methods but it also create a contract for other classes that might want to provide menu option information.

http://sonata-project.org/bundles/admin/master/doc/reference/extensions.html#configuration

dbu commented 10 years ago

exactly what we should do, yes.

mkoosej commented 10 years ago

symfony-cmf/MenuBundle#197 I submitted the PR for the menuBundle. The same extension works fine with SimpleCms page node when it implements MenuOptionsInterface. I'll merge the SimpleCms part if you think the extension looks good.

dbu commented 10 years ago

fixed in #120