diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 1c7e3aa81e5..3c8def52a9b 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,7 +11,6 @@ This serves two purposes: ### Added - Added a new `\Hyde\Framework\Actions\PreBuildTasks\TransferMediaAssets` build task handle media assets transfers for site builds. -- Added a new `ExternalRoute` class to represent external routes. - Added a new `NavigationItem::getLink()` method contain the previous `NavigationItem::getDestination()` logic, to return the link URL. ### Changed diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php new file mode 100644 index 00000000000..bcf07e11763 --- /dev/null +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -0,0 +1,41 @@ +destination = $destination; + } + + public function __toString(): string + { + return $this->getLink(); + } + + public function getLink(): string + { + return (string) $this->destination; + } + + /** @experimental */ + public function getRoute(): ?Route + { + return $this->destination instanceof Route ? $this->destination : null; + } +} diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationGroup.php b/packages/framework/src/Framework/Features/Navigation/NavigationGroup.php index ebd076aa9af..5b9c0ea8896 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationGroup.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationGroup.php @@ -7,7 +7,6 @@ use Illuminate\Support\Str; use Illuminate\Support\Arr; use Hyde\Pages\DocumentationPage; -use Hyde\Support\Models\ExternalRoute; use function min; use function collect; @@ -79,7 +78,7 @@ protected function addItem(NavigationItem $item): void protected function containsOnlyDocumentationPages(): bool { return count($this->getItems()) && collect($this->getItems())->every(function (NavigationItem $child): bool { - return (! $child->getRoute() instanceof ExternalRoute) && $child->getRoute()->getPage() instanceof DocumentationPage; + return ($child->getRoute() !== null) && $child->getRoute()->getPage() instanceof DocumentationPage; }); } } diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 7d67811207a..867684ea5f5 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -9,7 +9,6 @@ use Hyde\Support\Models\Route; use Illuminate\Support\Str; use Stringable; -use Hyde\Support\Models\ExternalRoute; use function is_string; @@ -23,7 +22,7 @@ */ class NavigationItem implements NavigationElement, Stringable { - protected Route $route; + protected NavigationDestination $destination; protected string $label; protected int $priority; @@ -40,11 +39,8 @@ class NavigationItem implements NavigationElement, Stringable */ public function __construct(Route|string $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null) { - if (is_string($destination)) { - $destination = Routes::get($destination) ?? new ExternalRoute($destination); - } + $this->destination = new NavigationDestination($destination); - $this->route = $destination; $this->label = $label; $this->priority = $priority; @@ -63,11 +59,11 @@ public function __construct(Route|string $destination, string $label, int $prior */ public static function create(Route|string $destination, ?string $label = null, ?int $priority = null, ?string $group = null): self { - if (is_string($destination)) { - $destination = Routes::get($destination) ?? new ExternalRoute($destination); + if (is_string($destination) && Routes::has($destination)) { + $destination = Routes::get($destination); } - if ($destination instanceof Route && ! $destination instanceof ExternalRoute) { + if ($destination instanceof Route) { return new self( $destination, $label ?? $destination->getPage()->navigationMenuLabel(), @@ -89,18 +85,22 @@ public function __toString(): string /** * Get the destination route of the navigation item. For dropdowns, this will return null. + * + * @deprecated To simplify the class, we may remove this as we probably don't need it. */ public function getRoute(): ?Route { - return $this->route; + return $this->destination->getRoute(); } /** * Resolve the destination link of the navigation item. + * + * @deprecated May be renamed to getLink() in the future to better match its usage, and to match the Route class. */ public function getUrl(): string { - return (string) $this->route; + return $this->destination->getLink(); } /** @@ -137,7 +137,7 @@ public function getGroupKey(): ?string */ public function isActive(): bool { - return Hyde::currentRoute()->getLink() === $this->route->getLink(); + return Hyde::currentRoute()->getLink() === $this->getUrl(); } /** @return ($group is null ? null : string) */ diff --git a/packages/framework/src/Support/Models/ExternalRoute.php b/packages/framework/src/Support/Models/ExternalRoute.php deleted file mode 100644 index 721b89a7c13..00000000000 --- a/packages/framework/src/Support/Models/ExternalRoute.php +++ /dev/null @@ -1,28 +0,0 @@ -destination = $destination; - } - - public function getLink(): string - { - return $this->destination; - } -} diff --git a/packages/framework/tests/Feature/NavigationMenuTest.php b/packages/framework/tests/Feature/NavigationMenuTest.php index d0ede48df22..a98931a6e41 100644 --- a/packages/framework/tests/Feature/NavigationMenuTest.php +++ b/packages/framework/tests/Feature/NavigationMenuTest.php @@ -7,7 +7,6 @@ use Hyde\Hyde; use Hyde\Support\Models\Route; use Hyde\Foundation\Facades\Routes; -use Hyde\Support\Models\ExternalRoute; use Hyde\Framework\Features\Navigation\NavigationGroup; use Hyde\Framework\Features\Navigation\MainNavigationMenu; use Hyde\Framework\Features\Navigation\NavigationItem; @@ -268,7 +267,7 @@ public function testCanAddItemsToMainNavigationMenuResolvedFromContainer() Hyde::boot(); $navigation = app('navigation.main'); - $navigation->add(new NavigationItem(new ExternalRoute('/foo'), 'Foo')); + $navigation->add(new NavigationItem('/foo', 'Foo')); $this->assertCount(2, $navigation->getItems()); $this->assertSame('Foo', $navigation->getItems()->last()->getLabel()); diff --git a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php index 9b9c4b7bb6a..27e1e3826e3 100644 --- a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php +++ b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php @@ -6,7 +6,6 @@ use Hyde\Testing\UnitTestCase; use Illuminate\Support\Collection; -use Hyde\Support\Models\ExternalRoute; use Hyde\Framework\Features\Navigation\NavigationItem; use Hyde\Framework\Features\Navigation\NavigationGroup; use Hyde\Framework\Features\Navigation\DocumentationSidebar; @@ -21,6 +20,9 @@ */ class DocumentationSidebarUnitTest extends UnitTestCase { + protected static bool $needsKernel = true; + protected static bool $needsConfig = true; + // Base menu tests public function testCanConstruct() @@ -207,9 +209,6 @@ public function testHasGroupsReturnsFalseWhenNoItemsHaveChildren() public function testHasGroupsReturnsTrueWhenAtLeastOneItemIsNavigationGroupInstance() { - self::mockConfig(); - self::setupKernel(); - $menu = new DocumentationSidebar([ new NavigationGroup('foo', []), ]); @@ -228,6 +227,6 @@ protected function getItems(): array protected function item(string $destination, string $label, int $priority = 500): NavigationItem { - return new NavigationItem(new ExternalRoute($destination), $label, $priority); + return new NavigationItem($destination, $label, $priority); } } diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index b613557ac9b..209727ea153 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -5,7 +5,6 @@ namespace Hyde\Framework\Testing\Unit; use Hyde\Foundation\Facades\Routes; -use Hyde\Support\Models\ExternalRoute; use Hyde\Framework\Features\Navigation\NavigationItem; use Hyde\Pages\InMemoryPage; use Hyde\Pages\MarkdownPage; @@ -66,20 +65,18 @@ public function testPassingRouteKeyToConstructorUsesRouteInstance() $this->assertSame($route, (new NavigationItem('index', 'Home'))->getRoute()); } - public function testPassingUrlToConstructorUsesExternalRoute() + public function testPassingUrlToConstructorSetsRouteToNull() { $item = new NavigationItem('https://example.com', 'Home'); - $this->assertInstanceOf(ExternalRoute::class, $item->getRoute()); - $this->assertEquals(new ExternalRoute('https://example.com'), $item->getRoute()); - $this->assertSame('https://example.com', (string) $item->getRoute()); + $this->assertNull($item->getRoute()); + $this->assertSame('https://example.com', $item->getUrl()); } - public function testPassingUnknownRouteKeyToConstructorUsesExternalRoute() + public function testPassingUnknownRouteKeyToConstructorSetsRouteToNull() { $item = new NavigationItem('foo', 'Home'); - $this->assertInstanceOf(ExternalRoute::class, $item->getRoute()); - $this->assertEquals(new ExternalRoute('foo'), $item->getRoute()); - $this->assertSame('foo', (string) $item->getRoute()); + $this->assertNull($item->getRoute()); + $this->assertSame('foo', $item->getUrl()); } public function testGetDestination() @@ -133,7 +130,8 @@ public function testCreateWithLink() { $item = NavigationItem::create('foo', 'bar'); - $this->assertEquals(new ExternalRoute('foo'), $item->getRoute()); + $this->assertNull($item->getRoute()); + $this->assertSame('foo', $item->getUrl()); $this->assertSame('bar', $item->getLabel()); $this->assertSame(500, $item->getPriority()); } @@ -173,7 +171,7 @@ public function testCreateWithRouteKey() public function testCreateWithMissingRouteKey() { - $this->assertInstanceOf(ExternalRoute::class, NavigationItem::create('foo', 'foo')->getRoute()); + $this->assertNull(NavigationItem::create('foo', 'foo')->getRoute()); } public function testCreateWithCustomPriority() @@ -245,7 +243,7 @@ public function testIsCurrent() $this->assertFalse(NavigationItem::create(new Route(new InMemoryPage('bar')))->isActive()); } - public function testIsCurrentWithExternalRoute() + public function testIsCurrentWithLink() { Render::swap(Mockery::mock(RenderData::class, [ 'getRoute' => new Route(new InMemoryPage('foo')), diff --git a/packages/framework/tests/Unit/NavigationMenuUnitTest.php b/packages/framework/tests/Unit/NavigationMenuUnitTest.php index e90b159624f..8de2aba9229 100644 --- a/packages/framework/tests/Unit/NavigationMenuUnitTest.php +++ b/packages/framework/tests/Unit/NavigationMenuUnitTest.php @@ -6,7 +6,6 @@ use Hyde\Testing\UnitTestCase; use Illuminate\Support\Collection; -use Hyde\Support\Models\ExternalRoute; use Hyde\Framework\Features\Navigation\NavigationItem; use Hyde\Framework\Features\Navigation\MainNavigationMenu; @@ -19,6 +18,9 @@ */ class NavigationMenuUnitTest extends UnitTestCase { + protected static bool $needsKernel = true; + protected static bool $needsConfig = true; + // Base menu tests public function testCanConstruct() @@ -159,6 +161,6 @@ protected function getItems(): array protected function item(string $destination, string $label, int $priority = 500): NavigationItem { - return new NavigationItem(new ExternalRoute($destination), $label, $priority); + return new NavigationItem($destination, $label, $priority); } }