Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

allow defaults for the options #128

Closed
dbu opened this issue Sep 4, 2013 · 7 comments
Closed

allow defaults for the options #128

dbu opened this issue Sep 4, 2013 · 7 comments
Milestone

Comments

@dbu
Copy link
Member

dbu commented Sep 4, 2013

we could allow to configure defaults for things like childrenAttributes on the ContentAwareFactory. in knp menu, those must be set on the objects, but as menu items are created programmatically, that is ok. for the cmf however this would mean storing data into the database even if it is a default.

i would draw the line at saying we can configure defaults for the things mapped in phpcr that will be applied to all menu items being created if they do not themselves provide that option. any logic to only apply the defaults to some menu items will require to extend the factory in your project, otherwise it gets too complex imo.

alternatively we could think of having MenuItemEnhancer like we have RouteEnhancer...

@dbu
Copy link
Member Author

dbu commented Sep 4, 2013

/cc @elHornair

@kminh
Copy link

kminh commented Jan 2, 2015

Would like to second this as it makes this bundle more user-friendly.

@dbu
Copy link
Member Author

dbu commented Jan 4, 2015

this should probably build on #214 as the way items are created from the nodes is refactored with knp 2.0

i think you could propose a Extension in https://github.com/KnpLabs/KnpMenu/tree/master/src/Knp/Menu/Factory that accepts an array of defaults and maybe a merge policy (keep existing or overwrite, defaulting to keep existing options) and implements buildOptions to add the default options that are not set in $options. that extension can then be configured as service in KnpMenuBundle and have configuration in knp_menu

@lsmith77 lsmith77 added this to the 2.0 milestone Jun 17, 2015
@dbu dbu added the nicetohave label Aug 21, 2015
@wouterj
Copy link
Member

wouterj commented Aug 27, 2015

👍 does it make sense to add this to the KnpMenuBundle or this bundle?

@dbu
Copy link
Member Author

dbu commented Aug 27, 2015

i think this should go to KnpMenu, its not a specific need for the cmf

@wouterj
Copy link
Member

wouterj commented Aug 27, 2015

See KnpLabs/KnpMenuBundle#278

@dbu
Copy link
Member Author

dbu commented Aug 28, 2015

thanks

@dbu dbu closed this as completed Aug 28, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants