From 58be9bbfb2b88a9ae3bace3c71ae85c2bc036c40 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 11:23:57 +0100 Subject: [PATCH 01/32] Deprecate ExternalRoute class See https://github.com/hydephp/develop/pull/1635 --- packages/framework/src/Support/Models/ExternalRoute.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/framework/src/Support/Models/ExternalRoute.php b/packages/framework/src/Support/Models/ExternalRoute.php index f6ad450b071..f39ce405dae 100644 --- a/packages/framework/src/Support/Models/ExternalRoute.php +++ b/packages/framework/src/Support/Models/ExternalRoute.php @@ -8,6 +8,7 @@ * A route that leads to an external URI. * * @experimental Take caution when using this class, as it may be subject to change. + * @deprecated This class may be removed before release. */ class ExternalRoute extends Route { From 3bc2b90735ca6e5847bc01b741258c907ee16b4d Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 11:24:55 +0100 Subject: [PATCH 02/32] Create NavigationDestination.php --- .../Features/Navigation/NavigationDestination.php | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 packages/framework/src/Framework/Features/Navigation/NavigationDestination.php 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..875e9c7ebb7 --- /dev/null +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -0,0 +1,10 @@ + Date: Sun, 24 Mar 2024 11:25:09 +0100 Subject: [PATCH 03/32] Class `NavigationDestination` implements `Stringable` --- .../Framework/Features/Navigation/NavigationDestination.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index 875e9c7ebb7..87849d69473 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -4,7 +4,9 @@ namespace Hyde\Framework\Features\Navigation; -class NavigationDestination +use Stringable; + +class NavigationDestination implements Stringable { // } From 6f482348067dcf487642af97eb4ec5207cbd806a Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:33:18 +0100 Subject: [PATCH 04/32] Add destination property --- .../Framework/Features/Navigation/NavigationDestination.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index 87849d69473..ab873893c8f 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -5,8 +5,9 @@ namespace Hyde\Framework\Features\Navigation; use Stringable; +use Hyde\Support\Models\Route; class NavigationDestination implements Stringable { - // + protected Route|string $destination; } From c6f7a595a7ae32362093756d203f9502364b089d Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:33:27 +0100 Subject: [PATCH 05/32] Add constructor --- .../Framework/Features/Navigation/NavigationDestination.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index ab873893c8f..dfc1693712f 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -10,4 +10,9 @@ class NavigationDestination implements Stringable { protected Route|string $destination; + + public function __construct(Route|string $destination) + { + $this->destination = $destination; + } } From 84861054315805ac8829f69313c06b5d44766f1c Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:33:40 +0100 Subject: [PATCH 06/32] Implement stringable method --- .../Framework/Features/Navigation/NavigationDestination.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index dfc1693712f..48cedb49a28 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -15,4 +15,9 @@ public function __construct(Route|string $destination) { $this->destination = $destination; } + + public function __toString(): string + { + return (string) $this->destination; + } } From 954ccc78f175bcf43604520252fb2caf2e748272 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:34:49 +0100 Subject: [PATCH 07/32] Get route instance for route keys --- .../Features/Navigation/NavigationDestination.php | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index 48cedb49a28..b78a5d04ae1 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -6,6 +6,9 @@ use Stringable; use Hyde\Support\Models\Route; +use Hyde\Foundation\Facades\Routes; + +use function is_string; class NavigationDestination implements Stringable { @@ -13,6 +16,10 @@ class NavigationDestination implements Stringable public function __construct(Route|string $destination) { + if (is_string($destination) && Routes::has($destination)) { + $destination = Routes::get($destination); + } + $this->destination = $destination; } From 8da04dbbc0df1273bd05ca53d8f532f2950a85d9 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:35:15 +0100 Subject: [PATCH 08/32] Add get link method --- .../Framework/Features/Navigation/NavigationDestination.php | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index b78a5d04ae1..04b7d2f091a 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -24,6 +24,11 @@ public function __construct(Route|string $destination) } public function __toString(): string + { + return $this->getLink(); + } + + public function getLink(): string { return (string) $this->destination; } From fff2088833a97a9caeca9f05edf3f112e39df3d7 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:36:10 +0100 Subject: [PATCH 09/32] Deprecate route property --- .../src/Framework/Features/Navigation/NavigationItem.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 7d67811207a..8d7a2766640 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -23,6 +23,7 @@ */ class NavigationItem implements NavigationElement, Stringable { + /** @deprecated */ protected Route $route; protected string $label; protected int $priority; From 74a98f52754b96e09a8be2d48894af0b93edfee6 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:36:31 +0100 Subject: [PATCH 10/32] Add new destination property --- .../src/Framework/Features/Navigation/NavigationItem.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 8d7a2766640..7e4c1647f79 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -25,6 +25,7 @@ class NavigationItem implements NavigationElement, Stringable { /** @deprecated */ protected Route $route; + protected NavigationDestination $destination; protected string $label; protected int $priority; From a56db505bbb0ead19916d37ff27c3c9a723e2ee7 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:37:20 +0100 Subject: [PATCH 11/32] Construct destination --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 7e4c1647f79..339f018a473 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -42,6 +42,8 @@ class NavigationItem implements NavigationElement, Stringable */ public function __construct(Route|string $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null) { + $this->destination = new NavigationDestination($destination); + if (is_string($destination)) { $destination = Routes::get($destination) ?? new ExternalRoute($destination); } From b28c520afd8074461ea9b06d50fe42d39cf19a73 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:41:33 +0100 Subject: [PATCH 12/32] Get link using new destination class --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 339f018a473..f2064bf86d0 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -104,7 +104,7 @@ public function getRoute(): ?Route */ public function getUrl(): string { - return (string) $this->route; + return $this->destination->getLink(); } /** From e8c1f2f79cbbdc5d8ae3e0808acaa258d21ced3a Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:44:41 +0100 Subject: [PATCH 13/32] Add experimental route getter --- .../Framework/Features/Navigation/NavigationDestination.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php index 04b7d2f091a..bcf07e11763 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationDestination.php @@ -32,4 +32,10 @@ public function getLink(): string { return (string) $this->destination; } + + /** @experimental */ + public function getRoute(): ?Route + { + return $this->destination instanceof Route ? $this->destination : null; + } } From 5cc2163e2a2ddc6bee55bb20ea7e35d6ed4f48fe Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:45:00 +0100 Subject: [PATCH 14/32] Get route using destination --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index f2064bf86d0..b7b2ee9f0aa 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -96,7 +96,7 @@ public function __toString(): string */ public function getRoute(): ?Route { - return $this->route; + return $this->destination->getRoute(); } /** From 6efd8b909212d88e2768a488403e83528e220d7b Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:48:01 +0100 Subject: [PATCH 15/32] Skip tests needing to be reimplemented for https://github.com/hydephp/develop/pull/1636 --- packages/framework/tests/Unit/NavigationItemTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index b613557ac9b..6daf921bb83 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -68,6 +68,7 @@ public function testPassingRouteKeyToConstructorUsesRouteInstance() public function testPassingUrlToConstructorUsesExternalRoute() { + $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $item = new NavigationItem('https://example.com', 'Home'); $this->assertInstanceOf(ExternalRoute::class, $item->getRoute()); $this->assertEquals(new ExternalRoute('https://example.com'), $item->getRoute()); @@ -76,6 +77,7 @@ public function testPassingUrlToConstructorUsesExternalRoute() public function testPassingUnknownRouteKeyToConstructorUsesExternalRoute() { + $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $item = new NavigationItem('foo', 'Home'); $this->assertInstanceOf(ExternalRoute::class, $item->getRoute()); $this->assertEquals(new ExternalRoute('foo'), $item->getRoute()); From c5203902c6ad55a8ae3e5531894aad1526da37c1 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:55:14 +0100 Subject: [PATCH 16/32] Use null check instead of instanceof on deprecated class --- .../src/Framework/Features/Navigation/NavigationGroup.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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; }); } } From faea07b04747ed3ec1b616dc6b28f86696030800 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 12:56:59 +0100 Subject: [PATCH 17/32] Reimplement tests for changed types --- .../framework/tests/Unit/NavigationItemTest.php | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index 6daf921bb83..3cb99793a9c 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -66,22 +66,18 @@ public function testPassingRouteKeyToConstructorUsesRouteInstance() $this->assertSame($route, (new NavigationItem('index', 'Home'))->getRoute()); } - public function testPassingUrlToConstructorUsesExternalRoute() + public function testPassingUrlToConstructorSetsRouteToNull() { - $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $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() { - $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $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() From f5a356736b6fe7152dda673c3cd08fb4bb401ec8 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 13:06:03 +0100 Subject: [PATCH 18/32] Compare state using destination --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index b7b2ee9f0aa..db5b1526127 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -141,7 +141,7 @@ public function getGroupKey(): ?string */ public function isActive(): bool { - return Hyde::currentRoute()->getLink() === $this->route->getLink(); + return Hyde::currentRoute()->getLink() === $this->destination->getLink(); } /** @return ($group is null ? null : string) */ From 39108997d1d707bc9f948e267795b3a518ce99c3 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 13:06:59 +0100 Subject: [PATCH 19/32] Use accessor with same logic --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index db5b1526127..9d89484bdf9 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -141,7 +141,7 @@ public function getGroupKey(): ?string */ public function isActive(): bool { - return Hyde::currentRoute()->getLink() === $this->destination->getLink(); + return Hyde::currentRoute()->getLink() === $this->getUrl(); } /** @return ($group is null ? null : string) */ From cdceed7f81f011f347c8ba079a84a535a59a3b4a Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 13:07:51 +0100 Subject: [PATCH 20/32] Remove deprecated route property --- .../src/Framework/Features/Navigation/NavigationItem.php | 7 ------- 1 file changed, 7 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 9d89484bdf9..1cf373a5081 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -23,8 +23,6 @@ */ class NavigationItem implements NavigationElement, Stringable { - /** @deprecated */ - protected Route $route; protected NavigationDestination $destination; protected string $label; protected int $priority; @@ -44,11 +42,6 @@ public function __construct(Route|string $destination, string $label, int $prior { $this->destination = new NavigationDestination($destination); - if (is_string($destination)) { - $destination = Routes::get($destination) ?? new ExternalRoute($destination); - } - - $this->route = $destination; $this->label = $label; $this->priority = $priority; From d16368c378772d6bb8accabda2f6c7be20f793f4 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:30:35 +0100 Subject: [PATCH 21/32] Deprecate methods we probably don't actually need --- .../src/Framework/Features/Navigation/NavigationItem.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 1cf373a5081..6fec030a9f6 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -86,6 +86,8 @@ 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 { @@ -94,6 +96,8 @@ public function getRoute(): ?Route /** * Resolve the destination link of the navigation item. + * + * @deprecated May be renamed to getLink() in the future. */ public function getUrl(): string { From eb81668e91594e9944a5d72cffb5f7d29246f2f9 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:31:28 +0100 Subject: [PATCH 22/32] Skip tests needing to be reimplemented for #1636 --- packages/framework/tests/Unit/NavigationItemTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index 3cb99793a9c..002270845b2 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -129,6 +129,7 @@ public function testToString() public function testCreateWithLink() { + $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $item = NavigationItem::create('foo', 'bar'); $this->assertEquals(new ExternalRoute('foo'), $item->getRoute()); @@ -171,6 +172,7 @@ public function testCreateWithRouteKey() public function testCreateWithMissingRouteKey() { + $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $this->assertInstanceOf(ExternalRoute::class, NavigationItem::create('foo', 'foo')->getRoute()); } From 798acda5546fd8d09ca9ee25d3b5693fb22915a4 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:34:58 +0100 Subject: [PATCH 23/32] Normalize logic with new destination constructor --- .../src/Framework/Features/Navigation/NavigationItem.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 6fec030a9f6..4615fdbbdf2 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -60,8 +60,8 @@ 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) { From 6a64d131765e101171c9ef2a1694ed5f1207eb54 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:36:27 +0100 Subject: [PATCH 24/32] Remove type check for old class --- .../src/Framework/Features/Navigation/NavigationItem.php | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index 4615fdbbdf2..c3fa4d1973d 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; @@ -64,7 +63,7 @@ public static function create(Route|string $destination, ?string $label = null, $destination = Routes::get($destination); } - if ($destination instanceof Route && ! $destination instanceof ExternalRoute) { + if ($destination instanceof Route) { return new self( $destination, $label ?? $destination->getPage()->navigationMenuLabel(), From 1eea4dead92bd0c80208eac6b6fe160760e082a2 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:38:28 +0100 Subject: [PATCH 25/32] Update deprecation notice --- .../src/Framework/Features/Navigation/NavigationItem.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php index c3fa4d1973d..867684ea5f5 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationItem.php @@ -96,7 +96,7 @@ public function getRoute(): ?Route /** * Resolve the destination link of the navigation item. * - * @deprecated May be renamed to getLink() in the future. + * @deprecated May be renamed to getLink() in the future to better match its usage, and to match the Route class. */ public function getUrl(): string { From 218590dcc9f41daf390d209f1c299c7e5de3a28d Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:42:28 +0100 Subject: [PATCH 26/32] Replace dependences on deprecated class with inlined strings We need the kernel and config so we can check if the routes exist --- packages/framework/tests/Feature/NavigationMenuTest.php | 3 +-- .../framework/tests/Unit/DocumentationSidebarUnitTest.php | 6 ++++-- packages/framework/tests/Unit/NavigationMenuUnitTest.php | 6 ++++-- 3 files changed, 9 insertions(+), 6 deletions(-) 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..4c009f9382d 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() @@ -228,6 +230,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/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); } } From 67941949381dd87fea22fbe69131b5a23d17ffbb Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:44:43 +0100 Subject: [PATCH 27/32] Remove logic added to base of test --- packages/framework/tests/Unit/DocumentationSidebarUnitTest.php | 3 --- 1 file changed, 3 deletions(-) diff --git a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php index 4c009f9382d..27e1e3826e3 100644 --- a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php +++ b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php @@ -209,9 +209,6 @@ public function testHasGroupsReturnsFalseWhenNoItemsHaveChildren() public function testHasGroupsReturnsTrueWhenAtLeastOneItemIsNavigationGroupInstance() { - self::mockConfig(); - self::setupKernel(); - $menu = new DocumentationSidebar([ new NavigationGroup('foo', []), ]); From ba65f1c5d38c9cd786065fb9b424cb181e288b0f Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:48:30 +0100 Subject: [PATCH 28/32] Delete ExternalRoute.php --- .../src/Support/Models/ExternalRoute.php | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 packages/framework/src/Support/Models/ExternalRoute.php 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; - } -} From 72fce42a51c6730a8588dbf0b7d4663e14723c86 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:51:13 +0100 Subject: [PATCH 29/32] Revert "Skip tests needing to be reimplemented for #1636" This reverts commit eb81668e91594e9944a5d72cffb5f7d29246f2f9. --- packages/framework/tests/Unit/NavigationItemTest.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index 002270845b2..3cb99793a9c 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -129,7 +129,6 @@ public function testToString() public function testCreateWithLink() { - $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $item = NavigationItem::create('foo', 'bar'); $this->assertEquals(new ExternalRoute('foo'), $item->getRoute()); @@ -172,7 +171,6 @@ public function testCreateWithRouteKey() public function testCreateWithMissingRouteKey() { - $this->markTestSkipped('Todo: Reimplement this test for https://github.com/hydephp/develop/pull/1636'); $this->assertInstanceOf(ExternalRoute::class, NavigationItem::create('foo', 'foo')->getRoute()); } From cab21b70721c0a3d20c37334ab0252d5aab78c43 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:53:31 +0100 Subject: [PATCH 30/32] Reimplement tests for changed types --- packages/framework/tests/Unit/NavigationItemTest.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index 3cb99793a9c..a74e1d94d76 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; @@ -131,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()); } @@ -171,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() From 0dec761e7ba554dcee8ebff754f31d934a882ced Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:54:01 +0100 Subject: [PATCH 31/32] Rename test for removed class --- packages/framework/tests/Unit/NavigationItemTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/tests/Unit/NavigationItemTest.php b/packages/framework/tests/Unit/NavigationItemTest.php index a74e1d94d76..209727ea153 100644 --- a/packages/framework/tests/Unit/NavigationItemTest.php +++ b/packages/framework/tests/Unit/NavigationItemTest.php @@ -243,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')), From 976d7ff5dd46fa6d10118ee4c2a1084ee585c6c7 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Sun, 24 Mar 2024 15:54:12 +0100 Subject: [PATCH 32/32] Remove removed new class from release notes --- RELEASE_NOTES.md | 1 - 1 file changed, 1 deletion(-) 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