bug #6633 Fix menu item highlighting (javiereguiluz)

This PR was squashed before being merged into the 4.x branch. Discussion ---------- Fix menu item highlighting This fixes some issues with menu item highlighting when using submenus. Commits ------- 925a33d Fix menu item highlighting
EasyCorp · Dec 9, 2024 · 5cb5534 · 5cb5534
2 parents 8d6b02d + 925a33d
commit 5cb5534
Show file tree

Hide file tree

Showing 5 changed files with 100 additions and 29 deletions.
diff --git a/config/services.php b/config/services.php
@@ -226,6 +226,7 @@
             ->arg(4, service(MenuItemMatcherInterface::class))
 
         ->set(MenuItemMatcher::class)
+            ->arg(0, service(AdminUrlGenerator::class))
 
         ->alias(MenuItemMatcherInterface::class, MenuItemMatcher::class)
 

diff --git a/src/Menu/MenuItemMatcher.php b/src/Menu/MenuItemMatcher.php
@@ -2,17 +2,24 @@
 
 namespace EasyCorp\Bundle\EasyAdminBundle\Menu;
 
+use EasyCorp\Bundle\EasyAdminBundle\Config\Action;
 use EasyCorp\Bundle\EasyAdminBundle\Config\Crud;
 use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
 use EasyCorp\Bundle\EasyAdminBundle\Contracts\Menu\MenuItemMatcherInterface;
 use EasyCorp\Bundle\EasyAdminBundle\Dto\MenuItemDto;
