Merge pull request #1577 from hydephp/simplify-dropdown-data-handling

[2.x] Simplify dropdown data handling

Author: emmadesilva

packages/framework/src/Framework/Features/Navigation/GeneratesDocumentationSidebarMenu.php

@@ -4,6 +4,7 @@
 
 namespace Hyde\Framework\Features\Navigation;
 
+use Hyde\Hyde;
 use Hyde\Facades\Config;
 use Illuminate\Support\Str;
 use Hyde\Support\Models\Route;
@@ -12,7 +13,9 @@
 use Hyde\Foundation\Facades\Routes;
 use Hyde\Foundation\Kernel\RouteCollection;
 
+use function filled;
 use function collect;
+use function strtolower;
 
 /**
  * @experimental This class may change significantly before its release.
@@ -121,12 +124,30 @@ protected function getOrCreateGroupItem(string $groupName): NavItem
     protected function createGroupItem(string $identifier, string $groupName): NavItem
     {
         $label = $this->searchForGroupLabelInConfig($identifier) ?? $groupName;
+        $priority = $this->searchForGroupPriorityInConfig($identifier);
 
-        return NavItem::dropdown($label, []);
+        return NavItem::dropdown(static::normalizeGroupLabel($label), [], $priority);
     }
 
     protected function searchForGroupLabelInConfig(string $identifier): ?string
     {
         return Config::getArray('docs.sidebar_group_labels', [])[$identifier] ?? null;
     }
+
+    /** Todo: Move into shared class */
+    protected static function normalizeGroupLabel(string $label): string
+    {
+        // If there is no label, and the group is a slug, we can make a title from it
+        if ($label === strtolower($label)) {
+            return Hyde::makeTitle($label);
+        }
+
+        return $label;
+    }
+
+    /** Todo: Move into shared class */
+    protected static function searchForGroupPriorityInConfig(string $groupKey): ?int
+    {
+        return Config::getArray('docs.sidebar_order', [])[$groupKey] ?? null;
+    }
 }

packages/framework/src/Framework/Features/Navigation/GeneratesMainNavigationMenu.php

@@ -4,13 +4,15 @@
 
 namespace Hyde\Framework\Features\Navigation;
 
+use Hyde\Hyde;
 use Hyde\Facades\Config;
 use Hyde\Support\Models\Route;
 use Hyde\Pages\DocumentationPage;
 use Illuminate\Support\Collection;
 use Hyde\Foundation\Facades\Routes;
 
 use function collect;
+use function strtolower;
 
 /**
  * @experimental This class may change significantly before its release.
@@ -72,7 +74,7 @@ protected function moveGroupedItemsIntoDropdowns(): void
 
         foreach ($dropdowns as $group => $items) {
             // Create a new dropdown item containing the buffered items
-            $this->items->add(NavItem::dropdown($group, $items));
+            $this->items->add(NavItem::dropdown(static::normalizeGroupLabel($group), $items, static::searchForDropdownPriorityInConfig($group)));
         }
     }
 
@@ -90,4 +92,21 @@ protected function useSubdirectoriesAsDropdowns(): bool
     {
         return Config::getString('hyde.navigation.subdirectories', 'hidden') === 'dropdown';
     }
+
+    /** Todo: Move into shared class */
+    protected static function normalizeGroupLabel(string $label): string
+    {
+        // If there is no label, and the group is a slug, we can make a title from it
+        if ($label === strtolower($label)) {
+            return Hyde::makeTitle($label);
+        }
+
+        return $label;
+    }
+
+    /** Todo: Move into shared class */
+    protected static function searchForDropdownPriorityInConfig(string $groupKey): ?int
+    {
+        return Config::getArray('hyde.navigation.order', [])[$groupKey] ?? null;
+    }
 }

packages/framework/src/Framework/Features/Navigation/NavItem.php

@@ -4,7 +4,6 @@
 
 namespace Hyde\Framework\Features\Navigation;
 
-use Hyde\Facades\Config;
 use Hyde\Foundation\Facades\Routes;
 use Hyde\Hyde;
 use Hyde\Support\Models\Route;
@@ -13,7 +12,6 @@
 use Hyde\Support\Models\ExternalRoute;
 
 use function is_string;
