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] Improved navigation group label generation #1576

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
64f6634
Add accessor hook to make the group label
caendesilva Feb 20, 2024
54a82d0
Sketch out group label generator method
caendesilva Feb 20, 2024
0cb1fad
Inline adapted normalization method
caendesilva Feb 20, 2024
6e858fc
Remove label normalization at create time
caendesilva Feb 20, 2024
9e5dc91
Pass label as group key even for group parents
caendesilva Feb 20, 2024
7fd874e
Use group identifier for internal generator state
caendesilva Feb 20, 2024
29f78ea
Add auxiliary assertion case to improve accuracy
caendesilva Feb 20, 2024
edd20ab
Create titles when necessary
caendesilva Feb 20, 2024
9ab9700
Extract helper method
caendesilva Feb 20, 2024
2a1c38f
Normalize label at construct
caendesilva Feb 20, 2024
a9638ab
Remove redundant title generation
caendesilva Feb 20, 2024
b7e8588
Handle null case as property can be explicitly set to null
caendesilva Feb 20, 2024
9a228f1
Add todo
caendesilva Feb 20, 2024
18e2511
Coalesce to try label
caendesilva Feb 20, 2024
bb46d13
Update to expect pretty labels
caendesilva Feb 20, 2024
30373f7
Add label casing tests
caendesilva Feb 20, 2024
649142b
Revert "Normalize label at construct"
caendesilva Feb 20, 2024
ffe7314
Normalize group label at construct
caendesilva Feb 20, 2024
e283b5a
Revert "Update to expect pretty labels"
caendesilva Feb 20, 2024
008910c
Expect explicitly set front matter to be what the user wants
caendesilva Feb 20, 2024
fceca31
Expect dropdown label to be formatted
caendesilva Feb 20, 2024
2296d30
Rename back helper method name
caendesilva Feb 20, 2024
be55671
Switch order of helpers to group static methods
caendesilva Feb 20, 2024
ed38340
Call static method statically
caendesilva Feb 20, 2024
a297250
Move comment to more relevant location
caendesilva Feb 20, 2024
c6324bc
Import used functions
caendesilva Feb 20, 2024
36177fa
Simplify method return
caendesilva Feb 20, 2024
3035620
Remove unnecessary method call for already executed operation
caendesilva Feb 20, 2024
913e8b9
Revert "Handle null case as property can be explicitly set to null"
caendesilva Feb 20, 2024
0e55c2b
Remove unnecessary default values from config call
caendesilva Feb 20, 2024
8e1ab83
Simplify config return
caendesilva Feb 20, 2024
8398351
Normalize config access
caendesilva Feb 20, 2024
8cc5bd6
Format to single line
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,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 +58,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 = NavItem::dropdown($group, []);
}

$groupItem->addChild($item);

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

return;
Expand All @@ -93,13 +93,6 @@ protected function usesSidebarGroups(): bool
}) !== null;
}

protected function makeTitleForGroup(string $group): string
{
// Todo search for other labels in the group before slugifying them

return Hyde::makeTitle($group);
}

protected function canAddRoute(Route $route): bool
{
return $route->getPage()->showInNavigation() && ! $route->is(DocumentationPage::homeRouteName());
Expand Down
37 changes: 22 additions & 15 deletions packages/framework/src/Framework/Features/Navigation/NavItem.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@
use Stringable;
use Hyde\Support\Models\ExternalRoute;

use function is_string;
use function strtolower;

/**
* Abstraction for a navigation menu item. Used by the MainNavigationMenu and DocumentationSidebar classes.
*
Expand Down Expand Up @@ -98,7 +101,7 @@ public static function forRoute(Route|string $route, ?string $label = null, ?int
public static function dropdown(string $label, array $items, ?int $priority = null): static
{
// TODO resolve label from config here instead of view
return new static('', static::normalizeGroupLabel($label), $priority ?? static::searchForDropdownPriorityInNavigationConfig($label) ?? 999, null, $items);
return new static('', static::normalizeGroupLabel($label), $priority ?? static::searchForDropdownPriorityInNavigationConfig($label) ?? 999, $label, $items);
}

/**
Expand Down Expand Up @@ -130,6 +133,10 @@ public function getLink(): string
*/
public function getLabel(): string
{
if ($this->hasChildren()) {
return $this->makeGroupLabel();
}

return $this->label;
}

Expand Down Expand Up @@ -192,6 +199,8 @@ public function isCurrent(): bool
*/
public function addChild(NavItem $item): void
{
// Todo: Ensure that the item has a group identifier by creating it from the label if it doesn't exist?

$this->children[] = $item;
}

Expand All @@ -205,27 +214,25 @@ protected static function makeIdentifier(string $label): string
return Str::slug($label); // Todo: If it's a dropdown based on a subdirectory, we should use the subdirectory as the identifier
}