+use EasyCorp\Bundle\EasyAdminBundle\Router\AdminUrlGenerator;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
  * @author Javier Eguiluz <[email protected]>
  */
 class MenuItemMatcher implements MenuItemMatcherInterface
 {
+    public function __construct(
+        private AdminUrlGenerator $adminUrlGenerator,
+    ) {
+    }
+
     /**
      * Given the full list of menu items, this method finds which item should be
      * marked as 'selected' based on the current page being visited by the user.
@@ -158,11 +165,18 @@ private function doMarkSelectedPrettyUrlsMenuItem(array $menuItems, Request $req
     {
         // the menu-item matching is a 2-phase process:
         // 1) traverse all menu items and try to find an exact match with the current URL
-        // 2) if no exact match is found, traverse all menu items again and try to find a partial match:
-        // 2.1) strip the query string from the current URL and the menu item URL and look for a match
-        // 2.2) if no match is found, strip the action from the request (e.g. /admin/post/new -> /admin/post) and try to match again
+        // 2) if no exact match is found, traverse all menu items again and try to find a partial match
+        $currentUrlWithoutHost = $request->getPathInfo();
+        $currentUrlQueryParams = $request->query->all();
+        unset($currentUrlQueryParams['sort'], $currentUrlQueryParams['page']);
+        // sort them because menu items always have their query parameters sorted
+        ksort($currentUrlQueryParams);
+
+        $normalizedCurrentUrl = $currentUrlWithoutHost;
+        if ([] !== $currentUrlQueryParams) {
+            $normalizedCurrentUrl .= '?'.http_build_query($currentUrlQueryParams);
+        }
 
-        // try to find an exact match with the current URL
         foreach ($menuItems as $menuItemDto) {
             if ($menuItemDto->isMenuSection()) {
                 continue;
@@ -172,37 +186,38 @@ private function doMarkSelectedPrettyUrlsMenuItem(array $menuItems, Request $req
                 $menuItemDto->setSubItems($this->doMarkSelectedPrettyUrlsMenuItem($subItems, $request));
             }
 
-            // compare the ending of the URL instead of a strict equality because link URLs can be absolute URLs
-            if (str_ends_with($menuItemDto->getLinkUrl(), $request->getRequestUri())) {
+            if ($menuItemDto->getLinkUrl() === $normalizedCurrentUrl) {
                 $menuItemDto->setSelected(true);
 
                 return $menuItems;
             }
         }
 
-        // if no exact match is found, try to find a partial match
-        $currentRequestUri = $request->getUri();
-        $currentRequestUriWithoutQueryString = parse_url($currentRequestUri, \PHP_URL_PATH);
-        // remove the last segment of the URL but keep the trailing slash (e.g. /admin/post/new -> /admin/post/)
-        $currentRequestUriWithoutAction = preg_replace('#/[^/]+$#', '', $currentRequestUriWithoutQueryString);
-        // the edit URL is '/admin/foo/{id}/edit' so we need to remove the two last segments of the URL
-        if (str_ends_with($currentRequestUriWithoutQueryString, '/edit')) {
-            $currentRequestUriWithoutAction = preg_replace('#/[^/]+$#', '', $currentRequestUriWithoutAction);
+        // If the current URL is a CRUD URL and the action is not 'index', attempt
+        // to match the same URL with the 'index' action. This ensures e.g. that the
+        // /admin/post menu item is highlighted when visiting related URLs such as
+        // /admin/post/new, /admin/post/37/edit, etc.
+        if (null === $crudControllerFqcn = $request->attributes->get(EA::CRUD_CONTROLLER_FQCN)) {
+            return $menuItems;
         }
 
+        $currentUrlWithIndexCrudAction = $this->adminUrlGenerator->setAll(array_merge($currentUrlQueryParams, [
+            EA::DASHBOARD_CONTROLLER_FQCN => $request->attributes->get(EA::DASHBOARD_CONTROLLER_FQCN),
+            EA::CRUD_CONTROLLER_FQCN => $crudControllerFqcn,
+            EA::CRUD_ACTION => Action::INDEX,
+        ]))->generateUrl();
+
         foreach ($menuItems as $menuItemDto) {
             if ($menuItemDto->isMenuSection()) {
                 continue;
             }
 
-            $menuItemUrl = $menuItemDto->getLinkUrl();
-            $menuItemUrlWithoutQueryString = parse_url($menuItemUrl, \PHP_URL_PATH);
+            if ([] !== $subItems = $menuItemDto->getSubItems()) {
+                $menuItemDto->setSubItems($this->doMarkSelectedPrettyUrlsMenuItem($subItems, $request));
+            }
 
             // compare the ending of the URL instead of a strict equality because link URLs can be absolute URLs
-            if (str_ends_with($menuItemUrl, $currentRequestUriWithoutQueryString)
-                || ('' !== $currentRequestUriWithoutAction && str_ends_with($menuItemUrl, $currentRequestUriWithoutAction))
-                || str_ends_with($menuItemUrlWithoutQueryString, $currentRequestUriWithoutQueryString)
-                || ('' !== $currentRequestUriWithoutAction && str_ends_with($menuItemUrlWithoutQueryString, $currentRequestUriWithoutAction))) {
+            if ('' !== $menuItemDto->getLinkUrl() && str_ends_with($currentUrlWithIndexCrudAction, $menuItemDto->getLinkUrl())) {
                 $menuItemDto->setSelected(true);
 
                 return $menuItems;

diff --git a/tests/Controller/PrettyUrls/PrettyUrlsControllerTest.php b/tests/Controller/PrettyUrls/PrettyUrlsControllerTest.php
@@ -99,6 +99,37 @@ public function testCusomizedWelcomePage()
         $this->assertSelectorTextContains('h1', 'Welcome to EasyAdmin 4');
     }
 
+    // this test visits the two dashboards of the application to test that all
+    // the generated URLs are correct for the menu items and Dashboard links
+    public function testLinkToDashboard()
+    {
+        $client = static::createClient();
+        $client->followRedirects();
+
+        $crawler = $client->request('GET', '/admin/pretty/urls/category');
+
+        // assert the Dashboard link points to the right URL
+        $this->assertSame('/admin/pretty/urls', $crawler->filter('.menu-item a:contains("Dashboard")')->attr('href'));
+        // assert the main menu contains the right items pointing to the right URLs
+        $this->assertSelectorExists('.menu-item a:contains("Dashboard")');
+        $this->assertSelectorExists('.menu-item.active a:contains("Categories")');
+        $this->assertSame('http://localhost/admin/pretty/urls/category', $crawler->filter('.menu-item a:contains("Categories")')->attr('href'));
+        $this->assertSelectorExists('.menu-item a:contains("Blog Posts")');
+        $this->assertSame('http://localhost/admin/pretty/urls/blog-post', $crawler->filter('.menu-item a:contains("Blog Posts")')->attr('href'));
+        $this->assertSelectorNotExists('.menu-item a:contains("Users")');
+
+        $crawler = $client->request('GET', '/second/dashboard/user-editor/custom/path-for-index');
+
+        // assert the Dashboard link points to the right URL
+        $this->assertSame('/second/dashboard', $crawler->filter('.menu-item a:contains("Dashboard")')->attr('href'));
+        // assert the main menu contains the right items pointing to the right URLs
+        $this->assertSelectorExists('.menu-item a:contains("Dashboard")');
+        $this->assertSelectorNotExists('.menu-item.active a:contains("Categories")');
+        $this->assertSelectorNotExists('.menu-item a:contains("Blog Posts")');
+        $this->assertSelectorExists('.menu-item.active a:contains("Users")');
+        $this->assertSame('http://localhost/second/dashboard/user-editor/custom/path-for-index', $crawler->filter('.menu-item a:contains("Users")')->attr('href'));
+    }
+
     public function testDefaultCrudController()
     {
         $client = static::createClient();

diff --git a/tests/Menu/MenuItemMatcherTest.php b/tests/Menu/MenuItemMatcherTest.php
@@ -6,6 +6,7 @@
 use EasyCorp\Bundle\EasyAdminBundle\Config\Option\EA;
 use EasyCorp\Bundle\EasyAdminBundle\Dto\MenuItemDto;
 use EasyCorp\Bundle\EasyAdminBundle\Menu\MenuItemMatcher;
+use EasyCorp\Bundle\EasyAdminBundle\Router\AdminUrlGenerator;
 use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase;
 use Symfony\Component\HttpFoundation\InputBag;
 use Symfony\Component\HttpFoundation\Request;
@@ -15,7 +16,10 @@ class MenuItemMatcherTest extends KernelTestCase
     public function testIsSelectedWhenContextIsNull()
     {
         $request = $this->getRequestMock();
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItemDto = new MenuItemDto();
 
         $menuItemMatcher->markSelectedMenuItem([$menuItemDto], $request);
@@ -26,7 +30,10 @@ public function testIsSelectedWhenContextIsNull()
     public function testIsSelectedWhenMenuItemIsSection()
     {
         $request = $this->getRequestMock();
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItemDto = new MenuItemDto();
         $menuItemDto->setType(MenuItemDto::TYPE_SECTION);
 
@@ -40,7 +47,10 @@ public function testIsSelectedWithCrudControllers()
         $request = $this->getRequestMock(
             getControllerFqcn: 'App\Controller\Admin\SomeController',
         );
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItemDto = $this->getMenuItemDto();
         $menuItemMatcher->markSelectedMenuItem([$menuItemDto], $request);
 
@@ -88,7 +98,10 @@ public function testIsSelectedWithRoutes()
         $request = $this->getRequestMock(
             routeName: 'some_route',
         );
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItemDto = $this->getMenuItemDto(routeName: 'some_route');
 
         $menuItemMatcher->markSelectedMenuItem([$menuItemDto], $request);
@@ -126,7 +139,10 @@ public function testIsSelectedWithUrls()
         $request = $this->getRequestMock(
             getUri: 'https://example.com/foo?bar=baz',
         );
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItemDto = new MenuItemDto();
 
         $menuItemMatcher->markSelectedMenuItem([$menuItemDto], $request);
@@ -164,7 +180,9 @@ public function testMenuWithDashboardItem()
             routeName: 'item2',
         );
 
-        $menuItemMatcher = new MenuItemMatcher();
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
         $menuItems = $menuItemMatcher->markSelectedMenuItem($menuItems, $request);
 
         $this->assertSame('item2', $this->getSelectedMenuItemLabel($menuItems), 'Perfect match: Dashboard item');
@@ -176,7 +194,10 @@ public function testComplexMenu()
         $request = $this->getRequestMock(
             getControllerFqcn: 'App\Controller\Admin\Controller1',
         );
-        $menuItemMatcher = new MenuItemMatcher();
+
+        self::bootKernel();
+        $adminUrlGenerator = self::getContainer()->get(AdminUrlGenerator::class);
+        $menuItemMatcher = new MenuItemMatcher($adminUrlGenerator);
 
         $menuItems = $menuItemMatcher->markSelectedMenuItem($menuItems, $request);
 

diff --git a/tests/PrettyUrlsTestApplication/src/Controller/DashboardController.php b/tests/PrettyUrlsTestApplication/src/Controller/DashboardController.php
@@ -27,7 +27,10 @@ public function configureDashboard(): Dashboard
     public function configureMenuItems(): iterable
     {
         yield MenuItem::linktoDashboard('Dashboard', 'fa fa-home');
-        yield MenuItem::linkToCrud('Categories', 'fas fa-tags', Category::class);
-        yield MenuItem::linkToCrud('Blog Posts', 'fas fa-tags', BlogPost::class);
+
+        yield MenuItem::subMenu('Blog', 'fas fa-blog')->setSubItems([
+            MenuItem::linkToCrud('Categories', 'fas fa-tags', Category::class),
+            MenuItem::linkToCrud('Blog Posts', 'far fa-file-lines', BlogPost::class),
+        ]);
     }
 }