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.

Sign up for GitHub

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

Jump to bottom

[2.x] Replace navigation item group normalization juggling with looser comparison #1575

Closed
wants to merge 3 commits into from
Closed

[2.x] Replace navigation item group normalization juggling with looser comparison #1575

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2cf9364
Normalize group name at compare time
caendesilva Feb 20, 2024
e492d12
Normalize group keys to a proper label instead of slug
caendesilva Feb 20, 2024
8fde0f2
Update code
caendesilva Feb 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 9 additions & 5 deletions packages/framework/src/Framework/Features/Navigation/GeneratesDocumentationSidebarMenu.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Hyde\Framework\Features\Navigation;

use Hyde\Hyde;
use Illuminate\Support\Str;
use Hyde\Support\Models\Route;
use Hyde\Pages\DocumentationPage;
use Illuminate\Support\Collection;
Expand Down Expand Up @@ -58,16 +59,16 @@ protected function generate(): void
$group = 'Other';
}

$groupItem = $this->items->get($group);
$groupItem = $this->items->get(Str::slug($group));

if ($groupItem === null) {
$groupItem = NavItem::dropdown($this->makeTitleForGroup($group), []);
}

$groupItem->addChild($item);

if (! $this->items->has($group)) {
$this->items->put($group, $groupItem);
if (! $this->items->has(Str::slug($group))) {
$this->items->put(Str::slug($group), $groupItem);
}

return;
Expand Down Expand Up @@ -95,9 +96,12 @@ protected function usesSidebarGroups(): bool

protected function makeTitleForGroup(string $group): string
{
// Todo search for other labels in the group before slugifying them
// If the label is not formatted, we format it here
if ($group === strtolower($group)) {
return Hyde::makeTitle($group);
}

return Hyde::makeTitle($group);
return $group;
}

protected function canAddRoute(Route $route): bool
Expand Down
12 changes: 11 additions & 1 deletion packages/framework/src/Framework/Features/Navigation/NavItem.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,17 @@ public function addChild(NavItem $item): void

protected static function normalizeGroupKey(?string $group): ?string
{
return $group ? Str::slug($group) : null;
// If there is no group, we return null
if (! $group) {
return null;
}

// If the label is not formatted, we format it here
if ($group === strtolower($group)) {
return Hyde::makeTitle($group);
}

return $group;
}

protected static function makeIdentifier(string $label): string
Expand Down
25 changes: 11 additions & 14 deletions packages/framework/tests/Feature/AutomaticNavigationConfigurationsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ public function testMainNavigationMenuWithFrontMatterGroup()
// TODO: For new v2 system, this should insert a root item with the group name and the children as the pages

$this->assertMenuEquals([
['label' => 'Foo', 'group' => 'group-1'],
['label' => 'Bar', 'group' => 'group-1'],
['label' => 'Baz', 'group' => 'group-1'],
['label' => 'Foo', 'group' => 'Group 1'],
['label' => 'Bar', 'group' => 'Group 1'],
['label' => 'Baz', 'group' => 'Group 1'],
], [
new MarkdownPage('foo', ['navigation.group' => 'Group 1']),
new MarkdownPage('bar', ['navigation.group' => 'Group 1']),
Expand All @@ -218,9 +218,9 @@ public function testMainNavigationMenuWithFrontMatterCategory()
// TODO: For new v2 system, this should insert a root item with the group name and the children as the pages

$this->assertMenuEquals([
['label' => 'Foo', 'group' => 'group-1'],
['label' => 'Bar', 'group' => 'group-1'],
['label' => 'Baz', 'group' => 'group-1'],
['label' => 'Foo', 'group' => 'Group 1'],
['label' => 'Bar', 'group' => 'Group 1'],
['label' => 'Baz', 'group' => 'Group 1'],
], [
new MarkdownPage('foo', ['navigation.category' => 'Group 1']),
new MarkdownPage('bar', ['navigation.category' => 'Group 1']),
Expand Down Expand Up @@ -290,7 +290,7 @@ public function testMainNavigationMenuWithFrontMatterGroupAndCategory()
{
// Since the main key in the navigation schema is 'group', that takes precedence over its 'category' alias

$this->assertMenuEquals(array_fill(0, 3, ['group' => 'group-1']), [
$this->assertMenuEquals(array_fill(0, 3, ['group' => 'Group 1']), [
new MarkdownPage('foo', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
new MarkdownPage('bar', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
new MarkdownPage('baz', ['navigation.group' => 'Group 1', 'navigation.category' => 'Group 2']),
Expand Down Expand Up @@ -483,8 +483,8 @@ public function testMainNavigationMenuItemsWithTheSameLabelAreNotFilteredForDupl
public function testMainNavigationMenuItemsWithSameLabelButDifferentGroupsAreNotFiltered()
{
$this->assertMenuEquals([
['label' => 'Foo', 'group' => 'group-1'],
['label' => 'Foo', 'group' => 'group-2'],
['label' => 'Foo', 'group' => 'Group 1'],
['label' => 'Foo', 'group' => 'Group 2'],
], [
new MarkdownPage('foo', ['navigation.label' => 'Foo', 'navigation.group' => 'Group 1']),
new MarkdownPage('bar', ['navigation.label' => 'Foo', 'navigation.group' => 'Group 2']),
Expand All @@ -496,9 +496,8 @@ public function testMainNavigationMenuDropdownItemsWithSameLabelButDifferentGrou
config(['hyde.navigation.subdirectories' => 'dropdown']);

$this->assertMenuEquals([
// Todo: Should use proper group name
['label' => 'group-1', 'children' => ['Foo']],
['label' => 'group-2', 'children' => ['Foo']],
['label' => 'Group 1', 'children' => ['Foo']],
['label' => 'Group 2', 'children' => ['Foo']],
], [
new MarkdownPage('one/foo', ['navigation.group' => 'Group 1']),
new MarkdownPage('two/foo', ['navigation.group' => 'Group 2']),
Expand Down Expand Up @@ -980,8 +979,6 @@ public function testSidebarItemGroupingIsNormalized()

public function testSidebarLabelsRetainBestFormatting()
{
$this->markTestSkipped('Not yet implemented');

$this->assertSidebarEquals(['GitHub'], [
new DocumentationPage('foo', ['navigation.group' => 'GitHub']),
new DocumentationPage('bar', ['navigation.group' => 'github']),
Expand Down
4 changes: 2 additions & 2 deletions packages/framework/tests/Unit/NavItemTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,8 @@ public function testGetGroupForRouteWithGroup()

public function testGroupKeysAreNormalized()
{
$item = new NavItem(new Route(new MarkdownPage()), 'Test', 500, 'Foo Bar');
$this->assertSame('foo-bar', $item->getGroup());
$this->assertSame('Foo Bar', (new NavItem(new Route(new MarkdownPage()), 'Test', 500, 'Foo Bar'))->getGroup());
$this->assertSame('Foo Bar', (new NavItem(new Route(new MarkdownPage()), 'Test', 500, 'foo-bar'))->getGroup());
}

public function testIdentifier()
Expand Down
Loading