protected static function normalizeGroupLabel(string $label): string
// TODO: Consider moving all of these to a dropdown factory
protected static function searchForDropdownPriorityInNavigationConfig(string $groupKey): ?int
{
$configLabel = Config::getNullableString('docs.sidebar_group_labels.'.Str::slug($label));
return Config::getArray('hyde.navigation.order', [])[$groupKey] ?? null;
}

if ($configLabel) {
return $configLabel;
protected static function normalizeGroupLabel(string $label): string
{
// If there is no label, and the group is a slug, we can make a title from it
if ($label === strtolower($label)) {
return Hyde::makeTitle($label);
}

return $label;
}

// TODO: Consider moving all of these to a dropdown factory
protected static function searchForDropdownPriorityInNavigationConfig(string $groupKey): ?int
/** Find the best label for the group. */
protected function makeGroupLabel(): string
{
/** @var array<string, int> $config */
$config = Config::getArray('hyde.navigation.order', [
'index' => 0,
'posts' => 10,
'docs/index' => 100,
]);

return $config[$groupKey] ?? null;
return Config::getArray('docs.sidebar_group_labels', [])[Str::slug($this->identifier)] ?? $this->label;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -448,8 +448,7 @@ public function testPagesInSubdirectoriesAreAddedAsDropdownsWhenNavigationSubdir
config(['hyde.navigation.subdirectories' => 'dropdown']);

$this->assertMenuEquals([
['label' => 'about', 'children' => ['Foo', 'Bar', 'Baz']],
// TODO: Label should be About
['label' => 'About', 'children' => ['Foo', 'Bar', 'Baz']],
], [
new MarkdownPage('about/foo'),
new MarkdownPage('about/bar'),
Expand Down Expand Up @@ -496,9 +495,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 All @@ -510,9 +508,8 @@ public function testMainNavigationMenuAutomaticDropdownItemsWithSameLabelButDiff
config(['hyde.navigation.subdirectories' => 'dropdown']);

$this->assertMenuEquals([
// Todo: Should use proper group name
['label' => 'one', 'children' => ['Foo']],
['label' => 'two', 'children' => ['Foo']],
['label' => 'One', 'children' => ['Foo']],
['label' => 'Two', 'children' => ['Foo']],
], [
new MarkdownPage('one/foo'),
new MarkdownPage('two/foo'),
Expand Down Expand Up @@ -986,6 +983,11 @@ public function testSidebarLabelsRetainBestFormatting()
new DocumentationPage('foo', ['navigation.group' => 'GitHub']),
new DocumentationPage('bar', ['navigation.group' => 'github']),
]);

$this->assertSidebarEquals(['GitHub'], [
new DocumentationPage('foo', ['navigation.group' => 'github']),
new DocumentationPage('bar', ['navigation.group' => 'GitHub']),
]);
}

public function testSidebarLabelsCanBeSetInConfig()
Expand Down Expand Up @@ -1036,6 +1038,92 @@ public function testAllSidebarItemsArePlacedInGroupsWhenAtLeastOneItemIsGrouped(
]);
}

// Label casing tests

public function testMainMenuNavigationItemCasing()
{
// These labels are based on the page titles, which are made from the file names, so we try to format them

$this->assertMenuEquals(['Hello World'], [new MarkdownPage('Hello World')]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('hello-world')]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('hello world')]);
}

