Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Current automenu 'list-group' doesn't support link hover states. #1057

Closed
peshi opened this issue Mar 30, 2015 · 9 comments
Closed

Current automenu 'list-group' doesn't support link hover states. #1057

peshi opened this issue Mar 30, 2015 · 9 comments

Comments

@peshi
Copy link
Contributor

peshi commented Mar 30, 2015

There should be an option to be able to decorate automenu 'list-group' with div instead of ul in order to benefit from Bootstraps linked list items.

Ref: https://github.com/twbs/bootstrap-sass/blob/master/assets/stylesheets/bootstrap/_list-group.scss#L41

Example.

<div class="list-group">
   <a href="#" class="list-group-item">Link1</a>
   <a href="#" class="list-group-item active">Link2</a>
   <a href="#" class="list-group-item">Link3</a>
</div>
@peshi
Copy link
Contributor Author

peshi commented Mar 30, 2015

An example of how to accomplish the above

https://gist.github.com/peshi/82997acb75cd1107c959

{{ mopa_bootstrap_menu(['AcmeDemoBundle:Builder:sidebar', 'profile'], {'automenu': 'list-group'}) }}

@phiamo
Copy link
Owner

phiamo commented Apr 27, 2015

@peshi should we extend the template?

@peshi
Copy link
Contributor Author

peshi commented Apr 27, 2015

@phiamo I'll check so that it doesn't interfere with intended behaviour of automenu and create a PR.

@phiamo
Copy link
Owner

phiamo commented Apr 27, 2015

@peshi thanks a lot!

@phiamo
Copy link
Owner

phiamo commented May 29, 2015

@peshi did we already merge a pr for this ?

@peshi
Copy link
Contributor Author

peshi commented Jun 18, 2015

@phiamo Nope, that hasn't been merged, if I remember correctly we have to lift the 'list-group' from automenu and extend the template to replace <ul> for <div>.

@phiamo
Copy link
Owner

phiamo commented Jul 18, 2015

@peshi is there a PR

@peshi
Copy link
Contributor Author

peshi commented Aug 12, 2015

@phiamo Hi, I have a solution locally (twig based) - but it will replace the current implementation that is specified MenuDecorator and MenuConverter. I would like to get some input from the contributor of automenu regarding this issue, maybe there is a way not to use twig.

You could try the gist I pasted above.

@isometriks
Copy link
Collaborator

I had originally created the menu extension and then @phiamo created the auto menu I believe and moved the menu extension to a separate class so it could be used in a couple ways. I think Twig would be the best bet. If you check out: https://github.com/KnpLabs/KnpMenu/blob/master/src/Knp/Menu/Resources/views/knp_menu.html.twig#L25

We could override that block and change the ul to be a variable like list_element and then in the item block we could have like a child_element and be able to pass what it should be rendered as. This almost might be a better PR to KnpMenuBundle instead as we'd have to override that whole block. I don't think this can be done without changing the Twig template because the ul and li elements are hard coded in it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants