Merge pull request #1574 from hydephp/change-navitem-class-api-to-be-…

…fluent-instead-of-readonly [2.x] Change NavItem class API to be fluent instead of readonly
hydephp · Feb 19, 2024 · 35c1400 · 35c1400
2 parents b018e7d + 482ac8a
commit 35c1400
Show file tree

Hide file tree

Showing 9 changed files with 72 additions and 64 deletions.
diff --git a/packages/framework/resources/views/components/docs/sidebar-item.blade.php b/packages/framework/resources/views/components/docs/sidebar-item.blade.php
@@ -5,23 +5,23 @@
         : 'active bg-black/5 dark:bg-black/10' => $item->isCurrent()
     ]) role="listitem">
     @if($item->isCurrent())
-        <a href="{{ $item->destination }}" aria-current="true" @class([$grouped
+        <a href="{{ $item->getDestination() }}" aria-current="true" @class([$grouped
             ? '-ml-8 pl-4 py-1 px-2 block text-indigo-600 dark:text-indigo-400 dark:font-medium border-l-[0.325rem] border-indigo-500 transition-colors duration-300 ease-in-out hover:bg-black/10'
             : '-ml-4 p-2 block hover:bg-black/5 dark:hover:bg-black/10 text-indigo-600 dark:text-indigo-400 dark:font-medium border-l-[0.325rem] border-indigo-500 transition-colors duration-300 ease-in-out'
         ])>
-            {{ $item->label }}
+            {{ $item->getLabel() }}
         </a>
 
         @if(config('docs.table_of_contents.enabled', true))
             <span class="sr-only">Table of contents</span>
             {!! ($page->getTableOfContents()) !!}
         @endif
     @else
-        <a href="{{ $item->destination }}" @class([$grouped
+        <a href="{{ $item->getDestination() }}" @class([$grouped
             ? '-ml-8 pl-4 py-1 px-2 block border-l-[0.325rem] border-transparent transition-colors duration-300 ease-in-out hover:bg-black/10'
             : 'block -ml-4 p-2 border-l-[0.325rem] border-transparent hover:bg-black/5 dark:hover:bg-black/10'
         ])>
-            {{ $item->label }}
+            {{ $item->getLabel() }}
         </a>
     @endif
 </li>
diff --git a/packages/framework/resources/views/components/navigation/navigation-link.blade.php b/packages/framework/resources/views/components/navigation/navigation-link.blade.php
@@ -1,4 +1,4 @@
 <a href="{{ $item }}" {!! $item->isCurrent() ? 'aria-current="page"' : '' !!} @class([
     'block my-2 md:my-0 md:inline-block py-1 text-gray-700 hover:text-gray-900 dark:text-gray-100',
     'border-l-4 border-indigo-500 md:border-none font-medium -ml-6 pl-5 md:ml-0 md:pl-0 bg-gray-100 dark:bg-gray-800 md:bg-transparent dark:md:bg-transparent' => $item->isCurrent()
-])>{{ $item->label }}</a>
+])>{{ $item->getLabel() }}</a>
diff --git a/packages/framework/src/Framework/Features/Navigation/GeneratesDocumentationSidebarMenu.php b/packages/framework/src/Framework/Features/Navigation/GeneratesDocumentationSidebarMenu.php
@@ -87,6 +87,6 @@ protected function canAddRoute(Route $route): bool
 
     protected function sortByPriority(): void
     {
-        $this->items = $this->items->sortBy('priority')->values();
+        $this->items = $this->items->sortBy(fn (NavItem $item) => $item->getPriority())->values();
     }
 }
diff --git a/packages/framework/src/Framework/Features/Navigation/GeneratesMainNavigationMenu.php b/packages/framework/src/Framework/Features/Navigation/GeneratesMainNavigationMenu.php
@@ -94,6 +94,6 @@ protected function useSubdirectoriesAsDropdowns(): bool
 
     protected function sortByPriority(): void
     {
-        $this->items = $this->items->sortBy('priority')->values();
+        $this->items = $this->items->sortBy(fn (NavItem $item): int => $item->getPriority())->values();
     }
 }
diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php
@@ -24,16 +24,16 @@
  */
 class NavItem implements Stringable
 {
-    public readonly Route $destination;
-    public readonly string $label;
-    public readonly int $priority;
-    public readonly ?string $group;
+    protected Route $destination;
+    protected string $label;
+    protected int $priority;
+    protected ?string $group;
 
     /** The "slugified" version of the label. */
-    public readonly string $identifier;
+    protected string $identifier;
 
     /** @var array<\Hyde\Framework\Features\Navigation\NavItem> */
-    public readonly array $children;
+    protected array $children;
 
     /**
      * Create a new navigation menu item.
@@ -149,6 +149,14 @@ public function getGroup(): ?string
         return $this->group;
     }
 
+    /**
+     * Get the identifier of the navigation item.
+     */
+    public function getIdentifier(): string
+    {
+        return $this->identifier;
+    }
+
     /**
      * Get the children of the navigation item.
      *

diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationMenu.php b/packages/framework/src/Framework/Features/Navigation/NavigationMenu.php
@@ -36,7 +36,7 @@ public function __construct(Arrayable|array $items = [])
      */
     public function getItems(): Collection
     {
-        return $this->items->sortBy('priority')->values();
+        return $this->items->sortBy(fn (NavItem $item) => $item->getPriority())->values();
     }
 
     /**

diff --git a/packages/framework/tests/Feature/NavigationMenuTest.php b/packages/framework/tests/Feature/NavigationMenuTest.php
@@ -46,15 +46,15 @@ public function testItemsAreSortedByPriority()
         Routes::addRoute(new Route(new MarkdownPage('bar', ['navigation.priority' => 2])));
         Routes::addRoute(new Route(new MarkdownPage('baz', ['navigation.priority' => 3])));
 
-        $this->assertSame(['Home', 'Foo', 'Bar', 'Baz'], $this->createNavigationMenu()->getItems()->pluck('label')->toArray());
+        $this->assertSame(['Home', 'Foo', 'Bar', 'Baz'], $this->createNavigationMenu()->getItems()->map(fn ($item) => $item->getLabel())->toArray());
     }
 
     public function testItemsWithHiddenPropertySetToTrueAreNotAdded()
     {
         Routes::addRoute(new Route(new MarkdownPage('foo', ['navigation.hidden' => true])));
         Routes::addRoute(new Route(new MarkdownPage('bar', ['navigation.hidden' => false])));
 
-        $this->assertSame(['Home', 'Bar'], $this->createNavigationMenu()->getItems()->pluck('label')->toArray());
+        $this->assertSame(['Home', 'Bar'], $this->createNavigationMenu()->getItems()->map(fn ($item) => $item->getLabel())->toArray());
     }
 
     public function testCreatedCollectionIsSortedByNavigationMenuPriority()
@@ -266,7 +266,7 @@ public function testCanAddItemsToMainNavigationMenuResolvedFromContainer()
         $navigation->add(new NavItem(new ExternalRoute('/foo'), 'Foo'));
 
         $this->assertCount(2, $navigation->getItems());
-        $this->assertSame('Foo', $navigation->getItems()->last()->label);
+        $this->assertSame('Foo', $navigation->getItems()->last()->getLabel());
     }
 
     protected function createNavigationMenu(): NavigationMenu

diff --git a/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php b/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php
@@ -114,7 +114,7 @@ public function testSidebarItemPriorityCanBeSetInFrontMatter()
     {
         $this->makePage('foo', ['navigation.priority' => 25]);
 
-        $this->assertEquals(25, DocumentationSidebar::create()->getItems()->first()->priority);
+        $this->assertEquals(25, DocumentationSidebar::create()->getItems()->first()->getPriority());
     }
 
     public function testSidebarItemPrioritySetInConfigOverridesFrontMatter()
@@ -123,7 +123,7 @@ public function testSidebarItemPrioritySetInConfigOverridesFrontMatter()
 
         Config::set('docs.sidebar_order', ['foo']);
 
-        $this->assertEquals(25, DocumentationSidebar::create()->getItems()->first()->priority);
+        $this->assertEquals(25, DocumentationSidebar::create()->getItems()->first()->getPriority());
     }
 
     public function testSidebarPrioritiesCanBeSetInBothFrontMatterAndConfig()

diff --git a/packages/framework/tests/Unit/NavItemTest.php b/packages/framework/tests/Unit/NavItemTest.php
@@ -44,36 +44,36 @@ public function testConstruct()
         $route = new Route(new MarkdownPage());
         $item = new NavItem($route, 'Test', 500);
 
-        $this->assertSame($route, $item->destination);
+        $this->assertSame($route, $item->getDestination());
     }
 
     public function testPassingRouteInstanceToConstructorUsesRouteInstance()
     {
         $route = new Route(new MarkdownPage());
-        $this->assertSame($route, (new NavItem($route, 'Home'))->destination);
+        $this->assertSame($route, (new NavItem($route, 'Home'))->getDestination());
     }
 
     public function testPassingRouteKeyToConstructorUsesRouteInstance()
     {
         $route = Routes::get('index');
 
-        $this->assertSame($route, (new NavItem('index', 'Home'))->destination);
+        $this->assertSame($route, (new NavItem('index', 'Home'))->getDestination());
     }
 
     public function testPassingUrlToConstructorUsesExternalRoute()
     {
         $item = new NavItem('https://example.com', 'Home');
-        $this->assertInstanceOf(ExternalRoute::class, $item->destination);
-        $this->assertEquals(new ExternalRoute('https://example.com'), $item->destination);
-        $this->assertSame('https://example.com', (string) $item->destination);
+        $this->assertInstanceOf(ExternalRoute::class, $item->getDestination());
+        $this->assertEquals(new ExternalRoute('https://example.com'), $item->getDestination());
+        $this->assertSame('https://example.com', (string) $item->getDestination());
     }
 
     public function testPassingUnknownRouteKeyToConstructorUsesExternalRoute()
     {
         $item = new NavItem('foo', 'Home');
-        $this->assertInstanceOf(ExternalRoute::class, $item->destination);
-        $this->assertEquals(new ExternalRoute('foo'), $item->destination);
-        $this->assertSame('foo', (string) $item->destination);
+        $this->assertInstanceOf(ExternalRoute::class, $item->getDestination());
+        $this->assertEquals(new ExternalRoute('foo'), $item->getDestination());
+        $this->assertSame('foo', (string) $item->getDestination());
     }
 
     public function testCanConstructWithChildren()
@@ -85,21 +85,21 @@ public function testCanConstructWithChildren()
         ];
         $item = new NavItem($route, 'Test', 500, null, $children);
 
-        $this->assertSame('Test', $item->label);
-        $this->assertSame($route, $item->destination);
-        $this->assertSame(500, $item->priority);
+        $this->assertSame('Test', $item->getLabel());
+        $this->assertSame($route, $item->getDestination());
+        $this->assertSame(500, $item->getPriority());
 
-        $this->assertCount(2, $item->children);
-        $this->assertSame($children, $item->children);
+        $this->assertCount(2, $item->getChildren());
+        $this->assertSame($children, $item->getChildren());
 
-        $this->assertSame('Foo', $item->children[0]->label);
-        $this->assertSame('Bar', $item->children[1]->label);
+        $this->assertSame('Foo', $item->getChildren()[0]->getLabel());
+        $this->assertSame('Bar', $item->getChildren()[1]->getLabel());
 
-        $this->assertSame('foo.html', $item->children[0]->getLink());
-        $this->assertSame('bar.html', $item->children[1]->getLink());
+        $this->assertSame('foo.html', $item->getChildren()[0]->getLink());
+        $this->assertSame('bar.html', $item->getChildren()[1]->getLink());
 
-        $this->assertSame(500, $item->children[0]->priority);
-        $this->assertSame(500, $item->children[1]->priority);
+        $this->assertSame(500, $item->getChildren()[0]->getPriority());
+        $this->assertSame(500, $item->getChildren()[1]->getPriority());
     }
 
     public function testCanConstructWithChildrenWithoutRoute()
@@ -110,11 +110,11 @@ public function testCanConstructWithChildrenWithoutRoute()
         ];
         $item = new NavItem('', 'Test', 500, null, $children);
 
-        $this->assertSame('Test', $item->label);
-        $this->assertSame('', $item->destination->getLink());
+        $this->assertSame('Test', $item->getLabel());
+        $this->assertSame('', $item->getDestination()->getLink());
 
-        $this->assertCount(2, $item->children);
-        $this->assertSame($children, $item->children);
+        $this->assertCount(2, $item->getChildren());
+        $this->assertSame($children, $item->getChildren());
     }
 
     public function testGetDestination()
@@ -171,7 +171,7 @@ public function testFromRoute()
         $route = new Route(new MarkdownPage());
         $item = NavItem::fromRoute($route);
 
-        $this->assertSame($route, $item->destination);
+        $this->assertSame($route, $item->getDestination());
     }
 
     public function testToString()
@@ -185,34 +185,34 @@ public function testForLink()
     {
         $item = NavItem::forLink('foo', 'bar');
 
-        $this->assertEquals(new ExternalRoute('foo'), $item->destination);
-        $this->assertSame('bar', $item->label);
-        $this->assertSame(500, $item->priority);
+        $this->assertEquals(new ExternalRoute('foo'), $item->getDestination());
+        $this->assertSame('bar', $item->getLabel());
+        $this->assertSame(500, $item->getPriority());
     }
 
     public function testForLinkWithCustomPriority()
     {
-        $this->assertSame(100, NavItem::forLink('foo', 'bar', 100)->priority);
+        $this->assertSame(100, NavItem::forLink('foo', 'bar', 100)->getPriority());
     }
 
     public function testForRoute()
     {
         $route = Routes::get('404');
         $item = NavItem::forRoute($route, 'foo');
 
-        $this->assertSame($route, $item->destination);
-        $this->assertSame('foo', $item->label);
-        $this->assertSame(999, $item->priority);
+        $this->assertSame($route, $item->getDestination());
+        $this->assertSame('foo', $item->getLabel());
+        $this->assertSame(999, $item->getPriority());
     }
 
     public function testForIndexRoute()
     {
         $route = Routes::get('index');
         $item = NavItem::forRoute($route, 'foo');
 
-        $this->assertSame($route, $item->destination);
-        $this->assertSame('foo', $item->label);
-        $this->assertSame(0, $item->priority);
+        $this->assertSame($route, $item->getDestination());
+        $this->assertSame('foo', $item->getLabel());
+        $this->assertSame(0, $item->getPriority());
     }
 
     public function testForRouteWithRouteKey()
@@ -231,7 +231,7 @@ public function testForRouteWithMissingRouteKey()
 
     public function testForRouteWithCustomPriority()
     {
-        $this->assertSame(100, NavItem::forRoute(Routes::get('index'), 'foo', 100)->priority);
+        $this->assertSame(100, NavItem::forRoute(Routes::get('index'), 'foo', 100)->getPriority());
     }
 
     public function testRouteBasedNavItemDestinationsAreResolvedRelatively()
@@ -265,9 +265,9 @@ public function testDropdownFacade()
     {
         $item = NavItem::dropdown('foo', []);
 
-        $this->assertSame('foo', $item->label);
+        $this->assertSame('foo', $item->getLabel());
         $this->assertSame([], $item->getChildren());
-        $this->assertSame(999, $item->priority);
+        $this->assertSame(999, $item->getPriority());
     }
 
     public function testDropdownFacadeWithChildren()
@@ -278,14 +278,14 @@ public function testDropdownFacadeWithChildren()
 
         $item = NavItem::dropdown('foo', $children);
         $this->assertSame($children, $item->getChildren());
-        $this->assertSame(999, $item->priority);
+        $this->assertSame(999, $item->getPriority());
     }
 
     public function testDropdownFacadeWithCustomPriority()
     {
         $item = NavItem::dropdown('foo', [], 500);
 
-        $this->assertSame(500, $item->priority);
+        $this->assertSame(500, $item->getPriority());
     }
 
     public function testHasChildren()
@@ -354,34 +354,34 @@ public function testIdentifier()
         $route = new Route(new MarkdownPage());
         $item = new NavItem($route, 'Test', 500);
 
-        $this->assertSame('test', $item->identifier);
+        $this->assertSame('test', $item->getIdentifier());
     }
 
     public function testIdentifierWithCustomLabel()
     {
         $route = new Route(new MarkdownPage());
         $item = new NavItem($route, 'Foo Bar', 500);
 
-        $this->assertSame('foo-bar', $item->identifier);
+        $this->assertSame('foo-bar', $item->getIdentifier());
     }
 
     public function testIdentifierFromRouteKey()
     {
         $item = NavItem::fromRoute(Routes::get('index'));
-        $this->assertSame('home', $item->identifier);
+        $this->assertSame('home', $item->getIdentifier());
     }
 
     public function testIdentifierUsesLabelWhenRouteKeyIsFalsy()
     {
         $route = new Route(new MarkdownPage());
         $item = new NavItem($route, 'Foo Bar', 500);
 
-        $this->assertSame('foo-bar', $item->identifier);
+        $this->assertSame('foo-bar', $item->getIdentifier());
     }
 
     public function testIdentifierUsesLabelForExternalRoute()
     {
         $item = NavItem::forLink('https://example.com', 'Foo Bar');
-        $this->assertSame('foo-bar', $item->identifier);
+        $this->assertSame('foo-bar', $item->getIdentifier());
     }
 }
-Original file line number
+Diff line change
@@ Expand Up / @@ -36,7 +36,7 @@ public function __construct(Arrayable|array $items = []) @@
          */
         public function getItems(): Collection
         {
-            return $this->items->sortBy('priority')->values();
+            return $this->items->sortBy(fn (NavItem $item) => $item->getPriority())->values();
         }
         /**
@@ Expand Down @@