Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[2.x] Simplify the navigation items class #1637

Merged
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
@props(['grouped' => false])
@php /** @var \Hyde\Framework\Features\Navigation\NavigationItem $item */ @endphp
<li @class(['sidebar-item -ml-4 pl-4', $grouped
? 'active -ml-8 pl-8 bg-black/5 dark:bg-black/10'
: 'active bg-black/5 dark:bg-black/10' => $item->isActive()
]) role="listitem">
@if($item->isActive())
<a href="{{ $item->getRoute() }}" aria-current="true" @class([$grouped
<a href="{{ $item->getLink() }}" aria-current="true" @class([$grouped
? '-ml-8 pl-4 py-1 px-2 block text-indigo-600 dark:text-indigo-400 dark:font-medium border-l-[0.325rem] border-indigo-500 transition-colors duration-300 ease-in-out hover:bg-black/10'
: '-ml-4 p-2 block hover:bg-black/5 dark:hover:bg-black/10 text-indigo-600 dark:text-indigo-400 dark:font-medium border-l-[0.325rem] border-indigo-500 transition-colors duration-300 ease-in-out'
])>
Expand All @@ -16,7 +17,7 @@
{!! $page->getTableOfContents() !!}
@endif
@else
<a href="{{ $item->getRoute() }}" @class([$grouped
<a href="{{ $item->getLink() }}" @class([$grouped
? '-ml-8 pl-4 py-1 px-2 block border-l-[0.325rem] border-transparent transition-colors duration-300 ease-in-out hover:bg-black/10'
: 'block -ml-4 p-2 border-l-[0.325rem] border-transparent hover:bg-black/5 dark:hover:bg-black/10'
])>
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -78,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() !== null) && $child->getRoute()->getPage() instanceof DocumentationPage;
return $child->getPage() instanceof DocumentationPage;
});
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

namespace Hyde\Framework\Features\Navigation;

use Hyde\Pages\Concerns\HydePage;
use Hyde\Foundation\Facades\Routes;
use Hyde\Hyde;
use Hyde\Support\Models\Route;
Expand All @@ -22,7 +23,7 @@
*/
class NavigationItem implements NavigationElement, Stringable
{
protected NavigationDestination $destination;
protected string|Route $destination;
protected string $label;
protected int $priority;

Expand All @@ -39,14 +40,12 @@ 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);
$this->destination = $destination;

$this->label = $label;
$this->priority = $priority;

if ($group !== null) {
$this->group = static::normalizeGroupKey($group);
}
$this->group = static::normalizeGroupKey($group);
}

/**
Expand All @@ -57,50 +56,35 @@ public function __construct(Route|string $destination, string $label, int $prior
* @param string|null $label Leave blank to use the label of the route's corresponding page, if there is one tied to the route.
* @param string|null $group Leave blank to use the group of the route's corresponding page, if there is one tied to the route.
*/
public static function create(Route|string $destination, ?string $label = null, ?int $priority = null, ?string $group = null): self
public static function create(Route|string $destination, ?string $label = null, ?int $priority = null, ?string $group = null): static
{
if (is_string($destination) && Routes::has($destination)) {
$destination = Routes::get($destination);
}

if ($destination instanceof Route) {
return new self(
$destination,
$label ?? $destination->getPage()->navigationMenuLabel(),
$priority ?? $destination->getPage()->navigationMenuPriority(),
$group ?? $destination->getPage()->navigationMenuGroup(),
);
$label ??= $destination->getPage()->navigationMenuLabel();
$priority ??= $destination->getPage()->navigationMenuPriority();
$group ??= $destination->getPage()->navigationMenuGroup();
}

return new self($destination, $label ?? '', $priority ?? NavigationMenu::DEFAULT, $group);
return new static($destination, $label ?? '', $priority ?? NavigationMenu::DEFAULT, $group);
}

/**
* Resolve a link to the navigation item.
*/
public function __toString(): string
{
return $this->getUrl();
}

/**
* Get the destination route of the navigation item. For dropdowns, this will return null.
*
* @deprecated To simplify the class, we may remove this as we probably don't need it.
*/
public function getRoute(): ?Route
{
return $this->destination->getRoute();
return $this->getLink();
}

/**
* Resolve the destination link of the navigation item.
*
* @deprecated May be renamed to getLink() in the future to better match its usage, and to match the Route class.
*/
public function getUrl(): string
public function getLink(): string
{
return $this->destination->getLink();
return (string) $this->destination;
}

/**
Expand Down Expand Up @@ -132,12 +116,20 @@ public function getGroupKey(): ?string
return $this->group;
}

/**
* If the navigation item is a link to a routed page, get the corresponding page instance.
*/
public function getPage(): ?HydePage
{
return $this->destination instanceof Route ? $this->destination->getPage() : null;
}

/**
* Check if the NavigationItem instance is the current page being rendered.
*/
public function isActive(): bool
{
return Hyde::currentRoute()->getLink() === $this->getUrl();
return Hyde::currentRoute()->getLink() === $this->getLink();
}

/** @return ($group is null ? null : string) */
Expand Down
87 changes: 62 additions & 25 deletions packages/framework/tests/Unit/NavigationItemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,10 @@ protected function setUp(): void

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

$this->assertSame($route, $item->getRoute());
$this->assertInstanceOf(NavigationItem::class, new NavigationItem('foo', 'Test'));
$this->assertInstanceOf(NavigationItem::class, new NavigationItem(new Route(new MarkdownPage()), 'Test'));
$this->assertInstanceOf(NavigationItem::class, new NavigationItem(new Route(new MarkdownPage()), 'Test', 500));
$this->assertInstanceOf(NavigationItem::class, new NavigationItem(new Route(new MarkdownPage()), 'Test', 500, 'foo'));
}

