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

Add weight option #209

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
}
],
"require": {
"php": ">=5.3.0"
"php": ">=5.3.0",
"vanderlee/php-stable-sort-functions": "~1.0"
},
"require-dev": {
"pimple/pimple": "1.0.*",
Expand Down
15 changes: 15 additions & 0 deletions doc/01-Basic-Menus.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,21 @@ $renderer->render($menu);
Using the above controls, you can specify exactly which part of your menu
you need to render at any given time.

### Rendering with sorting by weight

The renderer with the option weight abides by the menu levels.

The default weight is 0. Positive weight are prioritized over negative (i.e : 1, 0, -1 is the order)

```php
<?php
// the first way
$menu->addChild('Home', array('weight' => -10));

// the second way
$menu['Home']->setWeight(-10);
```

### Other rendering options

Most renderers also support several other options, which can be passed as
Expand Down
2 changes: 2 additions & 0 deletions src/Knp/Menu/Factory/CoreExtension.php
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ public function buildOptions(array $options)
'current' => null,
'display' => true,
'displayChildren' => true,
'weight' => 0,
),
$options
);
Expand All @@ -53,6 +54,7 @@ public function buildItem(ItemInterface $item, array $options)
->setCurrent($options['current'])
->setDisplay($options['display'])
->setDisplayChildren($options['displayChildren'])
->setWeight($options['weight'])
;

$this->buildExtras($item, $options);
Expand Down
15 changes: 15 additions & 0 deletions src/Knp/Menu/ItemInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -451,4 +451,19 @@ public function actsLikeFirst();
* @return boolean
*/
public function actsLikeLast();

/**
* Define the position of the element in the menu in function level in the tree
* It can be positive or negative
*
* @param int $weight
*
* @return ItemInterface
*/
public function setWeight($weight);

/**
* @return int
*/
public function getWeight();
}
49 changes: 49 additions & 0 deletions src/Knp/Menu/MenuItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ class MenuItem implements ItemInterface
*/
protected $factory;

/**
* @var int
*/
protected $weight = 0;

/**
* Class constructor
*
Expand Down Expand Up @@ -338,6 +343,8 @@ public function addChild($child, array $options = array())

$this->children[$child->getName()] = $child;

$this->setChildren($this->applySortByWeight($this->children));

return $child;
}

Expand Down Expand Up @@ -593,4 +600,46 @@ public function offsetUnset($name)
{
$this->removeChild($name);
}

/**
* @param int $weight
*
* @return $this
*/
public function setWeight($weight)
{
$this->weight = $weight;

return $this;
}

/**
* @return int
*/
public function getWeight()
{
return $this->weight;
}

/**
* @param array <ItemInterface> $children
*
* @return array <ItemInterface>
*
* @throws \RuntimeException
*/
public function applySortByWeight(array $children)
{
if (!suasort($children, function(ItemInterface $itemA, ItemInterface $itemB) {
if ($itemA->getWeight() === $itemB->getWeight()) {
return 0;
}

return $itemA->getWeight() > $itemB->getWeight() ? -1 : 1;
})) {
throw new \RuntimeException('unable to sort those items by weight');
}

return $children;
}
}
1 change: 1 addition & 0 deletions src/Knp/Menu/Util/MenuManipulator.php
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ public function toArray(ItemInterface $item, $depth = null)
'extras' => $item->getExtras(),
'display' => $item->isDisplayed(),
'displayChildren' => $item->getDisplayChildren(),
'weight' => $item->getWeight(),
'current' => $item->isCurrent(),
);

Expand Down
10 changes: 1 addition & 9 deletions tests/Knp/Menu/Tests/Iterator/IteratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,7 @@ public function testIterator()

