Adding support for weight item option.

[akerouanton]

See #97.

Usage:

<?php

$factory = new MenuFactory();

$menu = $factory->createItem('My menu');
$menu->addChild('Home', array('uri' => '/', array('extras' => array('weight' => 10))));
$menu->addChild('Comments', array('uri' => '#comments'));
$menu->addChild('Symfony2', array('uri' => 'http://symfony-reloaded.org/'));
$menu->addChild('Coming soon', array('extras' => array('weight' => 10))));

$renderer = new SortingRenderer(new ListRenderer(new \Knp\Menu\Matcher\Matcher()), new MenuManipulator());
echo $renderer->render($menu);

[stof]

Given that the sorting is handled entirely outside the menu item, I'm wondering whether we actually need to make the weird a first-class attribute, which would be ignored most of the time. Storing it in the extras may be enough, allowing to have the feature built on top of the existing library.

[stof]

btw, @NiR-, it would be good to use a personal fork when sending PRs, to avoid duplicate Travis builds (for your branch and for the PR) and extra version polluting Packagist

[akerouanton]

@stof I will do it next time. I have moved the weight into extras options.

[Koc]

What about name it like priority?

[Nek-]

Hello,

First thank you @NiR- for the effort.

But sorry I'm 👎 with this implementation because to me it should not be a specific renderer that sort items. Why the twig renderer could not have them sorted ? (or any other renderer)
I mean, this is a feature of the menu item, not of a renderer.

To me, the entity that should handle that is the factory. But as in KnpMenu the menu item build itself the main part, it make it complicated to manage with a factory (or imply many changes in the way we create new children of the menu item).

The final solution and easiest is to put that treatment directly inside the MenuItem. If the weight is not precised this will not have any impact on the user default sort.

Btw I'm fighting myself for not changing the current factory system (that would potentially add many code). But what do you think about this solution ?

[EDIT] My bad I didn't saw that's a Decorator. Well so not a Renderer, isn't it ? But then why not. But that's a new concept in KnpMenu that need adaptations in dependencies. (KnpMenuBundle and others for cake php, and laravel)
I still would like to compare this one with another implementation :) .

@Koc about the name « priority », I also though about that, and I think there's no real « priority » here as we are speaking about tree and leaf... But if many people prefere « priority », why not. I really don't care..

[akerouanton]

@Nek- Yeah, SortingRenderer is surely a confusing name. Maybe SortingRendererDecorator ?

Comparing both implementations, I dislike the idea to put sorting into MenuItem because sorting will be done each time an item is added (and it should require to do the same at each item update).
Also, IMHO sorting is a rendering-only task and it should not become part of the tree manipulation. Except that, the sorting algorithm (stable sorting) used is the same but impl differs.

[lsmith77]

should be added to the 2.1 milestone I guess?

[Nek-]

@lsmith77 the related issue is already in the milestone.

@NiR- what about modifying the factory ? I mean, a decorator that should be systematically called is weird isn't it ?

Btw I'm for the new name that looks good to me.

[Nek-]

@NiR- updated your topic to have a better view of how it works. And asking for vote here: https://gitter.im/KnpLabs/KnpMenu

[dbu]

are #209 and this one different attempts to solve the same issue?

[akerouanton]

@dbu Yep there are. But it seems that neither will be merged. stof would like to give a try to another way to do it (using different models for building & rendering). See https://gitter.im/KnpLabs/KnpMenu ;)

[bobmulder]

I am looking for this option. Why isn't it merged? (aug 27 2015!!!)

[dbu]

@bobmulder likely because nobody is payed to work on this library. so priorities are defined by what the maintainers need and by contributions that are easy to merge and maintain.

this particular PR (and #209 as well) expand the scope of the library by adding another use case. this would also make the library more complex to use and understand and develop. maybe the best solution would be that you build a separate repository with the necessary classes - it looks like these things don't need changes to the library classes themselves.

[akerouanton]

I guess we're going nowhere in the near future on this issue/PR. As outlined during the discussion, it doesn't need to be part of KnpMenu at all. Because of this I finally released this decorator as a standalone library: nir/knp-menu-sorter.

Thank you everyone for your reviews/comments.

Cheers!