public function testMainMenuNavigationItemCasingUsingFrontMatter()
{
// If the user explicitly sets the label, we should respect that and assume it's already formatted correctly

$this->assertMenuEquals(['Hello World'], [new MarkdownPage('foo', ['title' => 'Hello World'])]);
$this->assertMenuEquals(['hello-world'], [new MarkdownPage('foo', ['title' => 'hello-world'])]);
$this->assertMenuEquals(['hello world'], [new MarkdownPage('foo', ['title' => 'hello world'])]);

$this->assertMenuEquals(['Hello World'], [new MarkdownPage('foo', ['navigation.label' => 'Hello World'])]);
$this->assertMenuEquals(['hello-world'], [new MarkdownPage('foo', ['navigation.label' => 'hello-world'])]);
$this->assertMenuEquals(['hello world'], [new MarkdownPage('foo', ['navigation.label' => 'hello world'])]);
}

public function testMainMenuNavigationGroupCasing()
{
config(['hyde.navigation.subdirectories' => 'dropdown']);

// When using subdirectory groupings, we try to format them the same way as the page titles

$this->assertMenuEquals(['Hello World'], [new MarkdownPage('Hello World/foo')]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('hello-world/foo')]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('hello world/foo')]);
}

public function testMainMenuNavigationGroupCasingUsingFrontMatter()
{
config(['hyde.navigation.subdirectories' => 'dropdown']); // TODO This should NOT be necessary when using front matter

// If the user explicitly sets the group, we should respect that and assume it's already formatted correctly

$this->assertMenuEquals(['Hello World'], [new MarkdownPage('foo', ['navigation.group' => 'Hello World'])]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('foo', ['navigation.group' => 'hello-world'])]);
$this->assertMenuEquals(['Hello World'], [new MarkdownPage('foo', ['navigation.group' => 'hello world'])]);
}

public function testSidebarItemCasing()
{
// These labels are based on the page titles, which are made from the file names, so we try to format them

$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('Hello World')]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('hello-world')]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('hello world')]);
}

public function testSidebarItemCasingUsingFrontMatter()
{
// If the user explicitly sets the label, we should respect that and assume it's already formatted correctly

$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('foo', ['title' => 'Hello World'])]);
$this->assertSidebarEquals(['hello-world'], [new DocumentationPage('foo', ['title' => 'hello-world'])]);
$this->assertSidebarEquals(['hello world'], [new DocumentationPage('foo', ['title' => 'hello world'])]);

$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('foo', ['navigation.label' => 'Hello World'])]);
$this->assertSidebarEquals(['hello-world'], [new DocumentationPage('foo', ['navigation.label' => 'hello-world'])]);
$this->assertSidebarEquals(['hello world'], [new DocumentationPage('foo', ['navigation.label' => 'hello world'])]);
}

public function testSidebarGroupCasing()
{
// When using subdirectory groupings, we try to format them the same way as the page titles

$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('Hello World/foo')]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('hello-world/foo')]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('hello world/foo')]);
}

public function testSidebarGroupCasingUsingFrontMatter()
{
// If the user explicitly sets the group, we should respect that and assume it's already formatted correctly

$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('foo', ['navigation.group' => 'Hello World'])]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('foo', ['navigation.group' => 'hello-world'])]);
$this->assertSidebarEquals(['Hello World'], [new DocumentationPage('foo', ['navigation.group' => 'hello world'])]);
}

// Testing helpers

protected function assertSidebarEquals(array $expected, array $menuPages): void
Expand Down
2 changes: 1 addition & 1 deletion packages/framework/tests/Unit/NavItemTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ public function testDropdownFacade()
{
$item = NavItem::dropdown('foo', []);

$this->assertSame('foo', $item->getLabel());
$this->assertSame('Foo', $item->getLabel());
$this->assertSame([], $item->getChildren());
$this->assertSame(999, $item->getPriority());
}
Expand Down
Loading