public function testRecursiveIterator()
{
// Adding an item which does not provide a RecursiveIterator to be sure it works properly.
$child = $this->getMock('Knp\Menu\ItemInterface');
$child->expects($this->any())
->method('getName')
->will($this->returnValue('Foo'));
$child->expects($this->any())
->method('getIterator')
->will($this->returnValue(new \EmptyIterator()));
$this->menu->addChild($child);
$this->menu->addChild('Foo');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you are killing the purpose of the test here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the sort on addChild, this test become impossible because prophecy use exceptions and php don't support it while sorting (it throw a warning and make tests fail).

https://bugs.php.net/bug.php?id=50688

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but the test is now useless. So this is the wrong way to fix the test.


$names = array();
foreach (new \RecursiveIteratorIterator(new RecursiveItemIterator($this->menu), \RecursiveIteratorIterator::SELF_FIRST) as $value) {
Expand Down
55 changes: 41 additions & 14 deletions tests/Knp/Menu/Tests/Util/MenuManipulatorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -186,20 +186,7 @@ public function testCallRecursively()
$menu = $factory->createItem('test_menu');

foreach (range(1, 2) as $i) {
$child = $this->getMock('Knp\Menu\ItemInterface');
$child->expects($this->any())
->method('getName')
->will($this->returnValue('Child '.$i))
;
$child->expects($this->once())
->method('setDisplay')
->with(false)
;
$child->expects($this->once())
->method('getChildren')
->will($this->returnValue(array()))
;
$menu->addChild($child);
$menu->addChild('Child '.$i, array('display' => false));
}

$manipulator = new MenuManipulator();
Expand Down Expand Up @@ -236,6 +223,7 @@ public function testToArrayWithChildren()
'display' => true,
'displayChildren' => true,
'current' => null,
'weight' => 0,
'children' => array(
'jack' => array(
'name' => 'jack',
Expand All @@ -249,6 +237,7 @@ public function testToArrayWithChildren()
'display' => false,
'displayChildren' => true,
'current' => null,
'weight' => 0,
'children' => array(
'john' => array(
'name' => 'john',
Expand All @@ -263,6 +252,7 @@ public function testToArrayWithChildren()
'displayChildren' => true,
'children' => array(),
'current' => true,
'weight' => 0,
),
),
),
Expand All @@ -279,6 +269,7 @@ public function testToArrayWithChildren()
'displayChildren' => false,
'children' => array(),
'current' => false,
'weight' => 0,
),
),
),
Expand Down Expand Up @@ -309,6 +300,7 @@ public function testToArrayWithLimitedChildren()
'display' => true,
'displayChildren' => true,
'current' => null,
'weight' => 0,
'children' => array(
'jack' => array(
'name' => 'jack',
Expand All @@ -322,6 +314,7 @@ public function testToArrayWithLimitedChildren()
'display' => false,
'displayChildren' => true,
'current' => null,
'weight' => 0,
),
'joe' => array(
'name' => 'joe',
Expand All @@ -335,6 +328,7 @@ public function testToArrayWithLimitedChildren()
'display' => true,
'displayChildren' => false,
'current' => null,
'weight' => 0,
),
),
),
Expand Down Expand Up @@ -363,6 +357,7 @@ public function testToArrayWithoutChildren()
'display' => true,
'displayChildren' => true,
'current' => null,
'weight' => 0,
),
$manipulator->toArray($menu, 0)
);
Expand All @@ -383,4 +378,36 @@ private function createMenu($name = 'test_menu', $uri = 'homepage', array $attri

return $factory->createItem($name, array('attributes' => $attributes, 'uri' => $uri));
}

public function testWeightOption()
{
// test stable sort with same weight
$menu = new MenuItem('root', new MenuFactory());
$menu->addChild('c1', array('weight' => 10));
$menu->addChild('c2', array('weight' => 10));
$menu->addChild('c3', array('weight' => 10));
$menu->addChild('c4', array('weight' => 10));
$menu->addChild('c5', array('weight' => 10));

$this->assertEquals(array('c1', 'c2', 'c3', 'c4', 'c5'), array_keys($menu->getChildren()));


// test juste one level
$menu = new MenuItem('root', new MenuFactory());
$c1 = $menu->addChild('c1', array('weight' => 50));
$menu->addChild('c2', array('weight' => 1));
$menu->addChild('c3', array('weight' => -10));
$menu->addChild('c4', array('weight' => 5));

$this->assertEquals(array('c1', 'c4', 'c2', 'c3'), array_keys($menu->getChildren()));

// test with add a second level in the menu
$c1->addChild('c11', array('weight' => -20));
$c1->addChild('c12', array('weight' => -5));
$c1->addChild('c13', array('weight' => 20));
$c1->addChild('c14', array('weight' => 1));

$this->assertEquals(array('c1', 'c4', 'c2', 'c3'), array_keys($menu->getChildren())); // must not change
$this->assertEquals(array('c13', 'c14', 'c12', 'c11'), array_keys($c1->getChildren()));
}
}