Merge pull request #1597 from hydephp/refactor-main-menu-generation-t…

…o-be-deterministic [2.x] Always place navigation menu items in dropdowns when set in front matter
hydephp · Feb 28, 2024 · 5eb64a1 · 5eb64a1
2 parents e2b14c9 + 353603b
commit 5eb64a1
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 19 deletions.
diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php
@@ -120,14 +120,14 @@ protected function canAddRoute(Route $route): bool
 
     protected function canGroupRoute(Route $route): bool
     {
-        if (! $this->usesGroups) {
-            return false;
-        }
-
         if (! $this->generatesSidebar) {
             return $route->getPage()->navigationMenuGroup() !== null;
         }
 
+        if (! $this->usesGroups) {
+            return false;
+        }
+
         return true;
     }
 

diff --git a/packages/framework/tests/Feature/AutomaticNavigationConfigurationsTest.php b/packages/framework/tests/Feature/AutomaticNavigationConfigurationsTest.php
@@ -200,29 +200,21 @@ public function testMainNavigationMenuWithFrontMatterVisible()
         ]);
     }
 
-    public function testMainNavigationMenuWithFrontMatterGroup()
+    public function testMainNavigationMenuWithExplicitFrontMatterGroupUsesDropdownsRegardlessOfConfigSetting()
     {
-        // TODO: For new v2 system, this should insert a root item with the group name and the children as the pages
-
         $this->assertMenuEquals([
-            ['label' => 'Foo', 'group' => 'group-1'],
-            ['label' => 'Bar', 'group' => 'group-1'],
-            ['label' => 'Baz', 'group' => 'group-1'],
+            ['label' => 'Group 1', 'children' => ['Foo', 'Bar', 'Baz']],
         ], [
             new MarkdownPage('foo', ['navigation.group' => 'Group 1']),
             new MarkdownPage('bar', ['navigation.group' => 'Group 1']),
             new MarkdownPage('baz', ['navigation.group' => 'Group 1']),
         ]);
     }
 
-    public function testMainNavigationMenuWithFrontMatterCategory()
+    public function testMainNavigationMenuWithExplicitFrontMatterCategoryUsesDropdownsRegardlessOfConfigSetting()
     {
-        // TODO: For new v2 system, this should insert a root item with the group name and the children as the pages
-
         $this->assertMenuEquals([
-            ['label' => 'Foo', 'group' => 'group-1'],
-            ['label' => 'Bar', 'group' => 'group-1'],
-            ['label' => 'Baz', 'group' => 'group-1'],
+            ['label' => 'Group 1', 'children' => ['Foo', 'Bar', 'Baz']],
         ], [
             new MarkdownPage('foo', ['navigation.category' => 'Group 1']),
             new MarkdownPage('bar', ['navigation.category' => 'Group 1']),
@@ -292,7 +284,10 @@ public function testMainNavigationMenuWithFrontMatterGroupAndCategory()
     {
         // Since the main key in the navigation schema is 'group', that takes precedence over its 'category' alias
 
-        $this->assertMenuEquals(array_fill(0, 3, ['group' => 'group-1']), [
+        $this->assertMenuEquals([[
+            'label' => 'Group 1',
+            'children' => ['Foo', 'Bar', 'Baz'],
+        ]], [
             new MarkdownPage('foo', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
             new MarkdownPage('bar', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
             new MarkdownPage('baz', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
@@ -494,8 +489,8 @@ public function testMainNavigationMenuItemsWithTheSameLabelAreNotFilteredForDupl
     public function testMainNavigationMenuItemsWithSameLabelButDifferentGroupsAreNotFiltered()
     {
         $this->assertMenuEquals([
-            ['label' => 'Foo', 'group' => 'group-1'],
-            ['label' => 'Foo', 'group' => 'group-2'],
+            ['label' => 'Group 1', 'children' => ['Foo']],
+            ['label' => 'Group 2', 'children' => ['Foo']],
         ], [
             new MarkdownPage('foo', ['navigation.label' => 'Foo', 'navigation.group' => 'Group 1']),
             new MarkdownPage('bar', ['navigation.label' => 'Foo', 'navigation.group' => 'Group 2']),