From 439d9c6992c6102d0c5202a40a57752c6feb2eca Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 18:37:09 +0100 Subject: [PATCH 01/35] Rename NavItem `dropdown` method to `forGroup` Also considered using "grouped" but "forGroup" matches the other facade methods. --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 +--- .../Features/Navigation/NavigationMenuGenerator.php | 2 +- packages/framework/tests/Feature/NavigationMenuTest.php | 4 ++-- .../tests/Feature/Services/DocumentationSidebarTest.php | 6 +++--- packages/framework/tests/Unit/NavItemTest.php | 6 +++--- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index e6ecb310e8d..ab4ca0bcabd 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -95,13 +95,11 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int /** * Create a new dropdown navigation menu item. * - * @TODO: Might be more semantic to have this named something else, as it also includes sidebars groups. (technically it only makes sense for the main navigation menu, but it's not enforced in the code, and the sidebar does use this method) - * * @param string $label The label of the dropdown item. * @param array $items The items to be included in the dropdown. * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function dropdown(string $label, array $items, ?int $priority = null): static + public static function forGroup(string $label, array $items, ?int $priority = null): static { return new static('', $label, $priority ?? NavigationMenu::LAST, $label, $items); } diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php index 00ed144add6..bf82a3bbd5c 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php @@ -160,7 +160,7 @@ protected function createGroupItem(string $groupKey, string $groupName): NavItem $priority = $this->searchForGroupPriorityInConfig($groupKey); - return NavItem::dropdown($this->normalizeGroupLabel($label), [], $priority); + return NavItem::forGroup($this->normalizeGroupLabel($label), [], $priority); } protected function normalizeGroupLabel(string $label): string diff --git a/packages/framework/tests/Feature/NavigationMenuTest.php b/packages/framework/tests/Feature/NavigationMenuTest.php index 0666afeb8b8..2163dce6d6d 100644 --- a/packages/framework/tests/Feature/NavigationMenuTest.php +++ b/packages/framework/tests/Feature/NavigationMenuTest.php @@ -230,7 +230,7 @@ public function testPagesInSubdirectoriesAreNotAddedToTheNavigationMenuWithConfi $menu = $this->createNavigationMenu(); $expected = collect([ NavItem::fromRoute(Routes::get('index')), - NavItem::dropdown('Foo', [ + NavItem::forGroup('Foo', [ NavItem::fromRoute(Routes::get('foo/bar')), ]), ]); @@ -251,7 +251,7 @@ public function testPagesInDropdownsDoNotGetAddedToTheMainNavigation() $this->assertEquals([ NavItem::fromRoute(Routes::get('index')), NavItem::fromRoute((new MarkdownPage('foo'))->getRoute()), - NavItem::dropdown('Bar', [ + NavItem::forGroup('Bar', [ NavItem::fromRoute((new MarkdownPage('bar/baz'))->getRoute()), ]), ], $menu->getItems()->all()); diff --git a/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php b/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php index 35de82258a4..b2da470a8c5 100644 --- a/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php +++ b/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php @@ -267,10 +267,10 @@ public function testCanHaveMultipleGroupedPagesWithTheSameNameLabels() $this->assertEquals( collect([ - NavItem::dropdown('Bar', [ + NavItem::forGroup('Bar', [ NavItem::fromRoute(Routes::get('docs/bar'), priority: 999), ]), - NavItem::dropdown('Foo', [ + NavItem::forGroup('Foo', [ NavItem::fromRoute(Routes::get('docs/foo'), priority: 999), ]), ]), @@ -288,7 +288,7 @@ public function testDuplicateLabelsWithinTheSameGroupAreNotRemoved() $this->assertEquals( collect([ - NavItem::dropdown('Foo', [ + NavItem::forGroup('Foo', [ NavItem::fromRoute(Routes::get('docs/bar'), priority: 999), NavItem::fromRoute(Routes::get('docs/foo'), priority: 999), ]), diff --git a/packages/framework/tests/Unit/NavItemTest.php b/packages/framework/tests/Unit/NavItemTest.php index b5ff9c6c0b7..1816633e0d1 100644 --- a/packages/framework/tests/Unit/NavItemTest.php +++ b/packages/framework/tests/Unit/NavItemTest.php @@ -263,7 +263,7 @@ public function testRouteBasedNavItemDestinationsAreResolvedRelatively() public function testDropdownFacade() { - $item = NavItem::dropdown('foo', []); + $item = NavItem::forGroup('foo', []); $this->assertSame('foo', $item->getLabel()); $this->assertSame([], $item->getChildren()); @@ -276,14 +276,14 @@ public function testDropdownFacadeWithChildren() new NavItem(new Route(new MarkdownPage()), 'bar'), ]; - $item = NavItem::dropdown('foo', $children); + $item = NavItem::forGroup('foo', $children); $this->assertSame($children, $item->getChildren()); $this->assertSame(999, $item->getPriority()); } public function testDropdownFacadeWithCustomPriority() { - $item = NavItem::dropdown('foo', [], 500); + $item = NavItem::forGroup('foo', [], 500); $this->assertSame(500, $item->getPriority()); } From 37cb4a83441be2099d592427b2e81c11303dfeff Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 18:56:02 +0100 Subject: [PATCH 02/35] Call static method statically --- .../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 ab4ca0bcabd..0394d192294 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -54,7 +54,7 @@ public function __construct(Route|string $destination, string $label, int $prior $this->label = $label; $this->priority = $priority; $this->group = static::normalizeGroupKey($group); - $this->identifier = $this->makeIdentifier($label); + $this->identifier = static::makeIdentifier($label); $this->children = $children; } From e28ae7c8ba55a6507f021f3955f41d025734e05e Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 18:56:49 +0100 Subject: [PATCH 03/35] Use explicit cast for equality comparison --- .../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 0394d192294..18ee08de286 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -197,7 +197,7 @@ public function hasChildren(): bool */ public function isCurrent(): bool { - return Hyde::currentRoute()->getLink() === (string) $this->destination; + return Hyde::currentRoute()->getLink() === $this->destination->getLink(); } /** From 228e3d4d59d15751332f32ec0f25d828f26be8a0 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 18:57:28 +0100 Subject: [PATCH 04/35] Use existing helper for equality comparison --- .../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 18ee08de286..50e2c563bf7 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -197,7 +197,7 @@ public function hasChildren(): bool */ public function isCurrent(): bool { - return Hyde::currentRoute()->getLink() === $this->destination->getLink(); + return $this->destination->is(Hyde::currentRoute()); } /** From 734c95e4f157ffd201b85444986868acbb6ab0b1 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:04:52 +0100 Subject: [PATCH 05/35] Revert "Use existing helper for equality comparison" This reverts commit 228e3d4d59d15751332f32ec0f25d828f26be8a0 as existing helper depends on route keys which do not exist on external routes. --- .../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 50e2c563bf7..18ee08de286 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -197,7 +197,7 @@ public function hasChildren(): bool */ public function isCurrent(): bool { - return $this->destination->is(Hyde::currentRoute()); + return Hyde::currentRoute()->getLink() === $this->destination->getLink(); } /** From faa87b4c403ba0797853b23482dcc7462560874b Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:05:28 +0100 Subject: [PATCH 06/35] Clarify method description context --- .../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 18ee08de286..17a8a750186 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -193,7 +193,7 @@ public function hasChildren(): bool } /** - * Check if the NavItem instance is the current page. + * Check if the NavItem instance is the current page being rendered. */ public function isCurrent(): bool { From 8c43c4f63670de6999cad50ce1d7758380812925 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:07:11 +0100 Subject: [PATCH 07/35] Move fallback value to default parameter value --- .../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 17a8a750186..b6181fb08e8 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -99,9 +99,9 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int * @param array $items The items to be included in the dropdown. * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, ?int $priority = null): static + public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static { - return new static('', $label, $priority ?? NavigationMenu::LAST, $label, $items); + return new static('', $label, $priority, $label, $items); } /** From 2d90f7cdc4801a6961586820fd3a623deb6968e1 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:07:52 +0100 Subject: [PATCH 08/35] Remove null type support from facade helper method --- .../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 b6181fb08e8..37686076f92 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -97,9 +97,9 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int * * @param string $label The label of the dropdown item. * @param array $items The items to be included in the dropdown. - * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. + * @param int $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static + public static function forGroup(string $label, array $items, int $priority = NavigationMenu::LAST): static { return new static('', $label, $priority, $label, $items); } From 9042071b6848715807e3ad3a294aeca42cf34ad6 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:11:23 +0100 Subject: [PATCH 09/35] Revert "Remove null type support from facade helper method" This reverts commit 2d90f7cdc4801a6961586820fd3a623deb6968e1. --- .../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 37686076f92..b6181fb08e8 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -97,9 +97,9 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int * * @param string $label The label of the dropdown item. * @param array $items The items to be included in the dropdown. - * @param int $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. + * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, int $priority = NavigationMenu::LAST): static + public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static { return new static('', $label, $priority, $label, $items); } From 12ac55f81d7eb5eb117355b2b8ad3a3ca3627212 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:11:26 +0100 Subject: [PATCH 10/35] Revert "Move fallback value to default parameter value" This reverts commit 8c43c4f63670de6999cad50ce1d7758380812925. --- .../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 b6181fb08e8..17a8a750186 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -99,9 +99,9 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int * @param array $items The items to be included in the dropdown. * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static + public static function forGroup(string $label, array $items, ?int $priority = null): static { - return new static('', $label, $priority, $label, $items); + return new static('', $label, $priority ?? NavigationMenu::LAST, $label, $items); } /** From 1eb85a6ba139d899b61a7ac9d065bf3d895c6f33 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:15:30 +0100 Subject: [PATCH 11/35] Move up method in source --- .../Framework/Features/Navigation/NavItem.php | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 17a8a750186..0b7f2e29cc1 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -71,14 +71,6 @@ public static function fromRoute(Route $route, ?string $label = null, ?int $prio ); } - /** - * Create a new navigation menu item leading to an external URI. - */ - public static function forLink(string $href, string $label, int $priority = NavigationMenu::DEFAULT): static - { - return new static($href, $label, $priority); - } - /** * Create a new navigation menu item leading to a Route instance. * @@ -92,6 +84,14 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int return static::fromRoute($route instanceof Route ? $route : Routes::getOrFail($route), $label, $priority, $group); } + /** + * Create a new navigation menu item leading to an external URI. + */ + public static function forLink(string $href, string $label, int $priority = NavigationMenu::DEFAULT): static + { + return new static($href, $label, $priority); + } + /** * Create a new dropdown navigation menu item. * From 6af588a94ea443c173bfeb85afcb069341f84d36 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:17:22 +0100 Subject: [PATCH 12/35] Deprecate `NavItem::fromRoute()` method We don't need two methods that do almost the same thing. I'm going to merge them, keeping the name of the other one to match the other similar helpers. --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 0b7f2e29cc1..429c4180153 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -60,6 +60,8 @@ public function __construct(Route|string $destination, string $label, int $prior /** * Create a new navigation menu item from a route. + * + * @deprecated Use NavItem::forRoute() instead. */ public static function fromRoute(Route $route, ?string $label = null, ?int $priority = null, ?string $group = null): static { From 6ddd33ed1770d770de9c9d5cfcb5d010f47dafe7 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:18:48 +0100 Subject: [PATCH 13/35] Inline usage of deprecated method for merged replacement --- .../src/Framework/Features/Navigation/NavItem.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 429c4180153..08fd6cb011d 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -83,7 +83,12 @@ public static function fromRoute(Route $route, ?string $label = null, ?int $prio */ public static function forRoute(Route|string $route, ?string $label = null, ?int $priority = null, ?string $group = null): static { - return static::fromRoute($route instanceof Route ? $route : Routes::getOrFail($route), $label, $priority, $group); + return new static( + $route instanceof Route ? $route : Routes::getOrFail($route), + $label ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuLabel(), + $priority ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuPriority(), + $group ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuGroup(), + ); } /** From b67d9a5304e3028862a585519699ed905b29a2c8 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:22:27 +0100 Subject: [PATCH 14/35] Revert "Inline usage of deprecated method for merged replacement" This reverts commit 6ddd33ed1770d770de9c9d5cfcb5d010f47dafe7. --- .../src/Framework/Features/Navigation/NavItem.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 08fd6cb011d..429c4180153 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -83,12 +83,7 @@ public static function fromRoute(Route $route, ?string $label = null, ?int $prio */ public static function forRoute(Route|string $route, ?string $label = null, ?int $priority = null, ?string $group = null): static { - return new static( - $route instanceof Route ? $route : Routes::getOrFail($route), - $label ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuLabel(), - $priority ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuPriority(), - $group ?? $route instanceof Route ? $route : Routes::getOrFail($route)->getPage()->navigationMenuGroup(), - ); + return static::fromRoute($route instanceof Route ? $route : Routes::getOrFail($route), $label, $priority, $group); } /** From 83235f0ee946819cec5836b8fcaaefa8cdaa1f99 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:27:37 +0100 Subject: [PATCH 15/35] Introduce local variable --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 429c4180153..42e351ba0f1 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -83,7 +83,9 @@ public static function fromRoute(Route $route, ?string $label = null, ?int $prio */ public static function forRoute(Route|string $route, ?string $label = null, ?int $priority = null, ?string $group = null): static { - return static::fromRoute($route instanceof Route ? $route : Routes::getOrFail($route), $label, $priority, $group); + $route = $route instanceof Route ? $route : Routes::getOrFail($route); + + return static::fromRoute($route, $label, $priority, $group); } /** From f14bc66a3573ea5b59fe6712a180257b718a10e2 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:27:58 +0100 Subject: [PATCH 16/35] Reinline usage of deprecated method for merged replacement --- .../src/Framework/Features/Navigation/NavItem.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 42e351ba0f1..7198c985f4a 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -85,7 +85,12 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int { $route = $route instanceof Route ? $route : Routes::getOrFail($route); - return static::fromRoute($route, $label, $priority, $group); + return new static( + $route, + $label ?? $route->getPage()->navigationMenuLabel(), + $priority ?? $route->getPage()->navigationMenuPriority(), + $group ?? $route->getPage()->navigationMenuGroup(), + ); } /** From 870e00400c132c8e290726e5d3e929eb0a7aeb7a Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:40:32 +0100 Subject: [PATCH 17/35] Update deprecated method to point to replacement --- .../src/Framework/Features/Navigation/NavItem.php | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 7198c985f4a..e10ce0feffd 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -65,12 +65,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, - $label ?? $route->getPage()->navigationMenuLabel(), - $priority ?? $route->getPage()->navigationMenuPriority(), - $group ?? $route->getPage()->navigationMenuGroup(), - ); + return static::forRoute($route, $label, $priority, $group); } /** From 1edb2f2255da8cec7060edddd9dff94114e2b54c Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 19:50:07 +0100 Subject: [PATCH 18/35] Replace all usages of `fromRoute` with `forRoute` --- .../Navigation/NavigationMenuGenerator.php | 6 +-- .../tests/Feature/NavigationMenuTest.php | 44 +++++++++---------- .../Services/DocumentationSidebarTest.php | 32 +++++++------- .../Unit/DocumentationSidebarUnitTest.php | 2 +- .../tests/Unit/NavItemIsCurrentHelperTest.php | 26 +++++------ packages/framework/tests/Unit/NavItemTest.php | 24 +++++----- 6 files changed, 67 insertions(+), 67 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php index bf82a3bbd5c..342a696d96b 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php @@ -71,7 +71,7 @@ protected function generate(): void if ($this->canGroupRoute($route)) { $this->addRouteToGroup($route); } else { - $this->items->put($route->getRouteKey(), NavItem::fromRoute($route)); + $this->items->put($route->getRouteKey(), NavItem::forRoute($route)); } } }); @@ -79,7 +79,7 @@ protected function generate(): void if ($this->generatesSidebar) { // If there are no pages other than the index page, we add it to the sidebar so that it's not empty if ($this->items->count() === 0 && DocumentationPage::home() !== null) { - $this->items->push(NavItem::fromRoute(DocumentationPage::home())); + $this->items->push(NavItem::forRoute(DocumentationPage::home())); } } else { collect(Config::getArray('hyde.navigation.custom', []))->each(function (NavItem $item): void { @@ -133,7 +133,7 @@ protected function canGroupRoute(Route $route): bool protected function addRouteToGroup(Route $route): void { - $item = NavItem::fromRoute($route); + $item = NavItem::forRoute($route); $groupName = $this->generatesSidebar ? ($item->getGroup() ?? 'Other') : $item->getGroup(); diff --git a/packages/framework/tests/Feature/NavigationMenuTest.php b/packages/framework/tests/Feature/NavigationMenuTest.php index 2163dce6d6d..2bd704cc40e 100644 --- a/packages/framework/tests/Feature/NavigationMenuTest.php +++ b/packages/framework/tests/Feature/NavigationMenuTest.php @@ -38,7 +38,7 @@ public function testGenerateMethodCreatesCollectionOfNavItems() public function testGetItemsReturnsItems() { $this->assertEquals(collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), ]), $this->createNavigationMenu()->getItems()); } @@ -67,9 +67,9 @@ public function testCreatedCollectionIsSortedByNavigationMenuPriority() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), - NavItem::fromRoute(Routes::get('foo')), - NavItem::fromRoute(Routes::get('docs/index')), + NavItem::forRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('foo')), + NavItem::forRoute(Routes::get('docs/index')), ]); $this->assertCount(count($expected), $menu->getItems()); @@ -83,8 +83,8 @@ public function testIsSortedAutomaticallyWhenUsingNavigationMenuCreate() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), - NavItem::fromRoute(Routes::get('foo')), + NavItem::forRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('foo')), ]); $this->assertCount(count($expected), $menu->getItems()); @@ -98,7 +98,7 @@ public function testExternalLinkCanBeAddedInConfig() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forLink('https://example.com', 'foo'), ]); @@ -113,7 +113,7 @@ public function testPathLinkCanBeAddedInConfig() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forLink('foo', 'foo'), ]); @@ -131,7 +131,7 @@ public function testDuplicatesAreNotRemovedWhenAddingInConfig() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forLink('foo', 'foo'), NavItem::forLink('foo', 'foo'), ]); @@ -150,7 +150,7 @@ public function testDuplicatesAreNotRemovedWhenAddingInConfigRegardlessOfDestina $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forLink('foo', 'foo'), NavItem::forLink('bar', 'foo'), ]); @@ -168,9 +168,9 @@ public function testConfigItemsTakePrecedenceOverGeneratedItems() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forLink('bar', 'Foo'), - NavItem::fromRoute(Routes::get('foo')), + NavItem::forRoute(Routes::get('foo')), ]); $this->assertCount(count($expected), $menu->getItems()); @@ -185,8 +185,8 @@ public function testDocumentationPagesThatAreNotIndexAreNotAddedToTheMenu() $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), - NavItem::fromRoute(Routes::get('docs/index')), + NavItem::forRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('docs/index')), ]); $this->assertCount(count($expected), $menu->getItems()); @@ -199,7 +199,7 @@ public function testPagesInSubdirectoriesAreNotAddedToTheNavigationMenu() $this->file('_pages/foo/bar.md'); $menu = $this->createNavigationMenu(); - $expected = collect([NavItem::fromRoute(Routes::get('index'))]); + $expected = collect([NavItem::forRoute(Routes::get('index'))]); $this->assertCount(count($expected), $menu->getItems()); $this->assertEquals($expected, $menu->getItems()); @@ -213,8 +213,8 @@ public function testPagesInSubdirectoriesCanBeAddedToTheNavigationMenuWithConfig $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), - NavItem::fromRoute(Routes::get('foo/bar')), + NavItem::forRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('foo/bar')), ]); $this->assertCount(count($expected), $menu->getItems()); @@ -229,9 +229,9 @@ public function testPagesInSubdirectoriesAreNotAddedToTheNavigationMenuWithConfi $menu = $this->createNavigationMenu(); $expected = collect([ - NavItem::fromRoute(Routes::get('index')), + NavItem::forRoute(Routes::get('index')), NavItem::forGroup('Foo', [ - NavItem::fromRoute(Routes::get('foo/bar')), + NavItem::forRoute(Routes::get('foo/bar')), ]), ]); @@ -249,10 +249,10 @@ public function testPagesInDropdownsDoNotGetAddedToTheMainNavigation() $this->assertCount(3, $menu->getItems()); $this->assertEquals([ - NavItem::fromRoute(Routes::get('index')), - NavItem::fromRoute((new MarkdownPage('foo'))->getRoute()), + NavItem::forRoute(Routes::get('index')), + NavItem::forRoute((new MarkdownPage('foo'))->getRoute()), NavItem::forGroup('Bar', [ - NavItem::fromRoute((new MarkdownPage('bar/baz'))->getRoute()), + NavItem::forRoute((new MarkdownPage('bar/baz'))->getRoute()), ]), ], $menu->getItems()->all()); } diff --git a/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php b/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php index b2da470a8c5..5195d1f3fdc 100644 --- a/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php +++ b/packages/framework/tests/Feature/Services/DocumentationSidebarTest.php @@ -86,9 +86,9 @@ public function testSidebarIsOrderedAlphabeticallyWhenNoOrderIsSetInConfig() $this->assertEquals( collect([ - NavItem::fromRoute(Routes::get('docs/a'), priority: 999), - NavItem::fromRoute(Routes::get('docs/b'), priority: 999), - NavItem::fromRoute(Routes::get('docs/c'), priority: 999), + NavItem::forRoute(Routes::get('docs/a'), priority: 999), + NavItem::forRoute(Routes::get('docs/b'), priority: 999), + NavItem::forRoute(Routes::get('docs/c'), priority: 999), ]), NavigationMenuGenerator::handle(DocumentationSidebar::class)->getItems() ); @@ -107,9 +107,9 @@ public function testSidebarIsOrderedByPriorityWhenPriorityIsSetInConfig() $this->assertEquals( collect([ - NavItem::fromRoute(Routes::get('docs/c'), priority: 250 + 250), - NavItem::fromRoute(Routes::get('docs/b'), priority: 250 + 251), - NavItem::fromRoute(Routes::get('docs/a'), priority: 250 + 252), + NavItem::forRoute(Routes::get('docs/c'), priority: 250 + 250), + NavItem::forRoute(Routes::get('docs/b'), priority: 250 + 251), + NavItem::forRoute(Routes::get('docs/a'), priority: 250 + 252), ]), NavigationMenuGenerator::handle(DocumentationSidebar::class)->getItems() ); @@ -146,9 +146,9 @@ public function testSidebarPrioritiesCanBeSetInBothFrontMatterAndConfig() $this->assertEquals( collect([ - NavItem::fromRoute(Routes::get('docs/first'), priority: 250 + 250), - NavItem::fromRoute(Routes::get('docs/second'), priority: 250 + 252), - NavItem::fromRoute(Routes::get('docs/third'), priority: 250 + 300), + NavItem::forRoute(Routes::get('docs/first'), priority: 250 + 250), + NavItem::forRoute(Routes::get('docs/second'), priority: 250 + 252), + NavItem::forRoute(Routes::get('docs/third'), priority: 250 + 300), ]), NavigationMenuGenerator::handle(DocumentationSidebar::class)->getItems() ); @@ -195,7 +195,7 @@ public function testGetItemsInGroupDoesNotIncludeDocsIndex() Filesystem::touch('_docs/index.md'); $this->assertEquals( - collect([NavItem::fromRoute(Routes::get('docs/foo'), priority: 999)]), + collect([NavItem::forRoute(Routes::get('docs/foo'), priority: 999)]), NavigationMenuGenerator::handle(DocumentationSidebar::class)->getItems() ); } @@ -268,10 +268,10 @@ public function testCanHaveMultipleGroupedPagesWithTheSameNameLabels() $this->assertEquals( collect([ NavItem::forGroup('Bar', [ - NavItem::fromRoute(Routes::get('docs/bar'), priority: 999), + NavItem::forRoute(Routes::get('docs/bar'), priority: 999), ]), NavItem::forGroup('Foo', [ - NavItem::fromRoute(Routes::get('docs/foo'), priority: 999), + NavItem::forRoute(Routes::get('docs/foo'), priority: 999), ]), ]), $sidebar->getItems() @@ -289,8 +289,8 @@ public function testDuplicateLabelsWithinTheSameGroupAreNotRemoved() $this->assertEquals( collect([ NavItem::forGroup('Foo', [ - NavItem::fromRoute(Routes::get('docs/bar'), priority: 999), - NavItem::fromRoute(Routes::get('docs/foo'), priority: 999), + NavItem::forRoute(Routes::get('docs/bar'), priority: 999), + NavItem::forRoute(Routes::get('docs/foo'), priority: 999), ]), ]), $sidebar->getItems() @@ -312,7 +312,7 @@ public function testIndexPageAddedToSidebarWhenItIsTheOnlyPage() $this->assertCount(1, $sidebar->getItems()); $this->assertEquals( - collect([NavItem::fromRoute(Routes::get('docs/index'))]), + collect([NavItem::forRoute(Routes::get('docs/index'))]), $sidebar->getItems() ); } @@ -325,7 +325,7 @@ public function testIndexPageNotAddedToSidebarWhenOtherPagesExist() $this->assertCount(1, $sidebar->getItems()); $this->assertEquals( - collect([NavItem::fromRoute(Routes::get('docs/test-0'))]), + collect([NavItem::forRoute(Routes::get('docs/test-0'))]), $sidebar->getItems() ); } diff --git a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php index fc3b74ea55f..ee75d74d943 100644 --- a/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php +++ b/packages/framework/tests/Unit/DocumentationSidebarUnitTest.php @@ -226,7 +226,7 @@ public function testHasGroupsReturnsTrueWhenAtLeastOneItemHasChildren() $page = new DocumentationPage('foo'); $child = new DocumentationPage('bar'); $menu = new DocumentationSidebar([ - NavItem::fromRoute($page->getRoute())->addChild(NavItem::fromRoute($child->getRoute())), + NavItem::forRoute($page->getRoute())->addChild(NavItem::forRoute($child->getRoute())), ]); $this->assertTrue($menu->hasGroups()); diff --git a/packages/framework/tests/Unit/NavItemIsCurrentHelperTest.php b/packages/framework/tests/Unit/NavItemIsCurrentHelperTest.php index d9500006247..97aa4de6f6f 100644 --- a/packages/framework/tests/Unit/NavItemIsCurrentHelperTest.php +++ b/packages/framework/tests/Unit/NavItemIsCurrentHelperTest.php @@ -34,19 +34,19 @@ protected function tearDown(): void public function testIsCurrent() { $this->mockRenderData($this->makeRoute('foo')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('bar'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('bar'))->isCurrent()); } public function testIsCurrentWhenCurrent() { $this->mockRenderData($this->makeRoute('foo')); - $this->assertTrue(NavItem::fromRoute($this->makeRoute('foo'))->isCurrent()); + $this->assertTrue(NavItem::forRoute($this->makeRoute('foo'))->isCurrent()); } public function testIsCurrentUsingCurrentRoute() { $this->mockRenderData($this->makeRoute('index')); - $this->assertTrue(NavItem::fromRoute(Routes::get('index'))->isCurrent()); + $this->assertTrue(NavItem::forRoute(Routes::get('index'))->isCurrent()); } public function testIsCurrentUsingCurrentLink() @@ -58,13 +58,13 @@ public function testIsCurrentUsingCurrentLink() public function testIsCurrentWhenNotCurrent() { $this->mockRenderData($this->makeRoute('foo')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('bar'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('bar'))->isCurrent()); } public function testIsCurrentUsingNotCurrentRoute() { $this->mockRenderData($this->makeRoute('foo')); - $this->assertFalse(NavItem::fromRoute(Routes::get('index'))->isCurrent()); + $this->assertFalse(NavItem::forRoute(Routes::get('index'))->isCurrent()); } public function testIsCurrentUsingNotCurrentLink() @@ -76,49 +76,49 @@ public function testIsCurrentUsingNotCurrentLink() public function testIsCurrentWithNestedCurrentPage() { $this->mockRenderData($this->makeRoute('foo/bar')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('bar'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('bar'))->isCurrent()); } public function testIsCurrentWhenCurrentWithNestedCurrentPage() { $this->mockRenderData($this->makeRoute('foo/bar')); - $this->assertTrue(NavItem::fromRoute($this->makeRoute('foo/bar'))->isCurrent()); + $this->assertTrue(NavItem::forRoute($this->makeRoute('foo/bar'))->isCurrent()); } public function testIsCurrentWithNestedCurrentPageWhenNested() { $this->mockRenderData($this->makeRoute('foo/bar')); - $this->assertTrue(NavItem::fromRoute($this->makeRoute('foo/bar'))->isCurrent()); + $this->assertTrue(NavItem::forRoute($this->makeRoute('foo/bar'))->isCurrent()); } public function testIsCurrentWhenCurrentWithNestedCurrentPageWhenNested() { $this->mockRenderData($this->makeRoute('foo/bar')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('foo/baz'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('foo/baz'))->isCurrent()); } public function testIsCurrentWithNestedCurrentPageWhenVeryNested() { $this->mockRenderData($this->makeRoute('foo/bar/baz')); - $this->assertTrue(NavItem::fromRoute($this->makeRoute('foo/bar/baz'))->isCurrent()); + $this->assertTrue(NavItem::forRoute($this->makeRoute('foo/bar/baz'))->isCurrent()); } public function testIsCurrentWhenCurrentWithNestedCurrentPageWhenVeryNested() { $this->mockRenderData($this->makeRoute('foo/bar/baz')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('foo/baz/bar'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('foo/baz/bar'))->isCurrent()); } public function testIsCurrentWithNestedCurrentPageWhenVeryDifferingNested() { $this->mockRenderData($this->makeRoute('foo')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('foo/bar/baz'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('foo/bar/baz'))->isCurrent()); } public function testIsCurrentWithNestedCurrentPageWhenVeryDifferingNestedInverse() { $this->mockRenderData($this->makeRoute('foo/bar/baz')); - $this->assertFalse(NavItem::fromRoute($this->makeRoute('foo'))->isCurrent()); + $this->assertFalse(NavItem::forRoute($this->makeRoute('foo'))->isCurrent()); } public function testIsCurrentUsingCurrentLinkWithNestedCurrentPage() diff --git a/packages/framework/tests/Unit/NavItemTest.php b/packages/framework/tests/Unit/NavItemTest.php index 1816633e0d1..525dad28367 100644 --- a/packages/framework/tests/Unit/NavItemTest.php +++ b/packages/framework/tests/Unit/NavItemTest.php @@ -169,7 +169,7 @@ public function testGetChildrenWithNoChildren() public function testFromRoute() { $route = new Route(new MarkdownPage()); - $item = NavItem::fromRoute($route); + $item = NavItem::forRoute($route); $this->assertSame($route, $item->getDestination()); } @@ -178,7 +178,7 @@ public function testToString() { Render::shouldReceive('getRouteKey')->once()->andReturn('index'); - $this->assertSame('index.html', (string) NavItem::fromRoute(Routes::get('index'))); + $this->assertSame('index.html', (string) NavItem::forRoute(Routes::get('index'))); } public function testForLink() @@ -241,24 +241,24 @@ public function testRouteBasedNavItemDestinationsAreResolvedRelatively() 'getRouteKey' => 'foo', ])); - $this->assertSame('foo.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo')))); - $this->assertSame('foo/bar.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo/bar')))); + $this->assertSame('foo.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo')))); + $this->assertSame('foo/bar.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo/bar')))); Render::swap(Mockery::mock(RenderData::class, [ 'getRoute' => new Route(new InMemoryPage('foo/bar')), 'getRouteKey' => 'foo/bar', ])); - $this->assertSame('../foo.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo')))); - $this->assertSame('../foo/bar.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo/bar')))); + $this->assertSame('../foo.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo')))); + $this->assertSame('../foo/bar.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo/bar')))); Render::swap(Mockery::mock(RenderData::class, [ 'getRoute' => new Route(new InMemoryPage('foo/bar/baz')), 'getRouteKey' => 'foo/bar/baz', ])); - $this->assertSame('../../foo.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo')))); - $this->assertSame('../../foo/bar.html', (string) NavItem::fromRoute(new Route(new InMemoryPage('foo/bar')))); + $this->assertSame('../../foo.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo')))); + $this->assertSame('../../foo/bar.html', (string) NavItem::forRoute(new Route(new InMemoryPage('foo/bar')))); } public function testDropdownFacade() @@ -309,8 +309,8 @@ public function testIsCurrent() 'getRoute' => new Route(new InMemoryPage('foo')), 'getRouteKey' => 'foo', ])); - $this->assertTrue(NavItem::fromRoute(new Route(new InMemoryPage('foo')))->isCurrent()); - $this->assertFalse(NavItem::fromRoute(new Route(new InMemoryPage('bar')))->isCurrent()); + $this->assertTrue(NavItem::forRoute(new Route(new InMemoryPage('foo')))->isCurrent()); + $this->assertFalse(NavItem::forRoute(new Route(new InMemoryPage('bar')))->isCurrent()); } public function testIsCurrentWithExternalRoute() @@ -335,7 +335,7 @@ public function testGetGroupWithGroup() public function testGetGroupFromRouteWithGroup() { - $this->assertSame('foo', NavItem::fromRoute(new Route(new MarkdownPage(matter: ['navigation.group' => 'foo'])))->getGroup()); + $this->assertSame('foo', NavItem::forRoute(new Route(new MarkdownPage(matter: ['navigation.group' => 'foo'])))->getGroup()); } public function testGetGroupForRouteWithGroup() @@ -367,7 +367,7 @@ public function testIdentifierWithCustomLabel() public function testIdentifierFromRouteKey() { - $item = NavItem::fromRoute(Routes::get('index')); + $item = NavItem::forRoute(Routes::get('index')); $this->assertSame('home', $item->getIdentifier()); } From e9cbf99d60f746da1592defc18410aa0406ab10c Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 20:39:46 +0100 Subject: [PATCH 19/35] Breaking: Remove method `NavItem::fromRoute()` --- .../src/Framework/Features/Navigation/NavItem.php | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index e10ce0feffd..8db55c61800 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -58,16 +58,6 @@ public function __construct(Route|string $destination, string $label, int $prior $this->children = $children; } - /** - * Create a new navigation menu item from a route. - * - * @deprecated Use NavItem::forRoute() instead. - */ - public static function fromRoute(Route $route, ?string $label = null, ?int $priority = null, ?string $group = null): static - { - return static::forRoute($route, $label, $priority, $group); - } - /** * Create a new navigation menu item leading to a Route instance. * From 5970a041a45879e218392a5d04c7e9f0156be331 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 20:51:26 +0100 Subject: [PATCH 20/35] Annotate child array generics --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 8db55c61800..15761616734 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -43,6 +43,8 @@ class NavItem implements Stringable /** * Create a new navigation menu item. + * + * @param array<\Hyde\Framework\Features\Navigation\NavItem> $children */ public function __construct(Route|string $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null, array $children = []) { @@ -208,6 +210,8 @@ public function addChild(NavItem $item): static /** * Add multiple navigation items to the children of the navigation item. + * + * @param array<\Hyde\Framework\Features\Navigation\NavItem> $items */ public function addChildren(array $items): static { From 87dbc8029904cb5f19c528c575af77081926c7d8 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:10:34 +0100 Subject: [PATCH 21/35] Update `NavItem::$destination` property to be nullable --- .../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 15761616734..8d7c0c9b57b 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 { - protected Route $destination; + protected ?Route $destination; protected string $label; protected int $priority; protected ?string $group; @@ -46,7 +46,7 @@ class NavItem implements Stringable * * @param array<\Hyde\Framework\Features\Navigation\NavItem> $children */ - public function __construct(Route|string $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null, array $children = []) + public function __construct(Route|string|null $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null, array $children = []) { if (is_string($destination)) { $destination = Routes::get($destination) ?? new ExternalRoute($destination); From ab95f50e5217dd8f4896db8a096b2f40cfe09586 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:30:19 +0100 Subject: [PATCH 22/35] Set destination to null when creating group items --- .../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 8d7c0c9b57b..7bd1b7dc451 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -97,7 +97,7 @@ public static function forLink(string $href, string $label, int $priority = Navi */ public static function forGroup(string $label, array $items, ?int $priority = null): static { - return new static('', $label, $priority ?? NavigationMenu::LAST, $label, $items); + return new static(null, $label, $priority ?? NavigationMenu::LAST, $label, $items); } /** From fa9c01d05deab0984d7d6794fb0a2e07e6eb85a8 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:38:05 +0100 Subject: [PATCH 23/35] Document null state reasoning --- .../framework/src/Framework/Features/Navigation/NavItem.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 7bd1b7dc451..bd2925125cf 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -27,6 +27,7 @@ * Navigation items can be turned into dropdowns or sidebar groups by adding children. * Note that doing so will mean that any link on the parent will no longer be clickable, * as clicking the parent label will open the dropdown instead of leading to the destination. + * For this reason, dropdown items will have their destination set to null. */ class NavItem implements Stringable { @@ -109,7 +110,7 @@ public function __toString(): string } /** - * Get the destination route of the navigation item. + * Get the destination route of the navigation item. For dropdowns, this will return null. */ public function getDestination(): ?Route { From f1d9ca7fdb8f4f99f40af54b9efaef5b7eb2ebba Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:41:32 +0100 Subject: [PATCH 24/35] Enforce group destination null state when adding children Provides a consistent state --- packages/framework/src/Framework/Features/Navigation/NavItem.php | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index bd2925125cf..cf385c108c9 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -205,6 +205,7 @@ public function addChild(NavItem $item): static $item->group ??= $this->group; $this->children[] = $item; + $this->destination = null; return $this; } From c12f3de85d422708d9f0567f1df9c7a19eeacc88 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:41:40 +0100 Subject: [PATCH 25/35] Document null state enforcement --- .../framework/src/Framework/Features/Navigation/NavItem.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index cf385c108c9..c963e0dc6fc 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -199,6 +199,8 @@ public function isCurrent(): bool /** * Add a navigation item to the children of the navigation item. + * + * This will turn the parent item into a dropdown. Its destination will be set to null. */ public function addChild(NavItem $item): static { From 7c08dc66edee0824861e0e70b1ea184ea8932dd5 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 21:57:14 +0100 Subject: [PATCH 26/35] Construct children using helper to enforce type state --- .../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 c963e0dc6fc..77047479849 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -58,7 +58,7 @@ public function __construct(Route|string|null $destination, string $label, int $ $this->priority = $priority; $this->group = static::normalizeGroupKey($group); $this->identifier = static::makeIdentifier($label); - $this->children = $children; + $this->addChildren($children); } /** From 46dd08dd0ad2ab210bf3b6c01649e2c97b605504 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Thu, 29 Feb 2024 22:05:06 +0100 Subject: [PATCH 27/35] Ensure children are constructed --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 77047479849..e09a420f998 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -206,6 +206,10 @@ public function addChild(NavItem $item): static { $item->group ??= $this->group; + if (!isset($this->children)) { + $this->children = []; + } + $this->children[] = $item; $this->destination = null; From abf031618c82ac91f4bd7a610aaaa9b4f4d31f83 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:02:12 +0100 Subject: [PATCH 28/35] Revert "Ensure children are constructed" This reverts commit 46dd08dd0ad2ab210bf3b6c01649e2c97b605504. --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index e09a420f998..77047479849 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -206,10 +206,6 @@ public function addChild(NavItem $item): static { $item->group ??= $this->group; - if (!isset($this->children)) { - $this->children = []; - } - $this->children[] = $item; $this->destination = null; From 5b43c7c0eaeb3aedbe0bdca2e9d42e3932ddb573 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:02:39 +0100 Subject: [PATCH 29/35] Initialize children property --- .../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 77047479849..31ccb67b004 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -40,7 +40,7 @@ class NavItem implements Stringable protected string $identifier; /** @var array<\Hyde\Framework\Features\Navigation\NavItem> */ - protected array $children; + protected array $children = []; /** * Create a new navigation menu item. From aeff6c45919f135da7e4032e76214a2107c91294 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:04:40 +0100 Subject: [PATCH 30/35] Remove redundant null return for null type --- .../framework/src/Framework/Features/Navigation/NavItem.php | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index 31ccb67b004..d71e642c7e6 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -114,10 +114,6 @@ public function __toString(): string */ public function getDestination(): ?Route { - if ($this->hasChildren()) { - return null; - } - return $this->destination; } From c76ec470fd9f48305e08e2d8b72e22d091bbf95b Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:12:28 +0100 Subject: [PATCH 31/35] Add default value to method signature --- .../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 d71e642c7e6..cbec59c07ba 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -96,7 +96,7 @@ public static function forLink(string $href, string $label, int $priority = Navi * @param array $items The items to be included in the dropdown. * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, ?int $priority = null): static + public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static { return new static(null, $label, $priority ?? NavigationMenu::LAST, $label, $items); } From edb8b50f9f073d67a351b64b510fe7e2b181dcf4 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:37:41 +0100 Subject: [PATCH 32/35] Add null coalesce to default priority in generator --- .../Framework/Features/Navigation/NavigationMenuGenerator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php index 342a696d96b..b407234ff98 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php +++ b/packages/framework/src/Framework/Features/Navigation/NavigationMenuGenerator.php @@ -160,7 +160,7 @@ protected function createGroupItem(string $groupKey, string $groupName): NavItem $priority = $this->searchForGroupPriorityInConfig($groupKey); - return NavItem::forGroup($this->normalizeGroupLabel($label), [], $priority); + return NavItem::forGroup($this->normalizeGroupLabel($label), [], $priority ?? NavigationMenu::LAST); } protected function normalizeGroupLabel(string $label): string From 305b53bb328c1bd377888af4ed2ec7024ccc357e Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:38:27 +0100 Subject: [PATCH 33/35] Remove null coalesce moved to default value --- .../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 cbec59c07ba..577ebbe19fd 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -98,7 +98,7 @@ public static function forLink(string $href, string $label, int $priority = Navi */ public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static { - return new static(null, $label, $priority ?? NavigationMenu::LAST, $label, $items); + return new static(null, $label, $priority, $label, $items); } /** From 4036c3c3174e497ca3388440624c19fca825f346 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 12:49:55 +0100 Subject: [PATCH 34/35] Remove nullable priority parameter 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 577ebbe19fd..c487e8094a2 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -94,9 +94,9 @@ public static function forLink(string $href, string $label, int $priority = Navi * * @param string $label The label of the dropdown item. * @param array $items The items to be included in the dropdown. - * @param int|null $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. + * @param int $priority The priority of the dropdown item. Leave blank to use the default priority, which is last in the menu. */ - public static function forGroup(string $label, array $items, ?int $priority = NavigationMenu::LAST): static + public static function forGroup(string $label, array $items, int $priority = NavigationMenu::LAST): static { return new static(null, $label, $priority, $label, $items); } From 418db0055b95d065628c03290f85d3a680647da2 Mon Sep 17 00:00:00 2001 From: Caen De Silva Date: Fri, 1 Mar 2024 13:02:18 +0100 Subject: [PATCH 35/35] Document constructor --- .../framework/src/Framework/Features/Navigation/NavItem.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/framework/src/Framework/Features/Navigation/NavItem.php b/packages/framework/src/Framework/Features/Navigation/NavItem.php index c487e8094a2..4132e9bd2f6 100644 --- a/packages/framework/src/Framework/Features/Navigation/NavItem.php +++ b/packages/framework/src/Framework/Features/Navigation/NavItem.php @@ -45,7 +45,11 @@ class NavItem implements Stringable /** * Create a new navigation menu item. * - * @param array<\Hyde\Framework\Features\Navigation\NavItem> $children + * @param \Hyde\Support\Models\Route|string|null $destination Route instance, route key, or external URI. For dropdowns/groups, this should be null. + * @param string $label The label of the navigation item. + * @param int $priority The priority to determine the order of the navigation item. + * @param string|null $group The dropdown/group identifier of the navigation item, if any. + * @param array<\Hyde\Framework\Features\Navigation\NavItem> $children If the item is a dropdown, these are the items to be included in the dropdown. */ public function __construct(Route|string|null $destination, string $label, int $priority = NavigationMenu::DEFAULT, ?string $group = null, array $children = []) {