From fdf73674187c08598d6b1edc8a0c451c93ee4eb0 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:54:54 +0100 Subject: [PATCH 01/13] Update internal destination state to support route instances --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 7a57b554282..b78e7858502 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -20,7 +20,7 @@ */ class NavItem implements Stringable { - public readonly string $destination; + public readonly Route|string $destination; public readonly string $label; public readonly int $priority; public readonly ?string $group; From 36f692d9305ca85cdbb61dd6196de41ae0e4f8a0 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:55:07 +0100 Subject: [PATCH 02/13] Remove string cast forcing early evaluation --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index b78e7858502..3bc79da925a 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -30,7 +30,7 @@ class NavItem implements Stringable */ public function __construct(Route|string $destination, string $label, int $priority = 500, ?string $group = null) { - $this->destination = (string) $destination; + $this->destination = $destination; $this->label = $label; $this->priority = $priority; $this->group = $group; From 29f29777f0245906880247e3c269bf0ef89fd989 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:55:26 +0100 Subject: [PATCH 03/13] Remove early route resolver --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 3bc79da925a..b3f85f5f09c 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -42,7 +42,7 @@ public function __construct(Route|string $destination, string $label, int $prior public static function fromRoute(Route $route, ?string $label = null, ?int $priority = null, ?string $group = null): static { return new static( - $route->getLink(), + $route, $label ?? $route->getPage()->navigationMenuLabel(), $priority ?? $route->getPage()->navigationMenuPriority(), $group ?? static::getRouteGroup($route), From 384a8228c014e72f7353411e32e7051c4263fc77 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:56:09 +0100 Subject: [PATCH 04/13] Add string cast to string resolver --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index b3f85f5f09c..aa37636499a 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -75,7 +75,7 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int */ public function __toString(): string { - return $this->destination; + return (string) $this->destination; } /** From 43d4aa2d4a7c27602d9d696b7f9c21932f5592b2 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:58:53 +0100 Subject: [PATCH 05/13] Compare current state as string --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index aa37636499a..a0a4e0d3237 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -120,7 +120,7 @@ public function getGroup(): ?string */ public function isCurrent(): bool { - return Hyde::currentRoute()->getLink() === $this->destination; + return Hyde::currentRoute()->getLink() === (string) $this->destination; } protected static function getRouteGroup(Route $route): ?string From 600c0e86bf3919154943c13fce5dc2aaf169d495 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 19:01:13 +0100 Subject: [PATCH 06/13] Union return type --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index a0a4e0d3237..4c13e78d17e 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -82,9 +82,9 @@ public function __toString(): string * Get the destination link of the navigation item. * * If the navigation item is an external link, this will return the link as is, - * if it's for a route, a resolved relative link will be returned. + * if it's for a route, the route instance will be returned. */ - public function getDestination(): string + public function getDestination(): Route|string { return $this->destination; } From 7edd5933669f9ab2ead2129e7c62d8de83d84cc3 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 19:06:54 +0100 Subject: [PATCH 07/13] Create new getLink method to match previous getDestination result --- RELEASE_NOTES.md | 7 +++++++ .../Framework/Features/Navigation/NavItem.php | 13 ++++++++++++- packages/framework/tests/Unit/NavItemTest.php | 18 +++++++++++++----- 3 files changed, 32 insertions(+), 6 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 42b678e6888..5d7529abbc7 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -11,10 +11,12 @@ 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 `NavItem::getLink()` method contain the previous `NavItem::getDestination()` logic, to return the link URL. ### Changed - Changed how the documentation search is generated, to be an `InMemoryPage` instead of a post-build task. - Media asset files are now copied using the new build task instead of the deprecated `BuildService::transferMediaAssets()` method. +- The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type. ### Deprecated - for soon-to-be removed features. @@ -54,6 +56,11 @@ For more information, see https://github.com/hydephp/develop/pull/1498. ## Low impact +### Navigation item changes + +The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type. +If you want to retain the previous state where a string is always returned, you can use the new `NavItem::getLink()` method instead. + ### New documentation search implementation As the new documentation search implementation brings changes to their code API you may need to adapt your code diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 4c13e78d17e..063b5e66035 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -79,7 +79,7 @@ public function __toString(): string } /** - * Get the destination link of the navigation item. + * Get the destination of the navigation item. * * If the navigation item is an external link, this will return the link as is, * if it's for a route, the route instance will be returned. @@ -89,6 +89,17 @@ public function getDestination(): Route|string return $this->destination; } + /** + * Get the destination link of the navigation item. + * + * If the navigation item is an external link, this will return the link as is, + * if it's for a route, the route's resolved link will be returned. + */ + public function getLink(): string + { + return (string) $this->destination; + } + /** * Get the label of the navigation item. */ diff --git a/packages/framework/tests/Unit/NavItemTest.php b/packages/framework/tests/Unit/NavItemTest.php index 2976278c85f..ec17247b674 100644 --- a/packages/framework/tests/Unit/NavItemTest.php +++ b/packages/framework/tests/Unit/NavItemTest.php @@ -43,13 +43,21 @@ public function test__construct() $route = new Route(new MarkdownPage()); $item = new NavItem($route, 'Test', 500); - $this->assertSame($route->getLink(), $item->destination); + $this->assertSame($route, $item->destination); } public function testGetDestination() + { + $route = new Route(new MarkdownPage()); + $item = new NavItem($route, 'Test', 500); + + $this->assertSame($route, $item->getDestination()); + } + + public function testGetLink() { $navItem = new NavItem(new Route(new InMemoryPage('foo')), 'Page', 500); - $this->assertSame('foo.html', $navItem->getDestination()); + $this->assertSame('foo.html', $navItem->getLink()); } public function testGetLabel() @@ -75,7 +83,7 @@ public function testFromRoute() $route = new Route(new MarkdownPage()); $item = NavItem::fromRoute($route); - $this->assertSame($route->getLink(), $item->destination); + $this->assertSame($route, $item->destination); } public function test__toString() @@ -104,7 +112,7 @@ public function testForRoute() $route = Routes::get('404'); $item = NavItem::forRoute($route, 'foo'); - $this->assertSame($route->getLink(), $item->destination); + $this->assertSame($route, $item->destination); $this->assertSame('foo', $item->label); $this->assertSame(999, $item->priority); } @@ -114,7 +122,7 @@ public function testForIndexRoute() $route = Routes::get('index'); $item = NavItem::forRoute($route, 'foo'); - $this->assertSame($route->getLink(), $item->destination); + $this->assertSame($route, $item->destination); $this->assertSame('foo', $item->label); $this->assertSame(0, $item->priority); } From 69ad6e4678f366b7943970058551a2c0d7b6f8a6 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 19:09:16 +0100 Subject: [PATCH 08/13] Boot the kernel Now needed in unit tests to get the navigation. Not an issue for actual sites. From 4de815096ee908d3f2a0310c975b3827ca7bbc35 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:54:54 +0100 Subject: [PATCH 09/13] Update internal destination state to support route instances From 1eaffe4c0877e438971f7c4e3c3b4aee5e0e78d8 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:55:07 +0100 Subject: [PATCH 10/13] Remove string cast forcing early evaluation From 34add32bfd7ba681b79dca050a22eec03b89a85f Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:55:26 +0100 Subject: [PATCH 11/13] Remove early route resolver From 0e6241d596085e7db172a2fe8c4fe45a40015689 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 18:56:09 +0100 Subject: [PATCH 12/13] Add string cast to string resolver From 43b93d41699a770d6b281351e2ef236298a85e8d Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 8 Feb 2024 19:30:10 +0100 Subject: [PATCH 13/13] Update RELEASE_NOTES.md --- RELEASE_NOTES.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 5d7529abbc7..e457cd069bd 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -58,8 +58,11 @@ For more information, see https://github.com/hydephp/develop/pull/1498. ### Navigation item changes -The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type. -If you want to retain the previous state where a string is always returned, you can use the new `NavItem::getLink()` method instead. +The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type. This allows for deferring the route evaluation. + +If you have previously used this method directly and expected a string to be returned, you may need to adapt your code to handle the new return type. + +If you want to retain the previous state where a string is always returned, you can use the new `NavItem::getLink()` method instead, which will resolve the route immediately. ### New documentation search implementation