Skip to content

Commit

Permalink
Merge pull request #1539 from hydephp/update-navitem-class-to-support…
Browse files Browse the repository at this point in the history
…-deferring-route-resolves

Update NavItem class to support deferring route resolves
  • Loading branch information
caendesilva authored Feb 8, 2024
2 parents 89730ae + 43b93d4 commit dfc5a58
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 12 deletions.
10 changes: 10 additions & 0 deletions RELEASE_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,12 @@ This serves two purposes:

### Added
- Added a new `\Hyde\Framework\Actions\PreBuildTasks\TransferMediaAssets` build task handle media assets transfers for site builds.
- Added a new `NavItem::getLink()` method contain the previous `NavItem::getDestination()` logic, to return the link URL.

### Changed
- Changed how the documentation search is generated, to be an `InMemoryPage` instead of a post-build task.
- Media asset files are now copied using the new build task instead of the deprecated `BuildService::transferMediaAssets()` method.
- The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type.

### Deprecated
- for soon-to-be removed features.
Expand Down Expand Up @@ -54,6 +56,14 @@ For more information, see https://github.com/hydephp/develop/pull/1498.

## Low impact

### Navigation item changes

The `NavItem::getDestination()` method now returns its `Route` instance if that is the destination type. This allows for deferring the route evaluation.

If you have previously used this method directly and expected a string to be returned, you may need to adapt your code to handle the new return type.

If you want to retain the previous state where a string is always returned, you can use the new `NavItem::getLink()` method instead, which will resolve the route immediately.

### New documentation search implementation

As the new documentation search implementation brings changes to their code API you may need to adapt your code
Expand Down
25 changes: 18 additions & 7 deletions packages/framework/src/Framework/Features/Navigation/NavItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
*/
class NavItem implements Stringable
{
public readonly string $destination;
public readonly Route|string $destination;
public readonly string $label;
public readonly int $priority;
public readonly ?string $group;
Expand All @@ -30,7 +30,7 @@ class NavItem implements Stringable
*/
public function __construct(Route|string $destination, string $label, int $priority = 500, ?string $group = null)
{
$this->destination = (string) $destination;
$this->destination = $destination;
$this->label = $label;
$this->priority = $priority;
$this->group = $group;
Expand All @@ -42,7 +42,7 @@ public function __construct(Route|string $destination, string $label, int $prior
public static function fromRoute(Route $route, ?string $label = null, ?int $priority = null, ?string $group = null): static
{
return new static(
$route->getLink(),
$route,
$label ?? $route->getPage()->navigationMenuLabel(),
$priority ?? $route->getPage()->navigationMenuPriority(),
$group ?? static::getRouteGroup($route),
Expand Down Expand Up @@ -74,6 +74,17 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int
* Resolve a link to the navigation item.
*/
public function __toString(): string
{
return (string) $this->destination;
}

/**
* Get the destination of the navigation item.
*
* If the navigation item is an external link, this will return the link as is,
* if it's for a route, the route instance will be returned.
*/
public function getDestination(): Route|string
{
return $this->destination;
}
Expand All @@ -82,11 +93,11 @@ public function __toString(): string
* Get the destination link of the navigation item.
*
* If the navigation item is an external link, this will return the link as is,
* if it's for a route, a resolved relative link will be returned.
* if it's for a route, the route's resolved link will be returned.
*/
public function getDestination(): string
public function getLink(): string
{
return $this->destination;
return (string) $this->destination;
}

/**
Expand Down Expand Up @@ -120,7 +131,7 @@ public function getGroup(): ?string
*/
public function isCurrent(): bool
{
return Hyde::currentRoute()->getLink() === $this->destination;
return Hyde::currentRoute()->getLink() === (string) $this->destination;
}

protected static function getRouteGroup(Route $route): ?string
Expand Down
18 changes: 13 additions & 5 deletions packages/framework/tests/Unit/NavItemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,21 @@ public function test__construct()
$route = new Route(new MarkdownPage());
$item = new NavItem($route, 'Test', 500);

$this->assertSame($route->getLink(), $item->destination);
$this->assertSame($route, $item->destination);
}

public function testGetDestination()
{
$route = new Route(new MarkdownPage());
$item = new NavItem($route, 'Test', 500);

$this->assertSame($route, $item->getDestination());
}

public function testGetLink()
{
$navItem = new NavItem(new Route(new InMemoryPage('foo')), 'Page', 500);
$this->assertSame('foo.html', $navItem->getDestination());
$this->assertSame('foo.html', $navItem->getLink());
}

public function testGetLabel()
Expand All @@ -75,7 +83,7 @@ public function testFromRoute()
$route = new Route(new MarkdownPage());
$item = NavItem::fromRoute($route);

$this->assertSame($route->getLink(), $item->destination);
$this->assertSame($route, $item->destination);
}

public function test__toString()
Expand Down Expand Up @@ -104,7 +112,7 @@ public function testForRoute()
$route = Routes::get('404');
$item = NavItem::forRoute($route, 'foo');

$this->assertSame($route->getLink(), $item->destination);
$this->assertSame($route, $item->destination);
$this->assertSame('foo', $item->label);
$this->assertSame(999, $item->priority);
}
Expand All @@ -114,7 +122,7 @@ public function testForIndexRoute()
$route = Routes::get('index');
$item = NavItem::forRoute($route, 'foo');

$this->assertSame($route->getLink(), $item->destination);
$this->assertSame($route, $item->destination);
$this->assertSame('foo', $item->label);
$this->assertSame(0, $item->priority);
}
Expand Down

0 comments on commit dfc5a58

Please sign in to comment.