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

Displaying menu for a certain level in the tree #26

Closed
petesiss opened this issue Feb 6, 2012 · 33 comments
Closed

Displaying menu for a certain level in the tree #26

petesiss opened this issue Feb 6, 2012 · 33 comments

Comments

@petesiss
Copy link

petesiss commented Feb 6, 2012

No matter what level the current page is, I am displaying all the level 1 items across the top of the page, then all the level 2 items in a separate LHS col menu.

This doesn't seem to be straight forward as there is no helper to render a menu at a certain level, and no means to easily return e.g. the level 2 ancestor for the current page, in order to use this to create the menu.

Currently in order to achieve this, I am working up the provided breadCrumb array and testing for the level at each point, only rendering the LHS menu when I find the ancestor with the required level.

This is really messy in twig... clearly not the right way to go about it.

So I am wondering about whether to add methods to the menuItems class to return information regarding ancestors.

e.g. would it be useful to have

/**
 * @param int $level     The level of the ancestor to be returned
 * @return \Knp\Menu\ItemInterface
 */
public function getAncestor($level)
{
}

Any thoughts?

Is there already an accepted way to do this kind of thing?

@petesiss
Copy link
Author

petesiss commented Feb 8, 2012

This would also need a companion 'getFullPath' method, as for any page lower than level1 you need to pass the path elements as an array to use it as the root of your rendered menu.

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

@petesiss to render only the first level of the form, pass the depth option when rendering the form (with a value 1).

Can you explain what you mean about rendering all level 2 items separately ? Do you want to render N different menus (one for each level 2 item) ?

@petesiss
Copy link
Author

Yes - when a first level page is selected, then its second level (and deeper) children should appear in a separate menu.

Maybe an example will help. Take a simple site structure like

Home
About Us
    - Location
    - Ethos
Meet the team
    - Person 1
    - Person 2

The 1st level menu across the top is

Home | About Us | Meet the Team

So click on Meet the Team in the top, and then on that page the separate left column menu becomes a menu of person1 person2 etc.

Click on About Us in the top, and then on the About Us page the left menu shows the 2nd level items location and ethos, and any children of those items.

So I think there should be a way to set the starting depth of the menu, as well as the number of depth levels to include in the menu.

Our actual use case is a site that reuses just a few page templates across a large number of content and app pages. The actual menu use is a top horizontal menu above the header (showing root node and level 1), a second horizontal menu below the header (showing the level 2 items of the selected level 1 item) and a further menu in the left col which shows the level 3 items and any other deeper children for the selected level 2 item.

It doesn't seem possible to set this up currently.

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

Well, but on a given page, are you rendering all level 2 menus (as you said in the first sentence of the issue) or only one of them ?

@petesiss
Copy link
Author

I meant all the level 2 items for the selected level 1. Sorry - I can see it wasn't clear.

E.g. If I am on a level 4 page, I want to look back up the ancestors till I get to level 2, then render a menu of that level 2 element and all of its siblings.

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

The only thing you need is to be able to know the corresponding level1 item and render it (what is rendered is an item, and this root only displays its children)

