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

[WIP] Breadcrumb renderer #161

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

Nek-
Copy link
Contributor

@Nek- Nek- commented Jun 24, 2014

This PR could be in 2.1 if it doesn't pass before the end of the beta.

And she needs:

Problem: How to retrieve the current item without parsing the complete tree menu.

@Nek- Nek- changed the title Breadcrumb renderer [WIP] Breadcrumb renderer Jun 24, 2014
@Nek-
Copy link
Contributor Author

Nek- commented Jun 25, 2014

The way to do a Breadcrumb for now:

// Random example
$menu = $menuFactory->createItem('Homepage');

// Getting the last node aka "new message" here
$endOfTree = $menu->addChild('Forum')->addChild('Topic')->addChild('New Message');

$manipulator->getBreadcrumbsArray($endOfTree);

That mean that you need to have a direct access to the last node of a menu.

2 things are wrong in that sentence:

  • Having access to the last node implies that the manipulator will ask getParent to get all the tree and it's not at all the best way to through the tree.
  • It means also that you have an access to this node and not the whole menu.

New POV proposal on this feature:

  • Writing a new type of Item: a breadcrumb should not be a tree, it's too complexe for the thing.
  • Wrting a factory as well
  • Writing php & twig renderers

The usage would be quite similar.

<?php

$breadcrumb = $breadcrumbFactory->createItem('Homepage');
$breadcrumb->addChild('Forum')->addChild('Topic')->addChild('New Message');

$manipulator->getBreadcrumbsArray($endOfTree);

Differences:

  • The breadcrumb item return itself;
  • The children are added in top of the children array.

Limitations:

  • There is not a tree but more a simple object containing a single level of items.
  • There will never be more than one breadcrumb in the tree

Anything against this proposal ? I will be working on after some guys tell me what they think about.

@stof
Copy link
Collaborator

stof commented Jun 25, 2014

@Nek- The goal of this breadcrumb was to build the breadcrumb automatically based on the same menu you use for the navigation elsewhere. The breadcrumb is the path in the tree to reach the current item (which can be determined using the voter system, if you don't have a more efficient way to find it).

Your proposal looks weird, because it propose to use the ItemInterface in a way which does not respect the interface (ItemInterface is designed as a tree). ->addChild() does not return itself. It returns the newly added child.
If you don't want to reuse the same menu tree than for the navigation, it is probably way easier not to use the menu system to build the breadcrumb (which is indeed a list, not a tree).

2 things are wrong in that sentence:

  • Having access to the last node implies that the manipulator will ask getParent to get all the tree and it's not at all the best way to through the tree.

Getting the parent of a menu item is efficient (if I was designing the library from scratch, it may be different as I would probably try to avoid the circular reference in the object graph, but this is off-topic). The issue is more about getting the current item than about moving up in the tree.

  • It means also that you have an access to this node and not the whole menu.

when rendering a given item, you always have access only to the item being rendered right now, not to the whole menu. The difference is the way you move to another item.

@Nek-
Copy link
Contributor Author

Nek- commented Jun 25, 2014

Thank you for explainations about the feature :) . (I never used it)

But you never render a menu by yourself. And in a menu render you never need to render a breadcrumb. So how do you get the node in another part of your website ?

I made a little sample application and it was not so easy to use the breadcrumb renderer because of I needed the last node.

Do you have an example ?

@wouterj
Copy link
Collaborator

wouterj commented Jun 7, 2015

Can we maybe move this forward? I'm trying to update the CMF sandbox to use KnpMenuBundle 2.0 (as it's now required by Sonata). I'm currently unable to transform the code we used to render the breadcrumb with KnpMenu 1 to KnpMenu 2.

At the moment, one would need to iterate over the tree and use the matcher to get the current item and pass it to knp_menu_get_breadcrumbs_array. The iteration can't be done in a template, so we need to do some crazy iterator stuff to get things working. This doesn't seem very correct...

@jonny827
Copy link

jonny827 commented May 19, 2016

Is this PR focusing on building out a breadcrumb that has links or just a string? I think getPathAsString() works quite well as I suggest in Issue #224 for a great string based breadcrumb.

@dbu
Copy link
Collaborator

dbu commented Jun 21, 2016

ping @Nek- @jonny827 does one of you want to pick this up?

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

Successfully merging this pull request may close these issues.

6 participants