-use function strtolower;
 
 /**
  * Abstraction for a navigation menu item. Used by the MainNavigationMenu and DocumentationSidebar classes.
@@ -100,7 +98,7 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int
      */
     public static function dropdown(string $label, array $items, ?int $priority = null): static
     {
-        return new static('', static::normalizeGroupLabel($label), $priority ?? static::searchForDropdownPriorityInNavigationConfig($label) ?? 999, $label, $items);
+        return new static('', $label, $priority ?? 999, $label, $items);
     }
 
     /**
@@ -208,20 +206,4 @@ protected static function makeIdentifier(string $label): string
     {
         return Str::slug($label); // Todo: If it's a dropdown based on a subdirectory, we should use the subdirectory as the identifier
     }
-
-    // TODO: Consider moving all of these to a dropdown factory
-    protected static function searchForDropdownPriorityInNavigationConfig(string $groupKey): ?int
-    {
-        return Config::getArray('hyde.navigation.order', [])[$groupKey] ?? null;
-    }
-
-    protected static function normalizeGroupLabel(string $label): string
-    {
-        // If there is no label, and the group is a slug, we can make a title from it
-        if ($label === strtolower($label)) {
-            return Hyde::makeTitle($label);
-        }
-
-        return $label;
-    }
 }

packages/framework/tests/Feature/AutomaticNavigationConfigurationsTest.php

@@ -1038,6 +1038,50 @@ public function testAllSidebarItemsArePlacedInGroupsWhenAtLeastOneItemIsGrouped(
         ]);
     }
 
+    // Priority tests
+
+    public function testMainNavigationDropdownPriorityCanBeSetInConfig()
+    {
+        config(['hyde.navigation.subdirectories' => 'dropdown']);
+        config(['hyde.navigation.order' => ['foo' => 500]]);
+
+        $this->assertMenuEquals(
+            [['label' => 'Foo', 'priority' => 500]],
+            [new MarkdownPage('foo/bar')]
+        );
+    }
+
+    public function testMainNavigationDropdownPriorityCanBeSetInConfigUsingDifferingCases()
+    {
+        config(['hyde.navigation.subdirectories' => 'dropdown']);
+        config(['hyde.navigation.order' => ['hello-world' => 500]]);
+
+        $expected = [['label' => 'Hello World', 'priority' => 500]];
+        $this->assertMenuEquals($expected, [new MarkdownPage('Hello World/bar')]);
+        $this->assertMenuEquals($expected, [new MarkdownPage('hello-world/bar')]);
+        $this->assertMenuEquals($expected, [new MarkdownPage('hello world/bar')]);
+    }
+
+    public function testSidebarGroupPriorityCanBeSetInConfig()
+    {
+        config(['docs.sidebar_order' => ['foo' => 500]]);
+
+        $this->assertSidebarEquals(
+            [['label' => 'Foo', 'priority' => 500]],
+            [new DocumentationPage('foo/bar')]
+        );
+    }
+
+    public function testSidebarGroupPriorityCanBeSetInConfigUsingDifferingCases()
+    {
+        config(['docs.sidebar_order' => ['hello-world' => 500]]);
+
+        $expected = [['label' => 'Hello World', 'priority' => 500]];
+        $this->assertSidebarEquals($expected, [new DocumentationPage('Hello World/bar')]);
+        $this->assertSidebarEquals($expected, [new DocumentationPage('hello-world/bar')]);
+        $this->assertSidebarEquals($expected, [new DocumentationPage('hello world/bar')]);
+    }
+
     // Label casing tests
 
     public function testMainMenuNavigationItemCasing()

packages/framework/tests/Feature/NavigationMenuTest.php

@@ -228,7 +228,7 @@ public function testPagesInSubdirectoriesAreNotAddedToTheNavigationMenuWithConfi
         $menu = $this->createNavigationMenu();
         $expected = collect([
             NavItem::fromRoute(Routes::get('index')),
-            NavItem::dropdown('foo', [
+            NavItem::dropdown('Foo', [
                 NavItem::fromRoute(Routes::get('foo/bar')),
             ]),
         ]);
@@ -249,7 +249,7 @@ public function testPagesInDropdownsDoNotGetAddedToTheMainNavigation()
         $this->assertEquals([
             NavItem::fromRoute(Routes::get('index')),
             NavItem::fromRoute((new MarkdownPage('foo'))->getRoute()),
-            NavItem::dropdown('bar', [
+            NavItem::dropdown('Bar', [
                 NavItem::fromRoute((new MarkdownPage('bar/baz'))->getRoute()),
             ]),
         ], $menu->getItems()->all());

packages/framework/tests/Unit/NavItemTest.php

@@ -265,7 +265,7 @@ public function testDropdownFacade()
     {
         $item = NavItem::dropdown('foo', []);
 
-        $this->assertSame('Foo', $item->getLabel());
+        $this->assertSame('foo', $item->getLabel());
         $this->assertSame([], $item->getChildren());
         $this->assertSame(999, $item->getPriority());
     }
@@ -396,16 +396,4 @@ public function testCanAddItemToDropdown()
 
         $this->assertSame([$child], $parent->getChildren());
     }
-
-    public function testDefaultDropdownItemPriority()
-    {
-        $this->assertSame(999, NavItem::dropdown('foo', [])->getPriority());
-    }
-
-    public function testCanResolveDropdownItemPriorityFromConfig()
-    {
-        $this->mockConfig(['hyde.navigation.order' => ['foo' => 500]]);
-
-        $this->assertSame(500, NavItem::dropdown('foo', [])->getPriority());
-    }
 }