{# option 1: get the menu by hand by traversing the tree #}
{% set root = knp_menu_get('main') %}
{% set menu = root['about'] %}  {# the attribute() function could be used to get it according to a variable #}
{{ knp_menu_render(menu) }}

{# option 2: get the menu by path. 
See https://github.com/KnpLabs/KnpMenu/blob/master/doc/02-Twig-Integration.markdown
#}
{% set menu = knp_menu_get('main', ['about']) %}
{{ knp_menu_render(menu) }}


{# option 3: get the menu by path directly in the render call. #}
{{ knp_menu_render(['main', 'about']) }}

My example assumes that the name of the "About us" item is about

@petesiss
Copy link
Author

Well, I can see the different ways of passing path information in to the menu, but the question is how you get that path information so nicely in your twig template, if you are using and reusing your templates site-wide.

All three of those options do require me to know the path to my level N ancestor. So that's the question that I think sums this up:

How do you get the path to your current pages Level N ancestor?

Because you need this information, one way or the other, in order to render a standalone menu at that level N.

That is why I thought we might need a "getAncestor($level)" or "getAncestorPath($level)".

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

why getAncestor($level) ? This would assume you know the deep item, whereas what you can get easily in a template is the root item

@petesiss
Copy link
Author

Hm. Well yes I can easily get the root item, but I am not sure how does that help? As shown in all the menu options you demonstrated above I need to get the path for the level N-1 ancestor of my current page in order to display a stand-alone menu for that level N.

So in my template I have to get the menuItem object of the current page, check if it has level >= N, then if so I need to get the path to the level N-1 menuItem that is my current pages ancestor, so that I can use it as the root in my rendered menu.

How would you do this then?

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

My point is that I don't see how getAncestor would help you here as it would assume first that you are able to get the deep item (and so you need to know the path to this deep item)

@petesiss
Copy link
Author

Ah, ok. Well I had thought something like:

/**
 * @param int $level     The level of the ancestor to be returned
 * @return \Knp\Menu\ItemInterface
 */
public function getAncestor($level = null)
{
    if ($level) {
        $obj = $this;

        do {
            if($obj->getLevel() == $level) {
                return $obj;
            }   
        } while ($obj = $obj->getParent());
    }

    return false; 
}

Then in twig:

{% set currentItem = knp_menu_get('main').currentItem %}

{% set sideNav = currentItem.ancestor(2) %}

{{ knp_menu_render(sideNav) }}

I haven't tested any of the above. It's just a 'for instance'.

[edit : corrected obvious error in the twig syntax]

@stof
Copy link
Collaborator

stof commented Feb 13, 2012

Look at #32. getCurrentItem is lying and will probably be removed because of this (see also the related comments in previous discussion)

@petesiss
Copy link
Author

Didnt want to edit my post again - but the render wont work as it needs the path. But still - hope you can see the idea. getAncestor could return the path rather than the item object for instance.

@petesiss
Copy link
Author

Right... Well that wont work then if you cant use the currentItem.

Any other thoughts on how to achieve this? Or maybe then it needs holding till the question of how to properly get the current item is answered.

@petesiss
Copy link
Author

@stof would you consider a PR for the getAncestor($level) method now? getCurrentItem() can just be replaced with getOneCurrentItem() if / when that is implemented. Either way it remains the lib users responsibility to ensure it will return the required current item.

My reasoning is that it currently requires too much code in the template to loop up through ancestors in order to render a menu at a certain level - especially in a twig template it does not feel like the right solution.

But if we can fetch a level N ancestor of the current menuItem then it becomes trivial again. I think this is a common use case.

@stof
Copy link
Collaborator

stof commented Feb 23, 2012

I don't like getAncestor. Such method would need to return null if you require a higher level and would be quite inefficient: finding the level requires looping until the root each time (I don't realy like getLevel either btw)

@petesiss
Copy link
Author

Ok. Well at the moment, with no getAncestor method, I am doing exactly this but with twig in the template, which I am sure is even worse. So what is the alternative way to achieve the same result (re the original question of always displaying a level N menu in the template) ?

If the problem is just about performance, maybe we could use similar to getPathAsString to build the full path, then take the N segments to get the item at level N in one go.

Alternatively could use getLevelonly once by calling it on the current item, and then keep a separate counter of getParent iterations till we know we are on the required level.

@petesiss
Copy link
Author

Are there any thoughts on the best way to implement this now that the current matching has been reworked?

We are still using a very old commit of the lib with a getAncestor($level) method added to facilitate getting a node at a certain depth to render the menu from.

@dbu
Copy link
Collaborator

dbu commented Oct 10, 2012

i think for the cmf we will do something so that we can just get the menu item for the current content and then do things on that.

@peterrehm
Copy link

@stof What is then as of today the most efficient way to handle such two different menus in a single layout? I guess this is a common request

@webda2l
Copy link

webda2l commented Dec 4, 2012

A quick tips that I done is to include the path of the ancestor, directly in the name of your menuItem as "menuItemNameOfLevel0_menuItemNameOfLevel1_menuItemNameOfLevel2_...".
You have now easily ancestors and levels for your needs.

@peterrehm
Copy link

@webda2l But the issue if you seperate the mainmenu which contains only level 1 from the submenu which contains the sublevels, I dont see how your suggestion might help me

@ghost
Copy link

ghost commented Dec 29, 2012

how is one supposed to get the current item? The CurrentItemFilterIterator?

I found some fleshed out sample code here: KnpLabs/KnpMenuBundle#122 (comment)

The only thing i'm not completely sure of is how to strip the top most root after retrieving the current item and use that as the new root.

@sheeep
Copy link

sheeep commented Jan 8, 2013

@peterrehm Have you managed to solve your issue? And if yes: how?

@peterrehm
Copy link

@sheeep No, if you find out I am interested too.

@Mopster
Copy link

Mopster commented Jan 28, 2013

I'm not sure if it's the exact solution to your problem. But I had a similar problem (if I read this issue correctly). I have created a tree menu and only wanted to show part of the menu. The children of the active level one menu item. I did the following to resolve my problem :

I created 2 Twig extensions, one to get the current item, the second to get the level 1 menu item i needed :

use ArrayIterator;
use Knp\Menu\ItemInterface;
use Knp\Menu\Iterator\CurrentItemFilterIterator;
use Knp\Menu\Iterator\RecursiveItemIterator;
use Knp\Menu\Matcher\Matcher;

class MenuTwigExtension extends \Twig_Extension
{
    private $matcher;

    public function __construct(Matcher $matcher)
    {
        $this->matcher = $matcher;
    }

    /**
     * Get Twig functions defined in this extension.
     *
     * @return array
     */
    public function getFunctions()
    {
        return array(
            'admin_menu_current'  => new \Twig_Function_Method($this, 'getCurrentMenuItem'),
            'admin_menu_top'  => new \Twig_Function_Method($this, 'getTopMenuItem'),
        );
    }

    /**
     * Returns the current ItemMenu from given Menu
     * @param ItemInterface $menu
     *
     * @return string
     */
    public function getCurrentMenuItem(ItemInterface $menu)
    {
        $treeIterator = new RecursiveIteratorIterator(
            new RecursiveItemIterator(
                new ArrayIterator(array($menu))
            ),
            RecursiveIteratorIterator::CHILD_FIRST
        );

        $iterator = new CurrentItemFilterIterator($treeIterator, $this->matcher);

        $array = array();
        foreach ($iterator as $item) {
            $array[] = $item;
        }

        if (!empty($array)) {
            return $array[0];
        }

        return null;
    }

    /**
     * Returns the top MenuItem from given Menu
     *
     * @param ItemInterface $menu
     * @param int $level
     *
     * @return string
     */
    public function getTopMenuItem(ItemInterface $menu, $level)
    {
        $item = $this->getCurrentMenuItem($menu);
        $breadcrumbs = $item->getBreadcrumbsArray();

        return $breadcrumbs[$level];
    }

    /**
     * Get the Twig extension name.
     *
     * @return string
     */
    public function getName()
    {
        return 'adminmenu_twig_extension';
    }
}

In my twig template I used the following :

{% set menu = knp_menu_get('menu') %}
{% set topmenuitem = admin_menu_top(menu, 1) %}
{{ knp_menu_render([menu, topmenuitem['label']], { 'template': 'KunstmaanAdminBundle:Menu:knp_menu_treemenu.html.twig'}, 'bootstrap') }}

I hope this gets you a step closer to what you wanted.

@peterrehm
Copy link

@Mopster Thank you for the information, it does not totally suit my demand - since I was able to I just changed the menu structure to avoid this issue, with the next project I will dig into this again.

@Nek- Nek- closed this as completed Jan 31, 2014
@petesiss
Copy link
Author

Is this now possible?

@Nek-
Copy link
Contributor

Nek- commented Jan 31, 2014

We have to do something on this way for breadcrumb. Another issue will be write.

This issue is v1 related, so off.

@dbu
Copy link
Collaborator

dbu commented Jan 31, 2014

hi @Nek-
great to see some activity on knp menu again!
is there a plan when 2.0 should be out? if we can coordinate with the CmfMenuBundle that would be great.
cheers,david

@Nek-
Copy link
Contributor

Nek- commented Jan 31, 2014

Hi, for now we can't give any date for release, there is so much work to do (not all issues are written for know).

But i'm going to try making things goes quicker.

@Nielsb85
Copy link

@petesiss , for days I am trying to find a way of doing this. Have you figured out a way creating the structure this way? I would really like to know.

@marcel-burkhard
Copy link

@Nielsb85 maybe I can help you.

https://github.com/burki94/RecursiveMenuBundle/blob/master/README.md

Otherwise look for "matchingDepth" in the docs, they introduced that at some point this year.

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

10 participants