Improve menu item filters (#644)

* [src/Menu]: Improve and document the filters of the menu items. * [tests]: Improve tests for menu item filters to get better coverage over they. * [src/Menu]: Change the logic used on the Gate filter. * [General]: The SubmenuFilter is removed and the logic moved to the menu builder. This improves the filters interface because Builder instance is not required now. * [src/Menu/Builder]: Move isAllowed method to MenuItemHelper class and make some fixes/improvements. * [src/Menu]: Fixs and improvements on code documentation. * [src/Menu/Filters]: Improve the classes filter by moving some classes to blade layouts. * [src/Menu]: Some improvements on the menu filters. * Fix Code Style * Fix Code Style
jeroennoten · Jun 23, 2020 · c625297 · c625297
1 parent 7209504
commit c625297
Show file tree

Hide file tree

Showing 18 changed files with 356 additions and 186 deletions.
diff --git a/README.md b/README.md
@@ -885,7 +885,6 @@ And then add configuration to the `config/adminlte.php` file:
 'filters' => [
     JeroenNoten\LaravelAdminLte\Menu\Filters\ActiveFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\HrefFilter::class,
-    JeroenNoten\LaravelAdminLte\Menu\Filters\SubmenuFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\ClassesFilter::class,
     // Comment next line out.
     //JeroenNoten\LaravelAdminLte\Menu\Filters\GateFilter::class,
@@ -1050,7 +1049,6 @@ The default set of menu filters is:
     JeroenNoten\LaravelAdminLte\Menu\Filters\HrefFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\SearchFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\ActiveFilter::class,
-    JeroenNoten\LaravelAdminLte\Menu\Filters\SubmenuFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\ClassesFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\GateFilter::class,
     JeroenNoten\LaravelAdminLte\Menu\Filters\LangFilter::class,

diff --git a/config/adminlte.php b/config/adminlte.php
@@ -331,7 +331,6 @@
         JeroenNoten\LaravelAdminLte\Menu\Filters\HrefFilter::class,
         JeroenNoten\LaravelAdminLte\Menu\Filters\SearchFilter::class,
         JeroenNoten\LaravelAdminLte\Menu\Filters\ActiveFilter::class,
-        JeroenNoten\LaravelAdminLte\Menu\Filters\SubmenuFilter::class,
         JeroenNoten\LaravelAdminLte\Menu\Filters\ClassesFilter::class,
         JeroenNoten\LaravelAdminLte\Menu\Filters\GateFilter::class,
         JeroenNoten\LaravelAdminLte\Menu\Filters\LangFilter::class,

diff --git a/resources/views/partials/navbar/menu-item-dropdown-menu.blade.php b/resources/views/partials/navbar/menu-item-dropdown-menu.blade.php
@@ -1,4 +1,4 @@
-<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item {{ $item['top_nav_class'] }}">
+<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item dropdown">
 
     {{-- Menu toggler --}}
     <a class="nav-link dropdown-toggle" href=""

diff --git a/resources/views/partials/navbar/menu-item-link.blade.php b/resources/views/partials/navbar/menu-item-link.blade.php
@@ -1,6 +1,6 @@
-<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item {{ $item['top_nav_class'] }}">
+<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item">
 
-    <a class="nav-link" href="{{ $item['href'] }}"
+    <a class="nav-link {{ $item['class'] }}" href="{{ $item['href'] }}"
        @if(isset($item['target'])) target="{{ $item['target'] }}" @endif
        {!! $item['data-compiled'] ?? '' !!}>
 

diff --git a/resources/views/partials/sidebar/menu-item-treeview-menu.blade.php b/resources/views/partials/sidebar/menu-item-treeview-menu.blade.php
@@ -1,4 +1,4 @@
-<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item {{ $item['submenu_class'] }}">
+<li @if(isset($item['id'])) id="{{ $item['id'] }}" @endif class="nav-item has-treeview {{ $item['submenu_class'] }}">
 
     {{-- Menu toggler --}}
     <a class="nav-link {{ $item['class'] }} @if(isset($item['shift'])) {{ $item['shift'] }} @endif"

diff --git a/src/Helpers/MenuItemHelper.php b/src/Helpers/MenuItemHelper.php
@@ -53,6 +53,19 @@ public static function isSubmenu($item)
                is_array($item['submenu']);
     }
 
+    /**
+     * Check if a menu item is allowed to be shown.
+     *
+     * @param mixed $item
+     * @return bool
+     */
+    public static function isAllowed($item)
+    {
+        $isAllowed = ! (isset($item['restricted']) && $item['restricted']);
+
+        return $item && $isAllowed;
+    }
+
     /**
      * Check if a menu item is valid for the sidebar section.
      *

diff --git a/src/Menu/Builder.php b/src/Menu/Builder.php
@@ -3,6 +3,7 @@
 namespace JeroenNoten\LaravelAdminLte\Menu;
 
 use Illuminate\Support\Arr;
+use JeroenNoten\LaravelAdminLte\Helpers\MenuItemHelper;
 
 class Builder
 {
@@ -119,9 +120,12 @@ public function itemKeyExists($itemKey)
      * @param array $items An array with items to be transformed
      * @return array Array with the new transformed items
      */
-    public function transformItems($items)
+    protected function transformItems($items)
     {
-        return array_filter(array_map([$this, 'applyFilters'], $items));
+        return array_filter(
+            array_map([$this, 'applyFilters'], $items),
+            [MenuItemHelper::class, 'isAllowed']
+        );
     }
 
     /**
@@ -138,13 +142,13 @@ protected function findItem($itemKey, $items)
         foreach ($items as $key => $item) {
             if (isset($item['key']) && $item['key'] === $itemKey) {
                 return [$key];
-            } elseif (isset($item['submenu']) && is_array($item['submenu'])) {
+            } elseif (MenuItemHelper::isSubmenu($item)) {
 
                 // Do the recursive call to search on submenu. If we found the
                 // item, merge the path with the current one.
 
-                if ($newPath = $this->findItem($itemKey, $item['submenu'])) {
-                    return array_merge([$key, 'submenu'], $newPath);
+                if ($subPath = $this->findItem($itemKey, $item['submenu'])) {
+                    return array_merge([$key, 'submenu'], $subPath);
                 }
             }
         }
@@ -162,12 +166,22 @@ protected function findItem($itemKey, $items)
      */
     protected function applyFilters($item)
     {
-        if (is_string($item)) {
+        // Filters are only applied to array type menu items.
+
+        if (! is_array($item)) {
             return $item;
         }
 
+        // If the item is a submenu, transform all submenu items first.
+
+        if (MenuItemHelper::isSubmenu($item)) {
+            $item['submenu'] = $this->transformItems($item['submenu']);
+        }
+
+        // Now, apply all the filters on the item.
+
         foreach ($this->filters as $filter) {
-            $item = $filter->transform($item, $this);
+            $item = $filter->transform($item);
         }
 
         return $item;

diff --git a/src/Menu/Filters/ActiveFilter.php b/src/Menu/Filters/ActiveFilter.php
@@ -2,21 +2,37 @@
 
 namespace JeroenNoten\LaravelAdminLte\Menu\Filters;
 
+use JeroenNoten\LaravelAdminLte\Helpers\MenuItemHelper;
 use JeroenNoten\LaravelAdminLte\Menu\ActiveChecker;
-use JeroenNoten\LaravelAdminLte\Menu\Builder;
 
 class ActiveFilter implements FilterInterface
 {
-    private $activeChecker;
+    /**
+     * The active checker instance.
+     *
+     * @var ActiveChecker
+     */
+    protected $activeChecker;
 
+    /**
+     * Constructor.
+     *
+     * @param ActiveChecker $activeChecker
+     */
     public function __construct(ActiveChecker $activeChecker)
     {
         $this->activeChecker = $activeChecker;
     }
 
-    public function transform($item, Builder $builder)
+    /**
+     * Transforms a menu item. Adds the active attribute when suitable.
+     *
+     * @param array $item A menu item
+     * @return array The transformed menu item
+     */
+    public function transform($item)
     {
-        if (! isset($item['header'])) {
+        if (! MenuItemHelper::isHeader($item)) {
             $item['active'] = $this->activeChecker->isActive($item);
         }
 

diff --git a/src/Menu/Filters/ClassesFilter.php b/src/Menu/Filters/ClassesFilter.php
@@ -2,36 +2,62 @@
 
 namespace JeroenNoten\LaravelAdminLte\Menu\Filters;
 
-use JeroenNoten\LaravelAdminLte\Menu\Builder;
+use JeroenNoten\LaravelAdminLte\Helpers\MenuItemHelper;
 
 class ClassesFilter implements FilterInterface
 {
-    public function transform($item, Builder $builder)
+    /**
+     * Transforms a menu item. Add classes related attributes when suitable.
+     *
+     * @param array $item A menu item
+     * @return array The transformed menu item
+     */
+    public function transform($item)
     {
-        if (! isset($item['header'])) {
-            $item['classes'] = $this->makeClasses($item);
-            $item['class'] = implode(' ', $item['classes']);
-            $item['top_nav_classes'] = $this->makeClasses($item, true);
-            $item['top_nav_class'] = implode(' ', $item['top_nav_classes']);
+        if (! MenuItemHelper::isHeader($item)) {
+            $item['class'] = implode(' ', $this->makeClasses($item));
+        }
+
+        if (MenuItemHelper::isSubmenu($item)) {
+            $item['submenu_class'] = implode(' ', $this->makeSubmenuClasses($item));
         }
 
         return $item;
     }
 
-    protected function makeClasses($item, $topNav = false)
+    /**
+     * Make classes related to the components of a menu item.
+     *
+     * @param array $item A menu item
+     * @return array The array of classes
+     */
+    protected function makeClasses($item)
     {
         $classes = [];
 
+        // Add the active class when the item is active.
+
         if ($item['active']) {
             $classes[] = 'active';
         }
 
-        if (isset($item['submenu'])) {
-            if ($topNav) {
-                $classes[] = 'dropdown';
-            } else {
-                $classes[] = 'nav-item';
-            }
+        return $classes;
+    }
+
+    /**
+     * Make classes related to the components of a submenu item.
+     *
+     * @param array $item A menu item
+     * @return array The array of classes
+     */
+    protected function makeSubmenuClasses($item)
+    {
+        $classes = [];
+
+        // Add the menu-open class when a sidebar submenu is active.
+
+        if (MenuItemHelper::isSidebarItem($item) && $item['active']) {
+            $classes[] = 'menu-open';
         }
 
         return $classes;

diff --git a/src/Menu/Filters/DataFilter.php b/src/Menu/Filters/DataFilter.php
@@ -2,11 +2,15 @@
 
 namespace JeroenNoten\LaravelAdminLte\Menu\Filters;
 
-use JeroenNoten\LaravelAdminLte\Menu\Builder;
-
 class DataFilter implements FilterInterface
 {
-    public function transform($item, Builder $builder)
+    /**
+     * Transforms a menu item. Adds the compiled data attributes when suitable.
+     *
+     * @param array $item A menu item
+     * @return array The transformed menu item
+     */
+    public function transform($item)
     {
         if (isset($item['data']) && is_array($item['data'])) {
             $item['data-compiled'] = $this->compileData($item['data']);
@@ -15,6 +19,12 @@ public function transform($item, Builder $builder)
         return $item;
     }
 
+    /**
+     * Compile an array of data attributes into a data string.
+     *
+     * @param array $dataArray Array of html data attributes
+     * @return string The compiled version of data attributes
+     */
     protected function compileData($dataArray)
     {
         $compiled = [];

diff --git a/src/Menu/Filters/FilterInterface.php b/src/Menu/Filters/FilterInterface.php
@@ -2,9 +2,13 @@
 
 namespace JeroenNoten\LaravelAdminLte\Menu\Filters;
 
-use JeroenNoten\LaravelAdminLte\Menu\Builder;
-
 interface FilterInterface
 {
-    public function transform($item, Builder $builder);
+    /**
+     * Transforms a menu item in some way.
+     *
+     * @param array $item A menu item
+     * @return array The transformed menu item
+     */
+    public function transform($item);
 }
diff --git a/src/Menu/Filters/GateFilter.php b/src/Menu/Filters/GateFilter.php
@@ -3,48 +3,69 @@
 namespace JeroenNoten\LaravelAdminLte\Menu\Filters;
 
 use Illuminate\Contracts\Auth\Access\Gate;
-use JeroenNoten\LaravelAdminLte\Menu\Builder;
 
 class GateFilter implements FilterInterface
 {
+    /**
+     * The Laravel gate instance, used to check for permissions.
+     *
+     * @var Gate
+     */
     protected $gate;
 
+    /**
+     * Constructor.
+     *
+     * @param Gate $gate
+     */
     public function __construct(Gate $gate)
     {
         $this->gate = $gate;
     }
 
-    public function transform($item, Builder $builder)
+    /**
+     * Transforms a menu item. Add the restricted property to a menu item
+     * when situable.
+     *
+     * @param array $item A menu item
+     * @return array The transformed menu item
+     */
+    public function transform($item)
     {
-        if (! $this->isVisible($item)) {
-            return false;
+        // Set a special attribute when item is not allowed. Items with this
+        // attribute will be filtered out of the menu.
+
+        if (! $this->isAllowed($item)) {
+            $item['restricted'] = true;
         }
 
         return $item;
     }
 
-    protected function isVisible($item)
+    /**
+     * Check if a menu item is allowed for the current user.
+     *
+     * @param array $item A menu item
+     * @return bool
+     */
+    protected function isAllowed($item)
     {
-        if (! isset($item['can'])) {
+        // Check if there are any permission defined for the item.
+
+        if (empty($item['can'])) {
             return true;
         }
 
-        $args = [];
+        // Read the extra arguments (a db model instance can be used).
 
-        if (isset($item['model'])) {
-            $args = $item['model'];
-        }
+        $args = isset($item['model']) ? $item['model'] : [];
 
-        if (! is_array($item['can'])) {
-            return $this->gate->allows($item['can'], $args);
-        }
+        // Check if the current user can perform the configured permissions.
 
-        foreach ($item['can'] as $can) {
-            if ($this->gate->allows($can, $args)) {
-                return true;
-            }
+        if (is_string($item['can']) || is_array($item['can'])) {
+            return $this->gate->any($item['can'], $args);
         }
 
-        return false;
+        return true;
     }
 }