Fix #19060: Fix yii\widgets\Menu bug when using Closure for active …

…item and adding additional tests in `tests\framework\widgets\MenuTest`
yiisoft · Oct 20, 2023 · 5eeb12f · 5eeb12f
1 parent a4b5856
commit 5eeb12f
Show file tree

Hide file tree

Showing 3 changed files with 183 additions and 5 deletions.
diff --git a/framework/CHANGELOG.md b/framework/CHANGELOG.md
@@ -4,6 +4,7 @@ Yii Framework 2 Change Log
 2.0.50 under development
 ------------------------
 
+- Bug #19060: Fix `yii\widgets\Menu` bug when using Closure for active item and adding additional tests in `tests\framework\widgets\MenuTest` (atrandafir)
 - Bug #13920: Fixed erroneous validation for specific cases (tim-fischer-maschinensucher)
 - 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)
 - Bug #20002: Fixed superfluous query on HEAD request in serializer (xicond)

diff --git a/framework/widgets/Menu.php b/framework/widgets/Menu.php
@@ -283,7 +283,11 @@ protected function normalizeItems($items, &$active)
                     $items[$i]['active'] = false;
                 }
             } elseif ($item['active'] instanceof Closure) {
-                $active = $items[$i]['active'] = call_user_func($item['active'], $item, $hasActiveChild, $this->isItemActive($item), $this);
+                if (call_user_func($item['active'], $item, $hasActiveChild, $this->isItemActive($item), $this)) {
+                    $active = $items[$i]['active'] = true;
+                } else {
+                    $items[$i]['active'] = false;
+                }
             } elseif ($item['active']) {
                 $active = true;
             }

diff --git a/tests/framework/widgets/MenuTest.php b/tests/framework/widgets/MenuTest.php
@@ -18,7 +18,14 @@ class MenuTest extends \yiiunit\TestCase
     protected function setUp()
     {
         parent::setUp();
-        $this->mockApplication();
+        $this->mockWebApplication([
+            'components'=>[
+                'urlManager' => [
+                    'enablePrettyUrl' => true,
+                    'showScriptName' => false,
+                ],
+            ],
+        ]);
     }
 
     public function testEncodeLabel()
@@ -201,6 +208,149 @@ public function testActiveItemClosure()
         $this->assertEqualsWithoutLE($expected, $output);
     }
 
+    public function testActiveItemClosureWithLogic()
+    {
+        $output = Menu::widget([
+            'route' => 'test/logic',
+            'params' => [],
+            'linkTemplate' => '',
+            'labelTemplate' => '',
+            'items' => [
+                [
+                    'label' => 'logic item',
+                    'url' => 'test/logic',
+                    'template' => 'label: {label}; url: {url}',
+                    'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
+                        return $widget->route === 'test/logic';
+                    },
+                ],
+                [
+                    'label' => 'another item',
+                    'url' => 'test/another',
+                    'template' => 'label: {label}; url: {url}',
+                ]
+            ],
+        ]);
+
+        $expected = <<<'HTML'
+<ul><li class="active">label: logic item; url: test/logic</li>
+<li>label: another item; url: test/another</li></ul>
+HTML;
+
+        $this->assertEqualsWithoutLE($expected, $output);
+    }
+
+    public function testActiveItemClosureWithLogicParent()
+    {
+        $output = Menu::widget([
+            'route' => 'test/logic',
+            'params' => [],
+            'linkTemplate' => '',
+            'labelTemplate' => '',
+            'activateParents' => true,
+            'items' => [
+                [
+                    'label' => 'Home',
+                    'url' => 'test/home',
+                    'template' => 'label: {label}; url: {url}',
+                ],
+                [
+                    'label' => 'About',
+                    'url' => 'test/about',
+                    'template' => 'label: {label}; url: {url}',
+                ],
+                [
+                    'label' => 'Parent',
+                    'items' => [
+                        [
+                            'label' => 'logic item',
+                            'url' => 'test/logic',
+                            'template' => 'label: {label}; url: {url}',
+                            'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
+                                return $widget->route === 'test/logic';
+                            },
+                        ],
+                        [
+                            'label' => 'another item',
+                            'url' => 'test/another',
+                            'template' => 'label: {label}; url: {url}',
+                        ]
+                    ],
+                ],
+            ],
+        ]);
+
+        $expected = <<<'HTML'
+<ul><li>label: Home; url: test/home</li>
+<li>label: About; url: test/about</li>
+<li class="active">
+<ul>
+<li class="active">label: logic item; url: test/logic</li>
+<li>label: another item; url: test/another</li>
+</ul>
+</li></ul>
+HTML;
+
+        $this->assertEqualsWithoutLE($expected, $output);
+    }
+
+    public function testActiveItemClosureParentAnotherItem()
+    {
+        /** @see https://github.com/yiisoft/yii2/issues/19060 */
+        $output = Menu::widget([
+            'route' => 'test/another',
+            'params' => [],
+            'linkTemplate' => '',
+            'labelTemplate' => '',
+            'activateParents' => true,
+            'items' => [
+                [
+                    'label' => 'Home',
+                    'url' => 'test/home',
+                    'template' => 'label: {label}; url: {url}',
+                ],
+                [
+                    'label' => 'About',
+                    'url' => 'test/about',
+                    'template' => 'label: {label}; url: {url}',
+                ],
+                [
+                    'label' => 'Parent',
+                    'items' => [
+                        [
+                            'label' => 'another item',
+                            // use non relative route to avoid error in BaseUrl::normalizeRoute (missing controller)
+                            'url' => ['/test/another'], 
+                            'template' => 'label: {label}; url: {url}',
+                        ],
+                        [
+                            'label' => 'logic item',
+                            'url' => 'test/logic',
+                            'template' => 'label: {label}; url: {url}',
+                            'active' => function ($item, $hasActiveChild, $isItemActive, $widget) {
+                                return $widget->route === 'test/logic';
+                            },
+                        ],
+
+                    ],
+                ],
+            ],
+        ]);
+
+        $expected = <<<'HTML'
+<ul><li>label: Home; url: test/home</li>
+<li>label: About; url: test/about</li>
+<li class="active">
+<ul>
+<li class="active">label: another item; url: /test/another</li>
+<li>label: logic item; url: test/logic</li>
+</ul>
+</li></ul>
+HTML;
+
+        $this->assertEqualsWithoutLE($expected, $output);
+    }
+
     public function testItemClassAsArray()
     {
         $output = Menu::widget([
@@ -302,8 +452,31 @@ public function testItemClassAsString()
         $this->assertEqualsWithoutLE($expected, $output);
     }
 
-    /*public function testIsItemActive()
+    public function testIsItemActive()
     {
-        // TODO: implement test of protected method isItemActive()
-    }*/
+        $output = Menu::widget([
+            'route' => 'test/item2',
+            'params' => [
+                'page'=>'5',
+            ],
+            'items' => [
+                [
+                    'label' => 'item1',
+                    'url' => ['/test/item1'] 
+                ],
+                [
+                    'label' => 'item2',
+                    // use non relative route to avoid error in BaseUrl::normalizeRoute (missing controller)
+                    'url' => ['/test/item2','page'=>'5']
+                ],
+
+            ],
+        ]);
+
+        $expected = <<<'HTML'
+<ul><li><a href="/test/item1">item1</a></li>
+<li class="active"><a href="/test/item2?page=5">item2</a></li></ul>
+HTML;
+        $this->assertEqualsWithoutLE($expected, $output);
+    }
 }