Skip to content

Commit 5eeb12f

Browse files
atrandafiratrandafir
authored
Fix #19060: Fix yii\widgets\Menu bug when using Closure for active item and adding additional tests in tests\framework\widgets\MenuTest 
1 parent a4b5856 commit 5eeb12f

File tree

3 files changed

+183
-5
lines changed

3 files changed

+183
-5
lines changed

‎framework/CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
44
2.0.50 under development
55
------------------------
66

7+
- Bug #19060: Fix `yii\widgets\Menu` bug when using Closure for active item and adding additional tests in `tests\framework\widgets\MenuTest` (atrandafir)
78
- Bug #13920: Fixed erroneous validation for specific cases (tim-fischer-maschinensucher)
89
- Bug #19927: Fixed `console\controllers\MessageController` when saving translations to database: fixed FK error when adding new string and language at the same time, checking/regenerating all missing messages and dropping messages for unused languages (atrandafir)
910
- Bug #20002: Fixed superfluous query on HEAD request in serializer (xicond)

‎framework/widgets/Menu.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,11 @@ protected function normalizeItems($items, &$active)
283283
$items[$i]['active'] = false;
284284
}
285285
} elseif ($item['active'] instanceof Closure) {
286-
$active = $items[$i]['active'] = call_user_func($item['active'], $item, $hasActiveChild, $this->isItemActive($item), $this);
286+
if (call_user_func($item['active'], $item, $hasActiveChild, $this->isItemActive($item), $this)) {
287+
$active = $items[$i]['active'] = true;
288+
} else {
289+
$items[$i]['active'] = false;
290+
}
287291
} elseif ($item['active']) {
288292
$active = true;
289293
}

‎tests/framework/widgets/MenuTest.php‎

Lines changed: 177 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,14 @@ class MenuTest extends \yiiunit\TestCase
1818
protected function setUp()
1919
{
2020
parent::setUp();
21-
$this->mockApplication();
21+
$this->mockWebApplication([
22+
'components'=>[
23+
'urlManager' => [
24+
'enablePrettyUrl' => true,
25+
'showScriptName' => false,
26+
],
27+
],
28+
]);
2229
}
2330

2431
public function testEncodeLabel()
@@ -201,6 +208,149 @@ public function testActiveItemClosure()
201208
$this->assertEqualsWithoutLE($expected, $output);
202209
}
203210

