Merge pull request #1597 from hydephp/refactor-main-menu-generation-to-be-deterministic

[2.x] Always place navigation menu items in dropdowns when set in front matter

Author: emmadesilva

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;
     }
 

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']),