public function testIsInstanceOfNavigationElement()
Expand All @@ -55,42 +55,50 @@ public function testIsInstanceOfNavigationElement()
public function testPassingRouteInstanceToConstructorUsesRouteInstance()
{
$route = new Route(new MarkdownPage());
$this->assertSame($route, (new NavigationItem($route, 'Home'))->getRoute());
$this->assertEquals($route, (new NavigationItem($route, 'Home'))->getPage()->getRoute());
}

public function testPassingRouteKeyToConstructorUsesRouteInstance()
public function testPassingRouteKeyToConstructorUsesDestinationAsLink()
{
$route = Routes::get('index');

$this->assertSame($route, (new NavigationItem('index', 'Home'))->getRoute());
$item = new NavigationItem('index', 'Home');
$this->assertNull($item->getPage());
$this->assertSame('index', $item->getLink());
}

public function testPassingUrlToConstructorSetsRouteToNull()
{
$item = new NavigationItem('https://example.com', 'Home');
$this->assertNull($item->getRoute());
$this->assertSame('https://example.com', $item->getUrl());
$this->assertNull($item->getPage());
$this->assertSame('https://example.com', $item->getLink());
}

public function testPassingUnknownRouteKeyToConstructorSetsRouteToNull()
{
$item = new NavigationItem('foo', 'Home');
$this->assertNull($item->getRoute());
$this->assertSame('foo', $item->getUrl());
$this->assertNull($item->getPage());
$this->assertSame('foo', $item->getLink());
}

public function testGetDestination()
public function testCanGetPage()
{
$page = new MarkdownPage();
$item = new NavigationItem(new Route($page), 'Test', 500);

$this->assertSame($page, $item->getPage());
}

public function testGetPageRoute()
{
$route = new Route(new InMemoryPage('foo'));
$NavigationItem = new NavigationItem($route, 'Page', 500);

$this->assertSame($route, $NavigationItem->getRoute());
$this->assertEquals($route, $NavigationItem->getPage()->getRoute());
}

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

public function testGetLabel()
Expand All @@ -113,10 +121,12 @@ public function testGetGroup()

public function testFromRoute()
{
$route = new Route(new MarkdownPage());
$page = new MarkdownPage();
$route = new Route($page);
$item = NavigationItem::create($route);

$this->assertSame($route, $item->getRoute());
$this->assertSame($page, $item->getPage());
$this->assertEquals($route, $item->getPage()->getRoute());
}

public function testToString()
Expand All @@ -130,8 +140,8 @@ public function testCreateWithLink()
{
$item = NavigationItem::create('foo', 'bar');

$this->assertNull($item->getRoute());
$this->assertSame('foo', $item->getUrl());
$this->assertNull($item->getPage());
$this->assertSame('foo', $item->getLink());
$this->assertSame('bar', $item->getLabel());
$this->assertSame(500, $item->getPriority());
}
Expand All @@ -146,19 +156,38 @@ public function testCreate()
$route = Routes::get('404');
$item = NavigationItem::create($route, 'foo');

$this->assertSame($route, $item->getRoute());
$this->assertSame($route->getPage(), $item->getPage());
$this->assertSame('foo', $item->getLabel());
$this->assertSame(999, $item->getPriority());
$this->assertEquals($route, $item->getPage()->getRoute());
$this->assertSame('404.html', $item->getLink());
$this->assertSame('404.html', (string) $item);
$this->assertNull($item->getGroupKey());
}

public function testForIndexRoute()
{
$route = Routes::get('index');
$item = NavigationItem::create($route, 'foo');
$item = NavigationItem::create($route);

$this->assertSame($route, $item->getRoute());
$this->assertSame('foo', $item->getLabel());
$this->assertSame($route->getPage(), $item->getPage());
$this->assertSame('Home', $item->getLabel());
$this->assertSame(0, $item->getPriority());
$this->assertEquals($route, $item->getPage()->getRoute());
$this->assertSame('index.html', $item->getLink());
$this->assertSame('index.html', (string) $item);
$this->assertNull($item->getGroupKey());
}

public function testCreateWithLabels()
{
$route = Routes::get('404');
$item = NavigationItem::create($route, 'foo');
$this->assertSame('foo', $item->getLabel());

$route = Routes::get('index');
$item = NavigationItem::create($route);
$this->assertSame('Home', $item->getLabel());
}

public function testCreateWithRouteKey()
Expand All @@ -171,14 +200,22 @@ public function testCreateWithRouteKey()

public function testCreateWithMissingRouteKey()
{
$this->assertNull(NavigationItem::create('foo', 'foo')->getRoute());
$this->assertNull(NavigationItem::create('foo', 'foo')->getPage());
}

public function testCreateWithCustomPriority()
{
$this->assertSame(100, NavigationItem::create(Routes::get('index'), 'foo', 100)->getPriority());
}

public function testPassingRouteKeyToStaticConstructorUsesRouteInstance()
{
$route = Routes::get('index');
$item = NavigationItem::create('index', 'Home');
$this->assertNotNull($item->getPage());
$this->assertSame($route->getPage(), $item->getPage());
}

public function testRouteBasedNavigationItemDestinationsAreResolvedRelatively()
{
Render::swap(Mockery::mock(RenderData::class, [
Expand Down
Loading