211+
public function testActiveItemClosureWithLogic()
212+
{
213+
$output = Menu::widget([
214+
'route' => 'test/logic',
215+
'params' => [],
216+
'linkTemplate' => '',
217+
'labelTemplate' => '',
218+
'items' => [
219+
[
220+
'label' => 'logic item',
221+
'url' => 'test/logic',
222+
'template' => 'label: {label}; url: {url}',
223+
'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
224+
return $widget->route === 'test/logic';
225+
},
226+
],
227+
[
228+
'label' => 'another item',
229+
'url' => 'test/another',
230+
'template' => 'label: {label}; url: {url}',
231+
]
232+
],
233+
]);
234+
235+
$expected = <<<'HTML'
236+
<ul><li class="active">label: logic item; url: test/logic</li>
237+
<li>label: another item; url: test/another</li></ul>
238+
HTML;
239+
240+
$this->assertEqualsWithoutLE($expected, $output);
241+
}
242+
243+
public function testActiveItemClosureWithLogicParent()
244+
{
245+
$output = Menu::widget([
246+
'route' => 'test/logic',
247+
'params' => [],
248+
'linkTemplate' => '',
249+
'labelTemplate' => '',
250+
'activateParents' => true,
251+
'items' => [
252+
[
253+
'label' => 'Home',
254+
'url' => 'test/home',
255+
'template' => 'label: {label}; url: {url}',
256+
],
257+
[
258+
'label' => 'About',
259+
'url' => 'test/about',
260+
'template' => 'label: {label}; url: {url}',
261+
],
262+
[
263+
'label' => 'Parent',
264+
'items' => [
265+
[
266+
'label' => 'logic item',
267+
'url' => 'test/logic',
268+
'template' => 'label: {label}; url: {url}',
269+
'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
270+
return $widget->route === 'test/logic';
271+
},
272+
],
273+
[
274+
'label' => 'another item',
275+
'url' => 'test/another',
276+
'template' => 'label: {label}; url: {url}',
277+
]
278+
],
279+
],
280+
],
281+
]);
282+
283+
$expected = <<<'HTML'
284+
<ul><li>label: Home; url: test/home</li>
285+
<li>label: About; url: test/about</li>
286+
<li class="active">
287+
<ul>
288+
<li class="active">label: logic item; url: test/logic</li>
289+
<li>label: another item; url: test/another</li>
290+
</ul>
291+
</li></ul>
292+
HTML;
293+
294+
$this->assertEqualsWithoutLE($expected, $output);
295+
}
296+
297+
public function testActiveItemClosureParentAnotherItem()
298+
{
299+
/** @see https://github.com/yiisoft/yii2/issues/19060 */
300+
$output = Menu::widget([
301+
'route' => 'test/another',
302+
'params' => [],
303+
'linkTemplate' => '',
304+
'labelTemplate' => '',
305+
'activateParents' => true,
306+
'items' => [
307+
[
308+
'label' => 'Home',
309+
'url' => 'test/home',
310+
'template' => 'label: {label}; url: {url}',
311+
],
312+
[
313+
'label' => 'About',
314+
'url' => 'test/about',
315+
'template' => 'label: {label}; url: {url}',
316+
],
317+
[
318+
'label' => 'Parent',
319+
'items' => [
320+
[
321+
'label' => 'another item',
322+
// use non relative route to avoid error in BaseUrl::normalizeRoute (missing controller)
323+
'url' => ['/test/another'],
324+
'template' => 'label: {label}; url: {url}',
325+
],
326+
[
327+
'label' => 'logic item',
328+
'url' => 'test/logic',
329+
'template' => 'label: {label}; url: {url}',
330+
'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
331+
return $widget->route === 'test/logic';
332+
},
333+
],
334+
335+
],
336+
],
337+
],
338+
]);
339+
340+
$expected = <<<'HTML'
341+
<ul><li>label: Home; url: test/home</li>
342+
<li>label: About; url: test/about</li>
343+
<li class="active">
344+
<ul>
345+
<li class="active">label: another item; url: /test/another</li>
346+
<li>label: logic item; url: test/logic</li>
347+
</ul>
348+
</li></ul>
349+
HTML;
350+
351+
$this->assertEqualsWithoutLE($expected, $output);
352+
}
353+
204354
public function testItemClassAsArray()
205355
{
206356
$output = Menu::widget([
@@ -302,8 +452,31 @@ public function testItemClassAsString()
302452
$this->assertEqualsWithoutLE($expected, $output);
303453
}
304454

305-
/*public function testIsItemActive()
455+
public function testIsItemActive()
306456
{
307-
// TODO: implement test of protected method isItemActive()
308-
}*/
457+
$output = Menu::widget([
458+
'route' => 'test/item2',
459+
'params' => [
460+
'page'=>'5',
461+
],
462+
'items' => [
463+
[
464+
'label' => 'item1',
465+
'url' => ['/test/item1']
466+
],
467+
[
468+
'label' => 'item2',
469+
// use non relative route to avoid error in BaseUrl::normalizeRoute (missing controller)
470+
'url' => ['/test/item2','page'=>'5']
471+
],
472+
473+
],
474+
]);
475+
476+
$expected = <<<'HTML'
477+
<ul><li><a href="/test/item1">item1</a></li>
478+
<li class="active"><a href="/test/item2?page=5">item2</a></li></ul>
479+
HTML;
480+
$this->assertEqualsWithoutLE($expected, $output);
481+
}
309482
}

0 commit comments

Comments
 (0)