Skip to content

Commit 80f7e7f

Browse files
CybotTMCybotTM
committed
Keep navigation menu in toctree order when mixing internal and external entries
A toctree that mixes internal pages and external links built the navigation menu (the document entry's menu entries) with all external entries grouped first, even though the on-page toctree kept the authored order. The sidebar/navbar therefore disagreed with the page; this was visible for nested toctrees. Internal and external menu entries are attached to the document entry by two separate transformers (InternalMenuEntryNodeTransformer and ExternalMenuEntryNodeTransformer), and the compiler runs one full tree traversal per transformer, so every external entry is appended in one pass and every internal entry in another. The result is grouped by type instead of following the toctree. ToctreeSortingTransformer now realigns the document entry's menu entries with the authored toctree order; it already ran at the right point to handle the reversed option. Globbed toctrees are skipped, since their order comes from the glob expansion rather than an authored sequence. Reported downstream at TYPO3-Documentation/render-guides#1175 Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
1 parent 6d70127 commit 80f7e7f

7 files changed

Lines changed: 244 additions & 7 deletions

File tree

‎packages/guides/src/Compiler/NodeTransformers/MenuNodeTransformers/ToctreeSortingTransformer.php‎

Lines changed: 80 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,11 +15,17 @@
1515

1616
use phpDocumentor\Guides\Compiler\CompilerContext;
1717
use phpDocumentor\Guides\Compiler\NodeTransformer;
18+
use phpDocumentor\Guides\Nodes\DocumentTree\DocumentEntryNode;
19+
use phpDocumentor\Guides\Nodes\DocumentTree\ExternalEntryNode;
20+
use phpDocumentor\Guides\Nodes\Menu\MenuEntryNode;
1821
use phpDocumentor\Guides\Nodes\Menu\TocNode;
1922
use phpDocumentor\Guides\Nodes\Node;
2023

2124
use function array_reverse;
25+
use function array_values;
26+
use function count;
2227
use function is_array;
28+
use function ksort;
2329

2430
/** @implements NodeTransformer<TocNode> */
2531
final class ToctreeSortingTransformer implements NodeTransformer
@@ -35,24 +41,91 @@ public function enterNode(Node $node, CompilerContext $compilerContext): Node
3541
return $node;
3642
}
3743

