feature #971 Improved the way we deal with menu dividers (javiereguiluz)

This PR was merged into the master branch. Discussion ---------- Improved the way we deal with menu dividers This is an internal change which doesn't affect the end-users of the bundle. --- @yceruto this is what I was thinking about when I told you about simplifying the condition in the `menu.html.twig` template. You were right and we couldn't remove it ... but making some minor changes in the code, we can correctly handle "menu dividers" which are empty elements that doesn't define any submenus. Commits ------- eb72a36 Improved the way we deal with menu dividers
EasyCorp · Mar 9, 2016 · 5a4e6e5 · 5a4e6e5
2 parents 7364dc5 + eb72a36
commit 5a4e6e5
Show file tree

Hide file tree

Showing 4 changed files with 44 additions and 11 deletions.
diff --git a/Configuration/MenuConfigPass.php b/Configuration/MenuConfigPass.php
@@ -67,7 +67,7 @@ private function normalizeMenuConfig(array $menuConfig, array $backendConfig, $p
         // replaces the short config syntax:
         //   design.menu: ['Product', 'User']
         // by the expanded config syntax:
-        //   design.menu: []{ entity: 'Product' }, { entity: 'User' }]
+        //   design.menu: [{ entity: 'Product' }, { entity: 'User' }]
         foreach ($menuConfig as $i => $itemConfig) {
             if (is_string($itemConfig)) {
                 $itemConfig = array('entity' => $itemConfig);
@@ -158,9 +158,16 @@ private function processMenuConfig(array $menuConfig, array $backendConfig, $par
                 }
             }
 
-            // 4th level priority: if 'label' is defined (and not the previous options), this is an empty element
+            // 4th level priority: if 'label' is defined (and not the previous options),
+            // this is a menu divider of a submenu title
             elseif (isset($itemConfig['label'])) {
-                $itemConfig['type'] = 'empty';
+                if (empty($itemConfig['children'])) {
+                    // if the item doesn't define a submenu, this is a menu divider
+                    $itemConfig['type'] = 'divider';
+                } else {
+                    // if the item defines a submenu, this is the title of that submenu
+                    $itemConfig['type'] = 'empty';
+                }
             } else {
                 throw new \RuntimeException(sprintf('The configuration of the menu item in the position %d (being 0 the first item) must define at least one of these options: entity, url, route, label.', $i));
             }

diff --git a/Resources/views/default/menu.html.twig b/Resources/views/default/menu.html.twig
@@ -1,6 +1,6 @@
 {% macro render_menu_item(item) %}
-    {% if item.type == 'empty' and (item.children is not defined or item.children is empty) %}
-        <li class="header">{{ item.label|trans }}</li>
+    {% if item.type == 'divider' %}
+        {{ item.label|trans }}
     {% else %}
         {% set menu_params = { menuIndex: item.menu_index, submenuIndex: item.submenu_index } %}
         {% set path =
@@ -23,13 +23,13 @@
 <ul class="sidebar-menu">
     {% block main_menu %}
         {% for item in easyadmin_config('design.menu') %}
-            <li class="{{ item.children is not empty ? 'treeview' }} {{ app.request.query.get('menuIndex')|default(-1) == loop.index0 ? 'active' }} {{ app.request.query.get('submenuIndex')|default(-1) != -1 ? 'submenu-active' }}">
+            <li class="{{ item.type == 'divider' ? 'header' }} {{ item.children is not empty ? 'treeview' }} {{ app.request.query.get('menuIndex')|default(-1) == loop.index0 ? 'active' }} {{ app.request.query.get('submenuIndex')|default(-1) != -1 ? 'submenu-active' }}">
                 {{ helper.render_menu_item(item) }}
 
                 {% if item.children|default([]) is not empty %}
                     <ul class="treeview-menu">
                         {% for subitem in item.children %}
-                            <li class="{{ app.request.query.get('submenuIndex')|default(-1) == loop.index0 ? 'active' }}">
+                            <li class="{{ subitem.type == 'divider' ? 'header' }} {{ app.request.query.get('submenuIndex')|default(-1) == loop.index0 ? 'active' }}">
                                 {{ helper.render_menu_item(subitem) }}
                             </li>
                         {% endfor %}

diff --git a/Tests/Controller/CustomMenuTest.php b/Tests/Controller/CustomMenuTest.php
@@ -27,11 +27,12 @@ public function testCustomBackendHomepage()
         $this->client->request('GET', '/admin/');
 
         $this->assertEquals(
-            '/admin/?action=list&entity=Category&menuIndex=0&submenuIndex=2',
+            '/admin/?action=list&entity=Category&menuIndex=0&submenuIndex=3',
             $this->client->getResponse()->headers->get('location')
         );
 
         $crawler = $this->client->followRedirect();
+
         $this->assertEquals(
             'Products',
             trim($crawler->filter('.sidebar-menu li.active.submenu-active a')->text())
@@ -41,7 +42,7 @@ public function testCustomBackendHomepage()
             'Categories',
             trim($crawler->filter('.sidebar-menu .treeview-menu li.active a')->text())
         );
-    }
+   }
 
     public function testBackendHomepageConfig()
     {
@@ -66,14 +67,19 @@ public function testDefaultMenuItem()
         ), $backendConfig['default_menu_item']);
     }
 
-    public function testMenuHeaders()
+    public function testMenuDividers()
     {
         $crawler = $this->getBackendHomepage();
 
-        $this->assertEquals(
+        $this->assertContains(
             'header',
             $crawler->filter('.sidebar-menu li:contains("About EasyAdmin")')->attr('class')
         );
+
+        $this->assertContains(
+            'header',
+            $crawler->filter('.sidebar-menu .treeview-menu li:contains("Additional Items")')->attr('class')
+        );
     }
 
     public function testMenuIcons()
@@ -165,4 +171,22 @@ public function testMenuUrls()
             'First level menu, absolute URL'
         );
     }
+
+    public function testMenuItemTypes()
+    {
+        $expectedTypesMainMenu = array('empty', 'entity', 'entity', 'divider', 'link', 'link', 'link');
+        $expectedTypesSubMenu = array('entity', 'entity', 'divider', 'entity', 'link');
+
+        $crawler = $this->getBackendHomepage();
+        $backendConfig = $this->client->getContainer()->getParameter('easyadmin.config');
+        $menuConfig = $backendConfig['design']['menu'];
+
+        foreach ($menuConfig as $i => $itemConfig) {
+            $this->assertEquals($expectedTypesMainMenu[$i], $itemConfig['type']);
+        }
+
+        foreach ($menuConfig[0]['children'] as $i => $itemConfig) {
+            $this->assertEquals($expectedTypesSubMenu[$i], $itemConfig['type']);
+        }
+    }
 }
diff --git a/Tests/Fixtures/App/config/config_custom_menu.yml b/Tests/Fixtures/App/config/config_custom_menu.yml
@@ -9,7 +9,9 @@ easy_admin:
               children:
                   - { entity: 'Product', icon: 'th-list', label: 'List Products', params: { sortField: 'createdAt' } }
                   - { entity: 'Product', label: 'Add Product', params: { action: 'new' } }
+                  - { label: 'Additional Items' }
                   - { entity: 'Category', label: 'Categories', default: true, icon: '' }
+                  - { label: 'Absolute URL', url: 'https://github.com/javiereguiluz/EasyAdminBundle' }
             - { label: 'Images', entity: 'Image' }
             - { label: 'Purchases', entity: 'Purchase', icon: '', params: { sortField: 'deliveryDate' } }
             - { label: 'About EasyAdmin' }