KnpLabs / KnpMenuBundle

Object Oriented menus for your Symfony project.
http://knplabs.com
MIT License
1.4k stars 203 forks source link

Add Documentation about default twig renderer options #262

Open h4cc opened 9 years ago

h4cc commented 9 years ago

I am using Twitter Bootstrap and needed to change the "currentClass" to "active" for my whole site and found this quite easy way to do so:

# config.yml

parameters:
    knp_menu.renderer.twig.options:
        currentClass: 'active'

I think it would be cool to have such a info present in the documentation.

ajanssens commented 9 years ago

Thanks. You should create a pull request to add this. Very helpful !

wouterj commented 9 years ago

If this is really going to be a documented feature, it should be included in the bundle's config & set by the extension imo. You should never have to change bundle parameters

dbu commented 9 years ago

i think having such css classes in configuration would indeed be a good idea. css frameworks tend to need a specific class for that, not easy to remap on css side afaik.

h4cc commented 9 years ago

I dont need that value to be changes as container parameter. My problem was that simply changeing that common class was very hard to find.

dbu commented 9 years ago

the problem with handling that over such a parameter is that its very implicit. it does not show when you dump the configuration options, and if you mistype it, you never get any notification of invalid configuration, but it simply does not work.

xDaizu commented 8 years ago

@dbu I don't think such a feature being good or bad design is the question at hand. That would be a whole other interesting issue to debate.

I think that the idea of this "issue" is that, if exists, it should be documented. We have some serious "documentation lacking" problems in this bundle, because the KnpMenu documentation doesn't cover tons of Symfony-specific use cases, and the "Symfony documentation" link is very outdated/brief.

So +1. +1 and all the knights at my service to documenting every configuration option available.

dbu commented 8 years ago

i am not saying its wrong to document this. i am just saying that even better would be to build a proper solution that is safe against typo and will show up in the config dump console command for example. right now you need to dig in the code to find that parameter could exist, with the fix you could look at the Configuration class.

but for either, go for it, send your knights to do pull requests to improve the documentation and the bundle ;-)

xDaizu commented 8 years ago

@dbu I agree that would be better. Could we have that? Are your Coding Knights free or do we have to make do with what we have? What are the views of the bundle designers about this? Do we put our effort into it? Or do we document what we currently have?

As for my knights, by now they're busy retrieving information and building the Compendium of many Languages and keeping the supply line of the Caffeinated Substance of Many Colours, but as soon as they finish....