38-
if (!$node->isReversed()) {
44+
$entries = $node->getValue();
45+
if (!is_array($entries)) {
3946
return $node;
4047
}
4148

42-
$entries = $node->getValue();
4349
$documentEntry = $compilerContext->getDocumentNode()->getDocumentEntry();
44-
$documentMenuEntries = $documentEntry->getMenuEntries();
45-
if (is_array($entries)) {
50+
51+
if ($node->isReversed()) {
4652
$entries = array_reverse($entries);
47-
$documentMenuEntries = array_reverse($documentMenuEntries);
53+
$node->setValue($entries);
54+
$documentEntry->setMenuEntries(array_reverse($documentEntry->getMenuEntries()));
4855
}
4956

50-
$documentEntry->setMenuEntries($documentMenuEntries);
51-
$node->setValue($entries);
57+
// The document entry's menu entries (used to build the navigation menu)
58+
// are attached by separate transformers for internal and external menu
59+
// entries, each running in its own full tree traversal. As a result the
60+
// menu entries end up grouped by type instead of following the authored
61+
// toctree order, so the navigation menu disagrees with the order shown
62+
// on the page. Realign them with the toctree. Globbed toctrees are
63+
// skipped: their order is defined by the glob expansion, not by an
64+
// authored sequence.
65+
if (!$node->hasOption('glob')) {
66+
$documentEntry->setMenuEntries(
67+
$this->sortMenuEntriesByToctree($entries, $documentEntry->getMenuEntries()),
68+
);
69+
}
5270

5371
return $node;
5472
}
5573

74+
/**
75+
* Reorders the menu entries that belong to this toctree so they follow the
76+
* authored toctree order. Menu entries that belong to other toctrees of the
77+
* same document keep their position.
78+
*
79+
* @param array<MenuEntryNode> $tocEntries
80+
* @param array<DocumentEntryNode|ExternalEntryNode> $menuEntries
81+
*
82+
* @return array<DocumentEntryNode|ExternalEntryNode>
83+
*/
84+
private function sortMenuEntriesByToctree(array $tocEntries, array $menuEntries): array
85+
{
86+
$order = [];
87+
$position = 0;
88+
foreach ($tocEntries as $tocEntry) {
89+
if (!($tocEntry instanceof MenuEntryNode)) {
90+
continue;
91+
}
92+
93+
$order[$tocEntry->getUrl()] = $position++;
94+
}
95+
96+
$slots = [];
97+
$belonging = [];
98+
foreach ($menuEntries as $index => $menuEntry) {
99+
$key = self::menuEntryKey($menuEntry);
100+
if (!isset($order[$key])) {
101+
continue;
102+
}
103+
104+
$slots[] = $index;
105+
$belonging[$order[$key]] = $menuEntry;
106+
}
107+
108+
ksort($belonging);
109+
$belonging = array_values($belonging);
110+
111+
// Safety: only reorder when the slots and resolved entries line up
112+
// (they may not for the rare case of duplicate entries in a toctree).
113+
if (count($belonging) !== count($slots)) {
114+
return $menuEntries;
115+
}
116+
117+
foreach ($slots as $slotPosition => $index) {
118+
$menuEntries[$index] = $belonging[$slotPosition];
119+
}
120+
121+
return $menuEntries;
122+
}
123+
124+
private static function menuEntryKey(DocumentEntryNode|ExternalEntryNode $entry): string
125+
{
126+
return $entry instanceof DocumentEntryNode ? $entry->getFile() : $entry->getValue();
127+
}
128+
56129
public function leaveNode(Node $node, CompilerContext $compilerContext): Node|null
57130
{
58131
return $node;

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/expected/subpage/index.html‎

Lines changed: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
<!DOCTYPE html>
2+
<html class="no-js" lang="en">
3+
<head>
4+
<title>Subpage Title</title>
5+
<!-- Required meta tags -->
6+
<meta charset="utf-8">
7+
<meta name="viewport" content="width=device-width, initial-scale=1">
8+
9+
<link href="https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/css/bootstrap.min.css" rel="stylesheet" integrity="sha384-EVSTQN3/azprG1Anm3QDgpJLIm9Nao0Yz1ztcQTwFspd3yD65VohhpuuCOmLASjC" crossorigin="anonymous">
10+
</head>
11+
<body>
12+
<header class="">
13+
14+
<nav class="navbar navbar-expand-lg navbar-dark bg-primary">
15+
<div class="container">
16+
17+
<a class="navbar-brand" href="#">Navbar</a>
18+
<button class="navbar-toggler" type="button" data-bs-toggle="collapse" data-bs-target="#navbarSupportedContent" aria-controls="navbarSupportedContent" aria-expanded="false" aria-label="Toggle navigation">
19+
<span class="navbar-toggler-icon"></span>
20+
</button>
21+
<div class="collapse navbar-collapse" id="navbarSupportedContent">
22+
23+
<ul class="navbar-nav me-auto mb-2 mb-lg-0">
24+
<li class="nav-item">
25+
<a href="/subpage/index.html" class="nav-link current active" aria-current="page">
26+
Subpage Title
27+
</a>
28+
</li>
29+
<ul class="level-">
30+
<li class="nav-item">
31+
<a href="/subpage/index.html"
32+
class="nav-link current active" aria-current="page">Subpage Title</a>
33+
</li>
34+
</ul>
35+
</ul>
36+
37+
</div>
38+
</div>
39+
</nav>
40+
</header>
41+
<main id="main-content">
42+
<div class="container">
43+
<div class="container">
44+
<div class="row">
45+
<div class="col-lg-3">
46+
<nav class="nav flex-column">
47+
<ul class="menu-level-main">
48+
<li>
49+
<a href="/subpage/index.html"
50+
class="nav-link current active" aria-current="page">Subpage Title</a>
51+
<ul class="level-1">
52+
<li>
53+
<a href="/subpage/page1.html"
54+
class="nav-link">Page 1</a>
55+
</li>
56+
<li>
57+
<a href="https://example.com/a"
58+
class="nav-link">External A</a>
59+
</li>
60+
<li>
61+
<a href="/subpage/page2.html"
62+
class="nav-link">Page 2</a>
63+
</li>
64+
<li>
65+
<a href="https://example.org/b"
66+
class="nav-link">External B</a>
67+
</li>
68+
</ul>
69+
70+
</li>
71+
</ul>
72+
</nav>
73+
74+
</div>
75+
<div class="col-lg-9">
76+
77+
<nav aria-label="breadcrumb">
78+
<ol class="breadcrumb">
79+
<li class="breadcrumb-item"><a href="/index.html">Document Title</a></li>
80+
<li class="breadcrumb-item"><a href="/subpage/index.html">Subpage Title</a></li>
81+
</ol>
82+
</nav>
83+
<!-- content start -->
84+
<div class="section" id="subpage-title">
85+
<h1>Subpage Title</h1>
86+
<div class="toc">
87+
<p class="caption">Nested</p>
88+
<ul class="menu-level">
89+
<li class="toc-item">
90+
<a href="/subpage/page1.html#page-1">Page 1</a>
91+
92+
93+
</li>
94+
<li class="toc-item">
95+
<a href="https://example.com/a">External A</a>
96+
97+
98+
</li>
99+
<li class="toc-item">
100+
<a href="/subpage/page2.html#page-2">Page 2</a>
101+
102+
103+
</li>
104+
<li class="toc-item">
105+
<a href="https://example.org/b">External B</a>
106+
107+
108+
</li>
109+
</ul>
110+
</div>
111+
</div>
112+
<!-- content end -->
113+
</div>
114+
</div>
115+
</div>
116+
</div>
117+
</main>
118+
119+
<!-- Optional JavaScript; choose one of the two! -->
120+
121+
<!-- Option 1: Bootstrap Bundle with Popper -->
122+
<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/js/bootstrap.bundle.min.js" integrity="sha384-MrcW6ZMFYlzcLA8Nl+NtUVF0sA7MsXsP1UyJoMp4YLEuNSfAP+JcXn/tWtIaxVXM" crossorigin="anonymous"></script>
123+
124+
<!-- Option 2: Separate Popper and Bootstrap JS -->
125+
<!--
126+
<script src="https://cdn.jsdelivr.net/npm/@popperjs/core@2.9.2/dist/umd/popper.min.js" integrity="sha384-IQsoLXl5PILFhosVNubq5LC7Qb9DXgDA9i+tQ8Zj3iwWAwPtgFTxbJ8NT4GN1R8p" crossorigin="anonymous"></script>
127+
<script src="https://cdn.jsdelivr.net/npm/bootstrap@5.0.2/dist/js/bootstrap.min.js" integrity="sha384-cVKIPhGWiC2Al4u+LWgxfKTRIcfu0JTxR+EQDz/bgldoEyl4H0zUF0QKbrJ0EcQF" crossorigin="anonymous"></script>
128+
-->
129+
</body>
130+
</html>

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/input/guides.xml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
<?xml version="1.0" encoding="UTF-8" ?>
2+
<guides xmlns="https://www.phpdoc.org/guides"
3+
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
4+
xsi:schemaLocation="https://www.phpdoc.org/guides packages/guides-cli/resources/schema/guides.xsd"
5+
theme="bootstrap"
6+
>
7+
<extension class="phpDocumentor\Guides\Bootstrap"/>
8+
</guides>

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/input/index.rst‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
Document Title
2+
==============
3+
4+
.. toctree::
5+
6+
subpage/index

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/input/subpage/index.rst‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
Subpage Title
2+
=============
3+
4+
.. toctree::
5+
:caption: Nested
6+
7+
page1
8+
External A <https://example.com/a>
9+
page2
10+
External B <https://example.org/b>

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/input/subpage/page1.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
======
2+
Page 1
3+
======
4+
5+
Lorem Ipsum Dolor.

‎tests/Integration/tests-full/bootstrap/bootstrap-menu-nested-external-order/input/subpage/page2.rst‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
======
2+
Page 2
3+
======
4+
5+
Lorem Ipsum Dolor.

0 commit comments

Comments
 (0)